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

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jul 28, 2025 at 11:57:07AM -0700, Junio C Hamano wrote:
> 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