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

>On Mon, Sep 09, 2024 at 10:50:21AM +0800, Xing Xin wrote:
>> 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.