@matheusbsbs / matheus-bagliano / commits / 47d06bb010

commit-graph: detect read errors when verifying graph chain

Because it's OK to not have a graph file at all, the graph_verify() function needs to tell the difference between a missing file and a real error. So when loading a traditional graph file, we call open_commit_graph() separately from load_commit_graph_chain_fd_st(), and don't complain if the first one fails with ENOENT. When the function learned about chain files in 3da4b609bb (commit-graph: verify chains with --shallow mode, 2019-06-18), we couldn't be as careful, since the only way to load a chain was with read_commit_graph_one(), which did both the open/load as a single unit. So we'll miss errors in chain files we load, thinking instead that there was just no chain file at all. Note that we do still report some of these problems to stderr, as the loading function calls error() and warning(). But we'd exit with a successful exit code, which is wrong. We can fix that by using the recently split open/load functions for chains. That lets us treat the chain file just like a single file with respect to error handling here. An existing test (from 3da4b609bb) shows off the problem; we were expecting "commit-graph verify" to report success, but that makes no sense. We did not even verify the contents of the graph data, because we couldn't load it! I don't think this was an intentional exception, but rather just the test covering what happened to occur. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 28, 2023 at 00:38 UTC 47d06bb010867cd122f732be93e55a797371570c

2 files changed +18 -9

builtin/commit-graph.c

+16 -7

@@ -69,7 +69,8 @@ static int graph_verify(int argc, const char **argv, const char *prefix)

69 struct commit_graph *graph = NULL;

70 struct object_directory *odb = NULL;

71 char *graph_name;

72 - int open_ok;

72 + char *chain_name;

73 + enum { OPENED_NONE, OPENED_GRAPH, OPENED_CHAIN } opened = OPENED_NONE;

74 int fd;

75 struct stat st;

76 int flags = 0;

@@ -102,21 +103,29 @@ static int graph_verify(int argc, const char **argv, const char *prefix)

103

104 odb = find_odb(the_repository, opts.obj_dir);

105 graph_name = get_commit_graph_filename(odb);

105 - open_ok = open_commit_graph(graph_name, &fd, &st);

106 - if (!open_ok && errno != ENOENT)

106 + chain_name = get_commit_graph_chain_filename(odb);

107 + if (open_commit_graph(graph_name, &fd, &st))

108 + opened = OPENED_GRAPH;

109 + else if (errno != ENOENT)

110 die_errno(_("Could not open commit-graph '%s'"), graph_name);

111 + else if (open_commit_graph_chain(chain_name, &fd, &st))

112 + opened = OPENED_CHAIN;

113 + else if (errno != ENOENT)

114 + die_errno(_("could not open commit-graph chain '%s'"), chain_name);

115

116 FREE_AND_NULL(graph_name);

117 + FREE_AND_NULL(chain_name);

118 FREE_AND_NULL(options);

119

112 - if (open_ok)

120 + if (opened == OPENED_NONE)

121 + return 0;

122 + else if (opened == OPENED_GRAPH)

123 graph = load_commit_graph_one_fd_st(the_repository, fd, &st, odb);

124 else

115 - graph = read_commit_graph_one(the_repository, odb);

125 + graph = load_commit_graph_chain_fd_st(the_repository, fd, &st);

126

117 - /* Return failure if open_ok predicted success */

127 if (!graph)

119 - return !!open_ok;

128 + return 1;

129

130 ret = verify_commit_graph(the_repository, graph, flags);

131 free_commit_graph(graph);

t/t5324-split-commit-graph.sh

+2 -2

@@ -317,11 +317,11 @@ test_expect_success 'verify after commit-graph-chain corruption (base)' '

317 (

318 cd verify-chain-base &&

319 corrupt_file "$graphdir/commit-graph-chain" 30 "G" &&

320 - git commit-graph verify 2>test_err &&

320 + test_must_fail git commit-graph verify 2>test_err &&

321 grep -v "^+" test_err >err &&

322 test_i18ngrep "invalid commit-graph chain" err &&

323 corrupt_file "$graphdir/commit-graph-chain" 30 "A" &&

324 - git commit-graph verify 2>test_err &&

324 + test_must_fail git commit-graph verify 2>test_err &&

325 grep -v "^+" test_err >err &&

326 test_i18ngrep "unable to find all commit-graph files" err

327 )