Summary: | GD is declared as an optional dependency but Koha dies without it | ||
---|---|---|---|
Product: | Koha | Reporter: | Julian Maurice <julian.maurice> |
Component: | Architecture, internals, and plumbing | Assignee: | Julian Maurice <julian.maurice> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | 1joynelson, jonathan.druart, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
20.05.00
|
|
Attachments: |
Bug 24693: Move GD from recommends to requires in cpanfile
Bug 24693: Move GD from recommends to requires in cpanfile Bug 24693: Move GD from recommends to requires in cpanfile |
Description
Julian Maurice
2020-02-20 07:37:48 UTC
Created attachment 99299 [details] [review] Bug 24693: Move GD from recommends to requires in cpanfile Without it, catalogue/detail.pl dies (and probably other scripts too) Test plan: 1. Uninstall GD, reload starman 2. Go to catalogue/detail.pl. Confirm that it dies 3. Reinstall GD, reload starman 4. Go to catalogue/detail.pl. Confirm that it work normally Created attachment 99475 [details] [review] Bug 24693: Move GD from recommends to requires in cpanfile Without it, catalogue/detail.pl dies (and probably other scripts too) Test plan: 1. Uninstall GD, reload starman 2. Go to catalogue/detail.pl. Confirm that it dies 3. Reinstall GD, reload starman 4. Go to catalogue/detail.pl. Confirm that it work normally Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> catalogue/detail.pl dies without GD. Seems correct switch to require No errors It does not die for me, it dies if I enable LocalCoverImages, then try to upload an image. Maybe we should clarify what does *required* mean for us. Personally I believe that every Perl module that we directly `use` or `require` without checking its presence should be flagged as required. Even if the require is behind a syspref check or something like that. Put more simply, if Koha dies because of a missing Perl module, that module is a required dependency. Maybe later we can build something based on cpanfile's `feature` (https://metacpan.org/pod/distribution/Module-CPANfile/lib/cpanfile.pod#feature) to warn when an optional feature is enabled and the required modules for this feature are missing. (In reply to Julian Maurice from comment #4) > Maybe we should clarify what does *required* mean for us. > > Personally I believe that every Perl module that we directly `use` or > `require` without checking its presence should be flagged as required. Even > if the require is behind a syspref check or something like that. Put more > simply, if Koha dies because of a missing Perl module, that module is a > required dependency. > > Maybe later we can build something based on cpanfile's `feature` > (https://metacpan.org/pod/distribution/Module-CPANfile/lib/cpanfile. > pod#feature) to warn when an optional feature is enabled and the required > modules for this feature are missing. The whole 'feature' stuff from the cpanfile move came up earlier that I was expecting.. but I suspected it would raise it's head at some point. I'm really not sure what way to fall on this one.. I don't like that changing a syspref can completely 'break', i.e lead to a die, koha.. but at the same time if a set of functionality is optional, the dependencies it requires really aught to be too. (In reply to Jonathan Druart from comment #3) > It does not die for me, it dies if I enable LocalCoverImages, then try to > upload an image. That was wrong, I removed libgd-perl, that removed koha-common. The alias restart_all got broken, as it did not correctly restart plack, etc. So yes, detail.pl crashes with GD. Created attachment 99510 [details] [review] Bug 24693: Move GD from recommends to requires in cpanfile Without it, catalogue/detail.pl dies (and probably other scripts too) Test plan: 1. Uninstall GD, reload starman 2. Go to catalogue/detail.pl. Confirm that it dies 3. Reinstall GD, reload starman 4. Go to catalogue/detail.pl. Confirm that it work normally Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> catalogue/detail.pl dies without GD. Seems correct switch to require No errors Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> We will make it (really) optional if there is a real need for that. Nice work everyone! Pushed to master for 20.05 Does not apply to 19.11.x branch. Not backported |