From: Thomas Rast <trast@inf.ethz.ch>
To: <git@vger.kernel.org>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Junio C Hamano" <gitster@pobox.com>
Subject: [PATCH] log: use true parents for diff even when rewriting
Date: Mon, 22 Jul 2013 23:22:01 +0200 [thread overview]
Message-ID: <a598aec3e3c90de4d2c08e58ee0a4828edc80ac2.1374527806.git.trast@inf.ethz.ch> (raw)
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
next prev parent reply other threads:[~2013-07-22 21:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-22 9:08 git log anomalities Uwe Kleine-König
2013-07-22 10:40 ` Thomas Rast
2013-07-22 21:22 ` Thomas Rast [this message]
2013-07-22 21:48 ` [PATCH] log: use true parents for diff even when rewriting Junio C Hamano
2013-07-23 7:27 ` Thomas Rast
2013-07-23 7:49 ` Uwe Kleine-König
2013-07-23 19:55 ` Junio C Hamano
2013-07-31 20:13 ` [PATCH v2] " Thomas Rast
2013-07-31 22:55 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a598aec3e3c90de4d2c08e58ee0a4828edc80ac2.1374527806.git.trast@inf.ethz.ch \
--to=trast@inf.ethz.ch \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).