diff --follow: do call diffcore_std() as necessary

gitster · gitster · commit 44c48a909ae3 · 2010-08-13T12:17:45.000-07:00
Usually, diff frontends populate the output queue with filepairs without any rename information and call diffcore_std() to sort the renames out. When --follow is in effect, however, diff-tree family of frontend has a hack that looks like this: diff-tree frontend -> diff_tree_sha1() . populate diff_queued_diff . if --follow is in effect and there is only one change that creates the target path, then -> try_to_follow_renames() -> diff_tree_sha1() with no pathspec but with -C -> diffcore_std() to find renames . if rename is found, tweak diff_queued_diff and put a single filepair that records the found rename there -> diffcore_std() . tweak elements on diff_queued_diff by - rename detection - path ordering - pickaxe filtering We need to skip parts of the second call to diffcore_std() that is related to rename detection, and do so only when try_to_follow_renames() did find a rename. Earlier 1da6175 (Make diffcore_std only can run once before a diff_flush, 2010-05-06) tried to deal with this issue incorrectly; it unconditionally disabled any second call to diffcore_std(). This hopefully fixes the breakage. Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/diff.c b/diff.c
@@ -4064,33 +4064,32 @@ 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)
-		diffcore_break(options->break_opt);
-	if (options->detect_rename)
-		diffcore_rename(options);
-	if (options->break_opt != -1)
-		diffcore_merge_broken();
+	if (!options->found_follow) {
+		/* See try_to_follow_renames() in tree-diff.c */
+		if (options->break_opt != -1)
+			diffcore_break(options->break_opt);
+		if (options->detect_rename)
+			diffcore_rename(options);
+		if (options->break_opt != -1)
+			diffcore_merge_broken();
+	}
 	if (options->pickaxe)
 		diffcore_pickaxe(options->pickaxe, options->pickaxe_opts);
 	if (options->orderfile)
 		diffcore_order(options->orderfile);
-	diff_resolve_rename_copy();
+	if (!options->found_follow)
+		/* See try_to_follow_renames() in tree-diff.c */
+		diff_resolve_rename_copy();
 	diffcore_apply_filter(options->filter);
 
 	if (diff_queued_diff.nr && !DIFF_OPT_TST(options, DIFF_FROM_CONTENTS))
 		DIFF_OPT_SET(options, HAS_CHANGES);
 	else
 		DIFF_OPT_CLR(options, HAS_CHANGES);
 
-	diff_queued_diff.run = 1;
+	options->found_follow = 0;
 }
 
 int diff_result_code(struct diff_options *opt, int status)
diff --git a/diff.h b/diff.h
@@ -126,6 +126,9 @@ struct diff_options {
 	/* this is set by diffcore for DIFF_FORMAT_PATCH */
 	int found_changes;
 
+	/* to support internal diff recursion by --follow hack*/
+	int found_follow;
+
 	FILE *file;
 	int close_file;
 
diff --git a/diffcore.h b/diffcore.h
@@ -91,13 +91,11 @@ 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;
diff --git a/tree-diff.c b/tree-diff.c
@@ -359,6 +359,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	diff_tree_release_paths(&diff_opts);
 
 	/* Go through the new set of filepairing, and see if we find a more interesting one */
+	opt->found_follow = 0;
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 
@@ -376,6 +377,16 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 			diff_tree_release_paths(opt);
 			opt->paths[0] = xstrdup(p->one->path);
 			diff_tree_setup_paths(opt->paths, opt);
+
+			/*
+			 * The caller expects us to return a set of vanilla
+			 * filepairs to let a later call to diffcore_std()
+			 * it makes to sort the renames out (among other
+			 * things), but we already have found renames
+			 * ourselves; signal diffcore_std() not to muck with
+			 * rename information.
+			 */
+			opt->found_follow = 1;
 			break;
 		}
 	}
