Re: [PATCH 04/14] fetch: add object filtering for partial fetch
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Nov 3, 2017, 20:38 UTC
- Message-ID
- <20171103133845.b270485dd04c6c6c1b47d42a@google.com>
- In-Reply-To
- <20171102203129.59417-5-git@jeffhostetler.com>
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@jeffhostetler.com> wrote:
Show 14 quoted lines
> @@ -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.
Show 10 quoted lines
> 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.
Show 21 quoted lines
> @@ -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.