Re: Bugs with detection of renames that are also overwrites

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Feb 13, 2010 at 06:46:55PM -0500, Anders Kaseorg wrote:

> One can create a rename that also overwrites an existing file, for example 
> with ‘git mv -f’:
> 
> $ git init
> $ seq 100 200 > a; seq 300 400 > b; git add a b
> $ git commit -m foo; git tag foo
> $ git mv -f a b
> $ git commit -m bar; git tag bar
> 
> Git does not ordinarily detect this as a rename, even with -M.

Right. Git looks at only a subset of the files when looking for renames.
For straight renames, the set of possible sources is the list of deleted
files, and the possible destinations are the new files.

Finding copies ("-C") is similar, except that we also consider files
that were modified but not deleted. And --find-copies-harder will look
at _all_ files as sources, not just modified ones.

But what you are asking for is to expand the possible destination list
to include files that were modified but are not new. I don't think there
is currently a way to do that explicitly.

As you discovered, though, if either the source or destination file has
changed significantly, we should break them apart using "-B":

> But it can (sometimes*) be forced to detect the rename with -M -B.  
> 
> $ git diff --stat -M -B foo bar
>  a => b |    0
>  1 files changed, 0 insertions(+), 0 deletions(-)

In which case the rename engine sees the deletion and addition
separately (they just happen to have the same path name), and therefore
the path gets added to the source and destination lists.

> However, the resulting patch incorrectly omits the deletion of the 
> overwritten file!
> 
> $ git diff -M -B foo bar
> diff --git a/a b/b
> similarity index 100%
> rename from a
> rename to b
>
> Therefore, the patch can’t be applied to its own source tree.
> 
> $ git checkout foo
> $ git diff -M -B foo bar | git apply
> error: b: already exists in working directory

Hmm. I think this is a conflict between what the user wants to see and
what apply wants to see. From the user's perspective, thinking about the
diff representing a change, "b" was not really deleted. It was simply
overwritten. But from apply's perspective, the diff is a set of
instructions, and those instructions do not mention that "b" previously
existed and was overwritten. So it is right to be cautious and declare a
conflict.

I'm not sure just throwing a "delete" line in there would be the best
thing, though. The instructions for apply really need to be "if 'b' has
this sha1, then it is safe to delete and rename a into place. Otherwise
it is a conflict". And I'm not sure how we would represent that (I guess
with a full diff and an "index" line).

> Also, a question: is it possible to get ‘git merge’ to recognize this 
> rename?

No, I don't think there is a way currently. You would need to patch git
to set opts.break_opt in merge-recursive.c:get_renames, I think.

> (* I say “sometimes” because -B only detects the rewrite if it deems the 
> renamed file to be sufficiently different than the overwritten file.  If 
> you use 190 and 390 instead of 200 and 400 above, the rewrite and rename 
> will not be detected, even though the rename would be detected if it was 
> not an overwrite.  This also feels like a bug.)

I think you are getting into a philosophical discussion of what is a
rename, here. If "a" and "b" are very similar, "a" is removed, and "b"
has the same (or similar) content as "a", was it a rename from "a", or
was it simply that "b" was changed, possibly to incorporate the
duplicated items in "a"?

So I don't think it is a bug. But that isn't to say you can't come up
with a case where it would be nice, during a diff or a merge, to show
things that way. I mentioned at the beginning of the message that what
you wanted was to expand the list of destination possibilities. You
could have a "--find-overwrites" which puts all of the modified files
into the destination list. You would not want it on by default, though,
I think, as it would add a lot of computation time to find a somewhat
rare case.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]