[PATCH v2] Make git log --follow find copies among unmodified files.

[PATCH v2] Make git log --follow find copies among unmodified files.

[Bo Yang]

'git log --follow <path>' don't track copies from unmodified
files, and this patch fix it.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
---
 Documentation/git-log.txt           |    5 ++-
 diff.c                              |    3 ++
 t/t4205-log-follow-harder-copies.sh |   56 +++++++++++++++++++++++++++++++++++
 tree-diff.c                         |    2 +
 4 files changed, 65 insertions(+), 1 deletions(-)
 create mode 100755 t/t4205-log-follow-harder-copies.sh

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index fb184ba..682a85a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -56,7 +56,10 @@ include::diff-options.txt[]
 	commits, and doesn't limit diff for those commits.
 
 --follow::
-	Continue listing the history of a file beyond renames.
+	Continue listing the history of a file beyond renames/copies.
+	With this, all files in a commit will be searched for
+	renames/copies, and it is equal to specify '--follow' with
+	'--follow -M -C -C'.
 
 --log-size::
 	Before the log message print out its size in bytes. Intended
diff --git a/diff.c b/diff.c
index d0ecbc3..6982f79 100644
--- a/diff.c
+++ b/diff.c
@@ -2594,6 +2594,9 @@ int diff_setup_done(struct diff_options *options)
 	else
 		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
 
+	if (DIFF_OPT_TST(options, FOLLOW_RENAMES))
+		DIFF_OPT_SET(options, FIND_COPIES_HARDER);
+
 	if (DIFF_OPT_TST(options, FIND_COPIES_HARDER))
 		options->detect_rename = DIFF_DETECT_COPY;
 
diff --git a/t/t4205-log-follow-harder-copies.sh b/t/t4205-log-follow-harder-copies.sh
new file mode 100755
index 0000000..ad29e65
--- /dev/null
+++ b/t/t4205-log-follow-harder-copies.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+#
+# Copyright (c) 2010 Bo Yang
+#
+
+test_description='Test --follow should always find copies hard in git log.
+
+'
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/diff-lib.sh
+
+echo >path0 'Line 1
+Line 2
+Line 3
+'
+
+test_expect_success \
+    'add a file path0 and commit.' \
+    'git add path0 &&
+     git commit -m "Add path0"'
+
+echo >path0 'New line 1
+New line 2
+New line 3
+'
+test_expect_success \
+    'Change path0.' \
+    'git add path0 &&
+     git commit -m "Change path0"'
+
+cat <path0 >path1
+test_expect_success \
+    'copy path0 to path1.' \
+    'git add path1 &&
+     git commit -m "Copy path1 from path0"'
+
+test_expect_success \
+    'find the copy path0 -> path1 harder' \
+    'git log --follow --name-status --pretty="format:%s"  path1 > current'
+
+cat >expected <<\EOF
+Copy path1 from path0
+C100	path0	path1
+
+Change path0
+M	path0
+
+Add path0
+A	path0
+EOF
+
+test_expect_success \
+    'validate the output.' \
+    'compare_diff_patch current expected'
+
+test_done
diff --git a/tree-diff.c b/tree-diff.c
index fe9f52c..0dea53e 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -346,6 +346,8 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 
 	diff_setup(&diff_opts);
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	if (DIFF_OPT_TST(opt, FIND_COPIES_HARDER))
+		DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->paths[0];
-- 
1.7.0.2.273.gc2413.dirty

Re: [PATCH v2] Make git log --follow find copies among unmodified files.

[Junio C Hamano]

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

> @@ -56,7 +56,10 @@ include::diff-options.txt[]
>  	commits, and doesn't limit diff for those commits.
>  
>  --follow::
> -	Continue listing the history of a file beyond renames.
> +	Continue listing the history of a file beyond renames/copies.
> +	With this, all files in a commit will be searched for
> +	renames/copies, and it is equal to specify '--follow' with
> +	'--follow -M -C -C'.

Hmm.

Addition of "/copies" is fine, but I do not think the last three lines are
good.  The -M/-C options are about how the changes introduced by the found
commits are shown, and because the pathspec limitation applies _before_
all the diffcore transformations (see "Gaah. Why?" message from Linus in
the thread I pointed at you in another message), and because the --follow
option is defined to work _only_ when you give one single path (it is not
even a "pathspec" in the usual sense; see 750f7b6 (Finally implement "git
log --follow", 2007-06-19)), they are inherently mutually incompatible.

Cf. http://thread.gmane.org/gmane.comp.version-control.git/50512/focus=50513

> diff --git a/diff.c b/diff.c
> index d0ecbc3..6982f79 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2594,6 +2594,9 @@ int diff_setup_done(struct diff_options *options)
>  	else
>  		DIFF_OPT_CLR(options, DIFF_FROM_CONTENTS);
>  
> +	if (DIFF_OPT_TST(options, FOLLOW_RENAMES))
> +		DIFF_OPT_SET(options, FIND_COPIES_HARDER);

I don't think this is correct.  You are touching the diff options on the
"how do we show the found change" side here.

Because the user is fixated on one single path F, it is enough to say "F
is in the child but did not exist in the parent---it appears to have come
from E in the parent" without saying if it is copy or rename.  I think it
is Ok if the change is to pay the cost of full tree scanning for only
commits that try_to_follow_renames() deals with, but this instead makes it
in effect for _all_ the commits, not just the ones that created the path
that has been followed so far, which is unacceptable.

Why isn't it just a one-liner from your previous patch but done without
the conditional?  I think that would indeed be a fix.  With "--follow",
the user expressee that it is Ok to pay extra cost in order to _follow_
that single path, and we haven't been doing as thorough a job as we could.
With that single liner, you are fixing it to pay more attention to the
paths in the parent.

But that is _not_ about how the found commits are _shown_ (it is about how
the commits are _found_).

diff --git a/tree-diff.c b/tree-diff.c
index fe9f52c..0dea53e 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -346,6 +346,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 
 	diff_setup(&diff_opts);
 	DIFF_OPT_SET(&diff_opts, RECURSIVE);
+	DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->paths[0];
-- 
1.7.0.2.273.gc2413.dirty