Re: [PATCH v5 2/2] branch: add flags and config to inherit tracking
- From
Glen Choo <chooglen@google.com>
- Date
- Dec 8, 2021, 01:02 UTC
- Message-ID
- <kl6lfsr3c3j7.fsf@chooglen-macbookpro.roam.corp.google.com>
- In-Reply-To
- <c7e4af9a365cc61e682c3c73045fb1ee4b79cfbf.1638859949.git.steadmon@google.com>
Josh Steadmon <steadmon@google.com> writes:
Show 28 quoted lines
> +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()...
Show 21 quoted lines
> +
> /*
> * 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.
Show 15 quoted lines
>@@ -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.