Merge branch 'tc/fetch-bundle-uri' into seen

Allow "git fetch" take advantage of bundleURI feature.

* tc/fetch-bundle-uri:
  fetch: use bundle URIs when having creationToken heuristic
  transport: introduce transport_has_remote_bundle_uri()
  clone: remove double bundle list clear code

Author: gitster

builtin/clone.c

@@ -1409,25 +1409,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				bundle_uri);
 		else if (has_heuristic)
 			git_config_set_gently("fetch.bundleuri", bundle_uri);
-	} else {
-		/*
-		* Populate transport->got_remote_bundle_uri and
-		* transport->bundle_uri. We might get nothing.
-		*/
-		transport_get_remote_bundle_uri(transport);
-
-		if (transport->bundles &&
-		    hashmap_get_size(&transport->bundles->bundles)) {
-			/* At this point, we need the_repository to match the cloned repo. */
-			if (repo_init(the_repository, git_dir, work_tree))
-				warning(_("failed to initialize the repo, skipping bundle URI"));
-			else if (fetch_bundle_list(the_repository,
-						   transport->bundles))
-				warning(_("failed to fetch advertised bundles"));
-		} else {
-			clear_bundle_list(transport->bundles);
-			FREE_AND_NULL(transport->bundles);
-		}
+	} else if (transport_has_remote_bundle_uri(transport)) {
+		/* At this point, we need the_repository to match the cloned repo. */
+		if (repo_init(the_repository, git_dir, work_tree))
+			warning(_("failed to initialize the repo, skipping bundle URI"));
+		else if (fetch_bundle_list(the_repository,
+					   transport->bundles))
+			warning(_("failed to fetch advertised bundles"));
 	}
 
 	if (refs)

builtin/fetch.c

@@ -1695,6 +1695,19 @@ static int do_fetch(struct transport *transport,
 			retcode = 1;
 	}
 
+	if (transport_has_remote_bundle_uri(transport)) {
+		/*
+		 * Only use bundle-URIs when they use the creationToken
+		 * heuristic, this allows us to ensure not downloading bundles
+		 * we don't need. You can read the comments in
+		 * fetch_bundles_by_token() to understand how this works.
+		 */
+		if (transport->bundles->heuristic == BUNDLE_HEURISTIC_CREATIONTOKEN) {
+			if (fetch_bundle_list(the_repository, transport->bundles))
+				warning(_("failed to fetch advertised bundles"));
+		}
+	}
+
 	if (fetch_and_consume_refs(&display_state, transport, transaction, ref_map,
 				   &fetch_head, config)) {
 		retcode = 1;

t/helper/test-bundle-uri.c

@@ -90,7 +90,7 @@ static int cmd_ls_remote(int argc, const char **argv)
 	}
 
 	transport = transport_get(remote, NULL);
-	if (transport_get_remote_bundle_uri(transport) < 0) {
+	if (!transport_has_remote_bundle_uri(transport)) {
 		error(_("could not get the bundle-uri list"));
 		status = 1;
 		goto cleanup;

t/t5558-clone-bundle-uri.sh

@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test fetching bundles with --bundle-uri'
+test_description='test clone with use of bundle-uri'
 
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-bundle.sh
@@ -438,6 +438,32 @@ test_expect_success 'negotiation: bundle list with all wanted commits' '
 	test_grep ! "clone> want " trace-packet.txt
 '
 
+test_expect_success 'bundles advertised by the server' '
+	test_when_finished rm -f trace*.txt &&
+	git clone clone-from clone-advertiser &&
+	git -C clone-advertiser config uploadpack.advertiseBundleURIs true &&
+	git -C clone-advertiser config bundle.version 1 &&
+	git -C clone-advertiser config bundle.mode all &&
+	git -C clone-advertiser config bundle.bundle-1.uri "file://$(pwd)/clone-from/bundle-1.bundle" &&
+	git -C clone-advertiser config bundle.bundle-2.uri "file://$(pwd)/clone-from/bundle-2.bundle" &&
+	git -C clone-advertiser config bundle.bundle-3.uri "file://$(pwd)/clone-from/bundle-3.bundle" &&
+	git -C clone-advertiser config bundle.bundle-4.uri "file://$(pwd)/clone-from/bundle-4.bundle" &&
+
+	GIT_TRACE_PACKET="$(pwd)/trace-packet.txt" \
+	git -c transfer.bundleURI=true clone clone-advertiser clone-advertised &&
+	git -C clone-advertised for-each-ref --format="%(refname)" >refs &&
+	grep "refs/bundles/" refs >actual &&
+	cat >expect <<-\EOF &&
+	refs/bundles/base
+	refs/bundles/left
+	refs/bundles/merge
+	refs/bundles/right
+	EOF
+	test_cmp expect actual &&
+	# We already have all needed commits so no "want" needed.
+	test_grep ! "clone> want " trace-packet.txt
+'
+
 #########################################################################
 # HTTP tests begin here
 

t/t5584-fetch-bundle-uri.sh

@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='test use of bundle URI in "git fetch"'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'set up repos and bundles' '
+	git init source &&
+	test_commit -C source A &&
+	git clone --no-local source go-A-to-C &&
+	test_commit -C source B &&
+	git clone --no-local source go-B-to-C &&
+	git clone --no-local source go-B-to-D &&
+	git -C source bundle create B.bundle main &&
+	test_commit -C source C &&
+	git -C source bundle create B-to-C.bundle B..main &&
+	git -C source config uploadpack.advertiseBundleURIs true &&
+	git -C source config bundle.version 1 &&
+	git -C source config bundle.mode all &&
+	git -C source config bundle.heuristic creationToken &&
+	git -C source config bundle.bundle-B.uri "file://$(pwd)/source/B.bundle" &&
+	git -C source config bundle.bundle-B.creationToken 1 &&
+	git -C source config bundle.bundle-B-to-C.uri "file://$(pwd)/source/B-to-C.bundle" &&
+	git -C source config bundle.bundle-B-to-C.creationToken 2
+'
+
+test_expect_success 'fetches one bundle URI to get up-to-date' '
+	git -C go-B-to-C -c transfer.bundleURI=true fetch origin &&
+	test 1 = $(ls go-B-to-C/.git/objects/bundles | wc -l) &&
+	test 2 = $(git -C go-B-to-C config fetch.bundleCreationToken)
+'
+
+test_expect_success 'fetches two bundle URIs to get up-to-date' '
+	git -C go-A-to-C -c transfer.bundleURI=true fetch origin &&
+	test 2 = $(ls go-A-to-C/.git/objects/bundles | wc -l) &&
+	test 2 = $(git -C go-A-to-C config fetch.bundleCreationToken)
+'
+
+test_expect_success 'fetches one bundle URI and objects from remote' '
+	test_commit -C source D &&
+	git -C go-B-to-D -c transfer.bundleURI=true fetch origin &&
+	test 1 = $(ls go-B-to-D/.git/objects/bundles | wc -l) &&
+	test 2 = $(git -C go-B-to-D config fetch.bundleCreationToken)
+'
+
+test_done

transport.c

@@ -1538,7 +1538,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
 	return rc;
 }
 
-int transport_get_remote_bundle_uri(struct transport *transport)
+static int transport_get_remote_bundle_uri(struct transport *transport)
 {
 	int value = 0;
 	const struct transport_vtable *vtable = transport->vtable;
@@ -1563,6 +1563,18 @@ int transport_get_remote_bundle_uri(struct transport *transport)
 
 	if (vtable->get_bundle_uri(transport) < 0)
 		return error(_("could not retrieve server-advertised bundle-uri list"));
+
+	return 0;
+}
+
+int transport_has_remote_bundle_uri(struct transport *transport)
+{
+	transport_get_remote_bundle_uri(transport);
+
+	if (transport->bundles &&
+	    hashmap_get_size(&transport->bundles->bundles))
+		return 1;
+
 	return 0;
 }
 

transport.h

@@ -294,10 +294,11 @@ const struct ref *transport_get_remote_refs(struct transport *transport,
 					    struct transport_ls_refs_options *transport_options);
 
 /**
- * Retrieve bundle URI(s) from a remote. Populates "struct
- * transport"'s "bundle_uri" and "got_remote_bundle_uri".
+ * Try fetch bundle URI(s) from a remote and returns 1 if one or more
+ * bundle URI(s) are received from the server.
+ * Populates "struct transport"'s "bundles" and "got_remote_bundle_uri".
  */
-int transport_get_remote_bundle_uri(struct transport *transport);
+int transport_has_remote_bundle_uri(struct transport *transport);
 
 /*
  * Fetch the hash algorithm used by a remote.