Fix locking issues with git fetch --multiple --jobs=<n> and fetch.writeCommitGraph

The git fetch command recently learned to extend the --jobs=<n> option to cover the --multiple mode: it will run multiple fetches in parallel.

Together with the recent support to write commit-graphs automatically after each fetch by setting fetch.writeCommitGraph, this led to frequent issues where the commit-graph-chain.lock file could not be created because a parallel job had already created it.

This pair of patches first introduces the command-line option --write-commit-graph (together with the --no-* variant) and then uses it to avoid writing the commit-graph until all fetch jobs are complete.

I don't think that we will want to rush this into Git v2.24.0 because that release is imminent, and this is quite a corner case that I am fixing here. It's more of a FYI that I send this before v2.24.0 is available.

Johannes Schindelin (2):
fetch: add the command-line option --write-commit-graph
fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph

 Documentation/fetch-options.txt |  4 ++++
 builtin/fetch.c                 | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

base-commit: efd54442381a2792186abc994060b8f7dd8b834b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-443%2Fdscho%2Ffetch.writeCommitGraph-and-fetch-jobs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-443/dscho/fetch.writeCommitGraph-and-fetch-jobs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/443

gitgitgadget

From: Johannes Schindelin <[email protected]>

This option overrides the config setting fetch.writeCommitGraph, if both are set.

---
 Documentation/fetch-options.txt | 4 ++++
 builtin/fetch.c                 | 7 ++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

index 43b9ff3bce..a2f78624a2 100644
@@ -92,6 +92,10 @@ ifndef::git-pull[]
 	Run `git gc --auto` at the end to perform garbage collection
 	if needed. This is enabled by default.
 
+--[no-]write-commit-graph::
+	Write a commit-graph after fetching. This overrides the config
+	setting `fetch.writeCommitGraph`.
+
 -p::
 --prune::
 	Before fetching, remove any remote-tracking references that no

index 0c345b5dfe..ba2a81c392 100644
@@ -77,6 +77,7 @@ static struct refspec refmap = REFSPEC_INIT_FETCH;
 static struct list_objects_filter_options filter_options;
 static struct string_list server_options = STRING_LIST_INIT_DUP;
 static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
+static int fetch_write_commit_graph = -1;
 
 static int git_fetch_config(const char *k, const char *v, void *cb)
 {

@@ -198,6 +199,8 @@ static struct option builtin_fetch_options[] = {
 		 N_("run 'gc --auto' after fetching")),
 	OPT_BOOL(0, "show-forced-updates", &fetch_show_forced_updates,
 		 N_("check for forced-updates on all updated branches")),
+	OPT_BOOL(0, "write-commit-graph", &fetch_write_commit_graph,
+		 N_("write the commit-graph after fetching")),
 	OPT_END()
 };
 

@@ -1865,7 +1868,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	string_list_clear(&list, 0);
 
 	prepare_repo_settings(the_repository);
-	if (the_repository->settings.fetch_write_commit_graph) {
+	if (fetch_write_commit_graph > 0 ||
+	    (fetch_write_commit_graph < 0 &&
+	     the_repository->settings.fetch_write_commit_graph)) {
 		int commit_graph_flags = COMMIT_GRAPH_WRITE_SPLIT;
 		struct split_commit_graph_opts split_opts;
 		memset(&split_opts, 0, sizeof(struct split_commit_graph_opts));

gitgitgadget

From: Johannes Schindelin <[email protected]>

When both fetch.jobs and fetch.writeCommitGraph is set, we currently try to write the commit graph in each of the concurrent fetch jobs, which frequently leads to error messages like this one:

fatal: Unable to create '.../.git/objects/info/commit-graphs/commit-graph-chain.lock': File exists.

Let's avoid this by holding off from writing the commit graph until all fetch jobs are done.

---
 builtin/fetch.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index ba2a81c392..dc7dc303b1 100644
@@ -1602,7 +1602,8 @@ static int fetch_multiple(struct string_list *list, int max_children)
 			return errcode;
 	}
 
-	argv_array_pushl(&argv, "fetch", "--append", "--no-auto-gc", NULL);
+	argv_array_pushl(&argv, "fetch", "--append", "--no-auto-gc",
+			"--no-write-commit-graph", NULL);
 	add_options_to_argv(&argv);
 
 	if (max_children != 1 && list->nr != 1) {

gitgitgadget

Thanks, the whole thing looks clearly explained and the patches themselves look good. And having "--[no-]write-commit-graph" is a good thing even independent of the problem you're fixing.

I wondered if it was worth having a test in the second patch, but I think it would be inherently racy. So it's probably not worth the trouble.

-Peff

Thanks, both.

Hi Peff,

Yes, I gave testing a great deal of thought, and I failed at coming up with any way to automate it.

Thanks,
Dscho