Subject: Re: [PATCH v4 05/25] midx: clear auxiliary .rev after replacing the MIDX

From: Junio C Hamano <[email-masked]>
Date: Sun, 29 Aug 2021 15:56:31 -0700

Taylor Blau <[email-masked]> writes:

> On Thu, Aug 26, 2021 at 11:01:26PM -0700, Junio C Hamano wrote:
>> It seems that the *.rev test (probably added by the other topic that
>> is a single patch fix) fails under sha256 hash.  I am not going to
>> dig it any further myself, but for the interested, CI breakage is
>> here:
>>
>>   https://github.com/git/git/runs/3440068613?check_suite_focus=true#step:5:1219
>>
>> Thanks.
>
> I saw the same error myself when integrating that patch into my series.
> I discussed it more in [1], but the failure is basically caused by the
> midx code using the_hash_algo even when operating in a different
> repository via --object-dir.
>
> If the_hash_algo doesn't match (as is the case when using `--object-dir`
> to point at a SHA-256 repository when invoking the builtin from a
> repository using SHA-1 or outside of a repository altogether), then
> we'll fail when trying to open the pack indexes.

My recollection is that "--object-dir" is mostly about the alternate
odb usecase---am I correct?  It is unfortunate that we didn't start
with "alternate repository" and said "we only care about the objects
in the object store they have, and we do not have to care what refs
they point into their object database or what configuration they
have" instead.

I wonder if it is safe to assume that in practice a directory given
to the "--object-dir" option is always the "objects" subdirectory in
a repository, and it is an error if there is no "config" file next
to the directory.  Then, we could check ../config relative to the
given directory and error out if they use different hash.

I do not recall offhand how careful link_alt_odb_entries() is, but I
suspect it isn't at all (back when I invented it, there weren't need
for configuration to switch between hashes, and since then I do not
recall seeing any heavy update to the alternate odb code).  Perhaps
we should tighten it so that we check the accompanying "config" file
first and ignore the entry with incompatible "hash" (and we may
later discover other trait on a repository that is incompatible with
the current one)?

Thanks.