Re: [PATCH v3 1/2] merge: new autosetupmerge option 'simple' for matching branches

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 28, 2022, 10:39 UTC
Message-ID: <220228.86k0df5key.gmgdl@evledraar.gmail.com>
In-Reply-To: <0b5d47895120539d6a72a91398f33a0e33df7cd5.1646032466.git.gitgitgadget@gmail.com>

On Mon, Feb 28 2022, Tao Klerks via GitGitGadget wrote:

I think squashing 2/2 inot this would make this much easier to follow, i.e. to have tests along with the new feature.

Show 8 quoted lines

> +	/*
> +	 * This check does not apply to the BRANCH_TRACK_INHERIT
> +	 * option; you can inherit one or more tracking entries
> +	 * and the tracking.matches counter is not incremented.
> +	 */
>  	if (tracking.matches > 1)
>  		die(_("not tracking: ambiguous information for ref %s"),
>  		    orig_ref);

This function is the only user of find_tracked_branch(). For e.g. "git checkout we emit";

    fatal: builtin/checkout.c:1246: 'foo' matched multiple (4) remote tracking branches

Perhaps we can do something similar here, and even with some advise() emit information about what other branches conflicted.

Show 14 quoted lines

> +	if (track == BRANCH_TRACK_SIMPLE) {
> +		/*
> +		 * Only track if remote branch name matches.
> +		 * Reaching into items[0].string is safe because
> +		 * we know there is at least one and not more than
> +		 * one entry (because not BRANCH_TRACK_INHERIT).
> +		 */
> +		const char *tracked_branch;
> +		if (!skip_prefix(tracking.srcs->items[0].string,
> +				 "refs/heads/", &tracked_branch) ||
> +		    strcmp(tracked_branch, new_ref))
> +			return;
> +	}
> +

I wondered when reading this if there isn't a way to merge this and the "branch_get" call made in "inherit_tracking" earlier in this function in the "track != BRANCH_TRACK_INHERIT" case.

But maybe not, and that whole API entry point is a bit messy in needing to cover both the use-case of an existing branch & nonexisting (i.e. initial creation).