{"thread":{"id":"55213","subject":"[PATCH 0/2] Specify generation number version by config","startedAt":"2021-02-25T18:20:43Z","lastAt":"2021-02-25T18:20:43Z","messageCount":3,"participants":["Derrick Stolee via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"417827","messageId":"pull.886.git.1614277183.gitgitgadget@gmail.com","threadId":"55213","inReplyTo":null,"subject":"[PATCH 0/2] Specify generation number version by config","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-25T18:19:41Z","receivedAt":"2021-02-25T18:20:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"This is based on ds/chunked-file-api.\n\nGeneration number v2 (corrected commit dates) was introduced by\nak/corrected-commit-date. The new data could be opted-out only using the\nGIT_TEST_COMMIT_GRAPH_NO_GDAT environment variable.\n\nAs this feature gets deployed more widely, servers in particular might want\nmore control over when this format is used. Add a config option instead.\n\nThanks, -Stolee\n\nDerrick Stolee (2):\n  commit-graph: create local repository pointer\n  commit-graph: use config to specify generation type\n\n Documentation/config/commitgraph.txt |  6 ++++++\n commit-graph.c                       | 31 ++++++++++++++++++----------\n commit-graph.h                       |  1 -\n t/README                             |  3 ---\n t/t5318-commit-graph.sh              |  2 +-\n t/t5324-split-commit-graph.sh        |  4 ++--\n t/t6600-test-reach.sh                |  2 +-\n 7 files changed, 30 insertions(+), 19 deletions(-)\n\n\nbase-commit: c4ff24bbb354377a6a7937744fbbef2898243fc7\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-886%2Fderrickstolee%2Fcommit-graph-generation-config-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-886/derrickstolee/commit-graph-generation-config-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/886\n-- \ngitgitgadget\n"},{"id":"417828","messageId":"29b4ac5594dd039ee76f9f1b8630f13482aaa8f9.1614277183.git.gitgitgadget@gmail.com","threadId":"55213","inReplyTo":"pull.886.git.1614277183.gitgitgadget@gmail.com","subject":"[PATCH 1/2] commit-graph: create local repository pointer","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-25T18:19:42Z","receivedAt":"2021-02-25T18:20:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nThe write_commit_graph() method uses 'the_repository' in a few places. A\nnew need for a repository pointer is coming in the following change, so\ngroup these instances into a local variable 'r' that could eventually\nbecome part of the method signature, if so desired.\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n commit-graph.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex b9efeddeab6c..8b07a9a0a6df 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -2155,6 +2155,7 @@ int write_commit_graph(struct object_directory *odb,\n \t\t       enum commit_graph_write_flags flags,\n \t\t       const struct commit_graph_opts *opts)\n {\n+\tstruct repository *r = the_repository;\n \tstruct write_commit_graph_context *ctx;\n \tuint32_t i;\n \tint res = 0;\n@@ -2162,16 +2163,16 @@ int write_commit_graph(struct object_directory *odb,\n \tstruct bloom_filter_settings bloom_settings = DEFAULT_BLOOM_FILTER_SETTINGS;\n \tstruct topo_level_slab topo_levels;\n \n-\tprepare_repo_settings(the_repository);\n-\tif (!the_repository->settings.core_commit_graph) {\n+\tprepare_repo_settings(r);\n+\tif (!r->settings.core_commit_graph) {\n \t\twarning(_(\"attempting to write a commit-graph, but 'core.commitGraph' is disabled\"));\n \t\treturn 0;\n \t}\n-\tif (!commit_graph_compatible(the_repository))\n+\tif (!commit_graph_compatible(r))\n \t\treturn 0;\n \n \tctx = xcalloc(1, sizeof(struct write_commit_graph_context));\n-\tctx->r = the_repository;\n+\tctx->r = r;\n \tctx->odb = odb;\n \tctx->append = flags & COMMIT_GRAPH_WRITE_APPEND ? 1 : 0;\n \tctx->report_progress = flags & COMMIT_GRAPH_WRITE_PROGRESS ? 1 : 0;\n-- \ngitgitgadget\n\n"},{"id":"417829","messageId":"8eca11e6d4c9c1796418c28f13301223c11da01a.1614277183.git.gitgitgadget@gmail.com","threadId":"55213","inReplyTo":"pull.886.git.1614277183.gitgitgadget@gmail.com","subject":"[PATCH 2/2] commit-graph: use config to specify generation type","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-25T18:19:43Z","receivedAt":"2021-02-25T18:20:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee <dstolee@microsoft.com>\n\nWe have two established generation number versions:\n\n 1: topological levels\n 2: corrected commit dates\n\nThe corrected commit dates are enabled by default, but they also write\nextra data in the GDAT and GDOV chunks. Services that host Git data\nmight want to have more control over when this feature rolls out than\njust updating the Git binaries.\n\nAdd a new \"commitGraph.generationVersion\" config option that specifies\nthe intended generation number version. If this value is less than 2,\nthen the GDAT chunk is never written _or read_ from an existing file.\n\nThis can replace our use of the GIT_TEST_COMMIT_GRAPH_NO_GDAT\nenvironment variable in the test suite. Remove it.\n\nSigned-off-by: Derrick Stolee <dstolee@microsoft.com>\n---\n Documentation/config/commitgraph.txt |  6 ++++++\n commit-graph.c                       | 22 +++++++++++++++-------\n commit-graph.h                       |  1 -\n t/README                             |  3 ---\n t/t5318-commit-graph.sh              |  2 +-\n t/t5324-split-commit-graph.sh        |  4 ++--\n t/t6600-test-reach.sh                |  2 +-\n 7 files changed, 25 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/config/commitgraph.txt b/Documentation/config/commitgraph.txt\nindex 4582c39fc462..30604e4a4c29 100644\n--- a/Documentation/config/commitgraph.txt\n+++ b/Documentation/config/commitgraph.txt\n@@ -1,3 +1,9 @@\n+commitGraph.generationVersion::\n+\tSpecifies the type of generation number version to use when writing\n+\tor reading the commit-graph file. If version 1 is specified, then\n+\tthe corrected commit dates will not be written or read. Defaults to\n+\t2.\n+\n commitGraph.maxNewFilters::\n \tSpecifies the default value for the `--max-new-filters` option of `git\n \tcommit-graph write` (c.f., linkgit:git-commit-graph[1]).\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 8b07a9a0a6df..1e9b88c003d7 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -96,6 +96,13 @@ define_commit_slab(commit_graph_data_slab, struct commit_graph_data);\n static struct commit_graph_data_slab commit_graph_data_slab =\n \tCOMMIT_SLAB_INIT(1, commit_graph_data_slab);\n \n+static int get_configured_generation_version(struct repository *r)\n+{\n+\tint version = 2;\n+\trepo_config_get_int(r, \"commitgraph.generationversion\", &version);\n+\treturn version;\n+}\n+\n uint32_t commit_graph_position(const struct commit *c)\n {\n \tstruct commit_graph_data *data =\n@@ -394,10 +401,13 @@ struct commit_graph *parse_commit_graph(struct repository *r,\n \tpair_chunk(cf, GRAPH_CHUNKID_DATA, &graph->chunk_commit_data);\n \tpair_chunk(cf, GRAPH_CHUNKID_EXTRAEDGES, &graph->chunk_extra_edges);\n \tpair_chunk(cf, GRAPH_CHUNKID_BASE, &graph->chunk_base_graphs);\n-\tpair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,\n-\t\t   &graph->chunk_generation_data);\n-\tpair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,\n-\t\t   &graph->chunk_generation_data_overflow);\n+\n+\tif (get_configured_generation_version(r) >= 2) {\n+\t\tpair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,\n+\t\t\t&graph->chunk_generation_data);\n+\t\tpair_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA_OVERFLOW,\n+\t\t\t&graph->chunk_generation_data_overflow);\n+\t}\n \n \tif (r->settings.commit_graph_read_changed_paths) {\n \t\tpair_chunk(cf, GRAPH_CHUNKID_BLOOMINDEXES,\n@@ -1771,8 +1781,6 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx)\n \tadd_chunk(cf, GRAPH_CHUNKID_DATA, (hashsz + 16) * ctx->commits.nr,\n \t\t  write_graph_chunk_data);\n \n-\tif (git_env_bool(GIT_TEST_COMMIT_GRAPH_NO_GDAT, 0))\n-\t\tctx->write_generation_data = 0;\n \tif (ctx->write_generation_data)\n \t\tadd_chunk(cf, GRAPH_CHUNKID_GENERATION_DATA,\n \t\t\t  sizeof(uint32_t) * ctx->commits.nr,\n@@ -2179,7 +2187,7 @@ int write_commit_graph(struct object_directory *odb,\n \tctx->split = flags & COMMIT_GRAPH_WRITE_SPLIT ? 1 : 0;\n \tctx->opts = opts;\n \tctx->total_bloom_filter_data_size = 0;\n-\tctx->write_generation_data = 1;\n+\tctx->write_generation_data = (get_configured_generation_version(r) == 2);\n \tctx->num_generation_data_overflows = 0;\n \n \tbloom_settings.bits_per_entry = git_env_ulong(\"GIT_TEST_BLOOM_SETTINGS_BITS_PER_ENTRY\",\ndiff --git a/commit-graph.h b/commit-graph.h\nindex 97f3497c2790..96c24fb5777d 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -6,7 +6,6 @@\n #include \"oidset.h\"\n \n #define GIT_TEST_COMMIT_GRAPH \"GIT_TEST_COMMIT_GRAPH\"\n-#define GIT_TEST_COMMIT_GRAPH_NO_GDAT \"GIT_TEST_COMMIT_GRAPH_NO_GDAT\"\n #define GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE \"GIT_TEST_COMMIT_GRAPH_DIE_ON_PARSE\"\n #define GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS \"GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS\"\n \ndiff --git a/t/README b/t/README\nindex 8a121487279b..c730a7077057 100644\n--- a/t/README\n+++ b/t/README\n@@ -393,9 +393,6 @@ GIT_TEST_COMMIT_GRAPH=<boolean>, when true, forces the commit-graph to\n be written after every 'git commit' command, and overrides the\n 'core.commitGraph' setting to true.\n \n-GIT_TEST_COMMIT_GRAPH_NO_GDAT=<boolean>, when true, forces the\n-commit-graph to be written without generation data chunk.\n-\n GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=<boolean>, when true, forces\n commit-graph write to compute and write changed path Bloom filters for\n every 'git commit-graph write', as if the `--changed-paths` option was\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex c7da741284e5..6c051ca6bcf7 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -454,7 +454,7 @@ test_expect_success 'warn on improper hash version' '\n \n test_expect_success 'git commit-graph verify' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit rev-parse commits/8 | GIT_TEST_COMMIT_GRAPH_NO_GDAT=1 git commit-graph write --stdin-commits &&\n+\tgit rev-parse commits/8 | git -c commitGraph.generationVersion=1 commit-graph write --stdin-commits &&\n \tgit commit-graph verify >output &&\n \tgraph_read_expect 9 extra_edges\n '\ndiff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh\nindex 8e90f3423b8f..587226ed1032 100755\n--- a/t/t5324-split-commit-graph.sh\n+++ b/t/t5324-split-commit-graph.sh\n@@ -489,7 +489,7 @@ test_expect_success 'setup repo for mixed generation commit-graph-chain' '\n \t\t\ttest_commit $i &&\n \t\t\tgit branch commits/$i || return 1\n \t\tdone &&\n-\t\tgit commit-graph write --reachable --split &&\n+\t\tgit -c commitGraph.generationVersion=2 commit-graph write --reachable --split &&\n \t\tgraph_read_expect $NUM_FIRST_LAYER_COMMITS &&\n \t\ttest_line_count = 1 $graphdir/commit-graph-chain &&\n \t\tfor i in $(test_seq $SECOND_LAYER_SEQUENCE_START $SECOND_LAYER_SEQUENCE_END)\n@@ -497,7 +497,7 @@ test_expect_success 'setup repo for mixed generation commit-graph-chain' '\n \t\t\ttest_commit $i &&\n \t\t\tgit branch commits/$i || return 1\n \t\tdone &&\n-\t\tGIT_TEST_COMMIT_GRAPH_NO_GDAT=1 git commit-graph write --reachable --split=no-merge &&\n+\t\tgit -c commitGraph.generationVersion=1 commit-graph write --reachable --split=no-merge &&\n \t\ttest_line_count = 2 $graphdir/commit-graph-chain &&\n \t\ttest-tool read-graph >output &&\n \t\tcat >expect <<-EOF &&\ndiff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh\nindex e2d33a8a4c46..3d7a62ddab61 100755\n--- a/t/t6600-test-reach.sh\n+++ b/t/t6600-test-reach.sh\n@@ -55,7 +55,7 @@ test_expect_success 'setup' '\n \tgit show-ref -s commit-5-5 | git commit-graph write --stdin-commits &&\n \tmv .git/objects/info/commit-graph commit-graph-half &&\n \tchmod u+w commit-graph-half &&\n-\tGIT_TEST_COMMIT_GRAPH_NO_GDAT=1 git commit-graph write --reachable &&\n+\tgit -c commitGraph.generationVersion=1 commit-graph write --reachable &&\n \tmv .git/objects/info/commit-graph commit-graph-no-gdat &&\n \tchmod u+w commit-graph-no-gdat &&\n \tgit config core.commitGraph true\n-- \ngitgitgadget\n"}]}