Bug 5599 - koha_perl_deps.pl and PerlDependencies.pm don't compare $VERSION numbers correctly
Summary: koha_perl_deps.pl and PerlDependencies.pm don't compare $VERSION numbers corr...
Status: RESOLVED DUPLICATE of bug 17880
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Chris Nighswonger
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-07 22:15 UTC by Joe Atzberger
Modified: 2017-01-17 09:23 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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 ***