[PATCH] log: use true parents for diff even when rewriting
- From
Thomas Rast <trast@inf.ethz.ch>
- Date
- Jul 22, 2013, 21:22 UTC
- Message-ID
- <a598aec3e3c90de4d2c08e58ee0a4828edc80ac2.1374527806.git.trast@inf.ethz.ch>
- In-Reply-To
- <20130722090854.GA22222@pengutronix.de>
When using pathspec filtering in combination with diff-based log output, parent simplification happens before the diff is computed. The diff is therefore against the *simplified* parents.
This works okay, arguably by accident, in the normal case: the pruned commits did not affect the paths being filtered, so the diff against the prune-result is the same as against the diff against the true parents.
However, --full-diff breaks this guarantee, and indeed gives pretty spectacular results when comparing the output of
git log --graph --stat ... git log --graph --full-diff --stat ...
(--graph internally kicks in parent simplification, much like --parents).
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Signed-off-by: Thomas Rast <trast@inf.ethz.ch> ---
Perhaps like this. It's getting a bit late, so I'm not sure if I'm missing another user of the "true" parent list, but it does fix the issue you reported.
combine-diff.c | 3 ++- commit.c | 16 ++++++++++++++++ commit.h | 3 +++ log-tree.c | 2 +- revision.c | 30 +++++++++++++++++++++++++++++- revision.h | 15 +++++++++++++++ t/t6012-rev-list-simplify.sh | 6 ++++++ 7 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c index 6dc0609..abd2397 100644 --- 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(commit); struct sha1_array parents = SHA1_ARRAY_INIT; while (parent) { diff --git a/commit.c b/commit.c index e5862f6..5ecdb38 100644 --- 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 35cc4e2..ca766d2 100644 --- 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 a49d8e8..b1d5747 100644 --- 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(commit); if (!parents) { if (opt->show_root_diff) { diff_root_tree_sha1(sha1, "", &opt->diffopt); diff --git a/revision.c b/revision.c index 84ccc05..3fdd0ae 100644 --- 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,7 @@ 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)) { + save_parents(commit); if (rewrite_parents(revs, commit, rewrite_one) < 0) return commit_error; } @@ -3069,3 +3071,29 @@ 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 *); +struct saved_parents saved_parents_slab; +static int saved_parents_initialized; + +void save_parents(struct commit *commit) +{ + struct commit_list **pp; + + if (!saved_parents_initialized) { + init_saved_parents(&saved_parents_slab); + saved_parents_initialized = 1; + } + + pp = saved_parents_at(&saved_parents_slab, commit); + assert(*pp == NULL); + *pp = copy_commit_list(commit->parents); +} + +struct commit_list *get_saved_parents(struct commit *commit) +{ + if (!saved_parents_initialized) + return commit->parents; + + return *saved_parents_at(&saved_parents_slab, commit); +} diff --git a/revision.h b/revision.h index 95859ba..0717518 100644 --- a/revision.h +++ b/revision.h @@ -273,4 +273,19 @@ enum rewrite_result { 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_original_parents() will transparently return commit->parents if + * history simplification is off. + */ +extern void save_parents(struct commit *commit); +extern struct commit_list *get_original_parents(struct commit *commit); + #endif diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh index 57ce239..fde5e71 100755 --- 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
-- 1.8.3.3.1180.gbf33142