Re: [PATCH 3/2] Enhance --early-output format
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:43:47
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH 3/2] Enhance --early-output format · Linus Torvalds <torvalds@linux-foundation.org>
On Mon, 5 Nov 2007, Linus Torvalds wrote:
That said, I've often found the "TREECHANGE" bit annoying. The fact that we always have to test it together with testing "rev->prune_fn" and often also the "rev->dense" flag is just annoying. I'd almost like to just always set the bit if "rev->prune_fn" isn't set. Alternatively, the code could be rewritten to just have a few inline helper functions, and it could perhaps be written as if (!(flags & TREECHANGE) && revs->dense && single_parent(commit)) continue; I dunno.
Here's a possible cleanup patch. It's on top of the enhanced
--early-output format commit, and in fact fixes a stupid bug in that
commit ("return -1" vs "return NULL"), but that bug-fix is really an
independent thing.
It basically removes the unnecessary indirection of "revs->prune_fn",
since that function is always the same one (or NULL), and there is in fact
not even an abstraction reason to make it a function (ie its not called
from some other file and doesn't allow us to keep the function itself
static or anything like that).
It then just replaces it with a bit that says "prune or not", and if not
pruning, every commit gets TREECHANGE.
That in turn means that
- if (!revs->prune_fn || (flags & TREECHANGE))
- if (revs->prune_fn && !(flags & TREECHANGE))
just become
- if (flags & TREECHANGE)
- if (!(flags & TREECHANGE))
respectively.
Together with adding the "single_parent()" helper function, the "complex"
conditional now becomes
if (!(flags & TREECHANGE) && rev->dense && single_parent(commit))
continue;
Anyway, do with this as you will. I think it's ok, but apart from passing
the test-suite and looking obvious, this has gotten zero testing.
Linus
---
builtin-log.c | 6 ++----
builtin-rev-list.c | 14 +++++++-------
commit.h | 5 +++++
revision.c | 20 ++++++++++++--------
revision.h | 5 +----
5 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 981f388..76c84e2 100644
--- a/builtin-log.c
+++ b/builtin-log.c@@ -92,10 +92,8 @@ static int estimate_commit_count(struct rev_info *rev, struct commit_list *list) list = list->next; if (flags & UNINTERESTING) continue; - if (rev->prune_fn && rev->dense && !(flags & TREECHANGE)) { - if (commit->parents && !commit->parents->next) - continue; - } + if (!(flags & TREECHANGE) && rev->dense && single_parent(commit)) + continue; n++; } return n;
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 6970467..2dec887 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c@@ -142,7 +142,7 @@ static int count_distance(struct commit_list *entry) if (commit->object.flags & (UNINTERESTING | COUNTED)) break; - if (!revs.prune_fn || (commit->object.flags & TREECHANGE)) + if (commit->object.flags & TREECHANGE) nr++; commit->object.flags |= COUNTED; p = commit->parents;
@@ -198,7 +198,7 @@ static inline int halfway(struct commit_list *p, int nr) /* * Don't short-cut something we are not going to return! */ - if (revs.prune_fn && !(p->item->object.flags & TREECHANGE)) + if (!(p->item->object.flags & TREECHANGE)) return 0; if (DEBUG_BISECT) return 0;
@@ -268,7 +268,7 @@ static struct commit_list *best_bisection(struct commit_list *list, int nr) int distance; unsigned flags = p->item->object.flags; - if (revs.prune_fn && !(flags & TREECHANGE)) + if (!(flags & TREECHANGE)) continue; distance = weight(p); if (nr - distance < distance)
@@ -308,7 +308,7 @@ static struct commit_list *best_bisection_sorted(struct commit_list *list, int n int distance; unsigned flags = p->item->object.flags; - if (revs.prune_fn && !(flags & TREECHANGE)) + if (!(flags & TREECHANGE)) continue; distance = weight(p); if (nr - distance < distance)
@@ -362,7 +362,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list, p->item->util = &weights[n++]; switch (count_interesting_parents(commit)) { case 0: - if (!revs.prune_fn || (flags & TREECHANGE)) { + if (flags & TREECHANGE) { weight_set(p, 1); counted++; show_list("bisection 2 count one",
@@ -435,7 +435,7 @@ static struct commit_list *do_find_bisection(struct commit_list *list, * add one for p itself if p is to be counted, * otherwise inherit it from q directly. */ - if (!revs.prune_fn || (flags & TREECHANGE)) { + if (flags & TREECHANGE) { weight_set(p, weight(q)+1); counted++; show_list("bisection 2 count one",
@@ -482,7 +482,7 @@ static struct commit_list *find_bisection(struct commit_list *list, continue; p->next = last; last = p; - if (!revs.prune_fn || (flags & TREECHANGE)) + if (flags & TREECHANGE) nr++; on_list++; }
diff --git a/commit.h b/commit.h
index 4ed0c1c..aa67986 100644
--- a/commit.h
+++ b/commit.h@@ -117,4 +117,9 @@ extern int interactive_add(void); extern void add_files_to_cache(int verbose, const char *prefix, const char **files); extern int rerere(void); +static inline int single_parent(struct commit *commit) +{ + return commit->parents && !commit->parents->next; +} + #endif /* COMMIT_H */
diff --git a/revision.c b/revision.c
index 5d6f208..7a1ecba 100644
--- a/revision.c
+++ b/revision.c@@ -308,6 +308,14 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit) struct commit_list **pp, *parent; int tree_changed = 0, tree_same = 0; + /* + * If we don't do pruning, everything is interesting + */ + if (!revs->prune) { + commit->object.flags |= TREECHANGE; + return; + } + if (!commit->tree) return;
@@ -415,8 +423,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit, str * simplify the commit history and find the parent * that has no differences in the path set if one exists. */ - if (revs->prune_fn) - revs->prune_fn(revs, commit); + try_to_simplify_commit(revs, commit); if (revs->no_walk) return 0;
@@ -684,9 +691,6 @@ void init_revisions(struct rev_info *revs, const char *prefix) revs->skip_count = -1; revs->max_count = -1; - revs->prune_fn = NULL; - revs->prune_data = NULL; - revs->commit_format = CMIT_FMT_DEFAULT; diff_setup(&revs->diffopt);
@@ -1271,7 +1275,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch diff_tree_setup_paths(revs->prune_data, &revs->pruning); /* Can't prune commits with rename following: the paths change.. */ if (!revs->diffopt.follow_renames) - revs->prune_fn = try_to_simplify_commit; + revs->prune = 1; if (!revs->full_diff) diff_tree_setup_paths(revs->prune_data, &revs->diffopt); }
@@ -1412,7 +1416,7 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit) return commit_ignore; if (!commit_match(commit, revs)) return commit_ignore; - if (revs->prune_fn && revs->dense) { + if (revs->prune && revs->dense) { /* Commit without changes? */ if (!(commit->object.flags & TREECHANGE)) { /* drop merges unless we want parenthood */
@@ -1460,7 +1464,7 @@ static struct commit *get_revision_1(struct rev_info *revs) case commit_ignore: continue; case commit_error: - return -1; + return NULL; default: return commit; }
diff --git a/revision.h b/revision.h
index 2232247..a798514 100644
--- a/revision.h
+++ b/revision.h@@ -15,8 +15,6 @@ struct rev_info; struct log_info; -typedef void (prune_fn_t)(struct rev_info *revs, struct commit *commit); - struct rev_info { /* Starting list */ struct commit_list *commits;
@@ -28,12 +26,11 @@ struct rev_info { /* Basic information */ const char *prefix; void *prune_data; - prune_fn_t *prune_fn; - unsigned int early_output; /* Traversal flags */ unsigned int dense:1, + prune:1, no_merges:1, no_walk:1, remove_empty_trees:1,