Bug 5599

Summary: koha_perl_deps.pl and PerlDependencies.pm don't compare $VERSION numbers correctly
Product: Koha Reporter: Joe Atzberger <ohiocore>
Component: Installation and upgrade (command-line installer)Assignee: Chris Nighswonger <cnighswonger>
Status: RESOLVED DUPLICATE QA Contact: Bugs List <koha-bugs>
Severity: minor    
Priority: P5 - low CC: cnighswonger, jonathan.druart, veron
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12790
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16638
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:

Description Joe Atzberger 2011-01-07 22:15:38 UTC
Normal perl $VERSION numbers are not string sortable/comparable.  This is because they do not use leading zeroes or static numbers of digits for each part.  

But our logic in C4/Installer/PerlModules.pm relies on string comparison, so it does not regard 1.3.12 as being greater than 1.3.5, when in reality it is 7 iterations later.

Example:

$ ./koha_perl_deps.pl -u 

Module Name                                 Current Version                       Required Version         Module Required
--------------------------------------------------------------------------------------------------------------------------
Graphics::Magick                            1.3.12 *                              1.3.5                    No

--------------------------------------------------------------------------------------------------------------------------
Total modules reported: 1                                                     * Module is missing or requires an upgrade.



Specifically, these comparisons from lines 75 and 89 of C4/Installer/PerlModules.pm are incorrect:

 $params{'module'}->VERSION lt $PERL_DEPS->{$params{'module'}}->{'min_ver'}
...
 $pkg->VERSION lt $PERL_DEPS->{$_}->{'min_ver'}


I don't like the approach used by PerlModules.pm because the code is overwrought and it tries to reproduce core perl logic.  

The definitive test would rely on, for example, $pkg->VERSION($min) since VERSION() is a part of UNIVERSAL and it actually understands perl's lax version strings (and always will, even if they change format).  We can access that via UNIVERSAL::require($min).
Comment 1 Marc VĂ©ron 2014-08-20 06:49:37 UTC
I submitted patch for Bug 12790. It displays required version numbers on the 'About page. This way we can verify the required vs. installed version numbers without diving into the scripts.
Comment 2 Jonathan Druart 2015-03-26 14:18:17 UTC
The issue raised by the description is now fixed

commit 80ba84159ddbdd52394984c8078263066dcc7a7b
Author: Chris Cormack <chrisc@catalyst.net.nz>
Date:   Thu Jul 21 20:04:02 2011 +1200

    Fixing missing padding on module version


-        'min_ver'  => '1.3.5'
+        'min_ver'  => '1.3.05'

So it's not needed at the moment.
I decrease the priority.
Comment 3 Jonathan Druart 2017-01-17 09:23:33 UTC

*** This bug has been marked as a duplicate of bug 17880 ***