On Sat, Aug 02, 2025 at 06:45:33AM -0400, Jeff King wrote: > On Thu, Jul 31, 2025 at 04:56:53PM +0200, Patrick Steinhardt wrote: > > > There is one issue though with using atomic transactions: when nesting a > > remote into itself it can happen that renamed references conflict with > > the old referencse. For example, when we have a reference > > "refs/remotes/origin/foo" and we rename "origin" to "origin/foo", then > > we'll end up with an F/D conflict when we try to create the renamed > > reference "refs/remotes/origin/foo/foo". > > I think that was true even in the old code. E.g., if I do: > > git init server > git -C server commit --allow-empty -m foo > git -C server branch a > git -C server branch b > > git init > git remote add foo/b server > git fetch > git remote rename foo/b foo > > then I get (before your patches): > > error: 'refs/remotes/foo/b/main' exists; cannot create 'refs/remotes/foo/b' > fatal: renaming 'refs/remotes/foo/b/b' failed The test case I have added for this conflicting case only fails with the postimage of this series. So we hit some _new_ edge cases now that didn't exist before, but I think that's acceptable. > Worse, we moved "a" but not "b" (nor "main"/"master", which are > important because they are what's blocking the rename of "b"). So we are > left with a broken half-moved state. Yes, this half-applied state is quite awful. > After your patches we get a nicer hint message, and of course we retain > the unbroken state from prior to the rename. So IMHO it is strictly > better. Patrick
