Re: [PATCH 2/2] archive: avoid spawning `gzip`

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Apr 26, 2019, 14:54 UTC
Message-ID: <nycvar.QRO.7.76.6.1904261051310.45@tvgsbejvaqbjf.bet>
In-Reply-To: <20190413221646.GL12419@genre.crustytoothpaste.net>

Hi brian,

On Sat, 13 Apr 2019, brian m. carlson wrote:

Show 22 quoted lines

> On Fri, Apr 12, 2019 at 09:51:02PM -0400, Jeff King wrote:
> > I wondered how you were going to kick this in, since users can define
> > arbitrary filters. I think it's kind of neat to automagically convert
> > "gzip -cn" (which also happens to be the default). But I think we should
> > mention that in the Documentation, in case somebody tries to use a
> > custom version of gzip and wonders why it isn't kicking in.
> >
> > Likewise, it might make sense in the tests to put a poison gzip in the
> > $PATH so that we can be sure we're using our internal code, and not just
> > calling out to gzip (on platforms that have it, of course).
> >
> > The alternative is that we could use a special token like ":zlib" or
> > something to indicate that the internal implementation should be used
> > (and then tweak the baked-in default, too). That might be less
> > surprising for users, but most people would still get the benefit since
> > they'd be using the default config.
>
> I agree that a special value (or NULL, if that's possible) would be
> nicer here. That way, if someone does specify a custom gzip, we honor
> it, and it serves to document the code better. For example, if someone
> symlinked pigz to gzip and used "gzip -cn", then they might not get the
> parallelization benefits they expected.

I went with `:zlib`. The `NULL` value would not really work, as there is no way to specify that via `archive.tgz.command`.

About the symlinked thing: I do not really want to care to support such hacks. If you want a different compressor than the default (which can change), you should specify it specifically.

> I'm fine overall with the idea of bringing the compression into the
> binary using zlib, provided that we preserve the "-n" behavior
> (producing reproducible archives).

Thanks for voicing this concern. I had a look at zlib's source code, and it looks like it requires an extra function call (that we don't call) to make the resulting file non-reproducible. In other words, it has the opposite default behavior from `gzip`.

Ciao, Dscho