Merge branch 'tr/log-full-diff-keep-true-parents' into maint

Output from "git log --full-diff -- <pathspec>" looked strange,
because comparison was done with the previous ancestor that touched
the specified <pathspec>, causing the patches for paths outside the
pathspec to show more than the single commit has changed.

* tr/log-full-diff-keep-true-parents:
  log: use true parents for diff when walking reflogs
  log: use true parents for diff even when rewriting

diff --git a/combine-diff.c b/combine-diff.c
index 88525b37cf461ee922b81fd6848be5efc8e34542..4fc16ad4f35c558daf5bf96865801828d10b4b9b 100644 (file)
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -10,6 +10,7 @@
 #include "refs.h"
 #include "userdiff.h"
 #include "sha1-array.h"
+#include "revision.h"
 
 static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr, int n, int num_parent)
 {
@@ -1383,7 +1384,7 @@ void diff_tree_combined(const unsigned char *sha1,
 void diff_tree_combined_merge(const struct commit *commit, int dense,
                              struct rev_info *rev)
 {
-       struct commit_list *parent = commit->parents;
+       struct commit_list *parent = get_saved_parents(rev, commit);
        struct sha1_array parents = SHA1_ARRAY_INIT;
 
        while (parent) {

diff --git a/commit.c b/commit.c
index a575564a15c1c340b36d10c90da8fa343692264d..de16a3c0a2d6693173678247ba0755e4d7483a1c 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -377,6 +377,22 @@ unsigned commit_list_count(const struct commit_list *l)
        return c;
 }
 
+struct commit_list *copy_commit_list(struct commit_list *list)
+{
+       struct commit_list *head = NULL;
+       struct commit_list **pp = &head;
+       while (list) {
+               struct commit_list *new;
+               new = xmalloc(sizeof(struct commit_list));
+               new->item = list->item;
+               new->next = NULL;
+               *pp = new;
+               pp = &new->next;
+               list = list->next;
+       }
+       return head;
+}
+
 void free_commit_list(struct commit_list *list)
 {
        while (list) {

diff --git a/commit.h b/commit.h
index d912a9d4ac3fe36b2e69d48b1725f01bdf289b26..f9504f70cc5b57de590099024556b9d2b6b2cdce 100644 (file)
--- a/commit.h
+++ b/commit.h
@@ -62,6 +62,9 @@ struct commit_list *commit_list_insert_by_date(struct commit *item,
                                    struct commit_list **list);
 void commit_list_sort_by_date(struct commit_list **list);
 
+/* Shallow copy of the input list */
+struct commit_list *copy_commit_list(struct commit_list *list);
+
 void free_commit_list(struct commit_list *list);
 
 /* Commit formats */

diff --git a/log-tree.c b/log-tree.c
index a49d8e895d3ad24f00e8504ec3eccfbddead6b4c..8534d91826f3cf05f810a28728ee8bd51649d819 100644 (file)
--- a/log-tree.c
+++ b/log-tree.c
@@ -738,7 +738,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
        sha1 = commit->tree->object.sha1;
 
        /* Root commit? */
-       parents = commit->parents;
+       parents = get_saved_parents(opt, commit);
        if (!parents) {
                if (opt->show_root_diff) {
                        diff_root_tree_sha1(sha1, "", &opt->diffopt);

diff --git a/revision.c b/revision.c
index 84ccc0529b75508be5d1603a9e73790ca2b102fe..ac20d1aaed7b5e1a52682dfd196f87e2f94e87c7 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -15,6 +15,7 @@
 #include "string-list.h"
 #include "line-log.h"
 #include "mailmap.h"
+#include "commit-slab.h"
 
 volatile show_early_output_fn_t show_early_output;
 
@@ -2763,7 +2764,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
        return retval;
 }
 
-static inline int want_ancestry(struct rev_info *revs)
+static inline int want_ancestry(const struct rev_info *revs)
 {
        return (revs->rewrite_parents || revs->children.name);
 }
@@ -2820,6 +2821,14 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
        if (action == commit_show &&
            !revs->show_all &&
            revs->prune && revs->dense && want_ancestry(revs)) {
+               /*
+                * --full-diff on simplified parents is no good: it
+                * will show spurious changes from the commits that
+                * were elided.  So we save the parents on the side
+                * when --full-diff is in effect.
+                */
+               if (revs->full_diff)
+                       save_parents(revs, commit);
                if (rewrite_parents(revs, commit, rewrite_one) < 0)
                        return commit_error;
        }
@@ -2839,6 +2848,7 @@ static struct commit *get_revision_1(struct rev_info *revs)
                free(entry);
 
                if (revs->reflog_info) {
+                       save_parents(revs, commit);
                        fake_reflog_parent(revs->reflog_info, commit);
                        commit->object.flags &= ~(ADDED | SEEN | SHOWN);
                }
@@ -3038,6 +3048,8 @@ struct commit *get_revision(struct rev_info *revs)
        c = get_revision_internal(revs);
        if (c && revs->graph)
                graph_update(revs->graph, c);
+       if (!c)
+               free_saved_parents(revs);
        return c;
 }
 
@@ -3069,3 +3081,54 @@ void put_revision_mark(const struct rev_info *revs, const struct commit *commit)
        fputs(mark, stdout);
        putchar(' ');
 }
+
+define_commit_slab(saved_parents, struct commit_list *);
+
+#define EMPTY_PARENT_LIST ((struct commit_list *)-1)
+
+void save_parents(struct rev_info *revs, struct commit *commit)
+{
+       struct commit_list **pp;
+
+       if (!revs->saved_parents_slab) {
+               revs->saved_parents_slab = xmalloc(sizeof(struct saved_parents));
+               init_saved_parents(revs->saved_parents_slab);
+       }
+
+       pp = saved_parents_at(revs->saved_parents_slab, commit);
+
+       /*
+        * When walking with reflogs, we may visit the same commit
+        * several times: once for each appearance in the reflog.
+        *
+        * In this case, save_parents() will be called multiple times.
+        * We want to keep only the first set of parents.  We need to
+        * store a sentinel value for an empty (i.e., NULL) parent
+        * list to distinguish it from a not-yet-saved list, however.
+        */
+       if (*pp)
+               return;
+       if (commit->parents)
+               *pp = copy_commit_list(commit->parents);
+       else
+               *pp = EMPTY_PARENT_LIST;
+}
+
+struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit)
+{
+       struct commit_list *parents;
+
+       if (!revs->saved_parents_slab)
+               return commit->parents;
+
+       parents = *saved_parents_at(revs->saved_parents_slab, commit);
+       if (parents == EMPTY_PARENT_LIST)
+               return NULL;
+       return parents;
+}
+
+void free_saved_parents(struct rev_info *revs)
+{
+       if (revs->saved_parents_slab)
+               clear_saved_parents(revs->saved_parents_slab);
+}

diff --git a/revision.h b/revision.h
index 95859ba119033ba17346c701493df1474eec5df2..e7f1d211bf0a203978a3024bcbae5c98f25c60cf 100644 (file)
--- a/revision.h
+++ b/revision.h
@@ -25,6 +25,7 @@
 struct rev_info;
 struct log_info;
 struct string_list;
+struct saved_parents;
 
 struct rev_cmdline_info {
        unsigned int nr;
@@ -187,6 +188,9 @@ struct rev_info {
 
        /* line level range that we are chasing */
        struct decoration line_log_data;
+
+       /* copies of the parent lists, for --full-diff display */
+       struct saved_parents *saved_parents_slab;
 };
 
 #define REV_TREE_SAME          0
@@ -273,4 +277,20 @@ typedef enum rewrite_result (*rewrite_parent_fn_t)(struct rev_info *revs, struct
 
 extern int rewrite_parents(struct rev_info *revs, struct commit *commit,
        rewrite_parent_fn_t rewrite_parent);
+
+/*
+ * Save a copy of the parent list, and return the saved copy.  This is
+ * used by the log machinery to retrieve the original parents when
+ * commit->parents has been modified by history simpification.
+ *
+ * You may only call save_parents() once per commit (this is checked
+ * for non-root commits).
+ *
+ * get_saved_parents() will transparently return commit->parents if
+ * history simplification is off.
+ */
+extern void save_parents(struct rev_info *revs, struct commit *commit);
+extern struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
+extern void free_saved_parents(struct rev_info *revs);
+
 #endif

diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index 9a105fe21f0da2851fc0f2aae4ff10e2417c9842..6f47c0dd0ec9b5e59205ca98eea9bd729605e004 100755 (executable)
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -144,4 +144,26 @@ test_expect_success 'empty reflog file' '
        test_cmp expect actual
 '
 
+# This guards against the alternative of showing the diffs vs. the
+# reflog ancestor.  The reflog used is designed to list the commits
+# more than once, so as to exercise the corresponding logic.
+test_expect_success 'git log -g -p shows diffs vs. parents' '
+       test_commit two &&
+       git branch flipflop &&
+       git update-ref refs/heads/flipflop -m flip1 HEAD^ &&
+       git update-ref refs/heads/flipflop -m flop1 HEAD &&
+       git update-ref refs/heads/flipflop -m flip2 HEAD^ &&
+       git log -g -p flipflop >reflog &&
+       grep -v ^Reflog reflog >actual &&
+       git log -1 -p HEAD^ >log.one &&
+       git log -1 -p HEAD >log.two &&
+       (
+               cat log.one; echo
+               cat log.two; echo
+               cat log.one; echo
+               cat log.two
+       ) >expect &&
+       test_cmp expect actual
+'
+
 test_done

diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index 57ce2395d6738617797d0ba270874abd55a3b899..fde5e712eb512791c893de7b393fe36e382a3f6d 100755 (executable)
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -127,4 +127,10 @@ test_expect_success 'full history simplification without parent' '
        }
 '
 
+test_expect_success '--full-diff is not affected by --parents' '
+       git log -p --pretty="%H" --full-diff -- file >expected &&
+       git log -p --pretty="%H" --full-diff --parents -- file >actual &&
+       test_cmp expected actual
+'
+
 test_done