{"thread":{"id":"52933","subject":"[PATCH v3 3/3] builtin/commit-graph.c: support '--input=none'","startedAt":"2020-03-03T23:27:23Z","lastAt":"2020-04-01T22:49:18Z","messageCount":4,"participants":["Taylor Blau","Jeff King","SZEDER Gábor"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"392834","messageId":"7c47f37122ff2b03f0bccc48cb6ca022904f6353.1583278022.git.me@ttaylorr.com","threadId":"52933","inReplyTo":"4e85c6f7e40e7d6a8d93574645d65971b7cfa4f8.1581486293.git.me@ttaylorr.com/","subject":"[PATCH v3 3/3] builtin/commit-graph.c: support '--input=none'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-03-03T23:27:15Z","receivedAt":"2020-03-03T23:27:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In the previous commit, we introduced '--split=<no-merge|merge-all>',\nand alluded to the fact that '--split=merge-all' would be useful for\ncallers who wish to always trigger a merge of an incremental chain.\n\nThere is a problem with the above approach, which is that there is no\nway to specify to the commit-graph builtin that a caller only wants to\ninclude commits already in the graph. One can specify '--input=append'\nto include all commits in the existing graphs (that haven't since been\ndeleted from the object store), but the absence of\n'--input=stdin-{commits,packs}' causes the builtin to call\n'fill_oids_from_all_packs()'.\n\nPassing '--input=reachable' (as in 'git commit-graph write\n--split=merge-all --input=reachable --input=append') works around this\nissue by making '--input=reachable' effectively a no-op, but this can be\nprohibitively expensive in large repositories, making it an undesirable\nchoice for some users.\n\nTeach '--input=none' as an option to behave as if '--input=append' were\ngiven, but to consider no other sources in addition.\n\nThis, in conjunction with the option introduced in the previous patch\noffers the convenient way to force the commit-graph machinery to\ncondense a chain of incrementals without requiring any new commits:\n\n  $ git commit-graph write --split=merge-all --input=none\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-commit-graph.txt |  8 +++++++-\n builtin/commit-graph.c             | 11 ++++++++---\n commit-graph.c                     |  6 ++++--\n commit-graph.h                     |  3 ++-\n t/t5324-split-commit-graph.sh      | 26 ++++++++++++++++++++++++++\n 5 files changed, 47 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt\nindex 0a320cccdd..b210cef52f 100644\n--- a/Documentation/git-commit-graph.txt\n+++ b/Documentation/git-commit-graph.txt\n@@ -39,7 +39,7 @@ COMMANDS\n --------\n 'write'::\n\n-Write a commit-graph file based on the commits found in packfiles.\n+Write a commit-graph file based on the specified sources of input:\n +\n With the `--input=stdin-packs` option, generate the new commit graph by\n walking objects only in the specified pack-indexes. (Cannot be combined\n@@ -57,6 +57,12 @@ walking commits starting at all refs. (Cannot be combined with\n With the `--input=append` option, include all commits that are present\n in the existing commit-graph file.\n +\n+With the `--input=none` option, behave as if `--input=append` were\n+given, but do not walk other packs to find additional commits.\n++\n+If none of the above options are given, then generate the new\n+commit-graph by walking over all pack-indexes.\n++\n With the `--split[=<strategy>]` option, write the commit-graph as a\n chain of multiple commit-graph files stored in\n `<dir>/info/commit-graphs`. Commit-graph layers are merged based on the\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 0ff25896d0..a71af88815 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -11,7 +11,7 @@ static char const * const builtin_commit_graph_usage[] = {\n \tN_(\"git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]\"),\n \tN_(\"git commit-graph write [--object-dir <objdir>] \"\n \t   \"[--split[=<strategy>]] \"\n-\t   \"[--input=<reachable|stdin-packs|stdin-commits|append>] \"\n+\t   \"[--input=<reachable|stdin-packs|stdin-commits|append|none>] \"\n \t   \"[--[no-]progress] <split options>\"),\n \tNULL\n };\n@@ -24,7 +24,7 @@ static const char * const builtin_commit_graph_verify_usage[] = {\n static const char * const builtin_commit_graph_write_usage[] = {\n \tN_(\"git commit-graph write [--object-dir <objdir>] \"\n \t   \"[--split[=<strategy>]] \"\n-\t   \"[--input=<reachable|stdin-packs|stdin-commits|append>] \"\n+\t   \"[--input=<reachable|stdin-packs|stdin-commits|append|none>] \"\n \t   \"[--[no-]progress] <split options>\"),\n \tNULL\n };\n@@ -33,7 +33,8 @@ enum commit_graph_input {\n \tCOMMIT_GRAPH_INPUT_REACHABLE     = (1 << 1),\n \tCOMMIT_GRAPH_INPUT_STDIN_PACKS   = (1 << 2),\n \tCOMMIT_GRAPH_INPUT_STDIN_COMMITS = (1 << 3),\n-\tCOMMIT_GRAPH_INPUT_APPEND        = (1 << 4)\n+\tCOMMIT_GRAPH_INPUT_APPEND        = (1 << 4),\n+\tCOMMIT_GRAPH_INPUT_NONE          = (1 << 5)\n };\n\n static struct opts_commit_graph {\n@@ -80,6 +81,8 @@ static int option_parse_input(const struct option *opt, const char *arg,\n \t\t*to |= COMMIT_GRAPH_INPUT_STDIN_COMMITS;\n \telse if (!strcmp(arg, \"append\"))\n \t\t*to |= COMMIT_GRAPH_INPUT_APPEND;\n+\telse if (!strcmp(arg, \"none\"))\n+\t\t*to |= (COMMIT_GRAPH_INPUT_APPEND | COMMIT_GRAPH_INPUT_NONE);\n \telse\n \t\tdie(_(\"unrecognized --input source, %s\"), arg);\n \treturn 0;\n@@ -225,6 +228,8 @@ static int graph_write(int argc, const char **argv)\n \t\topts.obj_dir = get_object_directory();\n \tif (opts.input & COMMIT_GRAPH_INPUT_APPEND)\n \t\tflags |= COMMIT_GRAPH_WRITE_APPEND;\n+\tif (opts.input & COMMIT_GRAPH_INPUT_NONE)\n+\t\tflags |= COMMIT_GRAPH_WRITE_NO_INPUT;\n \tif (opts.split)\n \t\tflags |= COMMIT_GRAPH_WRITE_SPLIT;\n \tif (opts.progress)\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 3a5cb23cd7..417b7eac9c 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -788,7 +788,8 @@ struct write_commit_graph_context {\n \tunsigned append:1,\n \t\t report_progress:1,\n \t\t split:1,\n-\t\t check_oids:1;\n+\t\t check_oids:1,\n+\t\t no_input:1;\n\n \tconst struct split_commit_graph_opts *split_opts;\n };\n@@ -1785,6 +1786,7 @@ int write_commit_graph(struct object_directory *odb,\n \tctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;\n \tctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;\n \tctx->split_opts = split_opts;\n+\tctx->no_input = flags & COMMIT_GRAPH_WRITE_NO_INPUT ? 1 : 0;\n\n \tif (ctx->split) {\n \t\tstruct commit_graph *g;\n@@ -1843,7 +1845,7 @@ int write_commit_graph(struct object_directory *odb,\n \t\t\tgoto cleanup;\n \t}\n\n-\tif (!pack_indexes && !commit_hex)\n+\tif (!ctx->no_input && !pack_indexes && !commit_hex)\n \t\tfill_oids_from_all_packs(ctx);\n\n \tclose_reachable(ctx);\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 65a7d2edae..df7f3f5961 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -79,7 +79,8 @@ enum commit_graph_write_flags {\n \tCOMMIT_GRAPH_WRITE_PROGRESS   = (1 << 1),\n \tCOMMIT_GRAPH_WRITE_SPLIT      = (1 << 2),\n \t/* Make sure that each OID in the input is a valid commit OID. */\n-\tCOMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3)\n+\tCOMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3),\n+\tCOMMIT_GRAPH_WRITE_NO_INPUT   = (1 << 4)\n };\n\n enum commit_graph_split_flags {\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex 6894106727..7614f3915b 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -376,4 +376,30 @@ test_expect_success '--split=no-merge always writes an incremental' '\n \ttest_line_count = 2 $graphdir/commit-graph-chain\n '\n\n+test_expect_success '--split=no-merge, --input=none writes nothing' '\n+\ttest_when_finished rm -rf a graphs.before graphs.after &&\n+\trm -rf $graphdir &&\n+\tgit reset --hard commits/2 &&\n+\tgit rev-list -1 HEAD~1 >a &&\n+\tgit commit-graph write --split=no-merge --input=stdin-commits <a &&\n+\tls $graphdir/graph-*.graph >graphs.before &&\n+\ttest_line_count = 1 $graphdir/commit-graph-chain &&\n+\tgit commit-graph write --split --input=none &&\n+\tls $graphdir/graph-*.graph >graphs.after &&\n+\ttest_cmp graphs.before graphs.after\n+'\n+\n+test_expect_success '--split=merge-all, --input=none merges the chain' '\n+\ttest_when_finished rm -rf a b &&\n+\trm -rf $graphdir &&\n+\tgit reset --hard commits/2 &&\n+\tgit rev-list -1 HEAD~1 >a &&\n+\tgit rev-list -1 HEAD >b &&\n+\tgit commit-graph write --split=no-merge --input=stdin-commits <a &&\n+\tgit commit-graph write --split=no-merge --input=stdin-commits <b &&\n+\ttest_line_count = 2 $graphdir/commit-graph-chain &&\n+\tgit commit-graph write --split=merge-all --input=none &&\n+\ttest_line_count = 1 $graphdir/commit-graph-chain\n+'\n+\n test_done\n--\n2.24.1.2723.g1e7b7fcc88\n"},{"id":"393807","messageId":"e0f42a2f3c0162a5d43bb2bce0f69264b59f92e9.1584994172.git.me@ttaylorr.com","threadId":"52933","inReplyTo":"7c47f37122ff2b03f0bccc48cb6ca022904f6353.1583278022.git.me@ttaylorr.com","subject":"[PATCH v4] builtin/commit-graph.c: support '--input=graphed'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2020-03-23T20:12:19Z","receivedAt":"2020-03-23T20:12:26Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nIn response to some discussion in [1], here is another idea instead of\n'--input=none' that may make things a little clearer. Since it had been\na long time, I reminded myself that '--input=none' means\n\"--input=append, but don't look at packs as is usually the default\".\n\nIn [1], Gabor suggested that we could call this '--input=exists' or\n'--input=existing', but I think that 'graphed' may be clearer, since\nit is closer to \"only _graphed_ commits\".\n\nAnother option would be to call this '--input=only-graphed', but I think\nthat may be overly verbose for what we're going for here.\n\nLet me know what you think.\n\n[1]: https://lore.kernel.org/git/20200322110424.GC2224@szeder.dev/\n\n--- 8< ---\n\nIn the previous commit, we introduced '--split=<no-merge|merge-all>',\nand alluded to the fact that '--split=merge-all' would be useful for\ncallers who wish to always trigger a merge of an incremental chain.\n\nThere is a problem with the above approach, which is that there is no\nway to specify to the commit-graph builtin that a caller only wants to\ninclude commits already in the graph. One can specify '--input=append'\nto include all commits in the existing graphs, but the absence of\n'--input=stdin-{commits,packs}' causes the builtin to call\n'fill_oids_from_all_packs()'.\n\nPassing '--input=reachable' (as in 'git commit-graph write\n--split=merge-all --input=reachable --input=append') works around this\nissue by making '--input=reachable' effectively a no-op, but this can be\nprohibitively expensive in large repositories, making it an undesirable\nchoice for some users.\n\nTeach '--input=graphed' as an option to behave as if '--input=append' were\ngiven, but to consider no other sources in addition.\n\nThis, in conjunction with the option introduced in the previous patch\noffers the convenient way to force the commit-graph machinery to\ncondense a chain of incrementals without requiring any new commits:\n\n  $ git commit-graph write --split=merge-all --input=graphed\n\nSigned-off-by: Taylor Blau <me@ttaylorr.com>\n---\n Documentation/git-commit-graph.txt |  8 +++++++-\n builtin/commit-graph.c             | 11 ++++++++---\n commit-graph.c                     |  6 ++++--\n commit-graph.h                     |  3 ++-\n t/t5324-split-commit-graph.sh      | 26 ++++++++++++++++++++++++++\n 5 files changed, 47 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt\nindex 0a320cccdd..4d8fbbe8ff 100644\n--- a/Documentation/git-commit-graph.txt\n+++ b/Documentation/git-commit-graph.txt\n@@ -39,7 +39,7 @@ COMMANDS\n --------\n 'write'::\n\n-Write a commit-graph file based on the commits found in packfiles.\n+Write a commit-graph file based on the specified sources of input:\n +\n With the `--input=stdin-packs` option, generate the new commit graph by\n walking objects only in the specified pack-indexes. (Cannot be combined\n@@ -57,6 +57,12 @@ walking commits starting at all refs. (Cannot be combined with\n With the `--input=append` option, include all commits that are present\n in the existing commit-graph file.\n +\n+With the `--input=graphed` option, behave as if `--input=append` were\n+given, but do not walk other packs to find additional commits.\n++\n+If none of the above options are given, then generate the new\n+commit-graph by walking over all pack-indexes.\n++\n With the `--split[=<strategy>]` option, write the commit-graph as a\n chain of multiple commit-graph files stored in\n `<dir>/info/commit-graphs`. Commit-graph layers are merged based on the\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 0ff25896d0..dfb6c554ac 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -11,7 +11,7 @@ static char const * const builtin_commit_graph_usage[] = {\n \tN_(\"git commit-graph verify [--object-dir <objdir>] [--shallow] [--[no-]progress]\"),\n \tN_(\"git commit-graph write [--object-dir <objdir>] \"\n \t   \"[--split[=<strategy>]] \"\n-\t   \"[--input=<reachable|stdin-packs|stdin-commits|append>] \"\n+\t   \"[--input=<reachable|stdin-packs|stdin-commits|append|graphed>] \"\n \t   \"[--[no-]progress] <split options>\"),\n \tNULL\n };\n@@ -24,7 +24,7 @@ static const char * const builtin_commit_graph_verify_usage[] = {\n static const char * const builtin_commit_graph_write_usage[] = {\n \tN_(\"git commit-graph write [--object-dir <objdir>] \"\n \t   \"[--split[=<strategy>]] \"\n-\t   \"[--input=<reachable|stdin-packs|stdin-commits|append>] \"\n+\t   \"[--input=<reachable|stdin-packs|stdin-commits|append|graphed>] \"\n \t   \"[--[no-]progress] <split options>\"),\n \tNULL\n };\n@@ -33,7 +33,8 @@ enum commit_graph_input {\n \tCOMMIT_GRAPH_INPUT_REACHABLE     = (1 << 1),\n \tCOMMIT_GRAPH_INPUT_STDIN_PACKS   = (1 << 2),\n \tCOMMIT_GRAPH_INPUT_STDIN_COMMITS = (1 << 3),\n-\tCOMMIT_GRAPH_INPUT_APPEND        = (1 << 4)\n+\tCOMMIT_GRAPH_INPUT_APPEND        = (1 << 4),\n+\tCOMMIT_GRAPH_INPUT_GRAPHED       = (1 << 5)\n };\n\n static struct opts_commit_graph {\n@@ -80,6 +81,8 @@ static int option_parse_input(const struct option *opt, const char *arg,\n \t\t*to |= COMMIT_GRAPH_INPUT_STDIN_COMMITS;\n \telse if (!strcmp(arg, \"append\"))\n \t\t*to |= COMMIT_GRAPH_INPUT_APPEND;\n+\telse if (!strcmp(arg, \"graphed\"))\n+\t\t*to |= (COMMIT_GRAPH_INPUT_APPEND | COMMIT_GRAPH_INPUT_GRAPHED);\n \telse\n \t\tdie(_(\"unrecognized --input source, %s\"), arg);\n \treturn 0;\n@@ -225,6 +228,8 @@ static int graph_write(int argc, const char **argv)\n \t\topts.obj_dir = get_object_directory();\n \tif (opts.input & COMMIT_GRAPH_INPUT_APPEND)\n \t\tflags |= COMMIT_GRAPH_WRITE_APPEND;\n+\tif (opts.input & COMMIT_GRAPH_INPUT_GRAPHED)\n+\t\tflags |= COMMIT_GRAPH_WRITE_NO_INPUT;\n \tif (opts.split)\n \t\tflags |= COMMIT_GRAPH_WRITE_SPLIT;\n \tif (opts.progress)\ndiff --git a/commit-graph.c b/commit-graph.c\nindex c5a8ea244b..3da52847e4 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -788,7 +788,8 @@ struct write_commit_graph_context {\n \tunsigned append:1,\n \t\t report_progress:1,\n \t\t split:1,\n-\t\t check_oids:1;\n+\t\t check_oids:1,\n+\t\t no_input:1;\n\n \tconst struct split_commit_graph_opts *split_opts;\n };\n@@ -1781,6 +1782,7 @@ int write_commit_graph(struct object_directory *odb,\n \tctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;\n \tctx->check_oids = flags & COMMIT_GRAPH_WRITE_CHECK_OIDS ? 1 : 0;\n \tctx->split_opts = split_opts;\n+\tctx->no_input = flags & COMMIT_GRAPH_WRITE_NO_INPUT ? 1 : 0;\n\n \tif (ctx->split) {\n \t\tstruct commit_graph *g;\n@@ -1839,7 +1841,7 @@ int write_commit_graph(struct object_directory *odb,\n \t\t\tgoto cleanup;\n \t}\n\n-\tif (!pack_indexes && !commit_hex)\n+\tif (!ctx->no_input && !pack_indexes && !commit_hex)\n \t\tfill_oids_from_all_packs(ctx);\n\n \tclose_reachable(ctx);\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 65a7d2edae..df7f3f5961 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -79,7 +79,8 @@ enum commit_graph_write_flags {\n \tCOMMIT_GRAPH_WRITE_PROGRESS   = (1 << 1),\n \tCOMMIT_GRAPH_WRITE_SPLIT      = (1 << 2),\n \t/* Make sure that each OID in the input is a valid commit OID. */\n-\tCOMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3)\n+\tCOMMIT_GRAPH_WRITE_CHECK_OIDS = (1 << 3),\n+\tCOMMIT_GRAPH_WRITE_NO_INPUT   = (1 << 4)\n };\n\n enum commit_graph_split_flags {\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex 6894106727..6dda4c1f1c 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -376,4 +376,30 @@ test_expect_success '--split=no-merge always writes an incremental' '\n \ttest_line_count = 2 $graphdir/commit-graph-chain\n '\n\n+test_expect_success '--split=no-merge, --input=graphed writes nothing' '\n+\ttest_when_finished rm -rf a graphs.before graphs.after &&\n+\trm -rf $graphdir &&\n+\tgit reset --hard commits/2 &&\n+\tgit rev-list -1 HEAD~1 >a &&\n+\tgit commit-graph write --split=no-merge --input=stdin-commits <a &&\n+\tls $graphdir/graph-*.graph >graphs.before &&\n+\ttest_line_count = 1 $graphdir/commit-graph-chain &&\n+\tgit commit-graph write --split --input=graphed &&\n+\tls $graphdir/graph-*.graph >graphs.after &&\n+\ttest_cmp graphs.before graphs.after\n+'\n+\n+test_expect_success '--split=merge-all, --input=graphed merges the chain' '\n+\ttest_when_finished rm -rf a b &&\n+\trm -rf $graphdir &&\n+\tgit reset --hard commits/2 &&\n+\tgit rev-list -1 HEAD~1 >a &&\n+\tgit rev-list -1 HEAD >b &&\n+\tgit commit-graph write --split=no-merge --input=stdin-commits <a &&\n+\tgit commit-graph write --split=no-merge --input=stdin-commits <b &&\n+\ttest_line_count = 2 $graphdir/commit-graph-chain &&\n+\tgit commit-graph write --split=merge-all --input=graphed &&\n+\ttest_line_count = 1 $graphdir/commit-graph-chain\n+'\n+\n test_done\n--\n2.26.0.rc2.311.g2e49f7a131\n"},{"id":"394161","messageId":"20200327091336.GC610157@coredump.intra.peff.net","threadId":"52933","inReplyTo":"e0f42a2f3c0162a5d43bb2bce0f69264b59f92e9.1584994172.git.me@ttaylorr.com","subject":"Re: [PATCH v4] builtin/commit-graph.c: support '--input=graphed'","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-27T09:13:36Z","receivedAt":"2020-03-27T09:13:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 23, 2020 at 02:12:19PM -0600, Taylor Blau wrote:\n\n> Hi,\n> \n> In response to some discussion in [1], here is another idea instead of\n> '--input=none' that may make things a little clearer. Since it had been\n> a long time, I reminded myself that '--input=none' means\n> \"--input=append, but don't look at packs as is usually the default\".\n> \n> In [1], Gabor suggested that we could call this '--input=exists' or\n> '--input=existing', but I think that 'graphed' may be clearer, since\n> it is closer to \"only _graphed_ commits\".\n\nYeah, this name is much more clear to me than \"none\" (and would have\nprevented some mild confusion I had when experimenting the other day).\n\n> Another option would be to call this '--input=only-graphed', but I think\n> that may be overly verbose for what we're going for here.\n\nAgreed.\n\n-Peff\n"},{"id":"394532","messageId":"20200401224912.GI2224@szeder.dev","threadId":"52933","inReplyTo":"e0f42a2f3c0162a5d43bb2bce0f69264b59f92e9.1584994172.git.me@ttaylorr.com","subject":"Re: [PATCH v4] builtin/commit-graph.c: support '--input=graphed'","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2020-04-01T22:49:12Z","receivedAt":"2020-04-01T22:49:18Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Mar 23, 2020 at 02:12:19PM -0600, Taylor Blau wrote:\n> Hi,\n> \n> In response to some discussion in [1], here is another idea instead of\n> '--input=none' that may make things a little clearer. Since it had been\n> a long time, I reminded myself that '--input=none' means\n> \"--input=append, but don't look at packs as is usually the default\".\n> \n> In [1], Gabor suggested that we could call this '--input=exists' or\n> '--input=existing', but I think that 'graphed' may be clearer, since\n> it is closer to \"only _graphed_ commits\".\n> \n> Another option would be to call this '--input=only-graphed', but I think\n> that may be overly verbose for what we're going for here.\n> \n> Let me know what you think.\n> \n> [1]: https://lore.kernel.org/git/20200322110424.GC2224@szeder.dev/\n> \n> --- 8< ---\n> \n> In the previous commit, we introduced '--split=<no-merge|merge-all>',\n> and alluded to the fact that '--split=merge-all' would be useful for\n> callers who wish to always trigger a merge of an incremental chain.\n> \n> There is a problem with the above approach, which is that there is no\n> way to specify to the commit-graph builtin that a caller only wants to\n> include commits already in the graph. One can specify '--input=append'\n> to include all commits in the existing graphs, but the absence of\n> '--input=stdin-{commits,packs}' causes the builtin to call\n> 'fill_oids_from_all_packs()'.\n> \n> Passing '--input=reachable' (as in 'git commit-graph write\n> --split=merge-all --input=reachable --input=append') works around this\n> issue by making '--input=reachable' effectively a no-op, but this can be\n> prohibitively expensive in large repositories, making it an undesirable\n> choice for some users.\n> \n> Teach '--input=graphed' as an option to behave as if '--input=append' were\n> given, but to consider no other sources in addition.\n> \n> This, in conjunction with the option introduced in the previous patch\n> offers the convenient way to force the commit-graph machinery to\n> condense a chain of incrementals without requiring any new commits:\n> \n>   $ git commit-graph write --split=merge-all --input=graphed\n> \n> Signed-off-by: Taylor Blau <me@ttaylorr.com>\n> ---\n>  Documentation/git-commit-graph.txt |  8 +++++++-\n>  builtin/commit-graph.c             | 11 ++++++++---\n>  commit-graph.c                     |  6 ++++--\n>  commit-graph.h                     |  3 ++-\n>  t/t5324-split-commit-graph.sh      | 26 ++++++++++++++++++++++++++\n>  5 files changed, 47 insertions(+), 7 deletions(-)\n> \n> diff --git a/Documentation/git-commit-graph.txt b/Documentation/git-commit-graph.txt\n> index 0a320cccdd..4d8fbbe8ff 100644\n> --- a/Documentation/git-commit-graph.txt\n> +++ b/Documentation/git-commit-graph.txt\n> @@ -39,7 +39,7 @@ COMMANDS\n>  --------\n>  'write'::\n> \n> -Write a commit-graph file based on the commits found in packfiles.\n> +Write a commit-graph file based on the specified sources of input:\n>  +\n>  With the `--input=stdin-packs` option, generate the new commit graph by\n>  walking objects only in the specified pack-indexes. (Cannot be combined\n> @@ -57,6 +57,12 @@ walking commits starting at all refs. (Cannot be combined with\n>  With the `--input=append` option, include all commits that are present\n>  in the existing commit-graph file.\n>  +\n> +With the `--input=graphed` option, behave as if `--input=append` were\n> +given, but do not walk other packs to find additional commits.\n\ns/walk/scan/ would be more fitting, I think.\n\nIn an earlier version of these patches I asked for clarification about\nwhat happens with expired commits that are still included in the\ncommit-graph... and I do remember that you replied to that, but,\nunfortunately, not what your reply was.  And after reading this log\nmessage and the documentation update it's still not clear to me.\n\n> ++\n> +If none of the above options are given, then generate the new\n> +commit-graph by walking over all pack-indexes.\n\ns/walking/scanning/\n\n> ++\n>  With the `--split[=<strategy>]` option, write the commit-graph as a\n>  chain of multiple commit-graph files stored in\n>  `<dir>/info/commit-graphs`. Commit-graph layers are merged based on the\n"}]}