Re: [PATCH v2 8/9] bundle-uri: download bundles from an advertised list

[Victoria Dye <vdye@github.com>]

Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@github.com>
> 
> The logic in fetch_bundle_uri() is useful for the --bundle-uri option of
> 'git clone', but is not helpful when the clone operation discovers a
> list of URIs from the bundle-uri protocol v2 command. To actually
> download and unbundle the advertised bundles, we need a different
> mechanism.
> 
> Create the new fetch_bundle_list() method which is very similar to
> fetch_bundle_uri() except that it relies on download_bundle_list()
> instead of fetch_bundle_uri_internal(). The download_bundle_list()
> method will recursively call fetch_bundle_uri_internal() if any of the
> advertised URIs serve a bundle list instead of a bundle. This will also
> follow the bundle.list.mode setting from the input list: "any" will
> download only one such URI while "all" will download data from all of
> the URIs.
> 
> In an identical way to fetch_bundle_uri(), the bundles are unbundled
> after all of the bundle lists have been expanded and all necessary URIs.

This explanation is clear and matches the implementation below. I'll admit
it's a bit difficult to wrap my head around what's going on but, from what I
understand, it does what it needs to do to set up for the next patch.

There's no way to test this change in this patch (since
'fetch_bundle_list()' isn't called anywhere yet), but I think that's fine;
making it testable would probably make the patch too long/complicated to
follow.

> 
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  bundle-uri.c | 21 +++++++++++++++++++++
>  bundle-uri.h | 11 +++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/bundle-uri.c b/bundle-uri.c
> index ab91bb32e9b..5914d220c43 100644
> --- a/bundle-uri.c
> +++ b/bundle-uri.c
> @@ -577,6 +577,27 @@ cleanup:
>  	return result;
>  }
>  
> +int fetch_bundle_list(struct repository *r, const char *uri, struct bundle_list *list)
> +{
> +	int result;
> +	struct bundle_list global_list;
> +
> +	init_bundle_list(&global_list);
> +
> +	/* If a bundle is added to this global list, then it is required. */
> +	global_list.mode = BUNDLE_MODE_ALL;
> +
> +	if ((result = download_bundle_list(r, list, &global_list, 0)))
> +		goto cleanup;
> +
> +	result = unbundle_all_bundles(r, &global_list);
> +
> +cleanup:
> +	for_all_bundles_in_list(&global_list, unlink_bundle, NULL);
> +	clear_bundle_list(&global_list);
> +	return result;
> +}
> +
>  /**
>   * API for serve.c.
>   */
> diff --git a/bundle-uri.h b/bundle-uri.h
> index 7905e56732c..a75b68d2f5a 100644
> --- a/bundle-uri.h
> +++ b/bundle-uri.h
> @@ -102,6 +102,17 @@ int bundle_uri_parse_config_format(const char *uri,
>   */
>  int fetch_bundle_uri(struct repository *r, const char *uri);
>  
> +/**
> + * Given a bundle list that was already advertised (likely by the
> + * bundle-uri protocol v2 verb) at the given uri, fetch and unbundle the
> + * bundles according to the bundle strategy of that list.
> + *
> + * Returns non-zero if no bundle information is found at the given 'uri'.
> + */
> +int fetch_bundle_list(struct repository *r,
> +		      const char *uri,
> +		      struct bundle_list *list);
> +
>  /**
>   * API for serve.c.
>   */