Re: [PATCH 3/4] builtin/remote: rework how remote refs get renamed

From: Patrick Steinhardt <ps@pks.im>
Date: Jul 29, 2025, 08:43 UTC
Message-ID: <aIiJpUC5cn9oeMv9@pks.im>
In-Reply-To: <xmqqjz3suosc.fsf@gitster.g>

On Mon, Jul 28, 2025 at 11:57:07AM -0700, Junio C Hamano wrote:

Show 25 quoted lines

> Justin Tobler <jltobler@gmail.com> writes:
> 
> >> +	 * This split is required to avoid conflicting ref updates when a
> >> +	 * remote is being nested into itself or converted into its parent
> >> +	 * directory.
> >> +	 *
> >> +	 * Unfortunately this means that the operation isn't atomic. But we
> >> +	 * cannot avoid that, unless transactions learn to handle such
> >> +	 * conflicts one day.
> >>  	 */
> >
> > We could detect if the rename operation would result in a D/F conflict
> > upfront and special case it by using two transactions. If we know there
> > isn't a D/F conflict, I think a single transaction would be sufficient.
> 
> The right solution should be at the implementation of the
> transactions, not the application that uses the transaction
> mechanism, no?  So I would think the above workaround is actually
> counter-productive.
> 
> > That being said, it might be best to keep it simple for now and leave it
> > as-is.
> 
> Yes, we do not have to update the transaction layer to fix that D/F
> thing in this same series.

Ideally, yes, and with the "reftable" backend this is a trivial addition. But as ever so often, the problem is with the "files" backend: if we have "refs/heads/parent" and create "refs/heads/parent/child" we cannot create the lockfile for the latter ref.

There probably are ways to solve this, but we would have to add new locking semantics to the "files" backend to achieve it. And I am afraid that such new semantics might break existing implementations of the "files" backend that are outside of Git. So I'm not really sure whether this is even a possible route to go down, unfortunately.

So with that I don't think Justin's proposal is unreasonable. Some options:

  - We can check whether the old and new remote names are prefixes of
    one another. If so, we create two transactions, otherwise we do an
    atomic commit.

  - We can outright refuse to do a nesting/unnesting rename and just
    always use a single transaction where there cannot be any conflicts.

The latter is of course a backwards incompatible change. It feels quite tempting though, as hierarchical remotes are just so esoteric. And then renaming such hierarchical remotes with nesting/unnesting semantics is even more unlikely to ever happen.

A slightly less intrusive way could be to opportunistically rename refs in a single transaction, regardless of whether refs are nested or not. And if we do notice any conflict we abort the whole command and give the user guidance to please do `git remote rename parent/child unrelated && git remote rename unrelated parent`. Which is overall a bit of a stupid limitation, but maybe it's good enough given that it is so supremely unlikely that anyone will ever hit the issue?

Patrick