Subject: [PATCH v5 0/2] branch: inherit tracking configs

From: Josh Steadmon <[email-masked]>
Date: Mon, 6 Dec 2021 23:12:06 -0800

I've addressed feedback from V4. Since 2/3 reviewers seemed to (at least
slightly) prefer handling multiple upstream branches in the existing
tracking setup, I've gone that direction rather than repurposing the
branch copy code. None of the other issues were controversial.

In this version, I'd appreciate feedback mainly on patch 1:
* Is the combination of `git_config_set_gently()` +
  `git_config_set_multivar_gently() the best way to write multiple
  config entries for the same key?
* Does the reorganization of the BRANCH_CONFIG_VERBOSE output make
  things more readable, or less? Should I try to simplify the output
  here so that we don't end up with so many translatable variants of the
  same message?

Also, a question specifically for Junio: this will conflict with
gc/branch-recurse-submodules; should I rebase on that, or wait till it
hits next, or just ignore it for now?

Changes since V4:
* Add new patch (1/2) to refactor branch.c:install_branch_config() to
  accept multiple upstream refs
* When multiple upstream branches are set in the parent branch, inherit
  them all, instead of just the first
* Break out error string arguments for easier translation
* Don't ignore case for values of branch.autosetupmerge
* Move reference to git-pull out of usage string for --track into
  git-branch.txt
* Use test_config instead of `git config` in t2027
* Style fixes: add single-quotes around warning string arguments, remove
  unnecessary braces

Changes since V3:
* Use branch_get() instead of git_config_get_string() to look up branch
  configuration.
* Remove unnecessary string formatting in new error message in
  parse-options-cb.c.

Josh Steadmon (2):
  branch: accept multiple upstream branches for tracking
  branch: add flags and config to inherit tracking

 Documentation/config/branch.txt |   3 +-
 Documentation/git-branch.txt    |  24 +++--
 Documentation/git-checkout.txt  |   2 +-
 Documentation/git-switch.txt    |   2 +-
 branch.c                        | 169 ++++++++++++++++++++++++--------
 branch.h                        |   3 +-
 builtin/branch.c                |   6 +-
 builtin/checkout.c              |   6 +-
 config.c                        |   5 +-
 parse-options-cb.c              |  15 +++
 parse-options.h                 |   2 +
 t/t2017-checkout-orphan.sh      |   7 ++
 t/t2027-checkout-track.sh       |  23 +++++
 t/t2060-switch.sh               |  28 ++++++
 t/t3200-branch.sh               |  33 +++++++
 t/t7201-co.sh                   |  17 ++++
 16 files changed, 289 insertions(+), 56 deletions(-)

Range-diff against v4:
-:  ---------- > 1:  ba7d557725 branch: accept multiple upstream branches for tracking
1:  7ad7507f18 ! 2:  c7e4af9a36 branch: add flags and config to inherit tracking
    @@ Documentation/git-branch.txt: This option is only applicable in non-verbose mode
     +start-point is either a local or remote-tracking branch. Set it to
     +`inherit` if you want to copy the tracking configuration from the
     +branch point.
    +++
    ++See linkgit:git-pull[1] and linkgit:git-config[1] for additional discussion on
    ++how the `branch.<name>.remote` and `branch.<name>.merge` options are used.
      
      --no-track::
      	Do not set up "upstream" configuration, even if the
    @@ Documentation/git-switch.txt: should result in deletion of the path).
      	details.
     
      ## branch.c ##
    +@@
    + 
    + struct tracking {
    + 	struct refspec_item spec;
    +-	char *src;
    ++	struct string_list *srcs;
    + 	const char *remote;
    + 	int matches;
    + };
    +@@ branch.c: static int find_tracked_branch(struct remote *remote, void *priv)
    + 
    + 	if (!remote_find_tracking(remote, &tracking->spec)) {
    + 		if (++tracking->matches == 1) {
    +-			tracking->src = tracking->spec.src;
    ++			string_list_append(tracking->srcs, tracking->spec.src);
    + 			tracking->remote = remote->name;
    + 		} else {
    + 			free(tracking->spec.src);
    +-			FREE_AND_NULL(tracking->src);
    ++			string_list_clear(tracking->srcs, 0);
    + 		}
    + 		tracking->spec.src = NULL;
    + 	}
     @@ branch.c: int install_branch_config(int flag, const char *local, const char *origin, const
    - 	return -1;
    + 	string_list_clear(&remotes, 0);
      }
      
     +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"),
    ++		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"),
    ++		warning(_("asked to inherit tracking from '%s', but no merge configuration is set"),
     +			bare_ref);
     +		return -1;
     +	}
     +
     +	tracking->remote = xstrdup(branch->remote_name);
    -+	tracking->src = xstrdup(branch->merge_name[0]);
    ++	for (i = 0; i < branch->merge_nr; i++)
    ++		string_list_append(tracking->srcs, branch->merge_name[i]);
     +	tracking->matches = 1;
     +	return 0;
     +}
    @@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
       * 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
     @@ branch.c: 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))
    -+	if (track != BRANCH_TRACK_INHERIT) {
    ++	tracking.srcs = &tracking_srcs;
    ++	if (track != BRANCH_TRACK_INHERIT)
     +		for_each_remote(find_tracked_branch, &tracking);
    -+	} else if (inherit_tracking(&tracking, orig_ref))
    ++	else if (inherit_tracking(&tracking, orig_ref))
      		return;
      
      	if (!tracking.matches)
    +@@ branch.c: 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);
    + }
    + 
    + int read_branch_desc(struct strbuf *buf, const char *branch_name)
     
      ## branch.h ##
     @@ branch.h: enum branch_track {
    @@ builtin/branch.c: int cmd_branch(int argc, const char **argv, const char *prefix
     -		OPT_SET_INT('t', "track",  &track, N_("set up tracking mode (see git-pull(1))"),
     -			BRANCH_TRACK_EXPLICIT),
     +		OPT_CALLBACK_F('t', "track",  &track, "direct|inherit",
    -+			N_("set up tracking mode (see git-pull(1))"),
    ++			N_("set branch tracking configuration"),
     +			PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
     +			parse_opt_tracking_mode),
      		OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"),
    @@ builtin/checkout.c: static struct option *add_common_switch_branch_options(
      		OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unparented branch")),
     
      ## config.c ##
    -@@ config.c: static int git_default_branch_config(const char *var, const char *value)
    - 		if (value && !strcasecmp(value, "always")) {
    +@@ config.c: static int git_default_i18n_config(const char *var, const char *value)
    + static int git_default_branch_config(const char *var, const char *value)
    + {
    + 	if (!strcmp(var, "branch.autosetupmerge")) {
    +-		if (value && !strcasecmp(value, "always")) {
    ++		if (value && !strcmp(value, "always")) {
      			git_branch_track = BRANCH_TRACK_ALWAYS;
      			return 0;
    -+		} else if (value && !strcasecmp(value, "inherit")) {
    ++		} else if (value && !strcmp(value, "inherit")) {
     +			git_branch_track = BRANCH_TRACK_INHERIT;
     +			return 0;
      		}
    @@ parse-options-cb.c: int parse_opt_passthru_argv(const struct option *opt, const
     +	else if (!strcmp(arg, "inherit"))
     +		*(enum branch_track *)opt->value = BRANCH_TRACK_INHERIT;
     +	else
    -+		return error(_("option `--track' expects \"direct\" or \"inherit\""));
    ++		return error(_("option `%s' expects \"%s\" or \"%s\""),
    ++			     "--track", "direct", "inherit");
     +
     +	return 0;
     +}
    @@ t/t2027-checkout-track.sh: test_expect_success 'checkout --track -b rejects an e
      
     +test_expect_success 'checkout --track -b overrides autoSetupMerge=inherit' '
     +	# Set up tracking config on main
    -+	git config branch.main.remote origin &&
    -+	git config branch.main.merge refs/heads/main &&
    ++	test_config branch.main.remote origin &&
    ++	test_config branch.main.merge refs/heads/main &&
     +	test_config branch.autoSetupMerge inherit &&
     +	# With --track=inherit, we copy the tracking config from main
     +	git checkout --track=inherit -b b1 main &&

base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee
-- 
2.34.1.400.ga245620fadb-goog