Re: [PATCH v2] odb: do not list our own refs as alternate refs in the quarantine

From: Justin Tobler <jltobler@gmail.com>
Date: Nov 3, 2026, 21:53 UTC
Message-ID: <aupPATAGBWH1RIe6@denethor>
In-Reply-To: <aunS6dE0B-JAJnBk@pks.im>

On 26/11/03 01:26PM, Patrick Steinhardt wrote:

Show 14 quoted lines

> On Tue, Nov 03, 2026 at 01:05:29PM +0100, Kristofer Karlsson wrote:
> > That does sound like the cleaner fix.  Since you are already deep into
> > this area, I would be happy to drop the fix and wait for that instead.
> > I expect very few users to be affected in practice: mostly git hosts
> > with many hidden refs, which is the case bcec6780b2 was about.
> > 
> > In that case I could send just the tests, with the hidden refs case
> > marked as test_expect_failure, so that it is covered once a long-term
> > fix lands.  Or, if you would rather have a short-term fix in the
> > meantime, I can send a v3 with the change above.
> 
> I certainly do care about this case and would like to see this fixed :)
> I'd like to get Justin's opinion on that proposal first though, as he's
> worked on transactions quite extensively over the last couple months.

Regarding the fix proposed in this patch, I'm not quite sure this is really the right layer to address the problem. The root of the problem is that we are swapping around the "real" and quarantine object directories to facilitate writing to the "files" ODB transaction. IMO, this is already rather fragile. So I do agree with Patrick that I think the better long-term fix is to stop modifying the primary object directory altogether and instead propagate some context about the pending transaction via environment variables to the child process that it can use to reconstruct the proper environment. This is certainly a more involved change, but something that I've been wanting to do for a while now regardless.

If we are interested in a more simple fix for the short-term, we could adapt `odb_transaction_env()` to have a "read-only" mode flag that does not modify the primary object directory and only appends the quarantine directory as an alternate. For the connectivity check in git-receive-pack(1) we don't write any object anyways, so configuring the environment in this manner should be sufficient to fix the issue.

-Justin