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

[PATCH 8/8] commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag

From
Taylor Blau <me@ttaylorr.com>
Date
May 5, 2020, 01:14 UTC
Message-ID
<6c2d130b0cd4b6a8a541b362ae7dd44d4c282e3f.1588641176.git.me@ttaylorr.com>
In-Reply-To
<cover.1588641176.git.me@ttaylorr.com>

Since 7c5c9b9c57 (commit-graph: error out on invalid commit oids in 'write --stdin-commits', 2019-08-05), the commit-graph builtin dies on receiving non-commit OIDs as input to '--stdin-commits'.

This behavior can be cumbersome to work around in, say, the case of piping 'git for-each-ref' to 'git commit-graph write --stdin-commits' if the caller does not want to cull out non-commits themselves. In this situation, it would be ideal if 'git commit-graph write' wrote the graph containing the inputs that did pertain to commits, and silently ignored the remainder of the input.

Some options have been proposed to the effect of '--[no-]check-oids' which would allow callers to have the commit-graph builtin do just that. After some discussion, it is difficult to imagine a caller who wouldn't want to pass '--no-check-oids', suggesting that we should get rid of the behavior of complaining about non-commit inputs altogether.

If callers do wish to retain this behavior, they can easily work around this change by doing the following:

    git for-each-ref --format='%(objectname) %(objecttype) %(*objecttype)' |
    awk '/commit/ { print $1 }' |
    git commit-graph write --stdin-commits

To make it so that valid OIDs that refer to non-existent objects are indeed an error after loosening the error handling, perform an extra lookup to make sure that object indeed exists before sending it to the commit-graph internals.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
 Documentation/git-commit-graph.txt |  6 ++++--
 builtin/commit-graph.c             | 10 +++++-----
 commit-graph.c                     |  2 --
 commit-graph.h                     | 10 ++++------
 t/t5318-commit-graph.sh            | 15 +++++++++++----
 5 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt
index 53a650225a..fcac7d12e1 100644
--- a/Documentation/git-commit-graph.txt
+++ b/Documentation/git-commit-graph.txt
@@ -47,8 +47,10 @@ with `--stdin-commits` or `--reachable`.)
 +
 With the `--stdin-commits` option, generate the new commit graph by
 walking commits starting at the commits specified in stdin as a list
-of OIDs in hex, one OID per line. (Cannot be combined with
-`--stdin-packs` or `--reachable`.)
+of OIDs in hex, one OID per line. OIDs that resolve to non-commits
+(either directly, or by peeling tags) are silently ignored. OIDs that
+are malformed, or do not exist generate an error. (Cannot be combined
+with `--stdin-packs` or `--reachable`.)
 +
 With the `--reachable` option, generate the new commit graph by walking
 commits starting at all refs. (Cannot be combined with `--stdin-commits`
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 9eec68572f..3637d079fb 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -153,13 +153,14 @@ static int read_one_commit(struct oidset *commits, struct progress *progress,
 
 	display_progress(progress, oidset_size(commits) + 1);
 
+	if (oid_object_info(the_repository, &oid, NULL) < 0) {
+		error(_("object %s does not exist"), hash);
+		return 1;
+	}
+
 	result = lookup_commit_reference_gently(the_repository, &oid, 1);
 	if (result)
 		oidset_insert(commits, &result->object.oid);
-	else {
-		error(_("invalid commit object id: %s"), hash);
-		return 1;
-	}
 	return 0;
 }
 
@@ -239,7 +240,6 @@ static int graph_write(int argc, const char **argv)
 		struct strbuf buf = STRBUF_INIT;
 		if (opts.stdin_commits) {
 			oidset_init(&commits, 0);
-			flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
 			if (opts.progress)
 				progress = start_delayed_progress(
 					_("Analyzing commits from stdin"), 0);
diff --git a/commit-graph.c b/commit-graph.c
index 24c2d80935..4e5539bac5 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -880,7 +880,6 @@ struct write_commit_graph_context {
 	unsigned append:1,
 		 report_progress:1,
 		 split:1,
-		 check_oids:1,
 		 changed_paths:1,
 		 order_by_pack:1;
 
@@ -2001,7 +2000,6 @@ int write_commit_graph(struct object_directory *odb,
 	ctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;
 	ctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;
 	ctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;
-	ctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;
 	ctx->split_opts = split_opts;
 	ctx->changed_paths = flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS ? 1 : 0;
 	ctx->total_bloom_filter_data_size = 0;
diff --git a/commit-graph.h b/commit-graph.h
index 4212766a4f..659393e07f 100644
--- a/commit-graph.h
+++ b/commit-graph.h
@@ -88,12 +88,10 @@ struct commit_graph *parse_commit_graph(void *graph_map, size_t graph_size);
 int generation_numbers_enabled(struct repository *r);
 
 enum commit_graph_write_flags {
-	COMMIT_GRAPH_WRITE_APPEND     = (1 << 0),
-	COMMIT_GRAPH_WRITE_PROGRESS   = (1 << 1),
-	COMMIT_GRAPH_WRITE_SPLIT      = (1 << 2),
-	/* Make sure that each OID in the input is a valid commit OID. */
-	COMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3),
-	COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 4),
+	COMMIT_GRAPH_WRITE_APPEND        = (1 << 0),
+	COMMIT_GRAPH_WRITE_PROGRESS      = (1 << 1),
+	COMMIT_GRAPH_WRITE_SPLIT         = (1 << 2),
+	COMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 3)
 };
 
 enum commit_graph_split_flags {
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 89020d3d44..74f93f0a17 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -84,11 +84,18 @@ graph_read_expect() {
 
 test_expect_success 'exit with correct error on bad input to --stdin-commits' '
 	cd "$TRASH_DIRECTORY/full" &&
-	echo HEAD | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
+	# invalid, non-hex OID
+	echo HEAD >in &&
+	test_expect_code 1 git commit-graph write --stdin-commits <in 2>stderr &&
 	test_i18ngrep "unexpected non-hex object ID: HEAD" stderr &&
-	# valid tree OID, but not a commit OID
-	git rev-parse HEAD^{tree} | test_expect_code 1 git commit-graph write --stdin-commits 2>stderr &&
-	test_i18ngrep "invalid commit object id" stderr
+	# non-existent OID
+	echo $ZERO_OID >in &&
+	test_expect_code 1 git commit-graph write --stdin-commits <in 2>stderr &&
+	test_i18ngrep "does not exist" stderr &&
+	# valid commit and tree OID
+	git rev-parse HEAD HEAD^{tree} >in &&
+	git commit-graph write --stdin-commits <in &&
+	graph_read_expect 3
 '
 
 test_expect_success 'write graph' '

-- 
2.26.0.113.ge9739cdccc
Previous: Taylor BlauNext: Derrick Stolee
Message 21 of 59 in “commit-graph: drop CHECK_OIDS, peel in callers”
  1. 0/8 commit-graph: drop CHECK_OIDS, peel in callersTaylor Blau, May 5, 2020
  2. 1/8 commit-graph.c: extract 'refs_cb_data'Taylor Blau, May 5, 2020
  3. 2/8 commit-graph.c: show progress of finding reachable commitsTaylor Blau, May 5, 2020
  4. Derrick StoleeMay 5, 2020
  5. Taylor BlauMay 5, 2020
  6. 3/8 commit-graph.c: peel refs in 'add_ref_to_set'Taylor Blau, May 5, 2020
  7. Jeff KingMay 7, 2020
  8. Taylor BlauMay 13, 2020
  9. Jeff KingMay 13, 2020
  10. 4/8 builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau, May 5, 2020
  11. Jeff KingMay 7, 2020
  12. Taylor BlauMay 13, 2020
  13. 5/8 builtin/commit-graph.c: dereference tags in builtinTaylor Blau, May 5, 2020
  14. Derrick StoleeMay 5, 2020
  15. Taylor BlauMay 5, 2020
  16. Jeff KingMay 7, 2020
  17. 6/8 commit-graph.c: simplify 'fill_oids_from_commits'Taylor Blau, May 5, 2020
  18. Derrick StoleeMay 5, 2020
  19. Jeff KingMay 7, 2020
  20. 7/8 t5318: reorder test below 'graph_read_expect'Taylor Blau, May 5, 2020
  21. 8/8 commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau, May 5, 2020
  22. Derrick StoleeMay 5, 2020
  23. Taylor BlauMay 5, 2020
  24. Jeff KingMay 7, 2020
  25. Taylor BlauMay 13, 2020
  26. Derrick StoleeMay 5, 2020
  27. Taylor BlauMay 5, 2020
  28. 0/8 commit-graph: drop CHECK_OIDS, peel in callersTaylor Blau, May 6, 2020
  29. 1/8 commit-graph.c: extract 'refs_cb_data'Taylor Blau, May 6, 2020
  30. 2/8 commit-graph.c: show progress of finding reachable commitsTaylor Blau, May 6, 2020
  31. 3/8 commit-graph.c: peel refs in 'add_ref_to_set'Taylor Blau, May 6, 2020
  32. 4/8 builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau, May 6, 2020
  33. 5/8 builtin/commit-graph.c: dereference tags in builtinTaylor Blau, May 6, 2020
  34. 6/8 commit-graph.c: simplify 'fill_oids_from_commits'Taylor Blau, May 6, 2020
  35. 7/8 t5318: reorder test below 'graph_read_expect'Taylor Blau, May 6, 2020
  36. 8/8 commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau, May 6, 2020
  37. Jeff KingMay 7, 2020
  38. 0/8 commit-graph: drop CHECK_OIDS, peel in callersTaylor Blau, May 13, 2020
  39. 1/8 commit-graph.c: extract 'refs_cb_data'Taylor Blau, May 13, 2020
  40. 2/8 commit-graph.c: show progress of finding reachable commitsTaylor Blau, May 13, 2020
  41. 3/8 commit-graph.c: peel refs in 'add_ref_to_set'Taylor Blau, May 13, 2020
  42. 4/8 builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau, May 13, 2020
  43. Jeff KingMay 14, 2020
  44. Taylor BlauMay 14, 2020
  45. Junio C HamanoMay 14, 2020
  46. Taylor BlauMay 18, 2020
  47. 5/8 builtin/commit-graph.c: dereference tags in builtinTaylor Blau, May 13, 2020
  48. Jeff KingMay 14, 2020
  49. Taylor BlauMay 14, 2020
  50. commit-graph: fix "Collecting commits from input" progress lineSZEDER Gábor, Jul 10, 2020
  51. Taylor BlauJul 10, 2020
  52. SZEDER GáborJul 15, 2020
  53. Derrick StoleeJul 15, 2020
  54. Junio C HamanoJul 15, 2020
  55. 6/8 commit-graph.c: simplify 'fill_oids_from_commits'Taylor Blau, May 13, 2020
  56. 7/8 t5318: reorder test below 'graph_read_expect'Taylor Blau, May 13, 2020
  57. 8/8 commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau, May 13, 2020
  58. Jeff KingMay 14, 2020
  59. Jeff KingMay 14, 2020

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.