Re: [PATCH] import-tars: use Archive::Tar instead of unpack()
- From
- Michael Loeffler <zvpunry@zvpunry.de>
- Date
- Feb 14, 2007, 16:03 UTC
- Message-ID
- <1171468992.629.68.camel@ibook.zvpunry.de>
- In-Reply-To
- <20070212172848.GC29621@spearce.org>
Am Montag, den 12.02.2007, 12:28 -0500 schrieb Shawn O. Pearce: ...
> bzip2 and compress are popular formats applied to tars.
yes, and this should be supported. There is an simple example in the Archive::Tar manpage.
So something like "$tar = new Archive::Tar(*I)" is possible to use the original filehandle but I removed this code before reading the whole manpage.
Show 11 quoted lines
> > > @@ -10,6 +10,10 @@ > > ## > > > > use strict; > > +use Archive::Tar; > > +use Archive::Tar::File; > > +use Archive::Tar::Constant; > > + > > I did not apply this hunk. Not everyone has Archive::Tar installed.
And Archive::Tar is a little bit slower then your unpack() and it reads the whole file into memory which is not so good.
> But then again, not everyone will use this example program either.
I have used it to play a bit with this nice fast-import stuff.
> I'm debating it. Archive::Tar's parser will certainly be much > more robust than the one I hand-crafted. It might also let us deal > with symlinks. ;)
Maybe, but I haven't tested it with some bigger tar files.
Show 5 quoted lines
> I would considering applying something like this if it would also > support at least bz2. This is an example program meant to teach > people how to use fast-import, and maybe also to help someone who > wants to quickly import one or more .tar.gz for use with git-grep. > Requiring Archive::Tar here is not the end of Git as we know it. :)
I think we should stay with your unpack() version, it works, is faster and doesn't read the whole file into memory.
In the Archive::Tar manpage they use gunzip and uncompress instead of zcat or gzcat, so what do you think about the following patch?
diff --git a/contrib/fast-import/import-tars.perl b/contrib/fast-import/import-tars.perl index 990c9e7..5585a8b 100755 --- a/contrib/fast-import/import-tars.perl +++ b/contrib/fast-import/import-tars.perl @@ -25,11 +25,14 @@ foreach my $tar_file (@ARGV) my $tar_name = $1; if ($tar_name =~ s/\.(tar\.gz|tgz)$//) { - open(I, '-|', 'gzcat', $tar_file) or die "Unable to gzcat $tar_file: $!\n"; + open(I, '-|', 'gunzip', '-c', $tar_file) + or die "Unable to gunzip -c $tar_file: $!\n"; } elsif ($tar_name =~ s/\.(tar\.bz2|tbz2)$//) { - open(I, '-|', 'bzcat', $tar_file) or die "Unable to bzcat $tar_file: $!\n"; + open(I, '-|', 'bunzip2', '-c', $tar_file) + or die "Unable to bunzip2 -c $tar_file: $!\n"; } elsif ($tar_name =~ s/\.tar\.Z$//) { - open(I, '-|', 'zcat', $tar_file) or die "Unable to zcat $tar_file: $!\n"; + open(I, '-|', 'uncompress', '-c', $tar_file) + or die "Unable to uncompress -c $tar_file: $!\n"; } elsif ($tar_name =~ s/\.tar$//) { open(I, $tar_file) or die "Unable to open $tar_file: $!\n"; } else { ... the $git_mode patch ... > > This hunk is completely unrelated to the Archive::Tar rewrite. Yes, I had this change first in another patch but i forgot to mail it to the list. ... > I've applied this hunk (and only this hunk) to my fastimport tree > and pushed it out. Good, I no longer think this Archive::Tar stuff is such a good idea.