From: Kristofer Karlsson <krka@spotify.com>
Date: Wed, 04 Nov 2026 10:11:21 GMT
Subject: Re: [BUG] Lazy fetch in partial clone fails with "not our ref" since 5946a564 (odb/source: make read_object_info() pluggable)
Message-ID: <CAL71e4MqMdGk5ryhNxvVNN6Z02+45B933ZfUuvj4hM92N5a6hA@mail.gmail.com>
In-Reply-To: <yrjh1p92qc7w.fsf@pohly-mobl1.fritz.box>

On Tue, 3 Nov 2026 at 14:31, Patrick Ohly <patrick.ohly@intel.com> wrote:
>
> Hello,
>
> "git merge" in a blobless partial clone fails with "not our ref" while
> lazily fetching a missing object. This started to occur today in the
> Kubernetes CI, completely breaking the merging of PRs. We should have
> been using git 2.54.0, the affected version, for a few days at least
> without issues, so it can't be ruled out that this is at least partly
> caused by a change on the GitHub side. We are about to revert to git
> 2.52.0 to unblock the Kubernetes project.
>
> [...]
>
> I bisected v2.52.0 (good) to v2.54.0 (bad). The first bad commit is:
>
> 5946a564cddc0bf471f27ae4c3fe205441e3ef65
> odb/source: make `read_object_info()` function pluggable
>
> The parent, 3bc3177ad7 ( 2.53.0.185.g3bc3177ad7 ), is good.

Thanks for the detailed report.  I think I have found the cause: I
can reproduce the failure with your steps and make it go away with a
small change to the loose object cache.

The problem is in the semantics of the loose object cache, which
answers quick existence checks.  It is a two-layer cache: a bitmap
keyed by the first byte of the object ID, recording which
"objects/XX/" subdirectories have been listed, and an oidtree with
the objects found there.  The intended invariant is that once a
subdirectory has been listed, the oidtree holds every loose object
in it, so that "not in the oidtree" means "does not exist".

Writing a loose object does not update the cache, which breaks the
invariant if the subdirectory had already been listed.  In
practice, that means a quick lookup of any object in the same
subdirectory earlier in the same process.

In your example, the object the server refuses, 27b64cc8, is not in
either parent nor in the merge base.  It is the content-merged blob
that the merge itself just wrote as a loose object.  merge-ort's
prefetch had already listed "27/", so the checkout of the merge
result thinks the blob is missing and asks the server for an object
that only exists locally.  The server rejects the whole batch, and
we then die on the first object in it that really is missing,
3c804b88, which is why the two OIDs in the error differ.

The bug is older than 5946a564cd (odb/source: make
`read_object_info()` function pluggable), but that commit, which
matches the bisect result, is what exposed it.  Before it, any
non-quick lookup that missed called odb_reprepare(), and that also
threw away the whole loose cache.  A merge does such lookups between
writing the merged blob and checking out the result, so in practice
the stale cache was discarded before anything could use it.  Since
that commit, the retry only reprepares the packfile store, and the
stale cache survives until the checkout.

I think the fix is to maintain the invariant: after a successful
loose object write, if the subdirectory has already been listed,
insert the new OID into the oidtree.  (Clearing the subdirectory's
bit would also work, at the cost of listing it again.)

I have a local fix with a unit test for the loose source.  An
end-to-end test is awkward, since it needs a merged blob whose first
byte matches one of the prefetched objects.

I could polish this into a proper patch, but since Patrick
Steinhardt has been reworking this part of the code, I think his
thoughts on the approach would be useful first.

Thanks,
Kristofer