Re: [PATCH v6] tracking branches: add advice to ambiguous refspec error

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 31, 2022, 19:32 UTC
Message-ID: <xmqqpmm13okb.fsf@gitster.g>
In-Reply-To: <pull.1183.v6.git.1648742485887.gitgitgadget@gmail.com>

"Tao Klerks via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 17 quoted lines

>  	if (!remote_find_tracking(remote, &tracking->spec)) {
> -		if (++tracking->matches == 1) {
> +		switch (++tracking->matches) {
> +		case 1:
>  			string_list_append(tracking->srcs, tracking->spec.src);
>  			tracking->remote = remote->name;
> -		} else {
> +			break;
> +		case 2:
> +			/* there are at least two remotes; backfill the first one */
> +			string_list_append(&ftb->ambiguous_remotes, tracking->spec.src);
> +			/* fall through */
> +		default:
> +			string_list_append(&ftb->ambiguous_remotes, remote->name);
>  			free(tracking->spec.src);
>  			string_list_clear(tracking->srcs, 0);
> +		break;

Just to sanity check this part,

 - During the first iteration, we append tracking->spec.src to
   tracking->srcs, and set tracking->remote to remote->name;

 - In later iterations, we do not want to touch tracking->srcs, and
   want to collect remote->name.

And "backfill" assumes that tracking->spec.src at that point in the second iteration is the same as what we got in remote->name in the first round. If that were a correct assumption, then it is curious that the first iteration uses tracking->spec.src and remote->name separately for different purposes, which makes me want to double check if the assumption is indeed correct.

If it were tracking->remote (which was assigned the value of remote->name during the first iteration) that is used to backfill before we append remote->name in the second iteration, I wouldn't find it "curious", but the use of tracking->spec.src there makes me feel confused.

Thanks.