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.