git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/2] Enhance --early-output format

From
Linus Torvalds <torvalds@linux-foundation.org>
Date
Nov 5, 2007, 21:22 UTC
Message-ID
<alpine.LFD.0.999.0711051313350.15101@woody.linux-foundation.org>
In-Reply-To
<alpine.LFD.0.999.0711051233270.15101@woody.linux-foundation.org>
On Mon, 5 Nov 2007, Linus Torvalds wrote:
Show 12 quoted lines
> 
> 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,
Previous: Linus TorvaldsNext: Linus Torvalds
Message 30 of 57 in “New features in gitk”
  1. Paul MackerrasOct 28, 2007
  2. Linus TorvaldsOct 28, 2007
  3. Paul MackerrasOct 28, 2007
  4. Steffen ProhaskaOct 28, 2007
  5. Linus TorvaldsOct 28, 2007
  6. Paul MackerrasNov 1, 2007
  7. Linus TorvaldsNov 1, 2007
  8. Paul MackerrasNov 2, 2007
  9. Marco CostalbaNov 2, 2007
  10. Linus TorvaldsNov 2, 2007
  11. Marco CostalbaNov 2, 2007
  12. Linus TorvaldsNov 2, 2007
  13. 0/2 History replay supportLinus Torvalds, Nov 2, 2007
  14. 1/2 Simplify topo-sort logicLinus Torvalds, Nov 2, 2007
  15. 2/2 Support "history replay" for git log commandsLinus Torvalds, Nov 2, 2007
  16. Junio C HamanoNov 2, 2007
  17. Linus TorvaldsNov 2, 2007
  18. Linus TorvaldsNov 3, 2007
  19. Marco CostalbaNov 3, 2007
  20. 2/2 Add "--early-output" log flag for interactive GUI useLinus Torvalds, Nov 3, 2007
  21. Marco CostalbaNov 3, 2007
  22. Paul MackerrasNov 4, 2007
  23. Linus TorvaldsNov 4, 2007
  24. Paul MackerrasNov 4, 2007
  25. Marco CostalbaNov 4, 2007
  26. Linus TorvaldsNov 4, 2007
  27. 3/2 Enhance --early-output formatLinus Torvalds, Nov 4, 2007
  28. Junio C HamanoNov 5, 2007
  29. Linus TorvaldsNov 5, 2007
  30. Linus TorvaldsNov 5, 2007
  31. Linus TorvaldsNov 5, 2007
  32. 4/2 Fix parent rewriting in --early-outputLinus Torvalds, Nov 13, 2007
  33. Junio C HamanoNov 13, 2007
  34. Linus TorvaldsNov 13, 2007
  35. Linus TorvaldsNov 13, 2007
  36. Sven VerdoolaegeNov 13, 2007
  37. Junio C HamanoNov 13, 2007
  38. Shawn O. PearceNov 13, 2007
  39. Junio C HamanoNov 13, 2007
  40. Paul MackerrasNov 13, 2007
  41. Junio C HamanoNov 13, 2007
  42. Paul MackerrasNov 13, 2007
  43. Marco CostalbaNov 16, 2007
  44. Paul MackerrasNov 4, 2007
  45. Johannes SchindelinNov 2, 2007
  46. Linus TorvaldsNov 2, 2007
  47. Paul MackerrasNov 1, 2007
  48. Linus TorvaldsNov 1, 2007
  49. Linus TorvaldsNov 1, 2007
  50. Pierre HabouzitOct 28, 2007
  51. Mike HommeyOct 28, 2007
  52. Paul MackerrasOct 28, 2007
  53. Pierre HabouzitOct 29, 2007
  54. Jonathan del StrotherOct 29, 2007
  55. Pierre HabouzitOct 29, 2007
  56. Han-Wen NienhuysOct 29, 2007
  57. Michele BallabioOct 29, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.