Re: [PATCH v3 18/20] commit-graph: add '--reachable' option
On 6/2/2018 1:34 PM, Jakub Narebski wrote: Derrick Stolee writes: 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 write_commit_graph_reachable() after performing cleanup of the object database. Nice. The last sentence of the commit message is a bit long, though, in my opinion. Signed-off-by: Derrick Stolee --- Documentation/git-commit-graph.txt | 8 ++-- builtin/commit-graph.c | 16 commit-graph.c | 32 commit-graph.h | 1 + t/t5318-commit-graph.sh| 10 ++ 5 files changed, 61 insertions(+), 6 deletions(-) diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt index a222cfab08..dececb79d7 100644 --- a/Documentation/git-commit-graph.txt +++ b/Documentation/git-commit-graph.txt @@ -38,12 +38,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 `--stdin-packs`.) All right (though I wonder a bit about the restriction). I think it might be a good idea to describe all of this in the usage string for the 'git commit-graph write', instead of using '' placeholder, that is instead of current: 'git commit-graph write' [--object-dir ] use 'git commit-graph write' [--stdin-commits | --stdin-packs | --reachable] [--append] [--object-dir ] or something like that. + With the `--append` option, include all commits that are present in the existing commit-graph file. diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 0433dd6e20..20ce6437ae 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -9,7 +9,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] [--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append] [--reachable|--stdin-packs|--stdin-commits]"), All right, very straightforward. I guess they are put in [almost] alphabetical order, or is there some other reasoning behind the ordering used (which is different from the one in the manpage)? NULL }; @@ -24,12 +24,13 @@ 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] [--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append] [--reachable|--stdin-packs|--stdin-commits]"), The same. NULL }; static struct opts_commit_graph { const char *obj_dir; + int reachable; int stdin_packs; int stdin_commits; int append; @@ -130,6 +131,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")), Errr... does '--no-reachable' makes sense? Because if I am right currently it is supported, isn't it. True, but the same holds for many arguments in the codebase. Just looking at 'git apply' as a sample, it uses OPT_BOOL for "numstat", "summary", "check", "index", "cached", "apply", and "no-add" (so "--no-no-add" works?). I think accepting "--no-reachable" is fine, since it will set opts.reachable = 0 and we will not use the reachable option. 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, @@ -143,11 +146,16 @@ 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_comm
Re: [PATCH v3 18/20] commit-graph: add '--reachable' option
Derrick Stolee writes: > 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 > write_commit_graph_reachable() after performing cleanup of the object > database. Nice. The last sentence of the commit message is a bit long, though, in my opinion. > > Signed-off-by: Derrick Stolee > --- > Documentation/git-commit-graph.txt | 8 ++-- > builtin/commit-graph.c | 16 > commit-graph.c | 32 > commit-graph.h | 1 + > t/t5318-commit-graph.sh| 10 ++ > 5 files changed, 61 insertions(+), 6 deletions(-) > > diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > index a222cfab08..dececb79d7 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -38,12 +38,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 `--stdin-packs`.) All right (though I wonder a bit about the restriction). I think it might be a good idea to describe all of this in the usage string for the 'git commit-graph write', instead of using '' placeholder, that is instead of current: 'git commit-graph write' [--object-dir ] use 'git commit-graph write' [--stdin-commits | --stdin-packs | --reachable] [--append] [--object-dir ] or something like that. > + > With the `--append` option, include all commits that are present in the > existing commit-graph file. > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > index 0433dd6e20..20ce6437ae 100644 > --- a/builtin/commit-graph.c > +++ b/builtin/commit-graph.c > @@ -9,7 +9,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] > [--stdin-packs|--stdin-commits]"), > + N_("git commit-graph write [--object-dir ] [--append] > [--reachable|--stdin-packs|--stdin-commits]"), All right, very straightforward. I guess they are put in [almost] alphabetical order, or is there some other reasoning behind the ordering used (which is different from the one in the manpage)? > NULL > }; > > @@ -24,12 +24,13 @@ 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] > [--stdin-packs|--stdin-commits]"), > + N_("git commit-graph write [--object-dir ] [--append] > [--reachable|--stdin-packs|--stdin-commits]"), The same. > NULL > }; > > static struct opts_commit_graph { > const char *obj_dir; > + int reachable; > int stdin_packs; > int stdin_commits; > int append; > @@ -130,6 +131,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")), Errr... does '--no-reachable' makes sense? Because if I am right currently it is supported, isn't it. > 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, > @@ -143,11 +146,16 @@ 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) Nice trick. > + die(_("use at most one of --reachable, --stdin-commits, or > --stdin-packs")); It is a pity that parseopt API does not
[PATCH v3 18/20] commit-graph: add '--reachable' option
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 write_commit_graph_reachable() after performing cleanup of the object database. Signed-off-by: Derrick Stolee --- Documentation/git-commit-graph.txt | 8 ++-- builtin/commit-graph.c | 16 commit-graph.c | 32 commit-graph.h | 1 + t/t5318-commit-graph.sh| 10 ++ 5 files changed, 61 insertions(+), 6 deletions(-) diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt index a222cfab08..dececb79d7 100644 --- a/Documentation/git-commit-graph.txt +++ b/Documentation/git-commit-graph.txt @@ -38,12 +38,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 `--stdin-packs`.) + With the `--append` option, include all commits that are present in the existing commit-graph file. diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 0433dd6e20..20ce6437ae 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -9,7 +9,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] [--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append] [--reachable|--stdin-packs|--stdin-commits]"), NULL }; @@ -24,12 +24,13 @@ 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] [--stdin-packs|--stdin-commits]"), + N_("git commit-graph write [--object-dir ] [--append] [--reachable|--stdin-packs|--stdin-commits]"), NULL }; static struct opts_commit_graph { const char *obj_dir; + int reachable; int stdin_packs; int stdin_commits; int append; @@ -130,6 +131,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, @@ -143,11 +146,16 @@ 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")); if (!opts.obj_dir) opts.obj_dir = get_object_directory(); + if (opts.reachable) { + write_commit_graph_reachable(opts.obj_dir, opts.append); + return 0; + } + if (opts.stdin_packs || opts.stdin_commits) { struct strbuf buf = STRBUF_INIT; lines_nr = 0; diff --git a/commit-graph.c b/commit-graph.c index a33600c584..057d734926 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -6,6 +6,7 @@ #include "packfile.h" #include "commit.h" #include "object.h" +#include "refs.h" #include "revision.h" #include "sha1-lookup.h" #include "commit-graph.h" @@ -651,6 +652,37 @@ static void compute_generation_numbers(struct packed_commit_list* commits) } } +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_