Ævar Arnfjörð Bjarmason via GitGitGadget wrote:
> From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
> <avarab@xxxxxxxxx>
>
> The yet-to-be introduced client support for bundle-uri will always
> fall back on a full clone, but we'd still like to be able to ignore a
> server's bundle-uri advertisement entirely.
>
> The new transfer.bundleURI config option defaults to 'false', but a user
> can set it to 'true' to enable checking for bundle URIs from the origin
> Git server using protocol v2.
Thanks for adding this, an "opt-in" approach seems reasonable for
introducing this feature.
>
> To enable this setting by default in the correct tests, add a
> GIT_TEST_BUNDLE_URI environment variable.
This makes sense. I'm less concerned with this environment variable than
those in patch 2 [1], since it's in line with the test variables that
enable/disable whole features ('GIT_TEST_SPLIT_INDEX',
'GIT_TEST_COMMIT_GRAPH', etc.).
The only thing feedback I can think of would be that this patch could be
moved to earlier in the series (that is, immediately after creating
'transport_get_remote_bundle_uri()'). That said, I don't feel strongly
either way.
[1] https://lore.kernel.org/git/ca410bed-e8d1-415f-5235-b64fe18bed27@xxxxxxxxxx/
>
> Co-authored-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx>
The implementation and documentation below align with the commit message.
Looks good!
> ---
> Documentation/config/transfer.txt | 6 ++++++
> t/lib-t5730-protocol-v2-bundle-uri.sh | 3 +++
> transport.c | 10 +++++++---
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt
> index 264812cca4d..c3ac767d1e4 100644
> --- a/Documentation/config/transfer.txt
> +++ b/Documentation/config/transfer.txt
> @@ -115,3 +115,9 @@ transfer.unpackLimit::
> transfer.advertiseSID::
> Boolean. When true, client and server processes will advertise their
> unique session IDs to their remote counterpart. Defaults to false.
> +
> +transfer.bundleURI::
> + When `true`, local `git clone` commands will request bundle
> + information from the remote server (if advertised) and download
> + bundles before continuing the clone through the Git protocol.
> + Defaults to `false`.
> diff --git a/t/lib-t5730-protocol-v2-bundle-uri.sh b/t/lib-t5730-protocol-v2-bundle-uri.sh
> index 000fcc5e20b..872bc39ad1b 100644
> --- a/t/lib-t5730-protocol-v2-bundle-uri.sh
> +++ b/t/lib-t5730-protocol-v2-bundle-uri.sh
> @@ -1,5 +1,8 @@
> # Included from t573*-protocol-v2-bundle-uri-*.sh
>
> +GIT_TEST_BUNDLE_URI=1
> +export GIT_TEST_BUNDLE_URI
> +
> T5730_PARENT=
> T5730_URI=
> T5730_BUNDLE_URI=
> diff --git a/transport.c b/transport.c
> index 86460f5be28..b33180226ae 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1538,6 +1538,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
>
> int transport_get_remote_bundle_uri(struct transport *transport, int quiet)
> {
> + int value = 0;
> const struct transport_vtable *vtable = transport->vtable;
>
> /* Check config only once. */
> @@ -1545,10 +1546,13 @@ int transport_get_remote_bundle_uri(struct transport *transport, int quiet)
> return 0;
>
> /*
> - * This is intentionally below the transport.injectBundleURI,
> - * we want to be able to inject into protocol v0, or into the
> - * dialog of a server who doesn't support this.
> + * Don't use bundle-uri at all, if configured not to. Only proceed
> + * if GIT_TEST_BUNDLE_URI=1 or transfer.bundleURI=true.
> */
> + if (!git_env_bool("GIT_TEST_BUNDLE_URI", 0) &&> + (git_config_get_bool("transfer.bundleuri", &value) || !value))
> + return 0;
> +
> if (!vtable->get_bundle_uri) {
> if (quiet)
> return -1;
