Web lists-archives.com

Re: [PATCH 04/14] fetch: add object filtering for partial fetch




I did some of my own investigation and have a working (i.e. passing
tests) version of this patch here:

https://github.com/jonathantanmy/git/commits/pc20171103

If you want, you can use that, or incorporate the changes therein here.
I'll also remark on my findings inline.

On Thu,  2 Nov 2017 20:31:19 +0000
Jeff Hostetler <git@xxxxxxxxxxxxxxxxx> wrote:

> @@ -754,6 +757,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>  	const char *filename = dry_run ? "/dev/null" : git_path_fetch_head();
>  	int want_status;
>  	int summary_width = transport_summary_width(ref_map);
> +	struct check_connected_options opt = CHECK_CONNECTED_INIT;
>  
>  	fp = fopen(filename, "a");
>  	if (!fp)
> @@ -765,7 +769,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
>  		url = xstrdup("foreign");
>  
>  	rm = ref_map;
> -	if (check_connected(iterate_ref_map, &rm, NULL)) {
> +	if (check_connected(iterate_ref_map, &rm, &opt)) {

opt here is unchanged from CHECK_CONNECTED_INIT, so this change is unnecessary.

>  		rc = error(_("%s did not send all necessary objects\n"), url);
>  		goto abort;
>  	}
> @@ -1044,6 +1048,9 @@ static struct transport *prepare_transport(struct remote *remote, int deepen)
>  		set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, "yes");
>  	if (update_shallow)
>  		set_option(transport, TRANS_OPT_UPDATE_SHALLOW, "yes");
> +	if (filter_options.choice)
> +		set_option(transport, TRANS_OPT_LIST_OBJECTS_FILTER,
> +			   filter_options.raw_value);

You'll also need to set TRANS_OPT_FROM_PROMISOR.

> @@ -1242,6 +1249,20 @@ static int fetch_multiple(struct string_list *list)
>  	int i, result = 0;
>  	struct argv_array argv = ARGV_ARRAY_INIT;
>  
> +	if (filter_options.choice) {
> +		/*
> +		 * We currently only support partial-fetches to the remote
> +		 * used for the partial-clone because we only support 1
> +		 * promisor remote, so we DO NOT allow explicit command
> +		 * line filter arguments.
> +		 *
> +		 * Note that the loop below will spawn background fetches
> +		 * for each remote and one of them MAY INHERIT the proper
> +		 * partial-fetch settings, so everything is consistent.
> +		 */
> +		die(_("partial-fetch is not supported on multiple remotes"));
> +	}
> +
>  	if (!append && !dry_run) {
>  		int errcode = truncate_fetch_head();
>  		if (errcode)

My intention in doing the "fetch: refactor calculation of remote list"
patch is so that the interaction between the provided list of remotes
and the specification of the filter can be handled using the following
diff:

    -	if (remote)
    +	if (remote) {
    +		if (filter_options.choice &&
    +		    strcmp(remote->name, repository_format_partial_clone_remote))
    +			die(_("--blob-max-bytes can only be used with the remote configured in core.partialClone"));
     		result = fetch_one(remote, argc, argv);
    -	else
    +	} else {
    +		if (filter_options.choice)
    +			die(_("--blob-max-bytes can only be used with the remote configured in core.partialClone"));
     		result = fetch_multiple(&list);
    +	}

(Ignore the "blob-max-bytes" in the error message - that needs to be
updated.)

The GitHub link I provided above has this diff, and it seems to work.