tobyink / p5-dist-inkt

1 stars 2 forks source link

Stuffs full path into tarball with perl v5.28.1 #3

Open jonassmedegaard opened 5 years ago

jonassmedegaard commented 5 years ago

Running distinkt-dist on the included example on a Debian testing/unstable system with perl v5.28.1 fails:

Processing cpanfile
Finding provided packages
Finding provided scripts
Distribution seems suitable for static install
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/cpanfile
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/dist.ini
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/t/01basic.t
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/lib/Acme/Example/Dist.pm
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/INSTALL
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/README
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/LICENSE
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/doap.ttl
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.yml
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.json
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/Makefile.PL
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/MANIFEST
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001.tar.gz
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/INSTALL' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/LICENSE' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/MANIFEST' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.json' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.yml' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/Makefile.PL' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/README' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/cpanfile' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/dist.ini' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/doap.ttl' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/t/01basic.t' at /usr/share/perl5/Dist/Inkt.pm line 299.
No such file in archive: 'tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/lib/Acme/Example/Dist.pm' at /usr/share/perl5/Dist/Inkt.pm line 299.

For comparison, same succeeds on a stable Debian system with perl v5.24.1:

Processing cpanfile
Finding provided packages
Finding provided scripts
Distribution seems suitable for static install
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/cpanfile
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/dist.ini
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/t/01basic.t
Copying /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/lib/Acme/Example/Dist.pm
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/INSTALL
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/README
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/LICENSE
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/doap.ttl
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.yml
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.json
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/Makefile.PL
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/MANIFEST
Writing /tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001.tar.gz
/tmp/libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001
Acme-Example-Dist-0.001
INSTALL
LICENSE
MANIFEST
META.json
META.yml
Makefile.PL
README
cpanfile
dist.ini
doap.ttl
t/01basic.t
lib/Acme/Example/Dist.pm
kjetilk commented 5 years ago

It seems entirely possible that #1 has something to do with it?

kjetilk commented 5 years ago

Or perhaps it is earlier in the tarball building? At https://github.com/tobyink/p5-dist-inkt/blob/master/lib/Dist/Inkt.pm#L287, it seems the filename is OK, so everything seemed to have worked up to then. But then, it seems to completely rebuild the filename when it is actually adding to the tarball.

Alas, I don't have a perlbrew or a Debian Sid box up and running, but it seems to me that the problem is somewhere around in https://github.com/tobyink/p5-dist-inkt/blob/cec0bd2d890a927ead6dbd99831dd9e014f231c8/lib/Dist/Inkt.pm#L292-L300

jonassmedegaard commented 5 years ago

This change to /usr/share/perl/5.*/Archive/Tar/File.pm from perl 5.24.1 to perl 5.28.1, makes Dist::Inkt either succeed or fail:

@@ -13,7 +13,7 @@

 use vars qw[@ISA $VERSION];
 #@ISA        = qw[Archive::Tar];
-$VERSION    = '2.04_01';
+$VERSION    = '2.30';

 ### set value to 1 to oct() it during the unpack ###

@@ -396,12 +396,7 @@
     my $path = shift;

     my ($vol, $dirs, $file) = File::Spec->splitpath( $path, $self->is_dir );
-    my @dirs = File::Spec->splitdir( $dirs );
-
-    ### so sometimes the last element is '' -- probably when trailing
-    ### dir slashes are encountered... this is of course pointless,
-    ### so remove it
-    pop @dirs while @dirs and not length $dirs[-1];
+    my @dirs = File::Spec->splitdir( File::Spec->canonpath($dirs) );

     ### if it's a directory, then $file might be empty
     $file = pop @dirs if $self->is_dir and not length $file;
@@ -409,9 +404,7 @@
     ### splitting ../ gives you the relative path in native syntax
     map { $_ = '..' if $_  eq '-' } @dirs if ON_VMS;

-    my $prefix = File::Spec::Unix->catdir(
-                        grep { length } $vol, @dirs
-                    );
+    my $prefix = File::Spec::Unix->catdir(@dirs);
     return( $prefix, $file );
 }
jonassmedegaard commented 5 years ago

This commit to Archive::Tar: https://github.com/jib/archive-tar-new/commit/a00e030

jonassmedegaard commented 5 years ago

Here's a simple test to catch this bug:

use strict;
use warnings;

use Test::More;
use Test::Command::Simple;

use File::chdir;
use Path::Tiny;

# support overriding to instead test command installed into system
my @command = ($ENV{DISTINKT_DIST}) || ( 'perl', '../../script/distinkt-dist' );

my $sourcedir = path('examples/p5-acme-example-dist');

my $output = <<EOF
Processing cpanfile
Finding provided packages
Finding provided scripts
Distribution seems suitable for static install
Copying /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/cpanfile
Copying /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/dist.ini
Copying /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/t/01basic.t
Copying /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/lib/Acme/Example/Dist.pm
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/INSTALL
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/README
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/LICENSE
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/doap.ttl
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.yml
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/META.json
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/Makefile.PL
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001/MANIFEST
Writing /libdist-inkt-perl-0.024/examples/p5-acme-example-dist/Acme-Example-Dist-0.001.tar.gz
EOF
        ;

{
        local $CWD = $sourcedir;
        run_ok @command;
}
cmp_ok stdout, 'eq', '', 'nothing printed on stdout';
cmp_ok stderr, 'eq', $output, 'file list printed on stderr';
ok $sourcedir->child('Acme-Example-Dist-0.001.tar.gz')->remove,
        'remove created tarball';

done_testing;
jonassmedegaard commented 5 years ago
@@ -296,7 +296,7 @@
        {
                my $abs = path($_);
                $tar->add_files($abs);
-               $tar->rename(substr("$abs", 1), "$pfx/".$abs->relative($root));
+               $tar->rename("$abs", "$pfx/".$abs->relative($root));
        }

        $tar->write($file, Archive::Tar::COMPRESS_GZIP());

This works with Archive::Tar 2.28 and newer.

I will apply that for Debian, and restrict to recent releases of perl.

Upstream is probably needed to support both newer and older Archive::Tar...

jonassmedegaard commented 5 years ago

Here's the test I ended up including with the Debian package (corrected compared to above to not hardcode full paths): https://salsa.debian.org/perl-team/modules/packages/libdist-inkt-perl/blob/master/debian/tests/distinkt-dist.t