diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index c88389df24..f5e66e9969 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -69,10 +69,12 @@ static int graph_verify(int argc, const char **argv, const char *prefix) struct commit_graph *graph = NULL; struct object_directory *odb = NULL; char *graph_name; - int open_ok; + char *chain_name; + enum { OPENED_NONE, OPENED_GRAPH, OPENED_CHAIN } opened = OPENED_NONE; int fd; struct stat st; int flags = 0; + int incomplete_chain = 0; int ret; static struct option builtin_commit_graph_verify_options[] = { @@ -102,24 +104,39 @@ static int graph_verify(int argc, const char **argv, const char *prefix) odb = find_odb(the_repository, opts.obj_dir); graph_name = get_commit_graph_filename(odb); - open_ok = open_commit_graph(graph_name, &fd, &st); - if (!open_ok && errno != ENOENT) + chain_name = get_commit_graph_chain_filename(odb); + if (open_commit_graph(graph_name, &fd, &st)) + opened = OPENED_GRAPH; + else if (errno != ENOENT) die_errno(_("Could not open commit-graph '%s'"), graph_name); + else if (open_commit_graph_chain(chain_name, &fd, &st)) + opened = OPENED_CHAIN; + else if (errno != ENOENT) + die_errno(_("could not open commit-graph chain '%s'"), chain_name); FREE_AND_NULL(graph_name); + FREE_AND_NULL(chain_name); FREE_AND_NULL(options); - if (open_ok) + if (opened == OPENED_NONE) + return 0; + else if (opened == OPENED_GRAPH) graph = load_commit_graph_one_fd_st(the_repository, fd, &st, odb); else - graph = read_commit_graph_one(the_repository, odb); + graph = load_commit_graph_chain_fd_st(the_repository, fd, &st, + &incomplete_chain); - /* Return failure if open_ok predicted success */ if (!graph) - return !!open_ok; + return 1; ret = verify_commit_graph(the_repository, graph, flags); free_commit_graph(graph); + + if (incomplete_chain) { + error("one or more commit-graph chain files could not be loaded"); + ret |= 1; + } + return ret; } diff --git a/commit-graph.c b/commit-graph.c index 5e8a3a5085..1a56efcf69 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -473,6 +473,31 @@ static struct commit_graph *load_commit_graph_v1(struct repository *r, return g; } +/* + * returns 1 if and only if all graphs in the chain have + * corrected commit dates stored in the generation_data chunk. + */ +static int validate_mixed_generation_chain(struct commit_graph *g) +{ + int read_generation_data = 1; + struct commit_graph *p = g; + + while (read_generation_data && p) { + read_generation_data = p->read_generation_data; + p = p->base_graph; + } + + if (read_generation_data) + return 1; + + while (g) { + g->read_generation_data = 0; + g = g->base_graph; + } + + return 0; +} + static int add_graph_to_chain(struct commit_graph *g, struct commit_graph *chain, struct object_id *oids, @@ -513,31 +538,41 @@ static int add_graph_to_chain(struct commit_graph *g, return 1; } -static struct commit_graph *load_commit_graph_chain(struct repository *r, - struct object_directory *odb) +int open_commit_graph_chain(const char *chain_file, + int *fd, struct stat *st) +{ + *fd = git_open(chain_file); + if (*fd < 0) + return 0; + if (fstat(*fd, st)) { + close(*fd); + return 0; + } + if (st->st_size < the_hash_algo->hexsz) { + close(*fd); + if (!st->st_size) { + /* treat empty files the same as missing */ + errno = ENOENT; + } else { + warning("commit-graph chain file too small"); + errno = EINVAL; + } + return 0; + } + return 1; +} + +struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, + int fd, struct stat *st, + int *incomplete_chain) { struct commit_graph *graph_chain = NULL; struct strbuf line = STRBUF_INIT; - struct stat st; struct object_id *oids; int i = 0, valid = 1, count; - char *chain_name = get_commit_graph_chain_filename(odb); - FILE *fp; - int stat_res; + FILE *fp = xfdopen(fd, "r"); - fp = fopen(chain_name, "r"); - stat_res = stat(chain_name, &st); - free(chain_name); - - if (!fp) - return NULL; - if (stat_res || - st.st_size <= the_hash_algo->hexsz) { - fclose(fp); - return NULL; - } - - count = st.st_size / (the_hash_algo->hexsz + 1); + count = st->st_size / (the_hash_algo->hexsz + 1); CALLOC_ARRAY(oids, count); prepare_alt_odb(r); @@ -578,36 +613,32 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, } } + validate_mixed_generation_chain(graph_chain); + free(oids); fclose(fp); strbuf_release(&line); + *incomplete_chain = !valid; return graph_chain; } -/* - * returns 1 if and only if all graphs in the chain have - * corrected commit dates stored in the generation_data chunk. - */ -static int validate_mixed_generation_chain(struct commit_graph *g) +static struct commit_graph *load_commit_graph_chain(struct repository *r, + struct object_directory *odb) { - int read_generation_data = 1; - struct commit_graph *p = g; + char *chain_file = get_commit_graph_chain_filename(odb); + struct stat st; + int fd; + struct commit_graph *g = NULL; - while (read_generation_data && p) { - read_generation_data = p->read_generation_data; - p = p->base_graph; + if (open_commit_graph_chain(chain_file, &fd, &st)) { + int incomplete; + /* ownership of fd is taken over by load function */ + g = load_commit_graph_chain_fd_st(r, fd, &st, &incomplete); } - if (read_generation_data) - return 1; - - while (g) { - g->read_generation_data = 0; - g = g->base_graph; - } - - return 0; + free(chain_file); + return g; } struct commit_graph *read_commit_graph_one(struct repository *r, @@ -618,8 +649,6 @@ struct commit_graph *read_commit_graph_one(struct repository *r, if (!g) g = load_commit_graph_chain(r, odb); - validate_mixed_generation_chain(g); - return g; } diff --git a/commit-graph.h b/commit-graph.h index 5e534f0fcc..20ada7e891 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -26,6 +26,7 @@ struct string_list; char *get_commit_graph_filename(struct object_directory *odb); char *get_commit_graph_chain_filename(struct object_directory *odb); int open_commit_graph(const char *graph_file, int *fd, struct stat *st); +int open_commit_graph_chain(const char *chain_file, int *fd, struct stat *st); /* * Given a commit struct, try to fill the commit struct info, including: @@ -105,6 +106,9 @@ struct commit_graph { struct commit_graph *load_commit_graph_one_fd_st(struct repository *r, int fd, struct stat *st, struct object_directory *odb); +struct commit_graph *load_commit_graph_chain_fd_st(struct repository *r, + int fd, struct stat *st, + int *incomplete_chain); struct commit_graph *read_commit_graph_one(struct repository *r, struct object_directory *odb); diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh index 36c4141e67..06bb897f02 100755 --- a/t/t5324-split-commit-graph.sh +++ b/t/t5324-split-commit-graph.sh @@ -285,6 +285,32 @@ test_expect_success 'verify hashes along chain, even in shallow' ' ) ' +test_expect_success 'verify notices chain slice which is bogus (base)' ' + git clone --no-hardlinks . verify-chain-bogus-base && + ( + cd verify-chain-bogus-base && + git commit-graph verify && + base_file=$graphdir/graph-$(sed -n 1p $graphdir/commit-graph-chain).graph && + echo "garbage" >$base_file && + test_must_fail git commit-graph verify 2>test_err && + grep -v "^+" test_err >err && + grep "commit-graph file is too small" err + ) +' + +test_expect_success 'verify notices chain slice which is bogus (tip)' ' + git clone --no-hardlinks . verify-chain-bogus-tip && + ( + cd verify-chain-bogus-tip && + git commit-graph verify && + tip_file=$graphdir/graph-$(sed -n 2p $graphdir/commit-graph-chain).graph && + echo "garbage" >$tip_file && + test_must_fail git commit-graph verify 2>test_err && + grep -v "^+" test_err >err && + grep "commit-graph file is too small" err + ) +' + test_expect_success 'verify --shallow does not check base contents' ' git clone --no-hardlinks . verify-shallow && ( @@ -306,27 +332,54 @@ test_expect_success 'warn on base graph chunk incorrect' ' git commit-graph verify && base_file=$graphdir/graph-$(tail -n 1 $graphdir/commit-graph-chain).graph && corrupt_file "$base_file" $(test_oid base) "\01" && - git commit-graph verify --shallow 2>test_err && + test_must_fail git commit-graph verify --shallow 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "commit-graph chain does not match" err ) ' -test_expect_success 'verify after commit-graph-chain corruption' ' - git clone --no-hardlinks . verify-chain && +test_expect_success 'verify after commit-graph-chain corruption (base)' ' + git clone --no-hardlinks . verify-chain-base && ( - cd verify-chain && - corrupt_file "$graphdir/commit-graph-chain" 60 "G" && - git commit-graph verify 2>test_err && + cd verify-chain-base && + corrupt_file "$graphdir/commit-graph-chain" 30 "G" && + test_must_fail git commit-graph verify 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "invalid commit-graph chain" err && - corrupt_file "$graphdir/commit-graph-chain" 60 "A" && - git commit-graph verify 2>test_err && + corrupt_file "$graphdir/commit-graph-chain" 30 "A" && + test_must_fail git commit-graph verify 2>test_err && grep -v "^+" test_err >err && test_i18ngrep "unable to find all commit-graph files" err ) ' +test_expect_success 'verify after commit-graph-chain corruption (tip)' ' + git clone --no-hardlinks . verify-chain-tip && + ( + cd verify-chain-tip && + corrupt_file "$graphdir/commit-graph-chain" 70 "G" && + test_must_fail git commit-graph verify 2>test_err && + grep -v "^+" test_err >err && + test_i18ngrep "invalid commit-graph chain" err && + corrupt_file "$graphdir/commit-graph-chain" 70 "A" && + test_must_fail git commit-graph verify 2>test_err && + grep -v "^+" test_err >err && + test_i18ngrep "unable to find all commit-graph files" err + ) +' + +test_expect_success 'verify notices too-short chain file' ' + git clone --no-hardlinks . verify-chain-short && + ( + cd verify-chain-short && + git commit-graph verify && + echo "garbage" >$graphdir/commit-graph-chain && + test_must_fail git commit-graph verify 2>test_err && + grep -v "^+" test_err >err && + grep "commit-graph chain file too small" err + ) +' + test_expect_success 'verify across alternates' ' git clone --no-hardlinks . verify-alt && (