git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Josh Steadmon <steadmon@google.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, chooglen@google.com, emilyshaffer@google.com,
	avarab@gmail.com
Subject: [PATCH v5 0/2] branch: inherit tracking configs
Date: Mon,  6 Dec 2021 23:12:06 -0800	[thread overview]
Message-ID: <cover.1638859949.git.steadmon@google.com> (raw)
In-Reply-To: <9628d145881cb875f8e284967e10f587b9f686f9.1631126999.git.steadmon@google.com>

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


  parent reply	other threads:[~2021-12-07  7:12 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 20:15 [RFC PATCH] branch: add "inherit" option for branch.autoSetupMerge Josh Steadmon
2021-09-08 20:44 ` Josh Steadmon
2021-09-11  0:25 ` [PATCH v2] " Josh Steadmon
2021-09-11  0:52   ` Junio C Hamano
2021-10-17  4:35     ` Josh Steadmon
2021-10-17  5:50       ` Junio C Hamano
2021-11-15 21:57         ` Josh Steadmon
2021-10-17  4:45 ` [PATCH v3] branch: add flags and config to inherit tracking Josh Steadmon
2021-10-18 18:31   ` Ævar Arnfjörð Bjarmason
2021-10-18 21:44     ` Junio C Hamano
2021-10-18 22:11       ` Ævar Arnfjörð Bjarmason
2021-11-15 22:22     ` Josh Steadmon
2021-10-18 17:50 ` [RFC PATCH] branch: add "inherit" option for branch.autoSetupMerge Glen Choo
2021-10-18 18:08   ` Glen Choo
2021-11-15 21:44   ` Josh Steadmon
2021-11-16 18:25 ` [PATCH v4] branch: add flags and config to inherit tracking Josh Steadmon
2021-11-17  0:33   ` Glen Choo
2021-11-18 22:29   ` Junio C Hamano
2021-11-30 22:05     ` Josh Steadmon
2021-11-19  6:47   ` Ævar Arnfjörð Bjarmason
2021-11-30 21:34     ` Josh Steadmon
2021-12-01  9:11       ` Ævar Arnfjörð Bjarmason
2021-12-07  7:12 ` Josh Steadmon [this message]
2021-12-07  7:12   ` [PATCH v5 1/2] branch: accept multiple upstream branches for tracking Josh Steadmon
2021-12-07  8:57     ` Ævar Arnfjörð Bjarmason
2021-12-09 23:03       ` Josh Steadmon
2021-12-10  1:00         ` Ævar Arnfjörð Bjarmason
2021-12-07 19:28     ` Junio C Hamano
2021-12-14 20:35       ` Josh Steadmon
2021-12-08  0:16     ` Glen Choo
2021-12-08  0:17     ` Glen Choo
2021-12-09 22:45       ` Josh Steadmon
2021-12-09 23:47         ` Glen Choo
2021-12-10  1:03           ` Ævar Arnfjörð Bjarmason
2021-12-10 17:32             ` Glen Choo
2021-12-11  2:18               ` Ævar Arnfjörð Bjarmason
2021-12-08 23:53     ` Glen Choo
2021-12-09  0:08       ` Glen Choo
2021-12-09 22:49         ` Josh Steadmon
2021-12-09 23:43           ` Glen Choo
2021-12-07  7:12   ` [PATCH v5 2/2] branch: add flags and config to inherit tracking Josh Steadmon
2021-12-07  9:08     ` Ævar Arnfjörð Bjarmason
2021-12-08  0:35       ` Glen Choo
2021-12-14 22:15         ` Josh Steadmon
2021-12-14 22:27       ` Josh Steadmon
2021-12-07 19:41     ` Junio C Hamano
2021-12-14 20:37       ` Josh Steadmon
2021-12-08  1:02     ` Glen Choo
2021-12-14 22:10       ` Josh Steadmon
2021-12-07 18:52   ` [PATCH v5 0/2] branch: inherit tracking configs Junio C Hamano
2021-12-08 17:06     ` Glen Choo
2021-12-10 22:48     ` Johannes Schindelin
2021-12-14 22:11       ` Josh Steadmon
2021-12-14 23:44 ` [PATCH v6 0/3] " Josh Steadmon
2021-12-14 23:44   ` [PATCH v6 1/3] branch: accept multiple upstream branches for tracking Josh Steadmon
2021-12-15 21:30     ` Junio C Hamano
2021-12-16 19:57     ` Glen Choo
2021-12-17  5:10       ` Josh Steadmon
2021-12-20 18:29         ` Glen Choo
2021-12-21  3:27           ` Josh Steadmon
2021-12-14 23:44   ` [PATCH v6 2/3] branch: add flags and config to inherit tracking Josh Steadmon
2021-12-16 21:27     ` Glen Choo
2021-12-17  5:11       ` Josh Steadmon
2021-12-14 23:44   ` [PATCH v6 3/3] config: require lowercase for branch.autosetupmerge Josh Steadmon
2021-12-15  0:43   ` [PATCH v6 0/3] branch: inherit tracking configs Josh Steadmon
2021-12-16  0:02   ` Junio C Hamano
2021-12-16  0:37     ` Glen Choo
2021-12-16  1:20       ` Junio C Hamano
2021-12-17  5:12 ` [PATCH v7 " Josh Steadmon
2021-12-17  5:12   ` [PATCH v7 1/3] branch: accept multiple upstream branches for tracking Josh Steadmon
2021-12-17  5:12   ` [PATCH v7 2/3] branch: add flags and config to inherit tracking Josh Steadmon
2021-12-17  5:12   ` [PATCH v7 3/3] config: require lowercase for branch.*.autosetupmerge Josh Steadmon
2021-12-20 21:05   ` [PATCH v7 0/3] branch: inherit tracking configs Glen Choo
2021-12-21  3:30 ` [PATCH v8 " Josh Steadmon
2021-12-21  3:30   ` [PATCH v8 1/3] branch: accept multiple upstream branches for tracking Josh Steadmon
2021-12-21  6:55     ` Junio C Hamano
2021-12-21 18:25       ` Glen Choo
2021-12-21  3:30   ` [PATCH v8 2/3] branch: add flags and config to inherit tracking Josh Steadmon
2021-12-21 18:17     ` Glen Choo
2022-01-11  1:57     ` incorrect 'git (checkout|branch) -h' output with new --track modes (was: [PATCH v8 2/3] branch: add flags and config to inherit tracking) Ævar Arnfjörð Bjarmason
2022-01-18 20:49       ` [PATCH] branch,checkout: fix --track usage strings Josh Steadmon
2022-01-18 22:26         ` Junio C Hamano
2022-01-19 10:56           ` [PATCH] parse-options: document automatic PARSE_OPT_LITERAL_ARGHELP René Scharfe
2022-01-19 14:41             ` Ævar Arnfjörð Bjarmason
     [not found]             ` <CA++g3E-azP3wFTtNkbFdmT7VW3hvULL0WkkAdwfrMb6HDtcXdg@mail.gmail.com>
2022-01-19 15:30               ` René Scharfe
2022-01-19 18:16             ` Junio C Hamano
2022-01-20 10:30               ` René Scharfe
2022-01-20 18:25                 ` Junio C Hamano
2022-01-21  9:42                   ` René Scharfe
2022-01-21 20:59                     ` Junio C Hamano
2022-01-20 12:05         ` [PATCH] branch,checkout: fix --track usage strings Ævar Arnfjörð Bjarmason
2022-01-20 12:18           ` Andreas Schwab
2022-01-20 14:00             ` Ævar Arnfjörð Bjarmason
2022-01-20 18:38           ` Junio C Hamano
2022-01-21 11:27             ` Ævar Arnfjörð Bjarmason
2022-01-21 21:12               ` Junio C Hamano
2022-01-19 10:20       ` incorrect 'git (checkout|branch) -h' output with new --track modes (was: [PATCH v8 2/3] branch: add flags and config to inherit tracking) René Scharfe
2022-01-20 12:00         ` Ævar Arnfjörð Bjarmason
2022-01-20 12:35           ` [PATCH] branch,checkout: fix --track documentation René Scharfe
2022-01-20 13:57             ` Ævar Arnfjörð Bjarmason
2022-01-20 19:08             ` Junio C Hamano
2021-12-21  3:30   ` [PATCH v8 3/3] config: require lowercase for branch.*.autosetupmerge Josh Steadmon
2021-12-21 18:13   ` [PATCH v8 0/3] branch: inherit tracking configs Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1638859949.git.steadmon@google.com \
    --to=steadmon@google.com \
    --cc=avarab@gmail.com \
    --cc=chooglen@google.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).