Re: [RFC PATCH 10/12] commit-graph: add '--reachable' option
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Apr 20, 2018, 17:17 UTC
- Message-ID
- <864lk5er1j.fsf@gmail.com>
- In-Reply-To
- <20180417181028.198397-11-dstolee@microsoft.com>
Derrick Stolee <dstolee@microsoft.com> writes:
Show 8 quoted lines
> When writing commit-graph files, it can be convenient to ask for all > reachable commits (starting at the ref set) in the resulting file. This > is particularly helpful when writing to stdin is complicated, such as a > future integration with 'git gc' which will call > 'git commit-graph write --reachable' after performing cleanup of the > object database. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
For what it is worth, it looks good to me.
Show 26 quoted lines
> --- > Documentation/git-commit-graph.txt | 8 ++++-- > builtin/commit-graph.c | 41 +++++++++++++++++++++++++++--- > t/t5318-commit-graph.sh | 10 ++++++++ > 3 files changed, 53 insertions(+), 6 deletions(-) > > diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt > index 93c7841ba2..1b14d40590 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -37,12 +37,16 @@ Write a commit graph file based on the commits found in packfiles. > + > With the `--stdin-packs` option, generate the new commit graph by > walking objects only in the specified pack-indexes. (Cannot be combined > -with --stdin-commits.) > +with --stdin-commits or --reachable.) > + > With the `--stdin-commits` option, generate the new commit graph by > walking commits starting at the commits specified in stdin as a list > of OIDs in hex, one OID per line. (Cannot be combined with > ---stdin-packs.) > +--stdin-packs or --reachable.) > ++ > +With the `--reachable` option, generate the new commit graph by walking > +commits starting at all refs. (Cannot be combined with --stdin-commits > +or --stind-packs.)
I wonder if this "cannot be combined" is sustainable... ;-)
[...]
Show 43 quoted lines
> @@ -113,6 +115,25 @@ static int graph_read(int argc, const char **argv)
> return 0;
> }
>
> +struct hex_list {
> + char **hex_strs;
> + int hex_nr;
> + int hex_alloc;
> +};
> +
> +static int add_ref_to_list(const char *refname,
> + const struct object_id *oid,
> + int flags, void *cb_data)
> +{
> + struct hex_list *list = (struct hex_list*)cb_data;
> +
> + ALLOC_GROW(list->hex_strs, list->hex_nr + 1, list->hex_alloc);
> + list->hex_strs[list->hex_nr] = xcalloc(GIT_MAX_HEXSZ + 1, 1);
> + strcpy(list->hex_strs[list->hex_nr], oid_to_hex(oid));
> + list->hex_nr++;
> + return 0;
> +}
> +
> static int graph_write(int argc, const char **argv)
> {
> const char **pack_indexes = NULL;
> @@ -127,6 +148,8 @@ static int graph_write(int argc, const char **argv)
> OPT_STRING(0, "object-dir", &opts.obj_dir,
> N_("dir"),
> N_("The object directory to store the graph")),
> + OPT_BOOL(0, "reachable", &opts.reachable,
> + N_("start walk at all refs")),
> OPT_BOOL(0, "stdin-packs", &opts.stdin_packs,
> N_("scan pack-indexes listed by stdin for commits")),
> OPT_BOOL(0, "stdin-commits", &opts.stdin_commits,
> @@ -140,8 +163,8 @@ static int graph_write(int argc, const char **argv)
> builtin_commit_graph_write_options,
> builtin_commit_graph_write_usage, 0);
>
> - if (opts.stdin_packs && opts.stdin_commits)
> - die(_("cannot use both --stdin-commits and --stdin-packs"));
> + if (opts.reachable + opts.stdin_packs + opts.stdin_commits > 1)
> + die(_("use at most one of --reachable, --stdin-commits, or --stdin-packs"));Nice trick, but is it worth it (in place of boolean expression)? Though it lines up with the error message, though...
Show 17 quoted lines
> if (!opts.obj_dir)
> opts.obj_dir = get_object_directory();
>
> @@ -164,6 +187,16 @@ static int graph_write(int argc, const char **argv)
> commit_hex = lines;
> commits_nr = lines_nr;
> }
> + } else if (opts.reachable) {
> + struct hex_list list;
> + list.hex_nr = 0;
> + list.hex_alloc = 128;
> + ALLOC_ARRAY(list.hex_strs, list.hex_alloc);
> +
> + for_each_ref(add_ref_to_list, &list);
> +
> + commit_hex = (const char **)list.hex_strs;
> + commits_nr = list.hex_nr;Nice trick!
Show 24 quoted lines
> } > > write_commit_graph(opts.obj_dir, > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh > index e91053271a..ccadd22f57 100755 > --- a/t/t5318-commit-graph.sh > +++ b/t/t5318-commit-graph.sh > @@ -200,6 +200,16 @@ test_expect_success 'build graph from commits with append' ' > graph_git_behavior 'append graph, commit 8 vs merge 1' full commits/8 merge/1 > graph_git_behavior 'append graph, commit 8 vs merge 2' full commits/8 merge/2 > > +test_expect_success 'build graph using --reachable' ' > + cd "$TRASH_DIRECTORY/full" && > + git commit-graph write --reachable && > + test_path_is_file $objdir/info/commit-graph && > + graph_read_expect "11" "large_edges" > +' > + > +graph_git_behavior 'append graph, commit 8 vs merge 1' full commits/8 merge/1 > +graph_git_behavior 'append graph, commit 8 vs merge 2' full commits/8 merge/2 > + > test_expect_success 'setup bare repo' ' > cd "$TRASH_DIRECTORY" && > git clone --bare --no-local full bare &&