@matheusbsbs / matheus-bagliano / commits / f30e4d854b

fsck: verify commit graph when implicitly enabled

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Oct 15, 2021 at 13:16 UTC f30e4d854bb8462d1a4da697ad95501d33fe4425
3 files changed +29 -3
builtin/fsck.c
+2 -1
@@ -803,6 +803,7 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
803 fsck_enable_object_names(&fsck_walk_options);
804
805 git_config(git_fsck_config, &fsck_obj_options);
806 + prepare_repo_settings(the_repository);
807
808 if (connectivity_only) {
809 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)
909
910 check_connectivity();
911
911 - if (!git_config_get_bool("core.commitgraph", &i) && i) {
912 + if (the_repository->settings.core_commit_graph) {
913 struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
914 const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL };
915
t/t0410-partial-clone.sh
+5 -1
@@ -6,6 +6,10 @@ test_description='partial clone'
6
7 # missing promisor objects cause repacks which write bitmaps to fail
8 GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
9 +# When enabled, some commands will write commit-graphs. This causes fsck
10 +# to fail when delete_object() is called because fsck will attempt to
11 +# verify the out-of-sync commit graph.
12 +GIT_TEST_COMMIT_GRAPH=0
13
14 delete_object () {
15 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' '
326
327 git -C repo config core.repositoryformatversion 1 &&
328 git -C repo config extensions.partialclone "arbitrary string" &&
325 - GIT_TEST_COMMIT_GRAPH=0 git -C repo -c core.commitGraph=false rev-list --exclude-promisor-objects --objects bar >out &&
329 + git -C repo rev-list --exclude-promisor-objects --objects bar >out &&
330 grep $(git -C repo rev-parse bar) out &&
331 ! grep $FOO out
332 '
t/t5318-commit-graph.sh
+22 -1
@@ -694,12 +694,33 @@ test_expect_success 'detect incorrect chunk count' '
694 $GRAPH_CHUNK_LOOKUP_OFFSET
695 '
696
697 -test_expect_success 'git fsck (checks commit-graph)' '
697 +test_expect_success 'git fsck (checks commit-graph when config set to true)' '
698 cd "$TRASH_DIRECTORY/full" &&
699 git fsck &&
700 corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
701 "incorrect checksum" &&
702 cp commit-graph-pre-write-test $objdir/info/commit-graph &&
703 + test_must_fail git -c core.commitGraph=true fsck
704 +'
705 +
706 +test_expect_success 'git fsck (ignores commit-graph when config set to false)' '
707 + cd "$TRASH_DIRECTORY/full" &&
708 + git fsck &&
709 + corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
710 + "incorrect checksum" &&
711 + cp commit-graph-pre-write-test $objdir/info/commit-graph &&
712 + git -c core.commitGraph=false fsck
713 +'
714 +
715 +test_expect_success 'git fsck (checks commit-graph when config unset)' '
716 + cd "$TRASH_DIRECTORY/full" &&
717 + test_when_finished "git config core.commitGraph true" &&
718 +
719 + git fsck &&
720 + corrupt_graph_and_verify $GRAPH_BYTE_FOOTER "\00" \
721 + "incorrect checksum" &&
722 + test_unconfig core.commitGraph &&
723 + cp commit-graph-pre-write-test $objdir/info/commit-graph &&
724 test_must_fail git fsck
725 '
726