On 26/11/03 01:26PM, Patrick Steinhardt wrote:
On Tue, Nov 03, 2026 at 01:05:29PM +0100, Kristofer Karlsson wrote:
quoted
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