{"thread":{"id":"52177","subject":"[PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph","startedAt":"2019-11-03T00:22:04Z","lastAt":"2019-11-06T12:06:16Z","messageCount":6,"participants":["Johannes Schindelin via GitGitGadget","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"385326","messageId":"pull.443.git.1572740518.gitgitgadget@gmail.com","threadId":"52177","inReplyTo":null,"subject":"[PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-03T00:21:55Z","receivedAt":"2019-11-03T00:22:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The git fetch command recently learned to extend the --jobs= option to\ncover the --multiple mode: it will run multiple fetches in parallel.\n\nTogether with the recent support to write commit-graphs automatically after\neach fetch by setting fetch.writeCommitGraph, this led to frequent issues\nwhere the commit-graph-chain.lock file could not be created because a\nparallel job had already created it.\n\nThis pair of patches first introduces the command-line option \n--write-commit-graph (together with the --no-* variant) and then uses it to\navoid writing the commit-graph until all fetch jobs are complete.\n\nI don't think that we will want to rush this into Git v2.24.0 because that\nrelease is imminent, and this is quite a corner case that I am fixing here.\nIt's more of a FYI that I send this before v2.24.0 is available.\n\nJohannes Schindelin (2):\n fetch: add the command-line option `--write-commit-graph`\n fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph\n\n Documentation/fetch-options.txt | 4 ++++\n builtin/fetch.c | 10 ++++++++--\n 2 files changed, 12 insertions(+), 2 deletions(-)\n\n\nbase-commit: efd54442381a2792186abc994060b8f7dd8b834b\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-443%2Fdscho%2Ffetch.writeCommitGraph-and-fetch-jobs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-443/dscho/fetch.writeCommitGraph-and-fetch-jobs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/443\n-- \ngitgitgadget\n"},{"id":"385327","messageId":"3dbb1c0e0ca455801b54959816eb7888fc4e0c20.1572740518.git.gitgitgadget@gmail.com","threadId":"52177","inReplyTo":"pull.443.git.1572740518.gitgitgadget@gmail.com","subject":"[PATCH 1/2] fetch: add the command-line option `--write-commit-graph`","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-03T00:21:56Z","receivedAt":"2019-11-03T00:22:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis option overrides the config setting `fetch.writeCommitGraph`, if\nboth are set.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/fetch-options.txt | 4 ++++\n builtin/fetch.c | 7 ++++++-\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt\nindex 43b9ff3bce..a2f78624a2 100644\n--- a/Documentation/fetch-options.txt\n+++ b/Documentation/fetch-options.txt\n@@ -92,6 +92,10 @@ ifndef::git-pull[]\n \tRun `git gc --auto` at the end to perform garbage collection\n \tif needed. This is enabled by default.\n \n+--[no-]write-commit-graph::\n+\tWrite a commit-graph after fetching. This overrides the config\n+\tsetting `fetch.writeCommitGraph`.\n+\n -p::\n --prune::\n \tBefore fetching, remove any remote-tracking references that no\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 0c345b5dfe..ba2a81c392 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -77,6 +77,7 @@ static struct refspec refmap = REFSPEC_INIT_FETCH;\n static struct list_objects_filter_options filter_options;\n static struct string_list server_options = STRING_LIST_INIT_DUP;\n static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;\n+static int fetch_write_commit_graph = -1;\n \n static int git_fetch_config(const char *k, const char *v, void *cb)\n {\n@@ -198,6 +199,8 @@ static struct option builtin_fetch_options[] = {\n \t\t N_(\"run 'gc --auto' after fetching\")),\n \tOPT_BOOL(0, \"show-forced-updates\", &fetch_show_forced_updates,\n \t\t N_(\"check for forced-updates on all updated branches\")),\n+\tOPT_BOOL(0, \"write-commit-graph\", &fetch_write_commit_graph,\n+\t\t N_(\"write the commit-graph after fetching\")),\n \tOPT_END()\n };\n \n@@ -1865,7 +1868,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tstring_list_clear(&list, 0);\n \n \tprepare_repo_settings(the_repository);\n-\tif (the_repository->settings.fetch_write_commit_graph) {\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\tstruct split_commit_graph_opts split_opts;\n \t\tmemset(&split_opts, 0, sizeof(struct split_commit_graph_opts));\n-- \ngitgitgadget\n\n"},{"id":"385328","messageId":"f8157985dc5d59331a80d3c6034644bc3f6d8881.1572740518.git.gitgitgadget@gmail.com","threadId":"52177","inReplyTo":"pull.443.git.1572740518.gitgitgadget@gmail.com","subject":"[PATCH 2/2] fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-03T00:21:57Z","receivedAt":"2019-11-03T00:22:04Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nWhen both `fetch.jobs` and `fetch.writeCommitGraph` is set, we currently\ntry to write the commit graph in each of the concurrent fetch jobs,\nwhich frequently leads to error messages like this one:\n\nfatal: Unable to create '.../.git/objects/info/commit-graphs/commit-graph-chain.lock': File exists.\n\nLet's avoid this by holding off from writing the commit graph until all\nfetch jobs are done.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/fetch.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ba2a81c392..dc7dc303b1 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1602,7 +1602,8 @@ static int fetch_multiple(struct string_list *list, int max_children)\n \t\t\treturn errcode;\n \t}\n \n-\targv_array_pushl(&argv, \"fetch\", \"--append\", \"--no-auto-gc\", NULL);\n+\targv_array_pushl(&argv, \"fetch\", \"--append\", \"--no-auto-gc\",\n+\t\t\t\"--no-write-commit-graph\", NULL);\n \tadd_options_to_argv(&argv);\n \n \tif (max_children != 1 && list->nr != 1) {\n-- \ngitgitgadget\n"},{"id":"385419","messageId":"20191104195928.GC20900@sigill.intra.peff.net","threadId":"52177","inReplyTo":"pull.443.git.1572740518.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-04T19:59:28Z","receivedAt":"2019-11-04T19:59:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 03, 2019 at 12:21:55AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> The git fetch command recently learned to extend the --jobs= option to\n> cover the --multiple mode: it will run multiple fetches in parallel.\n> \n> Together with the recent support to write commit-graphs automatically after\n> each fetch by setting fetch.writeCommitGraph, this led to frequent issues\n> where the commit-graph-chain.lock file could not be created because a\n> parallel job had already created it.\n> \n> This pair of patches first introduces the command-line option \n> --write-commit-graph (together with the --no-* variant) and then uses it to\n> avoid writing the commit-graph until all fetch jobs are complete.\n\nThanks, the whole thing looks clearly explained and the patches\nthemselves look good. And having \"--[no-]write-commit-graph\" is a good\nthing even independent of the problem you're fixing.\n\nI wondered if it was worth having a test in the second patch, but I\nthink it would be inherently racy. So it's probably not worth the\ntrouble.\n\n-Peff\n"},{"id":"385511","messageId":"xmqqo8xpwys8.fsf@gitster-ct.c.googlers.com","threadId":"52177","inReplyTo":"20191104195928.GC20900@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-06T01:59:03Z","receivedAt":"2019-11-06T01:59:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Thanks, the whole thing looks clearly explained and the patches\n> themselves look good. And having \"--[no-]write-commit-graph\" is a good\n> thing even independent of the problem you're fixing.\n>\n> I wondered if it was worth having a test in the second patch, but I\n> think it would be inherently racy. So it's probably not worth the\n> trouble.\n\nThanks, both.\n"},{"id":"385586","messageId":"nycvar.QRO.7.76.6.1911061305060.46@tvgsbejvaqbjf.bet","threadId":"52177","inReplyTo":"20191104195928.GC20900@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-06T12:05:44Z","receivedAt":"2019-11-06T12:06:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Mon, 4 Nov 2019, Jeff King wrote:\n\n> On Sun, Nov 03, 2019 at 12:21:55AM +0000, Johannes Schindelin via GitGitGadget wrote:\n>\n> > The git fetch command recently learned to extend the --jobs= option to\n> > cover the --multiple mode: it will run multiple fetches in parallel.\n> >\n> > Together with the recent support to write commit-graphs automatically after\n> > each fetch by setting fetch.writeCommitGraph, this led to frequent issues\n> > where the commit-graph-chain.lock file could not be created because a\n> > parallel job had already created it.\n> >\n> > This pair of patches first introduces the command-line option\n> > --write-commit-graph (together with the --no-* variant) and then uses it to\n> > avoid writing the commit-graph until all fetch jobs are complete.\n>\n> Thanks, the whole thing looks clearly explained and the patches\n> themselves look good. And having \"--[no-]write-commit-graph\" is a good\n> thing even independent of the problem you're fixing.\n>\n> I wondered if it was worth having a test in the second patch, but I\n> think it would be inherently racy. So it's probably not worth the\n> trouble.\n\nYes, I gave testing a great deal of thought, and I failed at coming up\nwith any way to automate it.\n\nThanks,\nDscho\n"}]}