Re: [PATCH v5 2/2] branch: add flags and config to inherit tracking

[Glen Choo <chooglen@google.com>]

Josh Steadmon <steadmon@google.com> writes:

> +static int inherit_tracking(struct tracking *tracking, const char *orig_ref)
> +{
> +	const char *bare_ref;
> +	struct branch *branch;
> +	int i;
> +
> +	bare_ref = orig_ref;
> +	skip_prefix(orig_ref, "refs/heads/", &bare_ref);
> +
> +	branch = branch_get(bare_ref);
> +	if (!branch->remote_name) {
> +		warning(_("asked to inherit tracking from '%s', but no remote is set"),
> +			bare_ref);
> +		return -1;
> +	}
> +
> +	if (branch->merge_nr < 1 || !branch->merge_name || !branch->merge_name[0]) {
> +		warning(_("asked to inherit tracking from '%s', but no merge configuration is set"),
> +			bare_ref);
> +		return -1;
> +	}
> +
> +	tracking->remote = xstrdup(branch->remote_name);
> +	for (i = 0; i < branch->merge_nr; i++)
> +		string_list_append(tracking->srcs, branch->merge_name[i]);
> +	tracking->matches = 1;
> +	return 0;
> +}

tracking->matches is used to keep track of the number of matched remote
refs. I believe we set tracking->matches = 1 to fulfill two specific
conditions in setup_tracking()...

> +
>  /*
>   * This is called when new_ref is branched off of orig_ref, and tries
>   * to infer the settings for branch.<new_ref>.{remote,merge} from the
> @@ -189,11 +218,15 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
>  			   enum branch_track track, int quiet)
>  {
>  	struct tracking tracking;
> +	struct string_list tracking_srcs = STRING_LIST_INIT_DUP;
>  	int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;
>  
>  	memset(&tracking, 0, sizeof(tracking));
>  	tracking.spec.dst = (char *)orig_ref;
> -	if (for_each_remote(find_tracked_branch, &tracking))
> +	tracking.srcs = &tracking_srcs;
> +	if (track != BRANCH_TRACK_INHERIT)
> +		for_each_remote(find_tracked_branch, &tracking);
> +	else if (inherit_tracking(&tracking, orig_ref))
>  		return;
>  
>  	if (!tracking.matches)

*extra context*
	if (!tracking.matches)
		switch (track) {
		case BRANCH_TRACK_ALWAYS:
		case BRANCH_TRACK_EXPLICIT:
		case BRANCH_TRACK_OVERRIDE:
			break;
		default:
			return;
		}

First, tracking.matches > 0, because we want to do work if there are
branches to track.

Secondly,

*extra context*
	if (tracking.matches > 1)
		die(_("Not tracking: ambiguous information for ref %s"),
		    orig_ref);

tracking.matches <= 1, because we don't want to set up tracking if it's
not obvious what ref we want to track.

But as I understand it, BRANCH_TRACK_INHERIT should be unconditional, so
instead of fudging this behavior by setting the correct value for
tracking.matches (which is meant for matching remote refs), we can just
do what the other unconditional BRANCH_TRACK_* options do, which is to
to break instead of return, i.e.

	if (!tracking.matches)
		switch (track) {
		case BRANCH_TRACK_ALWAYS:
		case BRANCH_TRACK_EXPLICIT:
		case BRANCH_TRACK_OVERRIDE:
+   case BRANCH_TRACK_INHERIT:
			break;
		default:
			return;
		}

and BRANCH_TRACK_INHERIT won't have to pretend that tracking.matches is
meaningful to it.

>@@ -210,11 +243,13 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,
> 		die(_("Not tracking: ambiguous information for ref %s"),
> 		    orig_ref);
> 
>-	if (install_branch_config(config_flags, new_ref, tracking.remote,
>-			      tracking.src ? tracking.src : orig_ref) < 0)
>+	if (tracking.srcs->nr < 1)
>+		string_list_append(tracking.srcs, orig_ref);
>+	if (install_branch_config_multiple_remotes(config_flags, new_ref, tracking.remote,
>+			      tracking.srcs) < 0)
> 		exit(-1);
> 
>-	free(tracking.src);
>+	string_list_clear(tracking.srcs, 0);
> }

It looks like install_branch_config_multiple_remotes() can just replace
install_branch_config() in setup_tracking(), nice. This should make it
pretty easy for me to rebase gc/branch-recurse-submodules onto this.