threads / patch / 23562

v2Make git log --follow find copies among unmodified files.

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

## tl;dr

2 messages between Apr 22, 2010 and Apr 22, 2010. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Bo Yang· Apr 22, 2010, 03:50 UTC · lore

'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

Show changes to 4 files +65 −1

Documentation/git-log.txt, diff.c, t/t4205-log-follow-harder-copies.sh, tree-diff.c

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

Junio C Hamano· Apr 22, 2010, 06:26 UTC · re: Bo Yang · lore

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

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

Show 9 quoted lines

> @@ -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

Show 10 quoted lines

> 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_).

Show changes to tree-diff.c +1 −0

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