On Thu, Feb 02 2023, Phillip Wood wrote:

On 02/02/2023 09:32, Ævar Arnfjörð Bjarmason wrote:

quoted

As reported in
https://lore.kernel.org/git/a812a664-67ea-c0ba-599f-cb79e2d96694@gmail.com/ (local)
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).

quoted

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".