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

[PATCH v5 1/3] fsck: verify commit graph when implicitly enabled

From
Glen Choo <chooglen@google.com>
Date
Oct 15, 2021, 20:16 UTC
Message-ID
<20211015201631.91524-2-chooglen@google.com>
In-Reply-To
<20211015201631.91524-1-chooglen@google.com>

Change fsck to check the "core_commit_graph" variable set in "repo-settings.c" instead of reading the "core.commitGraph" variable. This fixes a bug where we wouldn't verify the commit-graph if the config key was missing. This bug was introduced in 31b1de6a09 (commit-graph: turn on commit-graph by default, 2019-08-13), where core.commitGraph was turned on by default.

Add tests to "t5318-commit-graph.sh" to verify that fsck checks the commit-graph as expected for the 3 values of core.commitGraph. Also, disable GIT_TEST_COMMIT_GRAPH in t/t0410-partial-clone.sh because some test cases use fsck in ways that assume that commit-graph checking is disabled.

Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Glen Choo <chooglen@google.com>
---
 builtin/fsck.c           |  3 ++-
 t/t0410-partial-clone.sh |  6 +++++-
 t/t5318-commit-graph.sh  | 23 ++++++++++++++++++++++-
 3 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index b42b6fe21f..1c4e485b66 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -803,6 +803,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 		fsck_enable_object_names(&fsck_walk_options);
 
 	git_config(git_fsck_config, &fsck_obj_options);
+	prepare_repo_settings(the_repository);
 
 	if (connectivity_only) {
 		for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
@@ -908,7 +909,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
 	check_connectivity();
 
-	if (!git_config_get_bool("core.commitgraph", &i) && i) {
+	if (the_repository->settings.core_commit_graph) {
 		struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
 		const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL };
 
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index bba679685f..c76485b1b6 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -6,6 +6,10 @@ test_description='partial clone'
 
 # missing promisor objects cause repacks which write bitmaps to fail
 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
+# When enabled, some commands will write commit-graphs. This causes fsck
+# to fail when delete_object() is called because fsck will attempt to
+# verify the out-of-sync commit graph.
+GIT_TEST_COMMIT_GRAPH=0
 
 delete_object () {
 	rm $1/.git/objects/$(echo $2 | sed -e 's|^..|&/|')
@@ -322,7 +326,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '
 
 	git -C repo config core.repositoryformatversion 1 &&
 	git -C repo config extensions.partialclone "arbitrary string" &&
-	GIT_TEST_COMMIT_GRAPH=0 git -C repo -c core.commitGraph=false rev-list --exclude-promisor-objects --objects bar >out &&
+	git -C repo rev-list --exclude-promisor-objects --objects bar >out &&
 	grep $(git -C repo rev-parse bar) out &&
 	! grep $FOO out
 '
diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
index 84d122a7ae..f516fda7cc 100755
--- a/t/t5318-commit-graph.sh
+++ b/t/t5318-commit-graph.sh
@@ -694,12 +694,33 @@ test_expect_success 'detect incorrect chunk count' '
 		$GRAPH_CHUNK_LOOKUP_OFFSET
 '
 
-test_expect_success 'git fsck (checks commit-graph)' '
+test_expect_success 'git fsck (checks commit-graph when config set to true)' '
 	cd "$TRASH_DIRECTORY/full" &&
 	git fsck &&
 	corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
 		"incorrect checksum" &&
 	cp commit-graph-pre-write-test $objdir/info/commit-graph &&
+	test_must_fail git -c core.commitGraph=true fsck
+'
+
+test_expect_success 'git fsck (ignores commit-graph when config set to false)' '
+	cd "$TRASH_DIRECTORY/full" &&
+	git fsck &&
+	corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
+		"incorrect checksum" &&
+	cp commit-graph-pre-write-test $objdir/info/commit-graph &&
+	git -c core.commitGraph=false fsck
+'
+
+test_expect_success 'git fsck (checks commit-graph when config unset)' '
+	cd "$TRASH_DIRECTORY/full" &&
+	test_when_finished "git config core.commitGraph true" &&
+
+	git fsck &&
+	corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
+		"incorrect checksum" &&
+	test_unconfig core.commitGraph &&
+	cp commit-graph-pre-write-test $objdir/info/commit-graph &&
 	test_must_fail git fsck
 '
 
-- 
2.33.0.1079.g6e70778dc9-goog
Previous: Glen ChooNext: Glen Choo
Message 62 of 65 in “Use default values from settings instead of config”
  1. 0/3 Use default values from settings instead of configGlen Choo, Sep 13, 2021
  2. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Sep 13, 2021
  3. Taylor BlauSep 13, 2021
  4. Eric SunshineSep 13, 2021
  5. Taylor BlauSep 13, 2021
  6. Glen ChooSep 13, 2021
  7. Eric SunshineSep 13, 2021
  8. Taylor BlauSep 14, 2021
  9. Eric SunshineSep 14, 2021
  10. Taylor BlauSep 14, 2021
  11. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Sep 13, 2021
  12. Taylor BlauSep 13, 2021
  13. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Sep 13, 2021
  14. Taylor BlauSep 13, 2021
  15. Glen ChooSep 14, 2021
  16. Bagas SanjayaSep 14, 2021
  17. Glen ChooSep 16, 2021
  18. Taylor BlauSep 13, 2021
  19. 0/3 Use default values from settings instead of configGlen Choo, Sep 17, 2021
  20. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Sep 17, 2021
  21. Eric SunshineSep 29, 2021
  22. Glen ChooSep 30, 2021
  23. Glen ChooSep 30, 2021
  24. Eric SunshineSep 30, 2021
  25. Glen ChooOct 1, 2021
  26. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Sep 17, 2021
  27. Eric SunshineSep 29, 2021
  28. Glen ChooSep 29, 2021
  29. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Sep 17, 2021
  30. Eric SunshineSep 29, 2021
  31. Glen ChooSep 27, 2021
  32. Eric SunshineSep 29, 2021
  33. Glen ChooSep 29, 2021
  34. 0/3 Use default values from settings instead of configGlen Choo, Oct 5, 2021
  35. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 5, 2021
  36. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 5, 2021
  37. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 5, 2021
  38. Ævar Arnfjörð BjarmasonOct 5, 2021
  39. Derrick StoleeOct 5, 2021
  40. Ævar Arnfjörð BjarmasonOct 5, 2021
  41. Glen ChooOct 5, 2021
  42. Junio C HamanoOct 9, 2021
  43. Glen ChooOct 11, 2021
  44. Junio C HamanoOct 11, 2021
  45. Glen ChooOct 11, 2021
  46. 0/3 Use default values from settings instead of configGlen Choo, Oct 12, 2021
  47. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 12, 2021
  48. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 12, 2021
  49. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 12, 2021
  50. Junio C HamanoOct 12, 2021
  51. Ævar Arnfjörð BjarmasonOct 12, 2021
  52. Glen ChooOct 12, 2021
  53. Ævar Arnfjörð BjarmasonOct 14, 2021
  54. Derrick StoleeOct 13, 2021
  55. Ævar Arnfjörð BjarmasonOct 13, 2021
  56. Derrick StoleeOct 14, 2021
  57. Glen ChooOct 14, 2021
  58. Ævar Arnfjörð BjarmasonOct 14, 2021
  59. Junio C HamanoOct 14, 2021
  60. Junio C HamanoOct 15, 2021
  61. 0/3 Use default values from settings instead of configGlen Choo, Oct 15, 2021
  62. 1/3 fsck: verify commit graph when implicitly enabledGlen Choo, Oct 15, 2021
  63. 2/3 fsck: verify multi-pack-index when implictly enabledGlen Choo, Oct 15, 2021
  64. 3/3 gc: perform incremental repack when implictly enabledGlen Choo, Oct 15, 2021
  65. Junio C HamanoOct 15, 2021

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.