Re: [PATCH v3 1/3] maintenance: simplify prefetch logic

[Tom Saeger <tom.saeger@oracle.com>]

On Sat, Apr 10, 2021 at 02:03:43AM +0000, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> The previous logic filled a string list with the names of each remote,
> but instead we could simply run the appropriate 'git fetch' data
> directly in the remote iterator. Do this for reduced code size, but also
> because it sets up an upcoming change to use the remote's refspec. This
> data is accessible from the 'struct remote' data that is now accessible
> in fetch_remote().
> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  builtin/gc.c | 33 ++++++++-------------------------
>  1 file changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index ef7226d7bca4..fa8128de9ae1 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -873,55 +873,38 @@ static int maintenance_task_commit_graph(struct maintenance_run_opts *opts)
>  	return 0;
>  }
>  
> -static int fetch_remote(const char *remote, struct maintenance_run_opts *opts)
> +static int fetch_remote(struct remote *remote, void *cbdata)
>  {
> +	struct maintenance_run_opts *opts = cbdata;
>  	struct child_process child = CHILD_PROCESS_INIT;


I think this might be appropriate to add:


       if (remote->skip_default_update)
               return 0;


maintenance prefetch is acting like `git fetch --all`
So it should also skip remotes with configs `skipfetchall = true`
Agree?

>  
>  	child.git_cmd = 1;
> -	strvec_pushl(&child.args, "fetch", remote, "--prune", "--no-tags",
> +	strvec_pushl(&child.args, "fetch", remote->name, "--prune", "--no-tags",
>  		     "--no-write-fetch-head", "--recurse-submodules=no",
>  		     "--refmap=", NULL);
>  
>  	if (opts->quiet)
>  		strvec_push(&child.args, "--quiet");
>  
> -	strvec_pushf(&child.args, "+refs/heads/*:refs/prefetch/%s/*", remote);
> +	strvec_pushf(&child.args, "+refs/heads/*:refs/prefetch/%s/*", remote->name);
>  
>  	return !!run_command(&child);
>  }
>  
> -static int append_remote(struct remote *remote, void *cbdata)
> -{
> -	struct string_list *remotes = (struct string_list *)cbdata;
> -
> -	string_list_append(remotes, remote->name);
> -	return 0;
> -}
> -
>  static int maintenance_task_prefetch(struct maintenance_run_opts *opts)
>  {
> -	int result = 0;
> -	struct string_list_item *item;
> -	struct string_list remotes = STRING_LIST_INIT_DUP;
> -
>  	git_config_set_multivar_gently("log.excludedecoration",
>  					"refs/prefetch/",
>  					"refs/prefetch/",
>  					CONFIG_FLAGS_FIXED_VALUE |
>  					CONFIG_FLAGS_MULTI_REPLACE);
>  
> -	if (for_each_remote(append_remote, &remotes)) {
> -		error(_("failed to fill remotes"));
> -		result = 1;
> -		goto cleanup;
> +	if (for_each_remote(fetch_remote, opts)) {
> +		error(_("failed to prefetch remotes"));
> +		return 1;
>  	}
>  
> -	for_each_string_list_item(item, &remotes)
> -		result |= fetch_remote(item->string, opts);
> -
> -cleanup:
> -	string_list_clear(&remotes, 0);
> -	return result;
> +	return 0;
>  }
>  
>  static int maintenance_task_gc(struct maintenance_run_opts *opts)
> -- 
> gitgitgadget
>