Summary: | Perl dependency evaluation incorrect | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Installation and upgrade (command-line installer) | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | jesus, jonathan.druart, kyle, m.de.rooy, mtj |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17880 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 17880 | ||
Bug Blocks: | 17990 | ||
Attachments: |
Bug 17986 - Perl dependency evaluation incorrect
Bug 17986 - Perl dependency evaluation incorrect Bug 17986: Perl dependency evaluation incorrect Bug 17986: Add tests Bug 17986: Perl dependency evaluation incorrect |
Description
David Cook
2017-01-24 23:22:59 UTC
Created attachment 59535 [details] [review] Bug 17986 - Perl dependency evaluation incorrect It looks like I made a copy/paste error in a previous patch. While the fix was working when you pass the param "module" to version_info, it wasn't populating the version correctly for the "all" param, which causes koha_perl_deps.pl to think all OK modules actually need an upgrade. TEST PLAN 0) Be on a system where you know your Koha Perl dependencies are mostly up-to-date 1) Run ./koha_perl_deps.pl -a -c 2) Note that most modules say they need an upgrade even when the installed version is the same as the minimum version 3) Apply patch 4) Run ./koha_perl_deps.pl -a -c 5) Note that most moduls say they're OK, especially when the installed version is the same or greater than the minimum version https://bugs.koha-community.org/show_bug.cgi?id=17880 Created attachment 59536 [details] [review] Bug 17986 - Perl dependency evaluation incorrect It looks like I made a copy/paste error in a previous patch. While the fix was working when you pass the param "module" to version_info, it wasn't populating the version correctly for the "all" param, which causes koha_perl_deps.pl to think all OK modules actually need an upgrade. TEST PLAN 0) Be on a system where you know your Koha Perl dependencies are mostly up-to-date 1) Run ./koha_perl_deps.pl -a -c 2) Note that most modules say they need an upgrade even when the installed version is the same as the minimum version 3) Apply patch 4) Run ./koha_perl_deps.pl -a -c 5) Note that most moduls say they're OK, especially when the installed version is the same or greater than the minimum version https://bugs.koha-community.org/show_bug.cgi?id=17880 TEST PLAN alternative Instead of running koha_perl_deps.pl, you can visit /cgi-bin/koha/about.pl and click on "Perl modules". Packaged koha-kommon included in Debian Wheezy seems to be affected by this bug. Upgrades to koha-kommon 16.05.05 package fail complaining of perl "virtual packages" unmet dependencies. Will this patch be commited to 16.11 or shall we wait to 17.05? Best, Jesus Created attachment 59538 [details] [review] Bug 17986: Perl dependency evaluation incorrect It looks like I made a copy/paste error in a previous patch. While the fix was working when you pass the param "module" to version_info, it wasn't populating the version correctly for the "all" param, which causes koha_perl_deps.pl to think all OK modules actually need an upgrade. TEST PLAN 0) Be on a system where you know your Koha Perl dependencies are mostly up-to-date 1) Run ./koha_perl_deps.pl -a -c 2) Note that most modules say they need an upgrade even when the installed version is the same as the minimum version 3) Apply patch 4) Run ./koha_perl_deps.pl -a -c 5) Note that most moduls say they're OK, especially when the installed version is the same or greater than the minimum version Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Running koha_perl_deps.pl -u convinced me. Created attachment 59544 [details] [review] Bug 17986: Add tests I am stuck, I tried to provide tests to highlight the problem, but I do not manage to make them fail. There is something wrong in the code: It seems that the same code is duplicated: if ( $params{module} ) { do something; } else { do almost the same thing; } Except that in the first block, $_ is used, but is always undefined (not in a loop). This code sounds ugly and buggy, it may be good to fix it. (In reply to Jonathan Druart from comment #7) > I am stuck, I tried to provide tests to highlight the problem, but I do not > manage to make them fail. > There is something wrong in the code: > It seems that the same code is duplicated: > if ( $params{module} ) { > do something; > } else { > do almost the same thing; > } > > Except that in the first block, $_ is used, but is always undefined (not in > a loop). > This code sounds ugly and buggy, it may be good to fix it. Yeah. I saw the code; it is horrible. But this seems to fix at least one issue. Ok, I am rewriting it, will move that to another bug report. Created attachment 59545 [details] [review] Bug 17986: Perl dependency evaluation incorrect It looks like I made a copy/paste error in a previous patch. While the fix was working when you pass the param "module" to version_info, it wasn't populating the version correctly for the "all" param, which causes koha_perl_deps.pl to think all OK modules actually need an upgrade. TEST PLAN 0) Be on a system where you know your Koha Perl dependencies are mostly up-to-date 1) Run ./koha_perl_deps.pl -a -c 2) Note that most modules say they need an upgrade even when the installed version is the same as the minimum version 3) Apply patch 4) Run ./koha_perl_deps.pl -a -c 5) Note that most moduls say they're OK, especially when the installed version is the same or greater than the minimum version Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Running koha_perl_deps.pl -u convinced me. (In reply to Jonathan Druart from comment #9) > Ok, I am rewriting it, will move that to another bug report. See bug 17990. (In reply to Jonathan Druart from comment #11) > (In reply to Jonathan Druart from comment #9) > > Ok, I am rewriting it, will move that to another bug report. > > See bug 17990. You're a champ, Jonathan! I have another two reports* that looks at fixing issues with this code, so cheers for rewriting it overall. *bug 17882 *bug 17887 Pushed to master for 17.05, thanks David! Much better. This patch has been pushed to 16.11.x and will be in 16.11.03. Pushed to 16.05.x, for 16.05.09 release |