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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 19, 2017, 21:44 UTC
Message-ID: <alpine.DEB.2.20.1701192241430.3469@virtualbox>
In-Reply-To: <20170119213100.g72ml7r2khu7bvey@sigill.intra.peff.net>

Hi Peff,

On Thu, 19 Jan 2017, Jeff King wrote:

Show 13 quoted lines

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index c85f3471d..9024cfffa 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1014,9 +1014,9 @@ static int add_remote_or_group(const char *name, struct string_list *list)
>  	git_config(get_remote_group, &g);
>  	if (list->nr == prev_nr) {
>  		struct remote *remote;
> -		if (!remote_is_configured(name))
> -			return 0;
>  		remote = remote_get(name);
> +		if (!remote->fetch_refspec_nr)
> +			return 0;

Please note that it is legitimate to fetch from foreign vcs, in which case the fetch refspecs may not be required (or even make sense).

> It's outside the scope of your patches, so I think we are OK to just
> ignore it. But if you want to pursue it, it avoids having to add the
> extra parameter to remote_is_configured().

Sure, it would avoid that. But that parameter makes semantic sense: some callers may want to have all remotes, even those configured via the command-line, and others really are only interested in knowing whether the current repository config already has settings for a remote of the given name.

> > Many thanks to Jeff King whose tireless review helped with settling for
> > nothing less than the current strategy.
> 
> Just how I wanted to be immortalized in git's commit history. ;)

You are welcome ;-)

Ciao, Johannes