Re: [PATCH] Make --follow support --find-copies-harder.

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 21, 2010, 09:24 UTC
Message-ID: <7vpr1tb1kk.fsf@alter.siamese.dyndns.org>
In-Reply-To: <i2r41f08ee11004210202r642aa25dy32b06c33ed98ba4c@mail.gmail.com>

Bo Yang <struggleyb.nku@gmail.com> writes:

Show 7 quoted lines

> I am really wondering, when -R is used, how the file rename/copy
> should defined? Now, I can make -R works with --follow, and it produce
> something like:
>
> byang@byang-laptop:~/git/git$ ./git diff --follow --name-status maint
> master -- builtin/log.c
> R089    builtin-log.c   builtin/log.c

As I already said, it is a bug that "diff" does not diagnose it an error when you give it the "--follow" option. It was designed to be used with "log" family, and never with "diff" family.

When a command from the "log" family traverses the history, it internally runs "diff-tree" between the commit C it is currently looking at, and its parents C^$n. When you give one path and --follow [*1*], it may notice that C has the named path and C^$n doesn't.

At that point, it internally runs "diff -M C^$n C" to see if there is a corresponding path in C^$n, and switch to follow the path it found in the parent commit.

The logic only detects the case where "new" side has a path that "old" side doesn't [*3*], and it is not even designed to be used with -R (where it needs to be given a path that does not exist anymore on the "new" side but used to exist in the "old" side).

Heck, it is not even designed to be used with "diff" as I already said twice ;-).

Even in the context of "log", it is a hack. It globally keeps one single path that it follows, which obviously would not work in a history with merges.

[Footnote]

*1* "log" command line parser enforces this "only one path" condition; "diff" doesn't even bother catching it as an error to give "--follow", so it lacks the logic to further catch it as an error to give more than one paths.

*2* No, I don't think there is an interface to tweak the -M to -C or -C -C; see tree-diff.c and look for try_to_follow_renames(). I think it is probably Ok to make this tweakable, and I suspect that is what your patch is about, but don't use "git diff" as an example nor in any of your tests.

*3* This is exactly why "diff --follow maint master -- builtin/log.c" appears to do something remotely sensible (notice that the path is what exists in the "new" side) but "maint master -- builtin-log.c" does not. The logic doesn't even care if the named path does not appear in the "new" side at all, because that is not useful at all in the way "log" internally uses "diff-tree" logic.