{"thread":{"id":"55138","subject":"[PATCH] commit-graph: when incompatible with graphs, indicate why","startedAt":"2021-02-11T15:42:22Z","lastAt":"2021-02-25T14:08:51Z","messageCount":4,"participants":["Johannes Schindelin via GitGitGadget","Derrick Stolee","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"416746","messageId":"pull.875.git.1613057954213.gitgitgadget@gmail.com","threadId":"55138","inReplyTo":null,"subject":"[PATCH] commit-graph: when incompatible with graphs, indicate why","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-02-11T15:39:13Z","receivedAt":"2021-02-11T15:42:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen `gc.writeCommitGraph = true`, it is possible that the commit-graph\nis _still_ not written: replace objects, grafts and shallow repositories\nare incompatible with the commit-graph feature.\n\nUnder such circumstances, we need to indicate to the user why the\ncommit-graph was not written instead of staying silent about it.\n\nSigned-off-by: Johannes Schindelin \n---\n Be clear why commit-graph was skipped\n \n After repairing my local checkout\n [https://github.com/gitgitgadget/git/pull/873], I was puzzled that the\n commit-graph file was not written. Turns out that I still had almost a\n dozen replace objects. But I only found out that they were blocking the\n commit-graph when I stepped through git gc in a debugger. This is my\n attempt to make it more straight-forward to recover from similar\n situations in the future.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-875%2Fdscho%2Fwarn-if-commit-graph-is-skipped-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-875/dscho/warn-if-commit-graph-is-skipped-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/875\n\n commit-graph.c | 14 +++++++++++---\n 1 file changed, 11 insertions(+), 3 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 65410602714e..9ad176fa7c8e 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -205,16 +205,24 @@ static int commit_graph_compatible(struct repository *r)\n \n \tif (read_replace_refs) {\n \t\tprepare_replace_object(r);\n-\t\tif (hashmap_get_size(&r->objects->replace_map->map))\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\treturn 0;\n+\t\t}\n \t}\n \n \tprepare_commit_graft(r);\n \tif (r->parsed_objects &&\n-\t (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent))\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\treturn 0;\n-\tif (is_repository_shallow(r))\n+\t}\n+\tif (is_repository_shallow(r)) {\n+\t\twarning(_(\"repository is shallow; skipping commit-graph\"));\n \t\treturn 0;\n+\t}\n \n \treturn 1;\n }\n\nbase-commit: f9f2520108bab26a750bcbb00518dc27672cf0a2\n-- \ngitgitgadget\n"},{"id":"416747","messageId":"aa368496-2875-c383-e862-06e9c587af60@gmail.com","threadId":"55138","inReplyTo":"pull.875.git.1613057954213.gitgitgadget@gmail.com","subject":"Re: [PATCH] commit-graph: when incompatible with graphs, indicate why","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-02-11T16:17:47Z","receivedAt":"2021-02-11T16:20:36Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 2/11/21 10:39 AM, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin \n> \n> When `gc.writeCommitGraph = true`, it is possible that the commit-graph\n> is _still_ not written: replace objects, grafts and shallow repositories\n> are incompatible with the commit-graph feature.\n> \n> Under such circumstances, we need to indicate to the user why the\n> commit-graph was not written instead of staying silent about it.\n\nThis feedback is valuable for these corner cases, especially now\nthat the commit-graph is getting less and less \"optional\" as time\ngoes on.\n\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...\n> +\t\twarning(_(\"repository is shallow; skipping commit-graph\"));\n\nThese warnings make sense to me.\n\nThanks,\n-Stolee\n"},{"id":"416750","messageId":"xmqq5z2yy0ga.fsf@gitster.c.googlers.com","threadId":"55138","inReplyTo":"aa368496-2875-c383-e862-06e9c587af60@gmail.com","subject":"Re: [PATCH] commit-graph: when incompatible with graphs, indicate why","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-02-11T17:32:37Z","receivedAt":"2021-02-11T17:38:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 2/11/21 10:39 AM, Johannes Schindelin via GitGitGadget wrote:\n>> From: Johannes Schindelin \n>> \n>> When `gc.writeCommitGraph = true`, it is possible that the commit-graph\n>> is _still_ not written: replace objects, grafts and shallow repositories\n>> are incompatible with the commit-graph feature.\n>> \n>> Under such circumstances, we need to indicate to the user why the\n>> commit-graph was not written instead of staying silent about it.\n>\n> This feedback is valuable for these corner cases, especially now\n> that the commit-graph is getting less and less \"optional\" as time\n> goes on.\n>\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> ...\n>> +\t\twarning(_(\"repository is shallow; skipping commit-graph\"));\n>\n> These warnings make sense to me.\n\nI take that as your Acked-by.\n\nThanks, both.\n"},{"id":"417817","messageId":"87pn0o6y1e.fsf@evledraar.gmail.com","threadId":"55138","inReplyTo":"pull.875.git.1613057954213.gitgitgadget@gmail.com","subject":"noisy \"warning: repository contains (deprecated) grafts; skipping commit-graph\" warnings","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-02-25T14:07:41Z","receivedAt":"2021-02-25T14:08:51Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Feb 11 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> When `gc.writeCommitGraph = true`, it is possible that the commit-graph\n> is _still_ not written: replace objects, grafts and shallow repositories\n> are incompatible with the commit-graph feature.\n>\n> Under such circumstances, we need to indicate to the user why the\n> commit-graph was not written instead of staying silent about it.\n\nThis change really needs to be changed in some way or other, but\nunfortunately the commit message has little/no information about when\nthese warnings are expected, and no tests.\n\nI'm assuming you were targeting the write_commit_graph() caller of\ncommit_graph_compatible(). In that case this somewhat makes sense I\nguess.\n\nBut we also call this at a distance when simply checking if we can\nlookup things in the commit-graph, observe this on the current git\nmaster:\n\n $ git clone --depth 1 --no-tags --single-branch --branch master https://github.com/git/git.git /tmp/git.git\n [...]\n warning: repository contains (deprecated) grafts; skipping commit-graph\n warning: repository contains (deprecated) grafts; skipping commit-graph\n\nIn that case we reach this via \"parse_object()\" in the \"clone\" process,\nand print it twice because our rev-list child also spews the warning at\nus.\n\nPerhaps a better approach here is to pass down some flag and e.g. write\nit only from \"git gc\" and friends?\n\n> Signed-off-by: Johannes Schindelin \n> ---\n> Be clear why commit-graph was skipped\n> \n> After repairing my local checkout\n> [https://github.com/gitgitgadget/git/pull/873], I was puzzled that the\n> commit-graph file was not written. Turns out that I still had almost a\n> dozen replace objects. But I only found out that they were blocking the\n> commit-graph when I stepped through git gc in a debugger. This is my\n> attempt to make it more straight-forward to recover from similar\n> situations in the future.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-875%2Fdscho%2Fwarn-if-commit-graph-is-skipped-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-875/dscho/warn-if-commit-graph-is-skipped-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/875\n>\n> commit-graph.c | 14 +++++++++++---\n> 1 file changed, 11 insertions(+), 3 deletions(-)\n>\n> diff --git a/commit-graph.c b/commit-graph.c\n> index 65410602714e..9ad176fa7c8e 100644\n> --- a/commit-graph.c\n> +++ b/commit-graph.c\n> @@ -205,16 +205,24 @@ static int commit_graph_compatible(struct repository *r)\n> \n> \tif (read_replace_refs) {\n> \t\tprepare_replace_object(r);\n> -\t\tif (hashmap_get_size(&r->objects->replace_map->map))\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\treturn 0;\n> +\t\t}\n> \t}\n> \n> \tprepare_commit_graft(r);\n> \tif (r->parsed_objects &&\n> -\t (r->parsed_objects->grafts_nr || r->parsed_objects->substituted_parent))\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\treturn 0;\n> -\tif (is_repository_shallow(r))\n> +\t}\n> +\tif (is_repository_shallow(r)) {\n> +\t\twarning(_(\"repository is shallow; skipping commit-graph\"));\n> \t\treturn 0;\n> +\t}\n> \n> \treturn 1;\n> }\n>\n> base-commit: f9f2520108bab26a750bcbb00518dc27672cf0a2\n\n"}]}