Re: [PATCH v6 1/3] branch: accept multiple upstream branches for tracking

[Junio C Hamano <gitster@pobox.com>]

Josh Steadmon <steadmon@google.com> writes:

> + * `origin` is the name of the remote owning the upstream branches. NULL means
> + * the upstream branches are local to this repo.
> + *
> + * `remotes` is a list of refs that are upstream of local
> + */
> +static int install_branch_config_multiple_remotes(int flag, const char *local,
> +		const char *origin, struct string_list *remotes)
>  {
>  	const char *shortname = NULL;
>  	struct strbuf key = STRBUF_INIT;
> +	struct string_list_item *item;
>  	int rebasing = should_setup_rebase(origin);
>  
> -	if (skip_prefix(remote, "refs/heads/", &shortname)
> -	    && !strcmp(local, shortname)
> -	    && !origin) {
> -		warning(_("Not setting branch %s as its own upstream."),
> -			local);
> -		return 0;
> -	}
> +	if (!remotes->nr)
> +		BUG("must provide at least one remote for branch config");
> +	if (rebasing && remotes->nr > 1)
> +		die(_("cannot inherit upstream tracking configuration when rebasing is requested"));
> +
> +	if (!origin)
> +		for_each_string_list_item(item, remotes)
> +			if (skip_prefix(item->string, "refs/heads/", &shortname)
> +			    && !strcmp(local, shortname)) {
> +				warning(_("not setting branch '%s' as its own upstream."),
> +					local);
> +				return 0;
> +			}

OK, if there is the current branch _among_ the remotes and we are
synching with the local repository, we reject to muck the
configuration for 'local' branch with any of the refs in 'remotes'.

Makes sense.

Just FYI (because there is nothing actionable on your part), there
is another topic in flight that wants to change the warning message
to be phrased like so:

		_("not setting branch '%s' as its own upstream")

i.e. without capitalizing the first word, and without finishing the
sentence with a full stop.  I'll make the conflict resolution to
read as such.

> -test_expect_success '--set-upstream-to notices an error to set branch as own upstream' '
> +test_expect_success '--set-upstream-to notices an error to set branch as own upstream' "
>  	git branch --set-upstream-to refs/heads/my13 my13 2>actual &&
>  	cat >expect <<-\EOF &&
> -	warning: Not setting branch my13 as its own upstream.
> +	warning: not setting branch 'my13' as its own upstream.

Likewise.

>  	EOF
>  	test_expect_code 1 git config branch.my13.remote &&
>  	test_expect_code 1 git config branch.my13.merge &&
>  	test_cmp expect actual
> -'
> +"

The above is currently correct but it often is an invitation for
future bugs to use double-quotes around the executable part of
test_expect_success.  We can always write ' --> '\'' to enclose the
thing in a pair of single quotes when we start needing to refer to
$shell_variables in the executable part, so let's take the above
patch as-is, but something to keep in mind.

Thanks, queued.