[PATCH 0/3 v4] Make git log --follow find copies among unmodified files.

[PATCH 0/3 v4] Make git log --follow find copies among unmodified files.

[Bo Yang]

I have tried to make --follow to support finding copies among unmodified files. And the first patch is to fix a bug introduced by '--follow' and 'git log' combination.
We use the code:

    else if (--p->one->rename_used > 0)
        p->status = DIFF_STATUS_COPIED;

to detect copies and renames. So, if diffcore_std run more than one time, p->one->rename_used will be reduced to a 'R' from 'C'. And this patch will fix this by allowing diffcore_std can only run once before a diff_flush, which seems rationale for our code.

Bo Yang (3):
  Add a macro DIFF_QUEUE_CLEAR.
  Make diffcore_std only can run once before a diff_flush
  Make git log --follow find copies among unmodified files.

 Documentation/git-log.txt           |    2 +-
 diff.c                              |   21 ++++++++-----
 diffcore-break.c                    |    6 +--
 diffcore-pickaxe.c                  |    3 +-
 diffcore-rename.c                   |    3 +-
 diffcore.h                          |    7 ++++
 t/t4205-log-follow-harder-copies.sh |   56 +++++++++++++++++++++++++++++++++++
 tree-diff.c                         |    2 +-
 8 files changed, 82 insertions(+), 18 deletions(-)
 create mode 100755 t/t4205-log-follow-harder-copies.sh

[PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.

[Bo Yang]

Refactor the diff_queue_struct code, this macro help
to reset the structure.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
---
 diff.c             |   13 +++++--------
 diffcore-break.c   |    6 ++----
 diffcore-pickaxe.c |    3 +--
 diffcore-rename.c  |    3 +--
 diffcore.h         |    5 +++++
 5 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/diff.c b/diff.c
index e40c127..4a350e3 100644
--- a/diff.c
+++ b/diff.c
@@ -2540,6 +2540,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 void diff_setup(struct diff_options *options)
 {
 	memset(options, 0, sizeof(*options));
+	memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));
 
 	options->file = stdout;
 
@@ -3457,8 +3458,7 @@ int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1)
 		diff_free_filepair(q->queue[i]);
 
 	free(q->queue);
-	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	DIFF_QUEUE_CLEAR(q);
 
 	return result;
 }
@@ -3586,8 +3586,7 @@ void diff_flush(struct diff_options *options)
 		diff_free_filepair(q->queue[i]);
 free_queue:
 	free(q->queue);
-	q->queue = NULL;
-	q->nr = q->alloc = 0;
+	DIFF_QUEUE_CLEAR(q);
 	if (options->close_file)
 		fclose(options->file);
 
@@ -3609,8 +3608,7 @@ static void diffcore_apply_filter(const char *filter)
 	int i;
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_queue_struct outq;
-	outq.queue = NULL;
-	outq.nr = outq.alloc = 0;
+	DIFF_QUEUE_CLEAR(&outq);
 
 	if (!filter)
 		return;
@@ -3678,8 +3676,7 @@ static void diffcore_skip_stat_unmatch(struct diff_options *diffopt)
 	int i;
 	struct diff_queue_struct *q = &diff_queued_diff;
 	struct diff_queue_struct outq;
-	outq.queue = NULL;
-	outq.nr = outq.alloc = 0;
+	DIFF_QUEUE_CLEAR(&outq);
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
diff --git a/diffcore-break.c b/diffcore-break.c
index 3a7b60a..44f8678 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -162,8 +162,7 @@ void diffcore_break(int break_score)
 	if (!merge_score)
 		merge_score = DEFAULT_MERGE_SCORE;
 
-	outq.nr = outq.alloc = 0;
-	outq.queue = NULL;
+	DIFF_QUEUE_CLEAR(&outq);
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
@@ -256,8 +255,7 @@ void diffcore_merge_broken(void)
 	struct diff_queue_struct outq;
 	int i, j;
 
-	outq.nr = outq.alloc = 0;
-	outq.queue = NULL;
+	DIFF_QUEUE_CLEAR(&outq);
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index d0ef839..929de15 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -55,8 +55,7 @@ void diffcore_pickaxe(const char *needle, int opts)
 	int i, has_changes;
 	regex_t regex, *regexp = NULL;
 	struct diff_queue_struct outq;
-	outq.queue = NULL;
-	outq.nr = outq.alloc = 0;
+	DIFF_QUEUE_CLEAR(&outq);
 
 	if (opts & DIFF_PICKAXE_REGEX) {
 		int err;
diff --git a/diffcore-rename.c b/diffcore-rename.c
index d6fd3ca..df41be5 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -569,8 +569,7 @@ void diffcore_rename(struct diff_options *options)
 	/* At this point, we have found some renames and copies and they
 	 * are recorded in rename_dst.  The original list is still in *q.
 	 */
-	outq.queue = NULL;
-	outq.nr = outq.alloc = 0;
+	DIFF_QUEUE_CLEAR(&outq);
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		struct diff_filepair *pair_to_free = NULL;
diff --git a/diffcore.h b/diffcore.h
index fcd00bf..5d05dea 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -92,6 +92,11 @@ struct diff_queue_struct {
 	int alloc;
 	int nr;
 };
+#define DIFF_QUEUE_CLEAR(q) \
+	do { \
+		(q)->queue = NULL; \
+		(q)->nr = (q)->alloc = 0; \
+	} while(0);
 
 extern struct diff_queue_struct diff_queued_diff;
 extern struct diff_filepair *diff_queue(struct diff_queue_struct *,
-- 
1.6.0.4

[PATCH 2/3 v4] Make diffcore_std only can run once before a diff_flush

[Bo Yang]

When file renames/copies detection is turned on, the
second diffcore_std will degrade a 'C' pair to a 'R' pair.

And this may happen when we run 'git log --follow' with
hard copies finding. That is, the try_to_follow_renames()
will run diffcore_std to find the copies, and then
'git log' will issue another diffcore_std, which will reduce
'src->rename_used' and recognize this copy as a rename.
This is not what we want.

So, I think we really don't need to run diffcore_std more
than one time.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
---
 diff.c     |    8 ++++++++
 diffcore.h |    2 ++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index 4a350e3..f0985bc 100644
--- a/diff.c
+++ b/diff.c
@@ -3737,6 +3737,12 @@ void diffcore_fix_diff_index(struct diff_options *options)
 
 void diffcore_std(struct diff_options *options)
 {
+	/* We never run this function more than one time, because the
+	 * rename/copy detection logic can only run once.
+	 */
+	if (diff_queued_diff.run)
+		return;
+
 	if (options->skip_stat_unmatch)
 		diffcore_skip_stat_unmatch(options);
 	if (options->break_opt != -1)
@@ -3756,6 +3762,8 @@ void diffcore_std(struct diff_options *options)
 		DIFF_OPT_SET(options, HAS_CHANGES);
 	else
 		DIFF_OPT_CLR(options, HAS_CHANGES);
+
+	diff_queued_diff.run = 1;
 }
 
 int diff_result_code(struct diff_options *opt, int status)
diff --git a/diffcore.h b/diffcore.h
index 5d05dea..491bea0 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -91,11 +91,13 @@ struct diff_queue_struct {
 	struct diff_filepair **queue;
 	int alloc;
 	int nr;
+	int run;
 };
 #define DIFF_QUEUE_CLEAR(q) \
 	do { \
 		(q)->queue = NULL; \
 		(q)->nr = (q)->alloc = 0; \
+		(q)->run = 0; \
 	} while(0);
 
 extern struct diff_queue_struct diff_queued_diff;
-- 
1.6.0.4

[PATCH 3/3 v4] 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           |    2 +-
 t/t4205-log-follow-harder-copies.sh |   56 +++++++++++++++++++++++++++++++++++
 tree-diff.c                         |    2 +-
 3 files changed, 58 insertions(+), 2 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..0727818 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -56,7 +56,7 @@ 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.
 
 --log-size::
 	Before the log message print out its size in bytes. Intended
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..1fb3e94 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -346,7 +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_opts.detect_rename = DIFF_DETECT_RENAME;
+	DIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->paths[0];
 	diff_opts.break_opt = opt->break_opt;
-- 
1.6.0.4

Re: [PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.

[Sven Verdoolaege]

On Thu, May 06, 2010 at 09:52:27PM -0700, Bo Yang wrote:
> Refactor the diff_queue_struct code, this macro help
> to reset the structure.
> 
[..]
> 
> diff --git a/diff.c b/diff.c
> index e40c127..4a350e3 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2540,6 +2540,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
>  void diff_setup(struct diff_options *options)
>  {
>  	memset(options, 0, sizeof(*options));
> +	memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));
>  

What's this line for?  It doesn't seem to be explained by the commit
message and it breaks "git diff-files -p --submodule".
Without this line, I get the following output in one of my projects:

    Submodule barvinok contains untracked content
    Submodule barvinok contains modified content
    Submodule barvinok e129555..833e4a6:
      > iscc: use simplified CLooG interface
    Submodule cloog contains untracked content
    Submodule cloog f083938..4684a24:
      > partial doc
      > cloog_names_read_strings: do not generate names if they cannot be read
      > cloog_program_read: separate reading from input from construction of CloogProgram
    Submodule cloog-polylib contains untracked content
    Submodule cloog-polylib contains modified content
    Submodule isl contains untracked content
    Submodule isl 892fb27..5292e00:
      > isl_transitive_closure.c: anonymize input map during incremental computation
      > isl_transitive_closure.c: keep track of domains for Floyd-Warshall
      > isl_dim_drop: always remove tuple name, even if number of dims to drop is zero
      > isl_dim_set_tuple_name: allow explicit removal of tuple name
    Submodule isl-polylib contains untracked content
    Submodule isl-polylib 531cb00..e9e2edf:
      > stop using isl_basic_map internals
    Submodule polylib contains untracked content

With the line, I only get

    Submodule barvinok contains untracked content
    Submodule barvinok contains modified content
    Submodule barvinok e129555..833e4a6:
      > iscc: use simplified CLooG interface

skimo

Re: [PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.

[Junio C Hamano]

Sven Verdoolaege <skimo@kotnet.org> writes:

> On Thu, May 06, 2010 at 09:52:27PM -0700, Bo Yang wrote:
>> Refactor the diff_queue_struct code, this macro help
>> to reset the structure.
>> 
> [..]
>> 
>> diff --git a/diff.c b/diff.c
>> index e40c127..4a350e3 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -2540,6 +2540,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
>>  void diff_setup(struct diff_options *options)
>>  {
>>  	memset(options, 0, sizeof(*options));
>> +	memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));
>>  
>
> What's this line for?

I don't think this change is warranted.  The macro was supposed to reduce
the repetition of assignment to q->queue, q->nr and q->alloc, and nothing
else.

Also the commit messages in this series are unreadable---I should have
been a bit more careful.

Sorry and thanks for noticing.

Re: [PATCH 1/3 v4] Add a macro DIFF_QUEUE_CLEAR.

[Bo Yang]

Hi Sven,
On Mon, Aug 2, 2010 at 8:47 PM, Sven Verdoolaege <skimo@kotnet.org> wrote:
> On Thu, May 06, 2010 at 09:52:27PM -0700, Bo Yang wrote:
>> Refactor the diff_queue_struct code, this macro help
>> to reset the structure.
>>
> [..]
>>
>> diff --git a/diff.c b/diff.c
>> index e40c127..4a350e3 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -2540,6 +2540,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
>>  void diff_setup(struct diff_options *options)
>>  {
>>       memset(options, 0, sizeof(*options));
>> +     memset(&diff_queued_diff, 0, sizeof(diff_queued_diff));
>>

Sorry about the broken code and the bad commit message...

This line is used to clear the global queue structure and make it
usable in next round of diff. I am wondering how the submodule part
use the diff API to cause such an issue. :)

-- 
Regards!
Bo
----------------------------
My blog: http://blog.morebits.org
Why Git: http://www.whygitisbetterthanx.com/