[PATCH] fetch: download bundles once, even with --all

From: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>
Date: Mar 31, 2023, 15:59 UTC
Message-ID: <pull.1508.git.1680278344173.gitgitgadget@gmail.com>

From: Derrick Stolee <derrickstolee@github.com>

When fetch.bundleURI is set, 'git fetch' downloads bundles from the given bundle URI before fetching from the specified remote. However, when using non-file remotes, 'git fetch --all' will launch 'git fetch' subprocesses which then read fetch.bundleURI and fetch the bundle list again. We do not expect the bundle list to have new information during these multiple runs, so avoid these extra calls by un-setting fetch.bundleURI in the subprocess arguments.

Be careful to skip fetching bundles for the empty bundle string. Fetching bundles from the empty list presents some interesting test failures.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
    fetch: download bundles once, even with --all
    
    I noticed this in local testing where I had set up a bundle server for
    my Git repo, but wrote the wrong port number so I saw multiple warning
    messages during a git fetch --all run.
    
    Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1508%2Fderrickstolee%2Ffetch-bundles-multiple-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1508/derrickstolee/fetch-bundles-multiple-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1508

 builtin/fetch.c             |  7 ++++++-
 bundle-uri.c                |  9 +++++++++
 t/t5558-clone-bundle-uri.sh | 34 ++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7221e57f352..8d8b2e0c26f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1955,7 +1955,12 @@ static int fetch_multiple(struct string_list *list, int max_children)
 			return errcode;
 	}
 
-	strvec_pushl(&argv, "fetch", "--append", "--no-auto-gc",
+	/*
+	 * Cancel out the fetch.bundleURI config when running subprocesses,
+	 * to avoid fetching from the same bundle list multiple times.
+	 */
+	strvec_pushl(&argv, "-c", "fetch.bundleURI=",
+		     "fetch", "--append", "--no-auto-gc",
 		     "--no-write-commit-graph", NULL);
 	add_options_to_argv(&argv);
 
diff --git a/bundle-uri.c b/bundle-uri.c
index 177c1810402..56390651b92 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -792,6 +792,15 @@ int fetch_bundle_uri(struct repository *r, const char *uri,
 
 	init_bundle_list(&list);
 
+	/*
+	 * Do not fetch a NULL or empty bundle URI. An empty bundle URI
+	 * could signal that a configured bundle URI has been disabled.
+	 */
+	if (!uri || !*uri) {
+		result = 0;
+		goto cleanup;
+	}
+
 	/* If a bundle is added to this global list, then it is required. */
 	list.mode = BUNDLE_MODE_ALL;
 
diff --git a/t/t5558-clone-bundle-uri.sh b/t/t5558-clone-bundle-uri.sh
index afd56926c53..996a08e90c9 100755
--- a/t/t5558-clone-bundle-uri.sh
+++ b/t/t5558-clone-bundle-uri.sh
@@ -1018,6 +1018,40 @@ test_expect_success 'creationToken heuristic with failed downloads (fetch)' '
 	test_cmp expect refs
 '
 
+test_expect_success 'bundles are downloaded once during fetch --all' '
+	test_when_finished rm -rf download-* trace*.txt fetch-mult &&
+
+	cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
+	[bundle]
+		version = 1
+		mode = all
+		heuristic = creationToken
+
+	[bundle "bundle-1"]
+		uri = bundle-1.bundle
+		creationToken = 1
+
+	[bundle "bundle-2"]
+		uri = bundle-2.bundle
+		creationToken = 2
+
+	[bundle "bundle-3"]
+		uri = bundle-3.bundle
+		creationToken = 3
+	EOF
+
+	git clone --single-branch --branch=left \
+		--bundle-uri="$HTTPD_URL/bundle-list" \
+		"$HTTPD_URL/smart/fetch.git" fetch-mult &&
+	git -C fetch-mult remote add dup1 "$HTTPD_URL/smart/fetch.git" &&
+	git -C fetch-mult remote add dup2 "$HTTPD_URL/smart/fetch.git" &&
+
+	GIT_TRACE2_EVENT="$(pwd)/trace-mult.txt" \
+		git -C fetch-mult fetch --all &&
+	grep "\"child_start\".*\"git-remote-https\",\"$HTTPD_URL/bundle-list\"" \
+		trace-mult.txt >bundle-fetches &&
+	test_line_count = 1 bundle-fetches
+'
 # Do not add tests here unless they use the HTTP server, as they will
 # not run unless the HTTP dependencies exist.
 

base-commit: 6369acd968d02899973a9a853c48029b92cea401

-- 
gitgitgadget