Re: Re: Re: Re: [PATCH 3/4] builtin/clone.c: recognize fetch.serverOption configuration

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 9, 2024, 11:49 UTC
Message-ID: <Zt7gufVmnMIGraNx@pks.im>
In-Reply-To: <3a7b1ced.364e.191d4b07a1a.Coremail.bupt_xingxin@163.com>

On Mon, Sep 09, 2024 at 10:50:21AM +0800, Xing Xin wrote:

Show 42 quoted lines

> At 2024-09-05 21:44:41, "Patrick Steinhardt" <ps@pks.im> wrote
> 
> [snip]
> 
> >> >> I named the new configuration `fetch.serverOption` mainly to follow the
> >> >> `push.pushOption` pattern.  Since which server options to support is
> >> >> actually server-specific, using `remote.<name>.serverOption` is a good
> >> >> idea for git-fetch. However, how should we design the configuration for
> >> >> git-ls-remote or git-clone, if we wanna provide all of them with a
> >> >> default list of server options to send?
> >> >
> >> >As mentioned in another reply, I think that putting this into the remote
> >> >configuration "remote.*.serverOption" might be a better solution, as it
> >> >also brings you the ability to set this per remote by default.
> >> 
> >> I agree that using "remote.*.serverOption" is better. In fact, I also
> >> think "push.pushOption" would be better as "remote.*.pushOption". What I'm
> >> contemplating is whether we need to add a configuration for a default
> >> list of server options, so that when "remote.origin.serverOption" is not
> >> present, we can fall back to use that as default.
> >
> >Junio proposed in [1] to introduce `[remote "*"]` syntax to supply
> >default values to all remotes. You could pick up that proposal and
> >implement it as part of your patch series.
> 
> Given the addition of a remote.*.<subkey> configuration in Git's global
> settings, such as:
> 
>     git config --global remote."*".demoConfigKey demoConfigValue
> 
> The current versions of Git may produce errors with certain commands.
> For example, running `git fetch --all` will result in:
> 
>     $ git fetch --all
>     Fetching *
>     fatal: '*' does not appear to be a git repository
>     fatal: Could not read from remote repository.
> 
>     Please make sure you have the correct access rights
>     and the repository exists.
>     error: could not fetch *
>     Fetching origin

We do not currently handle "remote.*.something", so we'd first have to add support for this syntax.

> This raises the question of whether this can be defined as an
> incompatibility or whether this is acceptable. If it isn't, I prefer
> using a `remote.defaultServerOption` instead, as it aligns with our
> existing use of `remote.pushDefault` anyway.

While for this config key it may work alright, I think supporting "remote.*.something" is preferable. Ideally, it would be generic enough to apply to all per-remote settings such that we don't have to add support for every single config key that applies to remotes.

Right now we only care about push options and server options. But somebody may want to globally configure proxies, tag options, partial clone filters or something else. And if we got all of that without having to introduce `remote.defaultPruneTags`, `remote.defaultPartialCloneFilter` and similar options I'd consider it a win.

Patrick