Item2609: extender.pl broken - Plugin installation fails if using tar instead of Archive::Tar
Priority: Normal
Current State: Closed
Released In: 1.0.9
Target Release: patch
Applies To: Engine
Component:
Branches:
If
extender.pl
calls the subroutine
untar
and Archive::Tar is not installed, it tries to use the commandline tool
tar
. Even if
tar
unpacks the archive succesfully, the subroutine
untar
reports failure (because of a misplaced
return 0
) and
extender.pl
fails. As a result, you cannot install plugins packed as tarballs if you don't have Archive::Tar installed. A simple patch is attached.
--
KerstinPuschke - 08 Jan 2010
I think your patch is better than current code but still fails if no tar but gtar is there.
Will look at it myself
This code has been fixed 3 or 4 times now. It must be possible to get it right.
--
KennethLavrsen - 08 Jan 2010
The code we are talking about is this
else {
print STDERR
"Archive::Tar is not installed: $@\n";
for my $tarBin ( qw( tar gtar ) ) {
print STDERR "Trying $tarBin on the command-line\n";
system $tarBin, "xvf$compressed", $archive and return 1;
if ($?) {
print STDERR "$tarBin failed: $?\n";
}
}
return 0;
}
return 1;
Let us go through it.
- We arrive at the else because Archive::Tar is not installed so we need to try tar first and then gtar
- We start the for loop with setting $tarbin = tar
- We print a little message to STDERR
- We perform the system call which will be "tar xvfz filename.tgz"
- If this returns a true value we continue "and" return 1. Ie if the system call succeeds we never reach the return 0 later. We return a 1.
- If the tar fails we continue and write the error code returned
- We now loop again trying gtar.
- Same steps - if we succed we return 1. If we fail we print error and return 0.
So moving the return inside the for loop is wrong.
As far as I can see the problem is that the tar fails because often the parameter f has to be followed by a file name.
So tar zxvf will work and tar xvfz will fail because we give it the filename z. It may not be all versions of tar that does this but I am sure I have made this mistake myself.
I need to try this myself. It is a time consuming test to do so I need some time.
Maybe some others can try to change
system $tarBin, "xvf$compressed", $archive and return 1;
to
system $tarBin, "$compressedxvf", $archive and return 1;
--
KennethLavrsen - 08 Jan 2010
I tried under my Centos 5.3 and there the f does not have to be at the end. But I am sure I experienced this on another Unix once.
For sure we should play it safe and change it.
Question is - what return code does it return. Always 0 on failure? Or 0 on success. And always on all OSes?
--
KennethLavrsen - 08 Jan 2010
I have been through some experiments.
I have renamed the Archive/Tar so perl cannot find it.
And I have tried to put a bogus program in the for loop
for my $tarBin ( qw( goof gtar ) ) {
The lack of a goof makes the script top looping in the for. It never tries gtar which is installed. And it fails
So the call to system fails in a way that does not silently make the code continue. It crashes. The tgz never gets extracted.
--
KennethLavrsen - 08 Jan 2010
Ha! The logic is reversed. The system call to tar returns a 0 if success.
Try
perl -e "print system tar, zxvf, 'AliasPlugin.tgz'"
assuming you have such a tgz in the directory
Result is 0!
So we do not return the 0. Getting close. Need to see how gtar behaves. Probably same program symlinked.
--
KennethLavrsen - 08 Jan 2010
Yes gtar also returns 0. So this is the fix.
I also check in the defensive change of having the f at the end of zxvf.
I believe it was on our old HPUX machines where I could not make tar work unless the f was last letter. So in case someone installs Foswiki on such a strange machine - let is make it work with that also. In any case - even if I remember wrong - it does not harm.
--
KennethLavrsen - 09 Jan 2010
No doubt this error also was the root cause of
Item1960 so it got closed along with this.
--
KennethLavrsen - 09 Jan 2010
good work, kenneth. though, i would prefer this phrasing to make the code read more naturally:
wbniv@wbniv-laptop:~/foswiki/Release01x00/core/tools$ svn diff
Index: extender.pl
===================================================================
--- extender.pl (revision 6002)
+++ extender.pl (working copy)
@@ -741,7 +741,7 @@
print STDERR "Trying $tarBin on the command-line\n";
# system call returns 0 if success. and error code if no success
# so we return 1 if the tarBin call succeed
- return 1 unless system $tarBin, "${compressed}xvf", $archive;
+ return 1 if system( $tarBin, "${compressed}xvf", $archive ) == 0;
# OK we failed. Report and loop on if more to loop
if ($?) {
print STDERR "$tarBin failed: $?\n";
--
WillNorris - 10 Jan 2010