C4::Installer::PerlModules compares a modules VERSION against the min_version specified in C4::Installer::PerlDependencies using the lexicographical comparison "lt". This means that 0.9.12 will be considered to be less than 0.9.3: PDF::Table 0.9.12 0.9.3 Yes The above line shows up in koha_perl_deps.pl when I look to see what modules I need to upgrade. It doesn't prevent me from installing/running Koha, but it's wrong. In theory, we could use version.pm for our comparisons (http://search.cpan.org/~jpeacock/version-0.9917/lib/version.pod#How_to_compare_version_objects). I think it can sometimes have surprising results, although in this case it should be OK since we're comparing version numbers against version numbers of the same module. So in theory it should have a consistent scheme...
I've actually written a patch for this, but I found another bug in C4::Installer::PerlModules::version_info() while writing a unit test, so I'll open another bug for that...
Removed the dependency on bug 17882 as they can actually be done separately...
(In reply to David Cook from comment #2) > Removed the dependency on bug 17882 as they can actually be done > separately... Although whichever one comes second will need to be rebased...
Created attachment 58857 [details] [review] Bug 17880 - Add test to check version number comparison
Created attachment 58858 [details] [review] Bug 17880 - Use version.pm to parse version numbers in C4::Installer::PerlModules
Test Plan: 1) Apply patch "Bug 17880 - Add test to check version number comparison" 2) Run "perl t/Installer_PerlModules.t" 3) Note the following output: not ok 19 - Version 0.9.13 is greater than 0.9.3, so no upgrade needed # Failed test 'Version 0.9.13 is greater than 0.9.3, so no upgrade needed' # at t/Installer_PerlModules.t line 51. # got: '1' # expected: '0' # Looks like you failed 1 test of 19. 4) Apply patch "Bug 17880 - Use version.pm to parse version numbers in C4::Installer::PerlModules" 5) Run "perl t/Installer_PerlModules.t" 6) Note the following output: ok 19 - Version 0.9.13 is greater than 0.9.3, so no upgrade needed
Created attachment 58998 [details] [review] Bug 17880 - Add test to check version number comparison Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Created attachment 58999 [details] [review] Bug 17880 - Use version.pm to parse version numbers in C4::Installer::PerlModules Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Oops. Didn't notice the Academy until after I signed off. Thanks for fixing this. I remember hitting this problem with some actual Koha stuff while playing between OS's and versions.
Created attachment 59074 [details] [review] Bug 17880 - Add test to check version number comparison Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 59075 [details] [review] Bug 17880 - Use version.pm to parse version numbers in C4::Installer::PerlModules Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
*** Bug 5599 has been marked as a duplicate of this bug. ***
*** Bug 16638 has been marked as a duplicate of this bug. ***
Pushed to master for 17.05, thanks David!
These patches have been pushed to 16.11.x and will be in 16.11.02. .
Ooops... it looks like I actually created a new bug while fixing that bug! The test works but I made a copy/paste error which means koha_perl_deps.pl now doesn't work correctly! I'm just about to attach a patch to fix that.
Created attachment 59533 [details] [review] Fix copy/paste error 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
Created attachment 59534 [details] [review] Bug 17880: Fix copy/paste error 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
See Bug 17986 for a separate bug report and fix
Comment on attachment 59534 [details] [review] Bug 17880: Fix copy/paste error Deleting this patch here, since it is on a new report
Pushed to 16.05.x, for 16.05.08 release
(In reply to Mason James from comment #21) > Pushed to 16.05.x, for 16.05.08 release Blocked by BZ-17986, skipping for 16.05.08
Pushed to 16.05.x, for 16.05.09 release