From e103f7276f0d809c2935ebc1a3d68c6bbfaed23d Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 12 Jun 2019 06:29:37 -0700 Subject: commit-graph: return with errors during write The write_commit_graph() method uses die() to report failure and exit when confronted with an unexpected condition. This use of die() in a library function is incorrect and is now replaced by error() statements and an int return type. Return zero on success and a negative value on failure. Now that we use 'goto cleanup' to jump to the terminal condition on an error, we have new paths that could lead to uninitialized values. New initializers are added to correct for this. The builtins 'commit-graph', 'gc', and 'commit' call these methods, so update them to check the return value. Test that 'git commit-graph write' returns a proper error code when hitting a failure condition in write_commit_graph(). Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'builtin/commit-graph.c') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 537fdfd0f0..2a1c4d701f 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -141,6 +141,7 @@ static int graph_write(int argc, const char **argv) struct string_list *pack_indexes = NULL; struct string_list *commit_hex = NULL; struct string_list lines; + int result = 0; static struct option builtin_commit_graph_write_options[] = { OPT_STRING(0, "object-dir", &opts.obj_dir, @@ -168,10 +169,8 @@ static int graph_write(int argc, const char **argv) read_replace_refs = 0; - if (opts.reachable) { - write_commit_graph_reachable(opts.obj_dir, opts.append, 1); - return 0; - } + if (opts.reachable) + return write_commit_graph_reachable(opts.obj_dir, opts.append, 1); string_list_init(&lines, 0); if (opts.stdin_packs || opts.stdin_commits) { @@ -188,14 +187,15 @@ static int graph_write(int argc, const char **argv) UNLEAK(buf); } - write_commit_graph(opts.obj_dir, - pack_indexes, - commit_hex, - opts.append, - 1); + if (write_commit_graph(opts.obj_dir, + pack_indexes, + commit_hex, + opts.append, + 1)) + result = 1; UNLEAK(lines); - return 0; + return result; } int cmd_commit_graph(int argc, const char **argv, const char *prefix) -- cgit v1.2.3 From 5af803945212af875670582ff153ee05ec368b83 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Wed, 12 Jun 2019 06:29:38 -0700 Subject: commit-graph: collapse parameters into flags The write_commit_graph() and write_commit_graph_reachable() methods currently take two boolean parameters: 'append' and 'report_progress'. As we update these methods, adding more parameters this way becomes cluttered and hard to maintain. Collapse these parameters into a 'flags' parameter, and adjust the callers to provide flags as necessary. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 8 +++++--- builtin/commit.c | 2 +- builtin/gc.c | 4 ++-- commit-graph.c | 9 +++++---- commit-graph.h | 8 +++++--- 5 files changed, 18 insertions(+), 13 deletions(-) (limited to 'builtin/commit-graph.c') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 2a1c4d701f..d8efa5bab2 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -142,6 +142,7 @@ static int graph_write(int argc, const char **argv) struct string_list *commit_hex = NULL; struct string_list lines; int result = 0; + unsigned int flags = COMMIT_GRAPH_PROGRESS; static struct option builtin_commit_graph_write_options[] = { OPT_STRING(0, "object-dir", &opts.obj_dir, @@ -166,11 +167,13 @@ static int graph_write(int argc, const char **argv) die(_("use at most one of --reachable, --stdin-commits, or --stdin-packs")); if (!opts.obj_dir) opts.obj_dir = get_object_directory(); + if (opts.append) + flags |= COMMIT_GRAPH_APPEND; read_replace_refs = 0; if (opts.reachable) - return write_commit_graph_reachable(opts.obj_dir, opts.append, 1); + return write_commit_graph_reachable(opts.obj_dir, flags); string_list_init(&lines, 0); if (opts.stdin_packs || opts.stdin_commits) { @@ -190,8 +193,7 @@ static int graph_write(int argc, const char **argv) if (write_commit_graph(opts.obj_dir, pack_indexes, commit_hex, - opts.append, - 1)) + flags)) result = 1; UNLEAK(lines); diff --git a/builtin/commit.c b/builtin/commit.c index b9ea7222fa..b001ef565d 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1670,7 +1670,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) "not exceeded, and then \"git reset HEAD\" to recover.")); if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) && - write_commit_graph_reachable(get_object_directory(), 0, 0)) + write_commit_graph_reachable(get_object_directory(), 0)) return 1; repo_rerere(the_repository, 0); diff --git a/builtin/gc.c b/builtin/gc.c index 3984addf73..df2573f124 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -665,8 +665,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix) } if (gc_write_commit_graph && - write_commit_graph_reachable(get_object_directory(), 0, - !quiet && !daemonized)) + write_commit_graph_reachable(get_object_directory(), + !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0)) return 1; if (auto_gc && too_many_loose_objects()) diff --git a/commit-graph.c b/commit-graph.c index 1b58d1da14..fc40b531af 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -851,15 +851,14 @@ static int add_ref_to_list(const char *refname, return 0; } -int write_commit_graph_reachable(const char *obj_dir, int append, - int report_progress) +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags) { struct string_list list = STRING_LIST_INIT_DUP; int result; for_each_ref(add_ref_to_list, &list); result = write_commit_graph(obj_dir, NULL, &list, - append, report_progress); + flags); string_list_clear(&list, 0); return result; @@ -868,7 +867,7 @@ int write_commit_graph_reachable(const char *obj_dir, int append, int write_commit_graph(const char *obj_dir, struct string_list *pack_indexes, struct string_list *commit_hex, - int append, int report_progress) + unsigned int flags) { struct packed_oid_list oids; struct packed_commit_list commits; @@ -887,6 +886,8 @@ int write_commit_graph(const char *obj_dir, struct strbuf progress_title = STRBUF_INIT; unsigned long approx_nr_objects; int res = 0; + int append = flags & COMMIT_GRAPH_APPEND; + int report_progress = flags & COMMIT_GRAPH_PROGRESS; if (!commit_graph_compatible(the_repository)) return 0; diff --git a/commit-graph.h b/commit-graph.h index 869717ca19..01538b5cf5 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -65,18 +65,20 @@ struct commit_graph *parse_commit_graph(void *graph_map, int fd, */ int generation_numbers_enabled(struct repository *r); +#define COMMIT_GRAPH_APPEND (1 << 0) +#define COMMIT_GRAPH_PROGRESS (1 << 1) + /* * The write_commit_graph* methods return zero on success * and a negative value on failure. Note that if the repository * is not compatible with the commit-graph feature, then the * methods will return 0 without writing a commit-graph. */ -int write_commit_graph_reachable(const char *obj_dir, int append, - int report_progress); +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags); int write_commit_graph(const char *obj_dir, struct string_list *pack_indexes, struct string_list *commit_hex, - int append, int report_progress); + unsigned int flags); int verify_commit_graph(struct repository *r, struct commit_graph *g); -- cgit v1.2.3 From 135a7123755bfdde05da18012bebd2776f82b26c Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 18 Jun 2019 11:14:28 -0700 Subject: commit-graph: add --split option to builtin Add a new "--split" option to the 'git commit-graph write' subcommand. This option allows the optional behavior of writing a commit-graph chain. The current behavior will add a tip commit-graph containing any commits that are not in the existing commit-graph or commit-graph chain. Later changes will allow merging the chain and expiring out-dated files. Add a new test script (t5324-split-commit-graph.sh) that demonstrates this behavior. Helped-by: Johannes Schindelin Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- builtin/commit-graph.c | 10 ++-- commit-graph.c | 14 +++-- t/t5324-split-commit-graph.sh | 122 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 8 deletions(-) create mode 100755 t/t5324-split-commit-graph.sh (limited to 'builtin/commit-graph.c') diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index d8efa5bab2..ff8bc3c8b9 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -10,7 +10,7 @@ static char const * const builtin_commit_graph_usage[] = { N_("git commit-graph [--object-dir ]"), N_("git commit-graph read [--object-dir ]"), N_("git commit-graph verify [--object-dir ]"), - N_("git commit-graph write [--object-dir ] [--append] [--reachable|--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits]"), NULL }; @@ -25,7 +25,7 @@ static const char * const builtin_commit_graph_read_usage[] = { }; static const char * const builtin_commit_graph_write_usage[] = { - N_("git commit-graph write [--object-dir ] [--append] [--reachable|--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits]"), NULL }; @@ -35,9 +35,9 @@ static struct opts_commit_graph { int stdin_packs; int stdin_commits; int append; + int split; } opts; - static int graph_verify(int argc, const char **argv) { struct commit_graph *graph = NULL; @@ -156,6 +156,8 @@ static int graph_write(int argc, const char **argv) N_("start walk at commits listed by stdin")), OPT_BOOL(0, "append", &opts.append, N_("include all commits already in the commit-graph file")), + OPT_BOOL(0, "split", &opts.split, + N_("allow writing an incremental commit-graph file")), OPT_END(), }; @@ -169,6 +171,8 @@ static int graph_write(int argc, const char **argv) opts.obj_dir = get_object_directory(); if (opts.append) flags |= COMMIT_GRAPH_APPEND; + if (opts.split) + flags |= COMMIT_GRAPH_SPLIT; read_replace_refs = 0; diff --git a/commit-graph.c b/commit-graph.c index f0698b0599..1224309e5f 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1472,12 +1472,16 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) } if (ctx->base_graph_name) { - result = rename(ctx->base_graph_name, - ctx->commit_graph_filenames_after[ctx->num_commit_graphs_after - 2]); + const char *dest = ctx->commit_graph_filenames_after[ + ctx->num_commit_graphs_after - 2]; - if (result) { - error(_("failed to rename base commit-graph file")); - return -1; + if (strcmp(ctx->base_graph_name, dest)) { + result = rename(ctx->base_graph_name, dest); + + if (result) { + error(_("failed to rename base commit-graph file")); + return -1; + } } } else { char *graph_name = get_commit_graph_filename(ctx->obj_dir); diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh new file mode 100755 index 0000000000..ccd24bd22b --- /dev/null +++ b/t/t5324-split-commit-graph.sh @@ -0,0 +1,122 @@ +#!/bin/sh + +test_description='split commit graph' +. ./test-lib.sh + +GIT_TEST_COMMIT_GRAPH=0 + +test_expect_success 'setup repo' ' + git init && + git config core.commitGraph true && + infodir=".git/objects/info" && + graphdir="$infodir/commit-graphs" && + test_oid_init +' + +graph_read_expect() { + NUM_BASE=0 + if test ! -z $2 + then + NUM_BASE=$2 + fi + cat >expect <<- EOF + header: 43475048 1 1 3 $NUM_BASE + num_commits: $1 + chunks: oid_fanout oid_lookup commit_metadata + EOF + git commit-graph read >output && + test_cmp expect output +} + +test_expect_success 'create commits and write commit-graph' ' + for i in $(test_seq 3) + do + test_commit $i && + git branch commits/$i || return 1 + done && + git commit-graph write --reachable && + test_path_is_file $infodir/commit-graph && + graph_read_expect 3 +' + +graph_git_two_modes() { + git -c core.commitGraph=true $1 >output + git -c core.commitGraph=false $1 >expect + test_cmp expect output +} + +graph_git_behavior() { + MSG=$1 + BRANCH=$2 + COMPARE=$3 + test_expect_success "check normal git operations: $MSG" ' + graph_git_two_modes "log --oneline $BRANCH" && + graph_git_two_modes "log --topo-order $BRANCH" && + graph_git_two_modes "log --graph $COMPARE..$BRANCH" && + graph_git_two_modes "branch -vv" && + graph_git_two_modes "merge-base -a $BRANCH $COMPARE" + ' +} + +graph_git_behavior 'graph exists' commits/3 commits/1 + +verify_chain_files_exist() { + for hash in $(cat $1/commit-graph-chain) + do + test_path_is_file $1/graph-$hash.graph || return 1 + done +} + +test_expect_success 'add more commits, and write a new base graph' ' + git reset --hard commits/1 && + for i in $(test_seq 4 5) + do + test_commit $i && + git branch commits/$i || return 1 + done && + git reset --hard commits/2 && + for i in $(test_seq 6 10) + do + test_commit $i && + git branch commits/$i || return 1 + done && + git reset --hard commits/2 && + git merge commits/4 && + git branch merge/1 && + git reset --hard commits/4 && + git merge commits/6 && + git branch merge/2 && + git commit-graph write --reachable && + graph_read_expect 12 +' + +test_expect_success 'add three more commits, write a tip graph' ' + git reset --hard commits/3 && + git merge merge/1 && + git merge commits/5 && + git merge merge/2 && + git branch merge/3 && + git commit-graph write --reachable --split && + test_path_is_missing $infodir/commit-graph && + test_path_is_file $graphdir/commit-graph-chain && + ls $graphdir/graph-*.graph >graph-files && + test_line_count = 2 graph-files && + verify_chain_files_exist $graphdir +' + +graph_git_behavior 'split commit-graph: merge 3 vs 2' merge/3 merge/2 + +test_expect_success 'add one commit, write a tip graph' ' + test_commit 11 && + git branch commits/11 && + git commit-graph write --reachable --split && + test_path_is_missing $infodir/commit-graph && + test_path_is_file $graphdir/commit-graph-chain && + ls $graphdir/graph-*.graph >graph-files && + test_line_count = 3 graph-files && + verify_chain_files_exist $graphdir +' + +graph_git_behavior 'three-layer commit-graph: commit 11 vs 6' commits/11 commits/6 + +test_done -- cgit v1.2.3 From c2bc6e6ab0ade70c475a73d06326e677e70840d2 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 18 Jun 2019 11:14:32 -0700 Subject: commit-graph: create options for split files The split commit-graph feature is now fully implemented, but needs some more run-time configurability. Allow direct callers to 'git commit-graph write --split' to specify the values used in the merge strategy and the expire time. Update the documentation to specify these values. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/git-commit-graph.txt | 21 +++++++++++++- Documentation/technical/commit-graph.txt | 7 +++-- builtin/commit-graph.c | 25 +++++++++++++---- builtin/commit.c | 2 +- builtin/gc.c | 3 +- commit-graph.c | 35 ++++++++++++++++-------- commit-graph.h | 12 ++++++-- t/t5324-split-commit-graph.sh | 47 ++++++++++++++++++++++++++++++++ 8 files changed, 128 insertions(+), 24 deletions(-) (limited to 'builtin/commit-graph.c') diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt index 624470e198..365e145e82 100644 --- a/Documentation/git-commit-graph.txt +++ b/Documentation/git-commit-graph.txt @@ -26,7 +26,7 @@ OPTIONS Use given directory for the location of packfiles and commit-graph file. This parameter exists to specify the location of an alternate that only has the objects directory, not a full `.git` directory. The - commit-graph file is expected to be at `/info/commit-graph` and + commit-graph file is expected to be in the `/info` directory and the packfiles are expected to be in `/pack`. @@ -51,6 +51,25 @@ or `--stdin-packs`.) + With the `--append` option, include all commits that are present in the existing commit-graph file. ++ +With the `--split` option, write the commit-graph as a chain of multiple +commit-graph files stored in `/info/commit-graphs`. The new commits +not already in the commit-graph are added in a new "tip" file. This file +is merged with the existing file if the following merge conditions are +met: ++ +* If `--size-multiple=` is not specified, let `X` equal 2. If the new +tip file would have `N` commits and the previous tip has `M` commits and +`X` times `N` is greater than `M`, instead merge the two files into a +single file. ++ +* If `--max-commits=` is specified with `M` a positive integer, and the +new tip file would have more than `M` commits, then instead merge the new +tip with the previous tip. ++ +Finally, if `--expire-time=` is not specified, let `datetime` +be the current time. After writing the split commit-graph, delete all +unused commit-graph whose modified times are older than `datetime`. 'read':: diff --git a/Documentation/technical/commit-graph.txt b/Documentation/technical/commit-graph.txt index aed4350a59..729fbcb32f 100644 --- a/Documentation/technical/commit-graph.txt +++ b/Documentation/technical/commit-graph.txt @@ -248,10 +248,11 @@ When writing a set of commits that do not exist in the commit-graph stack of height N, we default to creating a new file at level N + 1. We then decide to merge with the Nth level if one of two conditions hold: - 1. The expected file size for level N + 1 is at least half the file size for - level N. + 1. `--size-multiple=` is specified or X = 2, and the number of commits in + level N is less than X times the number of commits in level N + 1. - 2. Level N + 1 contains more than 64,0000 commits. + 2. `--max-commits=` is specified with non-zero C and the number of commits + in level N + 1 is more than C commits. This decision cascades down the levels: when we merge a level we create a new set of commits that then compares to the next level. diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index ff8bc3c8b9..6c0b5b17e0 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -10,7 +10,7 @@ static char const * const builtin_commit_graph_usage[] = { N_("git commit-graph [--object-dir ]"), N_("git commit-graph read [--object-dir ]"), N_("git commit-graph verify [--object-dir ]"), - N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] "), NULL }; @@ -25,7 +25,7 @@ static const char * const builtin_commit_graph_read_usage[] = { }; static const char * const builtin_commit_graph_write_usage[] = { - N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] "), NULL }; @@ -135,6 +135,7 @@ static int graph_read(int argc, const char **argv) } extern int read_replace_refs; +static struct split_commit_graph_opts split_opts; static int graph_write(int argc, const char **argv) { @@ -158,9 +159,19 @@ static int graph_write(int argc, const char **argv) N_("include all commits already in the commit-graph file")), OPT_BOOL(0, "split", &opts.split, N_("allow writing an incremental commit-graph file")), + OPT_INTEGER(0, "max-commits", &split_opts.max_commits, + N_("maximum number of commits in a non-base split commit-graph")), + OPT_INTEGER(0, "size-multiple", &split_opts.size_multiple, + N_("maximum ratio between two levels of a split commit-graph")), + OPT_EXPIRY_DATE(0, "expire-time", &split_opts.expire_time, + N_("maximum number of commits in a non-base split commit-graph")), OPT_END(), }; + split_opts.size_multiple = 2; + split_opts.max_commits = 0; + split_opts.expire_time = 0; + argc = parse_options(argc, argv, NULL, builtin_commit_graph_write_options, builtin_commit_graph_write_usage, 0); @@ -176,8 +187,11 @@ static int graph_write(int argc, const char **argv) read_replace_refs = 0; - if (opts.reachable) - return write_commit_graph_reachable(opts.obj_dir, flags); + if (opts.reachable) { + if (write_commit_graph_reachable(opts.obj_dir, flags, &split_opts)) + return 1; + return 0; + } string_list_init(&lines, 0); if (opts.stdin_packs || opts.stdin_commits) { @@ -197,7 +211,8 @@ static int graph_write(int argc, const char **argv) if (write_commit_graph(opts.obj_dir, pack_indexes, commit_hex, - flags)) + flags, + &split_opts)) result = 1; UNLEAK(lines); diff --git a/builtin/commit.c b/builtin/commit.c index b001ef565d..9216e9c043 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1670,7 +1670,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) "not exceeded, and then \"git reset HEAD\" to recover.")); if (git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) && - write_commit_graph_reachable(get_object_directory(), 0)) + write_commit_graph_reachable(get_object_directory(), 0, NULL)) return 1; repo_rerere(the_repository, 0); diff --git a/builtin/gc.c b/builtin/gc.c index 20c8f1bfe8..0aa8eac747 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -666,7 +666,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (gc_write_commit_graph && write_commit_graph_reachable(get_object_directory(), - !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0)) + !quiet && !daemonized ? COMMIT_GRAPH_PROGRESS : 0, + NULL)) return 1; if (auto_gc && too_many_loose_objects()) diff --git a/commit-graph.c b/commit-graph.c index 0cc2ceb349..315088d205 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -768,6 +768,8 @@ struct write_commit_graph_context { unsigned append:1, report_progress:1, split:1; + + const struct split_commit_graph_opts *split_opts; }; static void write_graph_chunk_fanout(struct hashfile *f, @@ -1116,14 +1118,15 @@ static int add_ref_to_list(const char *refname, return 0; } -int write_commit_graph_reachable(const char *obj_dir, unsigned int flags) +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags, + const struct split_commit_graph_opts *split_opts) { struct string_list list = STRING_LIST_INIT_DUP; int result; for_each_ref(add_ref_to_list, &list); result = write_commit_graph(obj_dir, NULL, &list, - flags); + flags, split_opts); string_list_clear(&list, 0); return result; @@ -1498,20 +1501,25 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) return 0; } -static int split_strategy_max_commits = 64000; -static float split_strategy_size_mult = 2.0f; - static void split_graph_merge_strategy(struct write_commit_graph_context *ctx) { struct commit_graph *g = ctx->r->objects->commit_graph; uint32_t num_commits = ctx->commits.nr; uint32_t i; + int max_commits = 0; + int size_mult = 2; + + if (ctx->split_opts) { + max_commits = ctx->split_opts->max_commits; + size_mult = ctx->split_opts->size_multiple; + } + g = ctx->r->objects->commit_graph; ctx->num_commit_graphs_after = ctx->num_commit_graphs_before + 1; - while (g && (g->num_commits <= split_strategy_size_mult * num_commits || - num_commits > split_strategy_max_commits)) { + while (g && (g->num_commits <= size_mult * num_commits || + (max_commits && num_commits > max_commits))) { if (strcmp(g->obj_dir, ctx->obj_dir)) break; @@ -1675,7 +1683,10 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) DIR *dir; struct dirent *de; size_t dirnamelen; - time_t expire_time = time(NULL); + timestamp_t expire_time = time(NULL); + + if (ctx->split_opts && ctx->split_opts->expire_time) + expire_time -= ctx->split_opts->expire_time; strbuf_addstr(&path, ctx->obj_dir); strbuf_addstr(&path, "/info/commit-graphs"); @@ -1719,7 +1730,8 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) int write_commit_graph(const char *obj_dir, struct string_list *pack_indexes, struct string_list *commit_hex, - unsigned int flags) + unsigned int flags, + const struct split_commit_graph_opts *split_opts) { struct write_commit_graph_context *ctx; uint32_t i, count_distinct = 0; @@ -1734,6 +1746,7 @@ int write_commit_graph(const char *obj_dir, ctx->append = flags & COMMIT_GRAPH_APPEND ? 1 : 0; ctx->report_progress = flags & COMMIT_GRAPH_PROGRESS ? 1 : 0; ctx->split = flags & COMMIT_GRAPH_SPLIT ? 1 : 0; + ctx->split_opts = split_opts; if (ctx->split) { struct commit_graph *g; @@ -1761,8 +1774,8 @@ int write_commit_graph(const char *obj_dir, ctx->approx_nr_objects = approximate_object_count(); ctx->oids.alloc = ctx->approx_nr_objects / 32; - if (ctx->split && ctx->oids.alloc > split_strategy_max_commits) - ctx->oids.alloc = split_strategy_max_commits; + if (ctx->split && split_opts && ctx->oids.alloc > split_opts->max_commits) + ctx->oids.alloc = split_opts->max_commits; if (ctx->append) { prepare_commit_graph_one(ctx->r, ctx->obj_dir); diff --git a/commit-graph.h b/commit-graph.h index 802d35254f..a84c22a560 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -75,17 +75,25 @@ int generation_numbers_enabled(struct repository *r); #define COMMIT_GRAPH_PROGRESS (1 << 1) #define COMMIT_GRAPH_SPLIT (1 << 2) +struct split_commit_graph_opts { + int size_multiple; + int max_commits; + timestamp_t expire_time; +}; + /* * The write_commit_graph* methods return zero on success * and a negative value on failure. Note that if the repository * is not compatible with the commit-graph feature, then the * methods will return 0 without writing a commit-graph. */ -int write_commit_graph_reachable(const char *obj_dir, unsigned int flags); +int write_commit_graph_reachable(const char *obj_dir, unsigned int flags, + const struct split_commit_graph_opts *split_opts); int write_commit_graph(const char *obj_dir, struct string_list *pack_indexes, struct string_list *commit_hex, - unsigned int flags); + unsigned int flags, + const struct split_commit_graph_opts *split_opts); int verify_commit_graph(struct repository *r, struct commit_graph *g); diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh index 76068ee407..1b699a543c 100755 --- a/t/t5324-split-commit-graph.sh +++ b/t/t5324-split-commit-graph.sh @@ -169,4 +169,51 @@ test_expect_success 'create fork and chain across alternate' ' graph_git_behavior 'alternate: commit 13 vs 6' commits/13 commits/6 +test_expect_success 'test merge stragety constants' ' + git clone . merge-2 && + ( + cd merge-2 && + git config core.commitGraph true && + test_line_count = 2 $graphdir/commit-graph-chain && + test_commit 14 && + git commit-graph write --reachable --split --size-multiple=2 && + test_line_count = 3 $graphdir/commit-graph-chain + + ) && + git clone . merge-10 && + ( + cd merge-10 && + git config core.commitGraph true && + test_line_count = 2 $graphdir/commit-graph-chain && + test_commit 14 && + git commit-graph write --reachable --split --size-multiple=10 && + test_line_count = 1 $graphdir/commit-graph-chain && + ls $graphdir/graph-*.graph >graph-files && + test_line_count = 1 graph-files + ) && + git clone . merge-10-expire && + ( + cd merge-10-expire && + git config core.commitGraph true && + test_line_count = 2 $graphdir/commit-graph-chain && + test_commit 15 && + git commit-graph write --reachable --split --size-multiple=10 --expire-time=1980-01-01 && + test_line_count = 1 $graphdir/commit-graph-chain && + ls $graphdir/graph-*.graph >graph-files && + test_line_count = 3 graph-files + ) && + git clone --no-hardlinks . max-commits && + ( + cd max-commits && + git config core.commitGraph true && + test_line_count = 2 $graphdir/commit-graph-chain && + test_commit 16 && + test_commit 17 && + git commit-graph write --reachable --split --max-commits=1 && + test_line_count = 1 $graphdir/commit-graph-chain && + ls $graphdir/graph-*.graph >graph-files && + test_line_count = 1 graph-files + ) +' + test_done -- cgit v1.2.3 From 3da4b609bb14b13672f64af908706462617f53cb Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 18 Jun 2019 11:14:32 -0700 Subject: commit-graph: verify chains with --shallow mode If we wrote a commit-graph chain, we only modified the tip file in the chain. It is valuable to verify what we wrote, but not waste time checking files we did not write. Add a '--shallow' option to the 'git commit-graph verify' subcommand and check that it does not read the base graph in a two-file chain. Making the verify subcommand read from a chain of commit-graphs takes some rearranging of the builtin code. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/git-commit-graph.txt | 5 ++- builtin/commit-graph.c | 27 ++++++++++++----- commit-graph.c | 15 +++++++-- commit-graph.h | 6 ++-- t/t5324-split-commit-graph.sh | 62 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 101 insertions(+), 14 deletions(-) (limited to 'builtin/commit-graph.c') diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt index 365e145e82..eb5e7865f0 100644 --- a/Documentation/git-commit-graph.txt +++ b/Documentation/git-commit-graph.txt @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] 'git commit-graph read' [--object-dir ] -'git commit-graph verify' [--object-dir ] +'git commit-graph verify' [--object-dir ] [--shallow] 'git commit-graph write' [--object-dir ] @@ -80,6 +80,9 @@ Used for debugging purposes. Read the commit-graph file and verify its contents against the object database. Used to check for corrupted data. ++ +With the `--shallow` option, only check the tip commit-graph file in +a chain of split commit-graphs. EXAMPLES diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 6c0b5b17e0..38027b83d9 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -5,17 +5,18 @@ #include "parse-options.h" #include "repository.h" #include "commit-graph.h" +#include "object-store.h" static char const * const builtin_commit_graph_usage[] = { N_("git commit-graph [--object-dir ]"), N_("git commit-graph read [--object-dir ]"), - N_("git commit-graph verify [--object-dir ]"), + N_("git commit-graph verify [--object-dir ] [--shallow]"), N_("git commit-graph write [--object-dir ] [--append|--split] [--reachable|--stdin-packs|--stdin-commits] "), NULL }; static const char * const builtin_commit_graph_verify_usage[] = { - N_("git commit-graph verify [--object-dir ]"), + N_("git commit-graph verify [--object-dir ] [--shallow]"), NULL }; @@ -36,6 +37,7 @@ static struct opts_commit_graph { int stdin_commits; int append; int split; + int shallow; } opts; static int graph_verify(int argc, const char **argv) @@ -45,11 +47,14 @@ static int graph_verify(int argc, const char **argv) int open_ok; int fd; struct stat st; + int flags = 0; static struct option builtin_commit_graph_verify_options[] = { OPT_STRING(0, "object-dir", &opts.obj_dir, N_("dir"), N_("The object directory to store the graph")), + OPT_BOOL(0, "shallow", &opts.shallow, + N_("if the commit-graph is split, only verify the tip file")), OPT_END(), }; @@ -59,21 +64,27 @@ static int graph_verify(int argc, const char **argv) if (!opts.obj_dir) opts.obj_dir = get_object_directory(); + if (opts.shallow) + flags |= COMMIT_GRAPH_VERIFY_SHALLOW; graph_name = get_commit_graph_filename(opts.obj_dir); open_ok = open_commit_graph(graph_name, &fd, &st); - if (!open_ok && errno == ENOENT) - return 0; - if (!open_ok) + if (!open_ok && errno != ENOENT) die_errno(_("Could not open commit-graph '%s'"), graph_name); - graph = load_commit_graph_one_fd_st(fd, &st); + FREE_AND_NULL(graph_name); + if (open_ok) + graph = load_commit_graph_one_fd_st(fd, &st); + else + graph = read_commit_graph_one(the_repository, opts.obj_dir); + + /* Return failure if open_ok predicted success */ if (!graph) - return 1; + return !!open_ok; UNLEAK(graph); - return verify_commit_graph(the_repository, graph); + return verify_commit_graph(the_repository, graph, flags); } static int graph_read(int argc, const char **argv) diff --git a/commit-graph.c b/commit-graph.c index 315088d205..f33f4fe009 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -428,7 +428,7 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r, const return graph_chain; } -static struct commit_graph *read_commit_graph_one(struct repository *r, const char *obj_dir) +struct commit_graph *read_commit_graph_one(struct repository *r, const char *obj_dir) { struct commit_graph *g = load_commit_graph_v1(r, obj_dir); @@ -1887,7 +1887,7 @@ static void graph_report(const char *fmt, ...) #define GENERATION_ZERO_EXISTS 1 #define GENERATION_NUMBER_EXISTS 2 -int verify_commit_graph(struct repository *r, struct commit_graph *g) +int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags) { uint32_t i, cur_fanout_pos = 0; struct object_id prev_oid, cur_oid, checksum; @@ -1895,6 +1895,7 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) struct hashfile *f; int devnull; struct progress *progress = NULL; + int local_error = 0; if (!g) { graph_report("no commit-graph file loaded"); @@ -1989,6 +1990,9 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) break; } + /* parse parent in case it is in a base graph */ + parse_commit_in_graph_one(r, g, graph_parents->item); + if (!oideq(&graph_parents->item->object.oid, &odb_parents->item->object.oid)) graph_report(_("commit-graph parent for %s is %s != %s"), oid_to_hex(&cur_oid), @@ -2040,7 +2044,12 @@ int verify_commit_graph(struct repository *r, struct commit_graph *g) } stop_progress(&progress); - return verify_commit_graph_error; + local_error = verify_commit_graph_error; + + if (!(flags & COMMIT_GRAPH_VERIFY_SHALLOW) && g->base_graph) + local_error |= verify_commit_graph(r, g->base_graph, flags); + + return local_error; } void free_commit_graph(struct commit_graph *g) diff --git a/commit-graph.h b/commit-graph.h index a84c22a560..df9a3b20e4 100644 --- a/commit-graph.h +++ b/commit-graph.h @@ -61,7 +61,7 @@ struct commit_graph { }; struct commit_graph *load_commit_graph_one_fd_st(int fd, struct stat *st); - +struct commit_graph *read_commit_graph_one(struct repository *r, const char *obj_dir); struct commit_graph *parse_commit_graph(void *graph_map, int fd, size_t graph_size); @@ -95,7 +95,9 @@ int write_commit_graph(const char *obj_dir, unsigned int flags, const struct split_commit_graph_opts *split_opts); -int verify_commit_graph(struct repository *r, struct commit_graph *g); +#define COMMIT_GRAPH_VERIFY_SHALLOW (1 << 0) + +int verify_commit_graph(struct repository *r, struct commit_graph *g, int flags); void close_commit_graph(struct raw_object_store *); void free_commit_graph(struct commit_graph *); diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh index 1b699a543c..3df90ae58f 100755 --- a/t/t5324-split-commit-graph.sh +++ b/t/t5324-split-commit-graph.sh @@ -216,4 +216,66 @@ test_expect_success 'test merge stragety constants' ' ) ' +corrupt_file() { + file=$1 + pos=$2 + data="${3:-\0}" + printf "$data" | dd of="$file" bs=1 seek="$pos" conv=notrunc +} + +test_expect_success 'verify hashes along chain, even in shallow' ' + git clone --no-hardlinks . verify && + ( + cd verify && + git commit-graph verify && + base_file=$graphdir/graph-$(head -n 1 $graphdir/commit-graph-chain).graph && + corrupt_file "$base_file" 1760 "\01" && + test_must_fail git commit-graph verify --shallow 2>test_err && + grep -v "^+" test_err >err && + test_i18ngrep "incorrect checksum" err + ) +' + +test_expect_success 'verify --shallow does not check base contents' ' + git clone --no-hardlinks . verify-shallow && + ( + cd verify-shallow && + git commit-graph verify && + base_file=$graphdir/graph-$(head -n 1 $graphdir/commit-graph-chain).graph && + corrupt_file "$base_file" 1000 "\01" && + git commit-graph verify --shallow && + test_must_fail git commit-graph verify 2>test_err && + grep -v "^+" test_err >err && + test_i18ngrep "incorrect checksum" err + ) +' + +test_expect_success 'warn on base graph chunk incorrect' ' + git clone --no-hardlinks . base-chunk && + ( + cd base-chunk && + git commit-graph verify && + base_file=$graphdir/graph-$(tail -n 1 $graphdir/commit-graph-chain).graph && + corrupt_file "$base_file" 1376 "\01" && + 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 && + ( + cd verify-chain && + corrupt_file "$graphdir/commit-graph-chain" 60 "G" && + 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 && + grep -v "^+" test_err >err && + test_i18ngrep "unable to find all commit-graph files" err + ) +' + test_done -- cgit v1.2.3