Item12225: Plugins using new version system break older versions Foswiki

Priority: Normal
Current State: Closed
Released In: n/a
Target Release: n/a
Applies To: Engine
Component:
Branches: Release01x01 trunk
Reported By: DavidKramer
Waiting For:
Last Change By: GeorgeClark
This problem presented itself after installing TaskManagementContrib in a 1.1.4 system. It installed SetVariablePlugin, which uses the new version system, which doesn't work with 1.1.4. To reproduce, install TaskManagementContrib in a 1.1.4 system and go to Install/Update plugins.

The error is Invalid version format (non-numeric data) at /home/neagile/foswiki/public_html/lib/Foswiki/Configure/UIs/EXTENSIONS.pm line 283.

The proposed fix is to check whether the version is an object before the comparison at that line. Here's the important bits of the conversation on #foswiki 11/04/12 11:00EST:
 
10:11:28 PM) dj_segfault: Interesting.  One of the dependencies is SetVariablePlugin, which is the last one my debugging code prints out
(10:11:55 PM) dj_segfault: According to the sparse documentation at http://foswiki.org/bin/viewauth/Extensions/TaskManagementContrib
(10:12:49 PM) gac410: Oh crap!!!    It's using the new style version strings.  which obviously breaks old foswiki.  That's not good.
(10:13:13 PM) dj_segfault: Which?  TMC or SVP?
(10:13:47 PM) gac410: SVP
(10:14:48 PM) gac410: dj_segfault:  Edit lib/Foswiki/Plugins/SetVariablePlugin.pm   Comment out and insert:
(10:14:50 PM) gac410: #use version; our $VERSION = version->declare("v2.30");
(10:14:50 PM) gac410: our $VERSION = '2.30';
(10:15:21 PM) dj_segfault: Is there some magic that knows not to install version >$x for release $y?
(10:15:32 PM) gac410: no.
(10:15:48 PM) gac410: This is bad.
(10:15:59 PM) ***gac410 needs to find out how badly things break.  
(10:18:01 PM) gac410: yeesh.  This breaks 1.1.6 too.
(10:18:11 PM) ***gac410 is looking pretty bad right now.
(10:19:54 PM) dj_segfault: Sorry, I'm confused.  Both of those lines are already in the file.  What am I commenting out and what am I putting back in?
(10:20:50 PM) gac410: Comment out the line   "use version; our $VERSION = version->declare("v2.30");
(10:21:04 PM) gac410: and add the line  "our version = "2.30";
(10:21:13 PM) gac410: er... but with the upper case
(10:21:30 PM) gac410: ie.  get rid of the "v"  and get rid of the declare stuff.
(10:25:13 PM) ***gac410 does not understand how if ( $ext->{installedVersion} eq 'HEAD' ) {   ... which is line 283 - can trigger that error
(10:27:43 PM) dj_segfault: That fixed it.  Thanks.
(10:30:42 PM) dj_segfault: Should I now uninstall those two plugins?
(10:31:18 PM) gac410: Up to you I guess.   If you don't need them..
(10:31:32 PM) gac410: I think I need to revert all the version stuff.   This is a disaster.
(10:31:44 PM) SvenDowideit: :/
(10:32:01 PM) SvenDowideit: what is it that causes the crash?
(10:32:38 PM) gac410: I don't understand ..  version/vpp.pm  is throwing the error.   Something with the way configure is calling the versionm as the string is correct, and it doesn't throw errors when using the module.
(10:33:12 PM) SvenDowideit: so it might be resolveable with a wrapperere
(10:33:31 PM) SvenDowideit: mmm, complicated
(10:33:40 PM) gac410: yeah.... I don't understand it,  ... AHHH...   I it's the compare.
(10:34:05 PM) gac410: I think perl knows it's a real version string and when we say ''eq HEAD"   it says   WHAT?
(10:37:11 PM) gac410: with perl version objects,  it "knows" how to compare them,  all built in.    Us doing strange things like expecting a VERSION string to be HEAD,  is just going to not work.
(10:37:30 PM) gac410: I obviously didn't understand the implications of using a version object.
(10:43:28 PM) dj_segfault: I get the sense I shouldn't file a bug for this because it's a larger-than-that-plugin issue, right?
(10:43:38 PM) SvenDowideit: go on, raise a bug:p
(10:44:09 PM) SvenDowideit: YAY.
(10:44:38 PM) gac410: Yeah.   I'll mea culpa it.    Basically the bug is in UIs/EXTENSIONS.pm     it needs to determine if the version isa version object.   and not compare it to HEAD if it's a real version object.
(10:44:59 PM) gac410: And then we need to provide a hotfix for the older code.   Or we abandon the whole idea of using real versions.
(10:45:01 PM) SvenDowideit: and then there's the problem of older installed :/
(10:45:22 PM) SvenDowideit: na, not abandon, just er, yeah, we may need to mumble.
(10:45:30 PM) SvenDowideit: ie - add the isa code
(10:45:54 PM) SvenDowideit: oooo, can we um detect what version of foswiki?
(10:46:06 PM) gac410: where
(10:46:12 PM) SvenDowideit: so that the plugin's code has a 'you're dumb' fallback
(10:46:19 PM) SvenDowideit: ie, in MyPlugin.pm
(10:46:36 PM) SvenDowideit: our $VERSION = your new hotness
...
(11:05:18 PM) gac410: Configure Extension list compares the version to HEAD .. to detect if it's pseudo installed.     Once $VERSION is a version object,  the comparison barfs because HEAD is non-numeric.
(11:05:59 PM) gac410: I think we have a solution.   in Plugins  (  If old foswiki,  $VERSION = "some string"   else $VERSION = version->declare("v1.2.3");
(11:06:24 PM) gac410: And then in new foswiki we need to figure a way around using HEAD to detect pseudo install.
(11:06:40 PM) timothe: That doesn't sound too hard to fix.  But don't forget the version stuff in CGISetup.pm also looks at version strings.
(11:06:59 PM) timothe: The way I detect pseudo-install is with -l
(11:08:08 PM) timothe: But we could (and maybe should) teach it to write a .pseudoinstalled in the entity's 'lib/'
(11:08:17 PM) gac410: Yes.   that's what Configure/Dependency.pm does.   Assigning {installedVersion} a value of HEAD is legal.     but comparing {installedVersion} eq 'HEAD' when {installedVersion} is a version object wil lcrash
(11:08:54 PM) gac410: The issue is not the pseudo installed stuff.    It's comparing a non-pseudo-installed version  to "HEAD".
(11:09:24 PM) gac410: We could do something like return version 0.0.0_000 if pseudo-installed.
(11:09:44 PM) timothe: Sure, but isn't it if( A eq 'HEAD' or B eq 'HEAD' ) then manual compare else $version ->?
(11:10:16 PM) gac410: No.   If installed eq "HEAD"   then it's pseudo installed and no furhter comparison is done.
(11:10:33 PM) timothe: Actually, pinstall implies a developer, so probably 9999.99_999 would be better.  Or 99999.svnrev (use svnversion cmd)
(11:10:35 PM) gac410: We don't attempt to display or compare pseudo versions.
(11:11:15 PM) gac410: Yeah that works too.   Any string that is also a legal version when the installed version happens to be a version object.
(11:11:18 PM) timothe: if you make pinstall look 'newest', then developers can live dangerously
(11:11:32 PM) gac410: good idea.  Thanks.  I'll play with that.
(11:12:30 PM) timothe: No problem.  Have a look at my latest commit; some will be familiar, but it's the first non-'validation' feedback button.
(11:13:26 PM) gac410: will do... But right now, anyone who installs SetVariablePlugin will totally disable install or remove extensions.   Need to fix that really fast.

-- DavidKramer - 05 Nov 2012

We've got two possible fixes.

  • Modify the $VERSION assignment in the extension to only create a version object on Foswiki 1.1.6 or newer. This works but results in a really messy module:

our $VERSION;
if ( substr( $Foswiki::VERSION, 0, 1 ) eq "v" ) {
    use version; $VERSION = version->declare("v2.31");
}
else {
    $VERSION = "v2.31";
}

  • The other option is to patch older Foswiki versions. One possible solution is to use a Contrib and a PREINSTALL script to patch Foswiki/Configure/UIs/EXTENSIONS.pm and Foswiki/Configure/Dependency.pm. We could use a PREUNINSTALL exit to "reverse" the patch when removing the extension.

-- GeorgeClark - 05 Nov 2012

Modifying the VERSION assignment works for now, but is a terribly ugly fix. The use of HEAD is mostly developer only, so going ahead with a tool to patch older installations would be preferable to mangling every new version string.

-- GeorgeClark - 07 Nov 2012

PatchFoswikiContrib released to resolve the issue on older Foswiki.

-- GeorgeClark - 31 Dec 2012
 
Topic revision: r18 - 31 Dec 2012, GeorgeClark
 
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. see CopyrightStatement. Creative Commons License