Subject: Re: [PATCH v4 4/4] maintenance: respect remote.*.skipFetchAll

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 16 Apr 2021 15:54:13 +0200

On Fri, Apr 16 2021, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <[email-masked]>
>
> If a remote has the skipFetchAll setting enabled, then that remote is
> not intended for frequent fetching. It makes sense to not fetch that
> data during the 'prefetch' maintenance task. Skip that remote in the
> iteration without error. The skip_default_update member is initialized
> in remote.c:handle_config() as part of initializing the 'struct remote'.
>
> Signed-off-by: Derrick Stolee <[email-masked]>
> ---
>  builtin/gc.c           | 3 +++
>  t/t7900-maintenance.sh | 8 +++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 9d35f7da50d8..98a803196b88 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -878,6 +878,9 @@ static int fetch_remote(struct remote *remote, void *cbdata)
>  	struct maintenance_run_opts *opts = cbdata;
>  	struct child_process child = CHILD_PROCESS_INIT;
>  
> +	if (remote->skip_default_update)
> +		return 0;
> +
>  	child.git_cmd = 1;
>  	strvec_pushl(&child.args, "fetch", remote->name,
>  		     "--prefetch", "--prune", "--no-tags",
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index eadb800c08cc..b93ae014ee58 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -153,7 +153,13 @@ test_expect_success 'prefetch multiple remotes' '
>  
>  	test_cmp_config refs/prefetch/ log.excludedecoration &&
>  	git log --oneline --decorate --all >log &&
> -	! grep "prefetch" log
> +	! grep "prefetch" log &&
> +
> +	test_when_finished git config --unset remote.remote1.skipFetchAll &&
> +	git config remote.remote1.skipFetchAll true &&
> +	GIT_TRACE2_EVENT="$(pwd)/skip-remote1.txt" git maintenance run --task=prefetch 2>/dev/null &&
> +	test_subcommand ! git fetch remote1 $fetchargs <skip-remote1.txt &&
> +	test_subcommand git fetch remote2 $fetchargs <skip-remote1.txt
>  '
>  
>  test_expect_success 'prefetch and existing log.excludeDecoration values' '

Without having read the code I'd have very much expected a
"remote.*.skipFetchAll" to impact:

    git fetch --all

Or:

    git remote update --all # --all does not exist yet

As e.g. remote.<name>.skipDefaultUpdate would do (i.e. impact "git
remote update" ...).

I suspect naming it like this started as a hack around the lack of
4-level .ini config keys, i.e. so we could do:

    maintenance.remote.<name>.skipFetchAll = true

But I wonder if we couldn't give this a less confusing name still, even
the pseudo-command form of:

    maintenanceRemote.<name>.skipFetchAll = true

Or something...