Re: [PATCH 2/2] Be more careful when determining whether a remote was configured

> On Wed, Jan 18, 2017 at 05:22:40PM +0100, Johannes Schindelin wrote:
>
>> > > I want to err on the side of caution. That's why.
>> > 
>> > I guess I just don't see why changing the behavior with respect to
>> > "prune" or "proxy" is any less conservative than changing the one for
>> > "refspec".
>> 
> I think _this_ is a much better way of framing the problem. It is not
> about which keys are set, but about _where_ they are set. IOW, a
> reasonable rule would be: if there is any remote.*.X in the repo config,
> then git-remote should consider it a configured repo. And otherwise, no
> matter what is in ~/.gitconfig or elsewhere, git-remote should proceed
> as if it doesn't exist (and repo-level config can take precedence over
> config defined elsewhere).
>
> I.e., something like this:
>
> diff --git a/remote.c b/remote.c
> index 298f2f93f..720d616be 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -373,6 +373,8 @@ static int handle_config(const char *key, const char *value, void *cb)
>  	}
>  	remote = make_remote(name, namelen);
>  	remote->origin = REMOTE_CONFIG;
> +	if (current_config_scope() == CONFIG_SCOPE_REPO)
> +		remote->configured = 1;
>  	if (!strcmp(subkey, "mirror"))
>  		remote->mirror = git_config_bool(key, value);
>  	else if (!strcmp(subkey, "skipdefaultupdate"))
>
> That doesn't make your test pass, but I think that is only because your
> test is not covering the interesting case (it puts the new config in the
> repo config, not in ~/.gitconfig).
>
> What do you think?
>
>> Originally, I would even have put the "vcs" into that set, as I could see
>> a legitimate use case for users to configure "remote.svn.vcs = vcs" in
>> ~/.gitconfig. But the regression test suite specifically tests for that
>> case, and I trust that there was a good reason, even if Thomas did not
>> describe that good reason in the commit message nor in any reply to this
>> patch pair.
>
> The config-scope thing above would allow "remote.svn.vcs" in
> ~/.gitconfig. But I don't think the test script actually checks that; it
> checks for the repo-level config. And we would continue to do the right
> thing there.