{"thread":{"id":"55223","subject":"[PATCH] commit-graph: warn about incompatibilities only when trying to write","startedAt":"2021-02-26T14:51:43Z","lastAt":"2021-03-01T22:26:15Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Ævar Arnfjörð Bjarmason","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"417893","messageId":"pull.888.git.1614351036334.gitgitgadget@gmail.com","threadId":"55223","inReplyTo":null,"subject":"[PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-26T14:50:35Z","receivedAt":"2021-02-26T14:51:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn c85eec7fc37 (commit-graph: when incompatible with graphs, indicate\nwhy, 2021-02-11), we started warning the user if they tried to write a\ncommit-graph in a shallow repository, or one containing replace objects.\n\nHowever, this patch was a bit overzealous, as Git now _also_ warns when\nmerely checking whether there _is_ a usable commit graph, not only when\nwriting one.\n\nLet's suppress that warning unless we want to write a commit-graph.\n\nReported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    commit-graph: warn about incompatibilities only when trying to write\n    \n    As pointed out by Ævar in\n    https://lore.kernel.org/git/87pn0o6y1e.fsf@evledraar.gmail.com, my\n    recent patch to trigger warnings in repositories that are incompatible\n    with the commit-graph was a bit too overzealous. Here is a fix for that.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-888%2Fdscho%2Fwarn-a-little-less-if-commit-graph-is-skipped-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-888/dscho/warn-a-little-less-if-commit-graph-is-skipped-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/888\n\n commit-graph.c          | 20 ++++++++++++--------\n t/t5318-commit-graph.sh | 13 +++++++++++++\n 2 files changed, 25 insertions(+), 8 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 8fd480434353..245b7108e0d1 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -198,7 +198,7 @@ static struct commit_graph *alloc_commit_graph(void)\n \n extern int read_replace_refs;\n \n-static int commit_graph_compatible(struct repository *r)\n+static int commit_graph_compatible(struct repository *r, int quiet)\n {\n \tif (!r->gitdir)\n \t\treturn 0;\n@@ -206,8 +206,9 @@ static int commit_graph_compatible(struct repository *r)\n \tif (read_replace_refs) {\n \t\tprepare_replace_object(r);\n \t\tif (hashmap_get_size(&r->objects->replace_map->map)) {\n-\t\t\twarning(_(\"repository contains replace objects; \"\n-\t\t\t       \"skipping commit-graph\"));\n+\t\t\tif (!quiet)\n+\t\t\t\twarning(_(\"repository contains replace \"\n+\t\t\t\t\t  \"objects; skipping commit-graph\"));\n \t\t\treturn 0;\n \t\t}\n \t}\n@@ -215,12 +216,15 @@ static int commit_graph_compatible(struct repository *r)\n \tprepare_commit_graft(r);\n \tif (r->parsed_objects &&\n \t    (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent)) {\n-\t\twarning(_(\"repository contains (deprecated) grafts; \"\n-\t\t       \"skipping commit-graph\"));\n+\t\tif (!quiet)\n+\t\t\twarning(_(\"repository contains (deprecated) grafts; \"\n+\t\t\t       \"skipping commit-graph\"));\n \t\treturn 0;\n \t}\n \tif (is_repository_shallow(r)) {\n-\t\twarning(_(\"repository is shallow; skipping commit-graph\"));\n+\t\tif (!quiet)\n+\t\t\twarning(_(\"repository is shallow; skipping \"\n+\t\t\t\t  \"commit-graph\"));\n \t\treturn 0;\n \t}\n \n@@ -652,7 +656,7 @@ static int prepare_commit_graph(struct repository *r)\n \t\t */\n \t\treturn 0;\n \n-\tif (!commit_graph_compatible(r))\n+\tif (!commit_graph_compatible(r, 1))\n \t\treturn 0;\n \n \tprepare_alt_odb(r);\n@@ -2123,7 +2127,7 @@ int write_commit_graph(struct object_directory *odb,\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(the_repository, 0))\n \t\treturn 0;\n \n \tctx = xcalloc(1, sizeof(struct write_commit_graph_context));\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 2ed0c1544da1..2699c55e9a93 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -741,4 +741,17 @@ test_expect_success 'corrupt commit-graph write (missing tree)' '\n \t)\n '\n \n+test_expect_success 'warn about incompatibilities (only) when writing' '\n+\tgit init warn &&\n+\ttest_commit -C warn initial &&\n+\ttest_commit -C warn second &&\n+\tgit -C warn replace --graft second &&\n+\ttest_config -C warn gc.writecommitgraph true &&\n+\n+\tgit -C warn gc 2>err &&\n+\ttest_i18ngrep \"skipping commit-graph\" err &&\n+\tgit -C warn rev-list -1 second 2>err &&\n+\ttest_i18ngrep ! \"skipping commit-graph\" err\n+'\n+\n test_done\n\nbase-commit: c85eec7fc37e1ca79072f263ae6ea1ee305ba38c\n-- \ngitgitgadget\n"},{"id":"417898","messageId":"87r1l27rae.fsf@evledraar.gmail.com","threadId":"55223","inReplyTo":"pull.888.git.1614351036334.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-26T16:00:25Z","receivedAt":"2021-02-26T16:01:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Feb 26 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> In c85eec7fc37 (commit-graph: when incompatible with graphs, indicate\n> why, 2021-02-11), we started warning the user if they tried to write a\n> commit-graph in a shallow repository, or one containing replace objects.\n>\n> However, this patch was a bit overzealous, as Git now _also_ warns when\n> merely checking whether there _is_ a usable commit graph, not only when\n> writing one.\n>\n> Let's suppress that warning unless we want to write a commit-graph.\n\nAh, so that's what it's for, as I suspected :) Unfortunately...\n\n> Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>     commit-graph: warn about incompatibilities only when trying to write\n>     \n>     As pointed out by Ævar in\n>     https://lore.kernel.org/git/87pn0o6y1e.fsf@evledraar.gmail.com, my\n>     recent patch to trigger warnings in repositories that are incompatible\n>     with the commit-graph was a bit too overzealous. Here is a fix for that.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-888%2Fdscho%2Fwarn-a-little-less-if-commit-graph-is-skipped-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-888/dscho/warn-a-little-less-if-commit-graph-is-skipped-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/888\n>\n>  commit-graph.c          | 20 ++++++++++++--------\n>  t/t5318-commit-graph.sh | 13 +++++++++++++\n>  2 files changed, 25 insertions(+), 8 deletions(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 8fd480434353..245b7108e0d1 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -198,7 +198,7 @@ static struct commit_graph *alloc_commit_graph(void)\n>  \n>  extern int read_replace_refs;\n>  \n> -static int commit_graph_compatible(struct repository *r)\n> +static int commit_graph_compatible(struct repository *r, int quiet)\n>  {\n>  \tif (!r->gitdir)\n>  \t\treturn 0;\n> @@ -206,8 +206,9 @@ static int commit_graph_compatible(struct repository *r)\n>  \tif (read_replace_refs) {\n>  \t\tprepare_replace_object(r);\n>  \t\tif (hashmap_get_size(&r->objects->replace_map->map)) {\n> -\t\t\twarning(_(\"repository contains replace objects; \"\n> -\t\t\t       \"skipping commit-graph\"));\n> +\t\t\tif (!quiet)\n> +\t\t\t\twarning(_(\"repository contains replace \"\n> +\t\t\t\t\t  \"objects; skipping commit-graph\"));\n>  \t\t\treturn 0;\n>  \t\t}\n>  \t}\n> @@ -215,12 +216,15 @@ static int commit_graph_compatible(struct repository *r)\n>  \tprepare_commit_graft(r);\n>  \tif (r->parsed_objects &&\n>  \t    (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent)) {\n> -\t\twarning(_(\"repository contains (deprecated) grafts; \"\n> -\t\t       \"skipping commit-graph\"));\n> +\t\tif (!quiet)\n> +\t\t\twarning(_(\"repository contains (deprecated) grafts; \"\n> +\t\t\t       \"skipping commit-graph\"));\n>  \t\treturn 0;\n>  \t}\n>  \tif (is_repository_shallow(r)) {\n> -\t\twarning(_(\"repository is shallow; skipping commit-graph\"));\n> +\t\tif (!quiet)\n> +\t\t\twarning(_(\"repository is shallow; skipping \"\n> +\t\t\t\t  \"commit-graph\"));\n>  \t\treturn 0;\n>  \t}\n>  \n> @@ -652,7 +656,7 @@ static int prepare_commit_graph(struct repository *r)\n>  \t\t */\n>  \t\treturn 0;\n>  \n> -\tif (!commit_graph_compatible(r))\n> +\tif (!commit_graph_compatible(r, 1))\n\n...doing this will cause \"git gc --auto\" to run into persistent\nwarnings. See a WIP patch-on-top in [1] below...\n\n>  \t\treturn 0;\n>  \n>  \tprepare_alt_odb(r);\n> @@ -2123,7 +2127,7 @@ int write_commit_graph(struct object_directory *odb,\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(the_repository, 0))\n>  \t\treturn 0;\n>  \n>  \tctx = xcalloc(1, sizeof(struct write_commit_graph_context));\n> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> index 2ed0c1544da1..2699c55e9a93 100755\n> --- a/t/t5318-commit-graph.sh\n> +++ b/t/t5318-commit-graph.sh\n> @@ -741,4 +741,17 @@ test_expect_success 'corrupt commit-graph write (missing tree)' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'warn about incompatibilities (only) when writing' '\n> +\tgit init warn &&\n> +\ttest_commit -C warn initial &&\n> +\ttest_commit -C warn second &&\n> +\tgit -C warn replace --graft second &&\n> +\ttest_config -C warn gc.writecommitgraph true &&\n> +\n> +\tgit -C warn gc 2>err &&\n> +\ttest_i18ngrep \"skipping commit-graph\" err &&\n> +\tgit -C warn rev-list -1 second 2>err &&\n> +\ttest_i18ngrep ! \"skipping commit-graph\" err\n\n...I think it makes sense to have a \"test_line_count = 1\" here for these\nsorts of warnings we know/suspect might come up N times if we don't\ncarefully tweak things.\n\nI think (but have not tested) that we'll still write it 2 times if you\nhave the graph configured to write on fetch, and we end up also running\n\"gc\" without gc.autoDetach.\n\nIn any case, here's a WIP patch to fix/demonstrate the bug here,\nconsider it to have my SOB (but no need to retain the authorship).\n\nObviously needs amending (e.g. the \"0 &&\" case, just to reproduce the\nbug), and you might not want the s/int/enum/ cleanup while we're at it.\n\nAlso makes sense to add to add this case to the \"gc --auto\" test,\ni.e. if we have writeCommitGraph=true && a --depth=1 repo do we have no\ngc.log at the end as we should.\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ecf85376050..73dacf927f9 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1924,13 +1924,13 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tif (fetch_write_commit_graph > 0 ||\n \t    (fetch_write_commit_graph < 0 &&\n \t     the_repository->settings.fetch_write_commit_graph)) {\n-\t\tint commit_graph_flags = COMMIT_GRAPH_WRITE_SPLIT;\n+\t\tenum commit_graph_write_flags flags = COMMIT_GRAPH_WRITE_SPLIT | COMMIT_GRAPH_WRITE_WARNINGS;\n \n \t\tif (progress)\n-\t\t\tcommit_graph_flags |= COMMIT_GRAPH_WRITE_PROGRESS;\n+\t\t\tflags |= COMMIT_GRAPH_WRITE_PROGRESS;\n \n \t\twrite_commit_graph_reachable(the_repository->objects->odb,\n-\t\t\t\t\t     commit_graph_flags,\n+\t\t\t\t\t     flags,\n \t\t\t\t\t     NULL);\n \t}\n \ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 64f2b52d6e2..9109898eacb 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -593,7 +593,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t/*\n \t\t * Auto-gc should be least intrusive as possible.\n \t\t */\n-\t\tif (!need_to_gc())\n+\t\tif (0 && !need_to_gc())\n \t\t\treturn 0;\n \t\tif (!quiet) {\n \t\t\tif (detach_auto)\n@@ -689,10 +689,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t}\n \n \tprepare_repo_settings(the_repository);\n-\tif (the_repository->settings.gc_write_commit_graph == 1)\n+\tif (the_repository->settings.gc_write_commit_graph) {\n+\t\tenum commit_graph_write_flags flags = 0;\n+\t\tif (!quiet && !daemonized)\n+\t\t\tflags |= (COMMIT_GRAPH_WRITE_PROGRESS |\n+\t\t\t\t  COMMIT_GRAPH_WRITE_WARNINGS);\n+\t\t\t\t\n \t\twrite_commit_graph_reachable(the_repository->objects->odb,\n-\t\t\t\t\t     !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,\n-\t\t\t\t\t     NULL);\n+\t\t\t\t\t     flags, NULL);\n+\t}\n \n \tif (auto_gc && too_many_loose_objects())\n \t\twarning(_(\"There are too many unreachable loose objects; \"\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 245b7108e0d..8afc75b2dbe 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -198,7 +198,8 @@ static struct commit_graph *alloc_commit_graph(void)\n \n extern int read_replace_refs;\n \n-static int commit_graph_compatible(struct repository *r, int quiet)\n+static int commit_graph_compatible(struct repository *r,\n+\t\t\t\t   enum commit_graph_write_flags flags)\n {\n \tif (!r->gitdir)\n \t\treturn 0;\n@@ -206,7 +207,7 @@ static int commit_graph_compatible(struct repository *r, int quiet)\n \tif (read_replace_refs) {\n \t\tprepare_replace_object(r);\n \t\tif (hashmap_get_size(&r->objects->replace_map->map)) {\n-\t\t\tif (!quiet)\n+\t\t\tif (flags & COMMIT_GRAPH_WRITE_WARNINGS)\n \t\t\t\twarning(_(\"repository contains replace \"\n \t\t\t\t\t  \"objects; skipping commit-graph\"));\n \t\t\treturn 0;\n@@ -216,13 +217,13 @@ static int commit_graph_compatible(struct repository *r, int quiet)\n \tprepare_commit_graft(r);\n \tif (r->parsed_objects &&\n \t    (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent)) {\n-\t\tif (!quiet)\n+\t\tif (flags & COMMIT_GRAPH_WRITE_WARNINGS)\n \t\t\twarning(_(\"repository contains (deprecated) grafts; \"\n \t\t\t       \"skipping commit-graph\"));\n \t\treturn 0;\n \t}\n \tif (is_repository_shallow(r)) {\n-\t\tif (!quiet)\n+\t\tif (flags & COMMIT_GRAPH_WRITE_WARNINGS)\n \t\t\twarning(_(\"repository is shallow; skipping \"\n \t\t\t\t  \"commit-graph\"));\n \t\treturn 0;\n@@ -2127,7 +2128,7 @@ int write_commit_graph(struct object_directory *odb,\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, 0))\n+\tif (!commit_graph_compatible(the_repository, flags))\n \t\treturn 0;\n \n \tctx = xcalloc(1, sizeof(struct write_commit_graph_context));\ndiff --git a/commit-graph.h b/commit-graph.h\nindex f8e92500c6e..0d2a12a5e58 100644\n--- a/commit-graph.h\n+++ b/commit-graph.h\n@@ -95,9 +95,10 @@ struct bloom_filter_settings *get_bloom_filter_settings(struct repository *r);\n enum commit_graph_write_flags {\n \tCOMMIT_GRAPH_WRITE_APPEND     = (1 << 0),\n \tCOMMIT_GRAPH_WRITE_PROGRESS   = (1 << 1),\n-\tCOMMIT_GRAPH_WRITE_SPLIT      = (1 << 2),\n-\tCOMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 3),\n-\tCOMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS = (1 << 4),\n+\tCOMMIT_GRAPH_WRITE_WARNINGS   = (1 << 2),\n+\tCOMMIT_GRAPH_WRITE_SPLIT      = (1 << 3),\n+\tCOMMIT_GRAPH_WRITE_BLOOM_FILTERS = (1 << 4),\n+\tCOMMIT_GRAPH_NO_WRITE_BLOOM_FILTERS = (1 << 5),\n };\n \n enum commit_graph_split_flags {\n"},{"id":"417918","messageId":"xmqqy2faqwr0.fsf@gitster.g","threadId":"55223","inReplyTo":"87r1l27rae.fsf@evledraar.gmail.com","subject":"Re: [PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-26T22:39:47Z","receivedAt":"2021-02-26T22:40:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Fri, Feb 26 2021, Johannes Schindelin via GitGitGadget wrote:\n>\n>> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>\n>> In c85eec7fc37 (commit-graph: when incompatible with graphs, indicate\n>> why, 2021-02-11), we started warning the user if they tried to write a\n>> commit-graph in a shallow repository, or one containing replace objects.\n>>\n>> However, this patch was a bit overzealous, as Git now _also_ warns when\n>> merely checking whether there _is_ a usable commit graph, not only when\n>> writing one.\n>>\n>> Let's suppress that warning unless we want to write a commit-graph.\n>\n> Ah, so that's what it's for, as I suspected :) Unfortunately...\n> ...\n> ...doing this will cause \"git gc --auto\" to run into persistent\n> warnings. See a WIP patch-on-top in [1] below...\n\nThanks for an input.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 64f2b52d6e2..9109898eacb 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -593,7 +593,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>  \t\t/*\n>  \t\t * Auto-gc should be least intrusive as possible.\n>  \t\t */\n> -\t\tif (!need_to_gc())\n> +\t\tif (0 && !need_to_gc())\n>  \t\t\treturn 0;\n\nLeftover debugging cruft?\n\n> -\t\t\tif (!quiet)\n> +\t\t\tif (flags & COMMIT_GRAPH_WRITE_WARNINGS)\n>  \t\t\t\twarning(_(\"repository contains replace \"\n>  \t\t\t\t\t  \"objects; skipping commit-graph\"));\n\nI would have expected the code would arrange to make the bit cleared\nin the \"flags\" that would be passed around to the next call that\ninspects the same bit, so that we won't need to see repeated\nwarning()s, but I do not see where it is done.\n\nI am tempted to say that we should revert c85eec7f (commit-graph:\nwhen incompatible with graphs, indicate why, 2021-02-11) for the\nupcoming release.  That would give us enough time to come up with\nand cook the solution in 'next' in the meantime.\n"},{"id":"417979","messageId":"8735xgkvuo.fsf@evledraar.gmail.com","threadId":"55223","inReplyTo":"xmqqy2faqwr0.fsf@gitster.g","subject":"Re: [PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-28T16:20:15Z","receivedAt":"2021-02-28T16:21:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Feb 26 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> On Fri, Feb 26 2021, Johannes Schindelin via GitGitGadget wrote:\n>>\n>>> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>>>\n>>> In c85eec7fc37 (commit-graph: when incompatible with graphs, indicate\n>>> why, 2021-02-11), we started warning the user if they tried to write a\n>>> commit-graph in a shallow repository, or one containing replace objects.\n>>>\n>>> However, this patch was a bit overzealous, as Git now _also_ warns when\n>>> merely checking whether there _is_ a usable commit graph, not only when\n>>> writing one.\n>>>\n>>> Let's suppress that warning unless we want to write a commit-graph.\n>>\n>> Ah, so that's what it's for, as I suspected :) Unfortunately...\n>> ...\n>> ...doing this will cause \"git gc --auto\" to run into persistent\n>> warnings. See a WIP patch-on-top in [1] below...\n>\n> Thanks for an input.\n>\n>> diff --git a/builtin/gc.c b/builtin/gc.c\n>> index 64f2b52d6e2..9109898eacb 100644\n>> --- a/builtin/gc.c\n>> +++ b/builtin/gc.c\n>> @@ -593,7 +593,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n>>  \t\t/*\n>>  \t\t * Auto-gc should be least intrusive as possible.\n>>  \t\t */\n>> -\t\tif (!need_to_gc())\n>> +\t\tif (0 && !need_to_gc())\n>>  \t\t\treturn 0;\n>\n> Leftover debugging cruft?\n\nYeah, see the \"Obviously needs amending\" later in my E-Mail.\n\n>> -\t\t\tif (!quiet)\n>> +\t\t\tif (flags & COMMIT_GRAPH_WRITE_WARNINGS)\n>>  \t\t\t\twarning(_(\"repository contains replace \"\n>>  \t\t\t\t\t  \"objects; skipping commit-graph\"));\n>\n> I would have expected the code would arrange to make the bit cleared\n> in the \"flags\" that would be passed around to the next call that\n> inspects the same bit, so that we won't need to see repeated\n> warning()s, but I do not see where it is done.\n\nRight, we could do that, but I think the path down my \"test_line_count =\n1\" is more useful.\n\nI.e. it's pretty easy to reason about us not calling\nwrite_commit_graph() twice in the same process, so I don't think such\nflag clearing is needed.\n\nBut the case the user cares about is getting this warning twice when\nthey invoke git, but we spawn two processes.\n\n> I am tempted to say that we should revert c85eec7f (commit-graph:\n> when incompatible with graphs, indicate why, 2021-02-11) for the\n> upcoming release.  That would give us enough time to come up with\n> and cook the solution in 'next' in the meantime.\n\nThat's probably sensible.\n\nAlso, I noticed that we went through this whole saga in the past, see\n25575015ca (repack: silence warnings when auto-enabled bitmaps cannot be\nbuilt, 2019-07-31), including breaking background gc.\n"},{"id":"418046","messageId":"xmqqft1essfz.fsf@gitster.c.googlers.com","threadId":"55223","inReplyTo":"8735xgkvuo.fsf@evledraar.gmail.com","subject":"Re: [PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-01T17:18:56Z","receivedAt":"2021-03-01T17:24:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>> I am tempted to say that we should revert c85eec7f (commit-graph:\n>> when incompatible with graphs, indicate why, 2021-02-11) for the\n>> upcoming release.  That would give us enough time to come up with\n>> and cook the solution in 'next' in the meantime.\n>\n> That's probably sensible.\n>\n> Also, I noticed that we went through this whole saga in the past, see\n> 25575015ca (repack: silence warnings when auto-enabled bitmaps cannot be\n> built, 2019-07-31), including breaking background gc.\n\nThanks and sigh ;-)\n"},{"id":"418080","messageId":"nycvar.QRO.7.76.6.2103012304580.57@tvgsbejvaqbjf.bet","threadId":"55223","inReplyTo":"8735xgkvuo.fsf@evledraar.gmail.com","subject":"Re: [PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-03-01T22:10:23Z","receivedAt":"2021-03-01T22:14:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 28 Feb 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> On Fri, Feb 26 2021, Junio C Hamano wrote:\n>\n> > I am tempted to say that we should revert c85eec7f (commit-graph:\n> > when incompatible with graphs, indicate why, 2021-02-11) for the\n> > upcoming release.  That would give us enough time to come up with\n> > and cook the solution in 'next' in the meantime.\n>\n> That's probably sensible.\n\nI agree. This close to v2.31.0, I do not want to rush things.\n\n> Also, I noticed that we went through this whole saga in the past, see\n> 25575015ca (repack: silence warnings when auto-enabled bitmaps cannot be\n> built, 2019-07-31), including breaking background gc.\n\nSounds like it.\n\nThank you for your valuable feedback!\n\nCiao,\nDscho\n"},{"id":"418084","messageId":"xmqqpn0ims5i.fsf@gitster.c.googlers.com","threadId":"55223","inReplyTo":"nycvar.QRO.7.76.6.2103012304580.57@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] commit-graph: warn about incompatibilities only when trying to write","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-01T22:21:45Z","receivedAt":"2021-03-01T22:26:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Sun, 28 Feb 2021, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Fri, Feb 26 2021, Junio C Hamano wrote:\n>>\n>> > I am tempted to say that we should revert c85eec7f (commit-graph:\n>> > when incompatible with graphs, indicate why, 2021-02-11) for the\n>> > upcoming release.  That would give us enough time to come up with\n>> > and cook the solution in 'next' in the meantime.\n>>\n>> That's probably sensible.\n>\n> I agree. This close to v2.31.0, I do not want to rush things.\n>\n>> Also, I noticed that we went through this whole saga in the past, see\n>> 25575015ca (repack: silence warnings when auto-enabled bitmaps cannot be\n>> built, 2019-07-31), including breaking background gc.\n>\n> Sounds like it.\n>\n> Thank you for your valuable feedback!\n>\n> Ciao,\n> Dscho\n\nThanks, reverted and started today's integration cycle on top.\n\n"}]}