Subject: Re: [PATCH 0/4] Check .gitmodules when using packfile URIs

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 18 Feb 2021 12:31:40 +0100

On Thu, Jan 28 2021, Jonathan Tan wrote:

>> Jonathan Tan <[email-masked]> writes:
>> 
>> > As part of this, index-pack has to output (1) the hash that goes into
>> > the name of the .pack/.idx file and (2) the hashes of all dangling
>> > .gitmodules. I just had (2) come after (1). If anyone has a better idea,
>> > I'm interested.
>> 
>> I have this feeling that the "blobs that need to be validated across
>> packs" will *not* be the last enhancement we'd need to make to the
>> output from index-pack to allow richer communication between it and
>> its invoker.  While there is no reason to change how the first line
>> of the output looks like, we'd probably want to make sure that the
>> future versions of Git can easily tell "list of blobs that require
>> further validation" from other additional information.
>> 
>> I am not comfortable to recommend "ok, then let's add a delimiter
>> line '---\n' if/when we need to have something after the list of
>> blobs and append more stuff in future versions of Git", because we
>> may find need to emit new kinds of info before the list of blobs
>> that needs further validation, for example, in future versions of
>> Git.
>> 
>> Having said all that, the internal communication between the
>> index-pack and its caller do not need as much care about
>> compatibility across versions as output visible to end-users, so
>> when a future version of Git needs to send different kinds of
>> information in different order from what you created here, we can do
>> so pretty much freely, I would guess.
>
> Yeah, that's what I thought too - since this is an internal interface,
> we can evolve them in lockstep. If we're really worried about the Git
> binaries (on a user's system) getting out of sync, 

I'm thinking in reading "getting out of sync" that you may be missing an
aspect of the issue here.

We're not talking about some abnormal error in some packaging system,
but how we'd expect all installations of git to behave if you update
them with *.rpm, *.deb etc, e.g. when your binaries are in
/usr/libexec/git-core. I suppose NixOS or something where there's
hash-based paths may be exempt from this.

On those systems if you've got a server serving concurrent traffic and
update the "git" package you could expect failure if any git process
invoked by another is incompatible during such an upgrade.

If you browse some of the recent GIT_CONFIG_PARAMETERS discussion this
was discussed there. I.e. even if GIT_CONFIG_PARAMETERS is internal-only
we bent over backwards not to change it in such a way as to have process
A invoking process B and the two not understanding each other because of
such an upgrade.

That's exactly because of this case, where receive-pack may be started
on version A, someone runs "apt install git" in the background
concurrently, and now a version A of that program is talking to a
version B index-pack.

> we could just make sure that subsequent updates to this protocol are
> non-backwards-compatible (e.g. have index-pack emit "foo <hash>",
> where "foo" is a string that describes the new check, so that current
> fetch-pack will reject "foo" since it is not a hash).

And then presumably index-pack would die and receive-pack would die on
the push or whatever, so the push fails for the end user.