Re: correct git merge behavior or corner case?

From: Jeff King <peff@peff.net>
Date: Apr 21, 2009, 19:11 UTC
Message-ID: <20090421191126.GA7632@coredump.intra.peff.net>
In-Reply-To: <20090421204701.1e0115c0@gmail.com>

On Tue, Apr 21, 2009 at 08:47:01PM +0200, Michał Kiedrowicz wrote:

Show 5 quoted lines

> > I thought the point was about _empty_ renames. This is a small but
> > non-zero rename.
> 
> Yes, you are right. But change these echos to touch and you'll see that
> this bug happens if date and LICENSE are the same, not necessary empty.

Right, because it is not exactly a bug, as I explained here:

  http://article.gmane.org/gmane.comp.version-control.git/117078

I say "not exactly a bug", because it is working as intended by the software authors, but because the intended behavior is to make a heuristic guess (some content moved from one filename to another, so we guess that a patch against the original content should actually go against the new location), the guess may not follow the user's expectations.

It is easy to see how the heuristic can be fooled in a toy example. But what we really care about is whether and how the heuristic is fooled in the real world. In the real world, there seems to be some non-trivial probability of removing an empty file, adding a new one elsewhere, and then merging with somebody who touched the empty file. So it may be worth improving the heuristic for this special case, especially because the harm done in a false negative is relatively small.

But what is the probability of doing the same thing to a file that has non-trivial contents? I would guess it is much less likely, and by special-casing it as a conflict, you have a much higher chance of bothering users who were relying on actual rename detection for their non-trivial case[1]. Of course, I don't have actual numbers, so I'm just guessing.

So my point is that while both are perhaps a failing of the heuristic, only one is going to be worth tweaking the heuristic for. So that is the one that should be included in the test case, since it is how somebody implementing a proposed tweak can test their tweak.

-Peff

[1] On a side note, this got me thinking about how git handles rename detection during merging. One of the things I like about git is that it tries to be very stupid: if something is questionable during a merge, it calls attention to it and makes it _easy_ for the user to access the versions and resolve (and I love mergetool for this). But renames are not like this: either they happen during auto-conflict-resolution, or they don't. I wonder if it might be a better strategy to barf on conflicts due removed files that could be resolved by questionable renames, but have a post-merge "renametool" that shows the user potential renames and lets them interactively specify the resolution. But maybe that would just be annoying, since 99% of the time, the rename detection gets it right.