Re: [PATCH 0/9] git archive: use gzip again by default, document output stabilty

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 3, 2023, 13:49 UTC
Message-ID: <230203.86fsbmbzwp.gmgdl@evledraar.gmail.com>
In-Reply-To: <771a98ca-9540-ad4e-dfba-9d304e1dff09@dunelm.org.uk>

On Thu, Feb 02 2023, Phillip Wood wrote:

Show 15 quoted lines

> On 02/02/2023 09:32, Ævar Arnfjörð Bjarmason wrote:
>> As reported in
>> https://lore.kernel.org/git/a812a664-67ea-c0ba-599f-cb79e2d96694@gmail.com/
>> changing the default "tgz" output method of from "gzip(1)" to our
>> internal "git archive gzip" (using zlib ) broke things for users in
>> the wild that assume that the "git archive" output is stable, most
>> notably GitHub: https://github.com/orgs/community/discussions/45830
>>
>> Leaving aside the larger question of whether we're going to promise
>> output stability for "git archive" in general, the motivation for that
>> change was to have a working compression method on systems that lacked
>> a gzip(1).
>
> As I recall the reduction in cpu time used to create a compressed
> archive was a factor in making it the default.

I read those references in 76d7602631a (archive-tar: add internal gzip implementation, 2022-06-15) more of a "it's not [much] slower", the flip to the default in 4f4be00d302 (archive-tar: use internal gzip by default, 2022-06-15) didn't discuss it.

So I didn't think it was important enough to mention (even though we're now back to the faster "gzip" method).

Show 9 quoted lines

>> As the disruption of changing the default isn't worth it, let's use
>> gzip(1) again by default, and only fall back on the new "git archive
>> gzip" if it isn't available.
>
> Playing devil's advocate for a moment as we're not going to promise
> that the compressed output of "git archive" will be stable in the
> future perhaps we should use this breakage as an opportunity to
> highlight that to users and to advertize the config setting that
> allows them to use gzip for compressing archives.

If we were trying to intentionally break things for those users we could do a lot better than "git archive gzip", whose output is mostly the same as "gzip", we could tweak one of the headers to make it different all the time.

But I think it's better to advocate for such intentional chaos-monkeying as a follow-up to this more conservative "oops, we broke stuff, it's easy not to break it, so let's not do it'.

> Reverting the change gives the misleading impression that we're making
> a commitment to keeping the output stable.

I don't see how you can conclude that from this series. It explicitly states that we make no such promises, what it does is go back to allowing the gzip(1) command to make its own promises.

> The focus of this thread seems to be the
> problems relating to github which they have already addressed.

Which they've addressed by reverting the change, but while they're a major user of git they're not the only one. They just happened to use "git archive".

I think it would be a mistake to conclude that everyone who's run into this has already done so, or is aware of it.

> I think there is general agreement that it is not practical to promise
> that the compressed output of "git archive" is stable so maybe it is
> better[...]

...better than what? This seems to imply that this series is making new promises about the output stability, which it isn't doing.

> [...]to make that clear now while users can work around it in the
> short term with a config setting rather than waiting until we're faced
> with some security or other issue that forces a change to the output
> which users cannot work around so easily.

I think it's always been clear that you can use that setting. For ages we've been saying:

	The `tar.gz` and `tgz` formats are defined automatically and use the
	command `gzip -cn` by default.

Then v2.38.0 changed it to:

	[...]
        magic command `git archive gzip` by default

Which IMO was easily missed among other "Performance, Internal Implementation, Development Support etc." items in the release notes, which said:

   Teach "git archive" to (optionally and then by default) avoid
   spawning an external "gzip" process when creating ".tar.gz" (and
   ".tgz") archives.

But I agree that all of this is subjective. To me a 2% reduction in CPU use (at the cost of ~20% increse in wallclock) & some unclear benefits to teaching users that they can't rely on our "gzip" output seems unclear or hypothetical.

Whereas the widespread breakage reported is very real, and we should consider GitHub as a canary for that, not the the stand & end of its potential impact.

As we didn't have a strong reason to change this in the first place (and as my series shows, we can have our cake & eat it too if we don't have a "gzip") I think the obvious choice is to go back to using "gzip".