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

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
index bf65892f78f84a31b6bf87b0f82cfc5938485862..93004922dee5b4f148dc8e6be5be469f1ffe75e6 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -4064,25 +4064,24 @@ 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))
@@ -4090,7 +4089,7 @@ void diffcore_std(struct diff_options *options)
        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
index 063d10ac2216071ef218fab2ad51c13787614acf..6fff024e3ad9462c717595cd90d7fcca1da6410c 100644 (file)
--- 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
index 05ebc115a13df5c1b80128489619d753ed107a6f..8b3241ad137f5934e32336cd1caf8d99ca11d1f5 100644 (file)
--- 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
index 5b68c0864cac22d083228d5966487913df5c6be2..cd659c6fe447b6fcdedee2843113feb358f23849 100644 (file)
--- 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;
                }
        }