Last Comment Bug 678772 - version.pm 0.92 and newer forbids negative values, making checksetup.pl fail with the error "Invalid version format"
: version.pm 0.92 and newer forbids negative values, making checksetup.pl fail ...
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Installation & Upgrading (show other bugs)
: 4.1.3
: All All
: -- major (vote)
: Bugzilla 4.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
: 679290 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-08-13 17:50 PDT by Frédéric Buclin
Modified: 2011-08-16 00:55 PDT (History)
3 users (show)
mkanat: approval+
mkanat: approval4.2+
LpSolit: blocking4.2+
mkanat: approval4.0+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (925 bytes, patch)
2011-08-13 18:24 PDT, Frédéric Buclin
mkanat: review+
Details | Diff | Splinter Review

Description Frédéric Buclin 2011-08-13 17:50:08 PDT
Perl 5.15.1 has version.pm 0.93, but ActiveState already offers it as an update via the Perl Package Manager (ppm) for at least Perl 5.14.1 (I didn't check for 5.12.4). Also, Linux distros could do the same if they want to (and some probably do). Since version.pm 0.92, negative values for $VERSION are no longer allowed, and an error is thrown if either $VERSION is negative or you try to compare $VERSION to a negative value. In this last case, the reason is that Perl automagically converts the negative value into a version object, but as version.pm now forbids them, an error is thrown. This makes checksetup.pl to die due to line 712 in Bugzilla::Install::Requirements::_checking_for():

  if ($found and $found eq "-1")

$found = $module->VERSION || -1; and so is a version object when the module being called returns a version object (most Perl modules still return a string, but some others return a version object, such as DBD::Pg). In that case, "-1" is converted into an illegal (negative) version object and checksetup.pl dies:

"Invalid version format (non-numeric data) at Bugzilla/Install/Requirements.pm line 712."

IMO, we shouldn't try to set $found to -1 when the module cannot be found. $found should simply be undefined. I know _checking_for() currently uses undefined for "module_unknown_version", but I doubt such modules exist (probably CPAN has a check to reject the upload of such modules).
Comment 1 Frédéric Buclin 2011-08-13 17:59:28 PDT
If you want to reproduce the error with version.pm 0.92 or newer, simply run this testcase from the shell:

perl -wE 'package Foo; use version; our $VERSION = qv("0.89.2"); my $ver = Foo->VERSION; say $ver unless $ver eq "-1"'
Comment 2 Frédéric Buclin 2011-08-13 18:24:18 PDT
Created attachment 552915 [details] [diff] [review]
patch, v1

If it's a reference, then it's a version object. In that case, it cannot be -1.
Comment 3 Max Kanat-Alexander 2011-08-15 16:07:10 PDT
Comment on attachment 552915 [details] [diff] [review]
patch, v1

Review of attachment 552915 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great.
Comment 4 Frédéric Buclin 2011-08-15 18:46:26 PDT
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified Bugzilla/Install/Requirements.pm
Committed revision 7923.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.2/
modified Bugzilla/Install/Requirements.pm
Committed revision 7903.

Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/4.0/
modified Bugzilla/Install/Requirements.pm
Committed revision 7644.
Comment 5 Teemu Mannermaa (:wicked) 2011-08-16 00:55:21 PDT
*** Bug 679290 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.