Re: [PATCH 1/2] fast-export: deletion action first

From: Jeff King <peff@peff.net>
Date: Apr 25, 2017, 04:46 UTC
Message-ID: <20170425044641.sx5uoql4oiug6iq7@sigill.intra.peff.net>
In-Reply-To: <xmqqfugxw1us.fsf@gitster.mtv.corp.google.com>

On Mon, Apr 24, 2017 at 09:24:59PM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> > So we sort deletions first. And the bit that the context doesn't quite
> > show here is that we then compare renames and push them to the end.
> > Everything else will compare equal.
> 
> Wait--we also allow renames?  Rename is like delete in the context
> of discussing d/f conflicts, in that it tells us that the source
> path will be missing in the end result.  If you rename a file "d" to
> "e", then there is a room for you to create a directory "d" to store
> a file "d/f" in.  Shouldn't it participate also in this "delete
> before add to avoid d/f conflict" logic?

Hrm. Yeah, I agree that case is problematic. But putting the renames early creates the opposite problem. If you delete "d/f" to make way for a rename to a file "d", then that deletion has to come first.

So naively you might think that pure deletions come first, then renames. But I think you could have dependencies within the renames. For instance:

  git init
  mkdir a b c
  seq 1 1000 >a/f
  seq 1001 2000 >b/f
  seq 2001 3000 >c/f
  git add .
  git commit -m base

  git mv a tmp
  git mv b/f a; rmdir b
  git mv c/f b; rmdir c
  git mv tmp/f c; rmdir tmp

There's no correct order there; it's a cycle.

So I suspect that any reader that accepts renames needs to be able to handle the inputs in any order (I'd also suspect that many implementations _don't_, but get by because people don't do silly things like this in practice).

Anyway. I don't think Miguel's patch needs to solve all of the lingering rename cases. But I am curious whether it makes some rename cases worse, because the depth-sorting was kicking in before and making them work.

-Peff