Re: [PATCH 2/2] archive: support gzipped tar files

[Jeff King <peff@github.com>]

On Tue, Jun 14, 2011 at 09:39:55PM +0200, René Scharfe wrote:

> > However, when running git-archive against a remote site,
> > having the remote side do the compression can save
> > considerable bandwidth. Service providers could always wrap
> > git-archive to provide that functionality, but this makes it
> > much simpler.
> 
> That's a good point and one that was overlooked when this topic came up
> earlier (see http://kerneltrap.org/mailarchive/git/2009/9/10/11507 and
> http://kerneltrap.org/mailarchive/git/2009/9/11/11577).

Hmph, I should have done my homework better. I totally missed that
thread.

Yeah, I am unsurprised that doing it in a single process is actually
slower. I do think because of the remote issue that we should provide
something like this. But we could implement it by piping to an external
gzip. That would make us just slightly less portable, but would give us
the multi-processor speedup, or even allow using something like pigz.

> > +static void output_gz(const char *buf, unsigned long len)
> > +{
> > +	if (!gzwrite(gz_file, buf, len))
> > +		die("unable to write compressed stream: %s",
> > +		    gzerror(gz_file, NULL));
> > +}
> 
> Does this do the right things when faced with interrupted writes or
> truncated pipes? I ask because the earlier attempt had a
> gzwrite_or_die() which did that, but I don't know anymore if that is
> strictly needed.

No, I blindly assumed that gzwrite was a little bit smart, but looking
at the zlib code, it really is just propagating whatever it got from
fwrite. I need to handle both errors and short writes myself. So we do
need gzwrite_or_die.

> Oh, and bridging the gap between unsigned long and int
> was certainly another reason for the existence of this function.

Ugh. I correctly saw that it took an unsigned long, but it actually
returns the number of bytes written as an int! Nice interface.

All of this can go away, though, if we switch to an external process.
It's tempting.

-Peff