From: Karthik Nayak Date: Wed, 12 Nov 2025 08:52:47 GMT Subject: Re: [PATCH v4 2/2] fetch: fix non-conflicting tags not being committed Message-ID: In-Reply-To: Patrick Steinhardt writes: > On Tue, Nov 11, 2025 at 02:27:08PM +0100, Karthik Nayak wrote: >> 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! > > Patrick