Thread (1 message) 1 message, 1 author, 2025-11-12

Re: [PATCH v4 2/2] fetch: fix non-conflicting tags not being committed

From: Junio C Hamano <hidden>
Date: 2025-11-12 16:34:08

Karthik Nayak [off-list ref] writes:
Patrick Steinhardt [off-list ref] writes:
quoted
On Tue, Nov 11, 2025 at 02:27:08PM +0100, Karthik Nayak wrote:
quoted
The cleanup section is reached with `retcode` set in several scenarios:

   - `truncate_fetch_head()` and `open_fetch_head()` both set `retcode`
     before the transaction is created, so no commit is attempted.

   - `prune_refs()` sets `retcode` after creating the transaction, so
     the commit will now proceed. Before batched updates, `prune_refs()`
     created its own transaction internally with all-or-nothing
     semantics. This was done since all deletions were made without an
     old OID, which meant they were assumed to never fail. This change
     allows partial deletions to succeed, consistent with how other
     reference updates behave during fetch.
Okay, so we do have a change in behaviour for `prune_refs()`. I guess
the reasoning is sound, but I was wondering why we don't have a test for
this.

I guess the reason is that, as you said, it should in theory always
succeed. But what if with the "files" backend one of the refs that we're
about to prune was locked? Would that be a case where we continue with
pruning the remaining refs now?
I was thinking of concurrent writes to lock the reference, and didn't
think of a nice way to do this. Your solution works and is indeed better.

I started writing the test and realized that the pruning happens before
we create the batched updates transaction. So I was _wrong_ and there is
no change for `prune_refs()` either, as the transaction is never defined
at this stage. Will amend and send in a new version.
Thanks, both.  I too was wondering what was going on in that part of
the flow.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help