It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach branch.autoSetupMerge a new "inherit" option. When this is set,
creating a new branch will cause the tracking configuration to default
to the configuration of the branch point, if set.
NEEDS WORK:
* this breaks `git checkout -b new-branch --recurse-submodules`
* add documentation
* add tests
* check corner cases, including whether this plays well with related
cmd-line options (switch, checkout, branch)
Signed-off-by: Josh Steadmon <redacted>
---
I'll be looking into the --recurse-submodules breakage today, and then
I'll work on polishing the patch after that's fixed. But I thought it's
worth getting an idea of how the list feels about the feature in general
while I sort through the issues.
branch.c | 36 +++++++++++++++++++++++++++++++++++-
branch.h | 3 ++-
config.c | 3 +++
3 files changed, 40 insertions(+), 2 deletions(-)
@@ -126,6 +126,38 @@ int install_branch_config(int flag, const char *local, const char *origin, constreturn-1;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+structstrbufkey=STRBUF_INIT;+char*remote;+constchar*bare_ref;++bare_ref=orig_ref;+skip_prefix(orig_ref,"refs/heads/",&bare_ref);++strbuf_addf(&key,"branch.%s.remote",bare_ref);+if(git_config_get_string(key.buf,&remote)){+warning("branch.autoSetupMerge=inherit, but could not find %s",+key.buf);+strbuf_release(&key);+return1;+}+tracking->remote=remote;++strbuf_reset(&key);+strbuf_addf(&key,"branch.%s.merge",bare_ref);+if(git_config_get_string(key.buf,&tracking->src)){+warning("branch.autoSetupMerge=inherit, but could not find %s",+key.buf);+strbuf_release(&key);+return1;+}++tracking->matches=1;+strbuf_release(&key);+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach branch.autoSetupMerge a new "inherit" option. When this is set,
creating a new branch will cause the tracking configuration to default
to the configuration of the branch point, if set.
For example, if branch.autoSetupMerge=inherit, branch "main" tracks
"origin/main", and we run `git checkout -b feature main`, then branch
"feature" will track "origin/main". Thus, `git status` will show us how
far ahead/behind we are from origin, and `git pull` will pull from
origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
After a bit of testing, I've verified that this still works as intended
even without the extra patch [1] linked above. I've added documentation
and tests.
Range-diff against v1:
1: 9628d14588 ! 1: 0346f44754 branch: add "inherit" option for branch.autoSetupMerge
@@ Commit message
creating a new branch will cause the tracking configuration to default
to the configuration of the branch point, if set.
- NEEDS WORK:
- * this breaks `git checkout -b new-branch --recurse-submodules`
- * add documentation
- * add tests
- * check corner cases, including whether this plays well with related
- cmd-line options (switch, checkout, branch)
+ For example, if branch.autoSetupMerge=inherit, branch "main" tracks
+ "origin/main", and we run `git checkout -b feature main`, then branch
+ "feature" will track "origin/main". Thus, `git status` will show us how
+ far ahead/behind we are from origin, and `git pull` will pull from
+ origin.
+
+ This is particularly useful when creating branches across many
+ submodules, such as with `git submodule foreach ...` (or if running with
+ a patch such as [1], which we use at $job), as it avoids having to
+ manually set tracking info for each submodule.
+
+ [1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
+ ## Documentation/config/branch.txt ##
+@@ Documentation/config/branch.txt: branch.autoSetupMerge::
+ automatic setup is done; `true` -- automatic setup is done when the
+ starting point is a remote-tracking branch; `always` --
+ automatic setup is done when the starting point is either a
+- local branch or remote-tracking
++ local branch or remote-tracking branch; `inherit` -- if the starting point
++ has a tracking configuration, it is copied to the new
+ branch. This option defaults to true.
+
+ branch.autoSetupRebase::
+
+ ## Documentation/git-branch.txt ##
+@@ Documentation/git-branch.txt: This behavior is the default when the start point is a remote-tracking branch.
+ Set the branch.autoSetupMerge configuration variable to `false` if you
+ want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track`
+ were given. Set it to `always` if you want this behavior when the
+-start-point is either a local or remote-tracking branch.
++start-point is either a local or remote-tracking branch. Set it to
++`inherit` if you want to copy the tracking configuration from the
++start point.
+
+ --no-track::
+ Do not set up "upstream" configuration, even if the
+
## branch.c ##
@@ branch.c: int install_branch_config(int flag, const char *local, const char *origin, const
return -1;
@@ config.c: static int git_default_branch_config(const char *var, const char *valu
}
git_branch_track = git_config_bool(var, value);
return 0;
+
+ ## t/t2017-checkout-orphan.sh ##
+@@ t/t2017-checkout-orphan.sh: test_expect_success '--orphan ignores branch.autosetupmerge' '
+ git checkout --orphan gamma &&
+ test -z "$(git config branch.gamma.merge)" &&
+ test refs/heads/gamma = "$(git symbolic-ref HEAD)" &&
++ test_must_fail git rev-parse --verify HEAD^ &&
++ git checkout main &&
++ git config branch.autosetupmerge inherit &&
++ git checkout --orphan eta &&
++ test -z "$(git config branch.eta.merge)" &&
++ test -z "$(git config branch.eta.remote)" &&
++ test refs/heads/eta = "$(git symbolic-ref HEAD)" &&
+ test_must_fail git rev-parse --verify HEAD^
+ '
+
+
+ ## t/t2027-checkout-track.sh ##
+@@ t/t2027-checkout-track.sh: test_expect_success 'checkout --track -b rejects an extra path argument' '
+ test_i18ngrep "cannot be used with updating paths" err
+ '
+
++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.autoSetupMerge inherit &&
++ # With branch.autoSetupMerge=inherit, we copy the tracking config
++ git checkout -b b1 main &&
++ test_cmp_config origin branch.b1.remote &&
++ test_cmp_config refs/heads/main branch.b1.merge &&
++ # But --track overrides this
++ git checkout --track -b b2 main &&
++ test_cmp_config . branch.b2.remote &&
++ test_cmp_config refs/heads/main branch.b2.merge
++'
++
+ test_done
+
+ ## t/t2060-switch.sh ##
+@@ t/t2060-switch.sh: test_expect_success 'not switching when something is in progress' '
+ test_must_fail git switch -d @^
+ '
+
++test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
++ # default config does not copy tracking info
++ git switch -c foo-no-inherit foo &&
++ test -z "$(git config branch.foo-no-inherit.remote)" &&
++ test -z "$(git config branch.foo-no-inherit.merge)" &&
++ # with autoSetupMerge=inherit, we copy tracking info from foo
++ test_config branch.autoSetupMerge inherit &&
++ git switch -c foo2 foo &&
++ test_cmp_config origin branch.foo2.remote &&
++ test_cmp_config refs/heads/foo branch.foo2.merge &&
++ # no tracking info to inherit from main
++ git switch -c main2 main &&
++ test -z "$(git config branch.main2.remote)" &&
++ test -z "$(git config branch.main2.merge)"
++'
++
+ test_done
+
+ ## t/t3200-branch.sh ##
+@@ t/t3200-branch.sh: test_expect_success 'invalid sort parameter in configuration' '
+ )
+ '
+
++test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
++ test_unconfig branch.autoSetupMerge &&
++ # default config does not copy tracking info
++ git branch foo-no-inherit my1 &&
++ test -z "$(git config branch.foo-no-inherit.remote)" &&
++ test -z "$(git config branch.foo-no-inherit.merge)" &&
++ # with autoSetupMerge=inherit, we copy tracking info from my1
++ test_config branch.autoSetupMerge inherit &&
++ git branch foo2 my1 &&
++ test_cmp_config local branch.foo2.remote &&
++ test_cmp_config refs/heads/main branch.foo2.merge &&
++ # no tracking info to inherit from main
++ git branch main2 main &&
++ test -z "$(git config branch.main2.remote)" &&
++ test -z "$(git config branch.main2.merge)"
++'
++
+ test_done
+
+ ## t/t7201-co.sh ##
+@@ t/t7201-co.sh: test_expect_success 'custom merge driver with checkout -m' '
+ test_cmp expect arm
+ '
+
++test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
++ git reset --hard main &&
++ # default config does not copy tracking info
++ git checkout -b foo-no-inherit koala/bear &&
++ test -z "$(git config branch.foo-no-inherit.remote)" &&
++ test -z "$(git config branch.foo-no-inherit.merge)" &&
++ # with autoSetupMerge=inherit, we copy tracking info from koala/bear
++ test_config branch.autoSetupMerge inherit &&
++ git checkout -b foo koala/bear &&
++ test_cmp_config origin branch.foo.remote &&
++ test_cmp_config refs/heads/koala/bear branch.foo.merge &&
++ # no tracking info to inherit from main
++ git checkout -b main2 main &&
++ test -z "$(git config branch.main2.remote)" &&
++ test -z "$(git config branch.main2.merge)"
++'
++
+ test_done
Documentation/config/branch.txt | 3 ++-
Documentation/git-branch.txt | 4 +++-
branch.c | 36 ++++++++++++++++++++++++++++++++-
branch.h | 3 ++-
config.c | 3 +++
t/t2017-checkout-orphan.sh | 7 +++++++
t/t2027-checkout-track.sh | 15 ++++++++++++++
t/t2060-switch.sh | 16 +++++++++++++++
t/t3200-branch.sh | 17 ++++++++++++++++
t/t7201-co.sh | 17 ++++++++++++++++
10 files changed, 117 insertions(+), 4 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -218,7 +218,9 @@ This behavior is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+start-point is either a local or remote-tracking branch. Set it to+`inherit` if you want to copy the tracking configuration from the+start point. --no-track:: Do not set up "upstream" configuration, even if the
@@ -126,6 +126,38 @@ int install_branch_config(int flag, const char *local, const char *origin, constreturn-1;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+structstrbufkey=STRBUF_INIT;+char*remote;+constchar*bare_ref;++bare_ref=orig_ref;+skip_prefix(orig_ref,"refs/heads/",&bare_ref);++strbuf_addf(&key,"branch.%s.remote",bare_ref);+if(git_config_get_string(key.buf,&remote)){+warning("branch.autoSetupMerge=inherit, but could not find %s",+key.buf);+strbuf_release(&key);+return1;+}+tracking->remote=remote;++strbuf_reset(&key);+strbuf_addf(&key,"branch.%s.merge",bare_ref);+if(git_config_get_string(key.buf,&tracking->src)){+warning("branch.autoSetupMerge=inherit, but could not find %s",+key.buf);+strbuf_release(&key);+return1;+}++tracking->matches=1;+strbuf_release(&key);+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -24,4 +24,19 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+gitconfigbranch.main.remoteorigin&&+gitconfigbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With branch.autoSetupMerge=inherit, we copy the tracking config+gitcheckout-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# But --track overrides this+gitcheckout--track-bb2main&&+test_cmp_config.branch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge+'+ test_done
@@ -107,4 +107,20 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from foo+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
@@ -1409,4 +1409,21 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
Finally, teach branch.autoSetupMerge a new "inherit" option. When this
is set, "--track=inherit" becomes the default behavior.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
Range-diff against v2:
1: 0346f44754 ! 1: b9356d9837 branch: add "inherit" option for branch.autoSetupMerge
@@ Metadata
Author: Josh Steadmon [off-list ref]
## Commit message ##
- branch: add "inherit" option for branch.autoSetupMerge
+ branch: add flags and config to inherit tracking
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
- Teach branch.autoSetupMerge a new "inherit" option. When this is set,
- creating a new branch will cause the tracking configuration to default
- to the configuration of the branch point, if set.
+ Teach git-{branch,checkout,switch} an "inherit" argument to the
+ "--track" option. When this is set, creating a new branch will cause the
+ tracking configuration to default to the configuration of the branch
+ point, if set.
- For example, if branch.autoSetupMerge=inherit, branch "main" tracks
- "origin/main", and we run `git checkout -b feature main`, then branch
- "feature" will track "origin/main". Thus, `git status` will show us how
- far ahead/behind we are from origin, and `git pull` will pull from
- origin.
+ For example, if branch "main" tracks "origin/main", and we run
+ `git checkout --track=inherit -b feature main`, then branch "feature"
+ will track "origin/main". Thus, `git status` will show us how far
+ ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
+ Since we've added an argument to "--track", also add "--track=direct" as
+ another way to explicitly get the original "--track" behavior ("--track"
+ without an argument still works as well).
+
+ Finally, teach branch.autoSetupMerge a new "inherit" option. When this
+ is set, "--track=inherit" becomes the default behavior.
+
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
@@ Documentation/config/branch.txt: branch.autoSetupMerge::
branch.autoSetupRebase::
## Documentation/git-branch.txt ##
-@@ Documentation/git-branch.txt: This behavior is the default when the start point is a remote-tracking branch.
+@@ Documentation/git-branch.txt: SYNOPSIS
+ [--points-at <object>] [--format=<format>]
+ [(-r | --remotes) | (-a | --all)]
+ [--list] [<pattern>...]
+-'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
++'git branch' [--track [direct|inherit] | --no-track] [-f] <branchname> [<start-point>]
+ 'git branch' (--set-upstream-to=<upstream> | -u <upstream>) [<branchname>]
+ 'git branch' --unset-upstream [<branchname>]
+ 'git branch' (-m | -M) [<oldbranch>] <newbranch>
+@@ Documentation/git-branch.txt: This option is only applicable in non-verbose mode.
+ Display the full sha1s in the output listing rather than abbreviating them.
+
+ -t::
+---track::
++--track [inherit|direct]::
+ When creating a new branch, set up `branch.<name>.remote` and
+- `branch.<name>.merge` configuration entries to mark the
+- start-point branch as "upstream" from the new branch. This
++ `branch.<name>.merge` configuration entries to set "upstream" tracking
++ configuration for the new branch. This
+ configuration will tell git to show the relationship between the
+ two branches in `git status` and `git branch -v`. Furthermore,
+ it directs `git pull` without arguments to pull from the
+ upstream when the new branch is checked out.
+ +
+-This behavior is the default when the start point is a remote-tracking branch.
++The exact upstream branch is chosen depending on the optional argument:
++`--track` or `--track direct` means to use the start-point branch itself as the
++upstream; `--track inherit` means to copy the upstream configuration of the
++start-point branch.
+++
++`--track direct` is the default when the start point is a remote-tracking branch.
Set the branch.autoSetupMerge configuration variable to `false` if you
want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track`
were given. Set it to `always` if you want this behavior when the
-start-point is either a local or remote-tracking branch.
+start-point is either a local or remote-tracking branch. Set it to
+`inherit` if you want to copy the tracking configuration from the
-+start point.
++branch point.
--no-track::
Do not set up "upstream" configuration, even if the
+- branch.autoSetupMerge configuration variable is true.
++ branch.autoSetupMerge configuration variable is set.
+
+ --set-upstream::
+ As this option had confusing syntax, it is no longer supported.
+
+ ## Documentation/git-checkout.txt ##
+@@ Documentation/git-checkout.txt: of it").
+ linkgit:git-branch[1] for details.
+
+ -t::
+---track::
++--track [direct|inherit]::
+ When creating a new branch, set up "upstream" configuration. See
+ "--track" in linkgit:git-branch[1] for details.
+ +
+
+ ## Documentation/git-switch.txt ##
+@@ Documentation/git-switch.txt: should result in deletion of the path).
+ attached to a terminal, regardless of `--quiet`.
+
+ -t::
+---track::
++--track [direct|inherit]::
+ When creating a new branch, set up "upstream" configuration.
+ `-c` is implied. See `--track` in linkgit:git-branch[1] for
+ details.
## branch.c ##
@@ branch.c: int install_branch_config(int flag, const char *local, const char *origin, const
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
+
+ strbuf_addf(&key, "branch.%s.remote", bare_ref);
+ if (git_config_get_string(key.buf, &remote)) {
-+ warning("branch.autoSetupMerge=inherit, but could not find %s",
-+ key.buf);
++ warning(_("asked to inherit tracking from %s, but could not find %s"),
++ bare_ref, key.buf);
+ strbuf_release(&key);
-+ return 1;
++ return -1;
+ }
-+ tracking->remote = remote;
+
+ strbuf_reset(&key);
+ strbuf_addf(&key, "branch.%s.merge", bare_ref);
+ if (git_config_get_string(key.buf, &tracking->src)) {
-+ warning("branch.autoSetupMerge=inherit, but could not find %s",
-+ key.buf);
++ warning(_("asked to inherit tracking from %s, but could not find %s"),
++ bare_ref, key.buf);
+ strbuf_release(&key);
-+ return 1;
++ free(remote);
++ return -1;
+ }
+
++ tracking->remote = remote;
+ tracking->matches = 1;
+ strbuf_release(&key);
+ return 0;
@@ branch.c: static void setup_tracking(const char *new_ref, const char *orig_ref,
memset(&tracking, 0, sizeof(tracking));
tracking.spec.dst = (char *)orig_ref;
- if (for_each_remote(find_tracked_branch, &tracking))
-+ if (track == BRANCH_TRACK_INHERIT && inherit_tracking(&tracking, orig_ref))
-+ return;
-+ else if (for_each_remote(find_tracked_branch, &tracking))
++ if (track != BRANCH_TRACK_INHERIT) {
++ for_each_remote(find_tracked_branch, &tracking);
++ } else if (inherit_tracking(&tracking, orig_ref))
return;
if (!tracking.matches)
@@ branch.h: enum branch_track {
extern enum branch_track git_branch_track;
+ ## builtin/branch.c ##
+@@ builtin/branch.c: int cmd_branch(int argc, const char **argv, const char *prefix)
+ OPT__VERBOSE(&filter.verbose,
+ N_("show hash and subject, give twice for upstream branch")),
+ OPT__QUIET(&quiet, N_("suppress informational messages")),
+- 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))"),
++ PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
++ parse_opt_tracking_mode),
+ OPT_SET_INT_F(0, "set-upstream", &track, N_("do not use"),
+ BRANCH_TRACK_OVERRIDE, PARSE_OPT_HIDDEN),
+ OPT_STRING('u', "set-upstream-to", &new_upstream, N_("upstream"), N_("change the upstream info")),
+
+ ## builtin/checkout.c ##
+@@ builtin/checkout.c: static struct option *add_common_switch_branch_options(
+ {
+ struct option options[] = {
+ OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
+- OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"),
+- BRANCH_TRACK_EXPLICIT),
++ OPT_CALLBACK_F('t', "track", &opts->track, "direct|inherit",
++ N_("set up tracking mode (see git-pull(1))"),
++ PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
++ parse_opt_tracking_mode),
+ OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"),
+ PARSE_OPT_NOCOMPLETE),
+ 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_branch_config(const char *var, const char *valu
git_branch_track = git_config_bool(var, value);
return 0;
+ ## parse-options-cb.c ##
+@@
+ #include "git-compat-util.h"
+ #include "parse-options.h"
++#include "branch.h"
+ #include "cache.h"
+ #include "commit.h"
+ #include "color.h"
+@@ parse-options-cb.c: int parse_opt_passthru_argv(const struct option *opt, const char *arg, int unset
+
+ return 0;
+ }
++
++int parse_opt_tracking_mode(const struct option *opt, const char *arg, int unset) {
++ if (unset)
++ *(enum branch_track *)opt->value = BRANCH_TRACK_NEVER;
++ else if (!arg || !strcmp(arg, "direct"))
++ *(enum branch_track *)opt->value = BRANCH_TRACK_EXPLICIT;
++ else if (!strcmp(arg, "inherit"))
++ *(enum branch_track *)opt->value = BRANCH_TRACK_INHERIT;
++ else
++ return error(_("option `%s' expects \"direct\" or \"inherit\""),
++ opt->long_name);
++
++ return 0;
++}
+
+ ## parse-options.h ##
+@@ parse-options.h: enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
+ const char *, int);
+ int parse_opt_passthru(const struct option *, const char *, int);
+ int parse_opt_passthru_argv(const struct option *, const char *, int);
++/* value is enum branch_track* */
++int parse_opt_tracking_mode(const struct option *, const char *, int);
+
+ #define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h))
+ #define OPT__QUIET(var, h) OPT_COUNTUP('q', "quiet", (var), (h))
+
## t/t2017-checkout-orphan.sh ##
@@ t/t2017-checkout-orphan.sh: test_expect_success '--orphan ignores branch.autosetupmerge' '
git checkout --orphan gamma &&
@@ t/t2027-checkout-track.sh: test_expect_success 'checkout --track -b rejects an e
+ git config branch.main.remote origin &&
+ git config branch.main.merge refs/heads/main &&
+ test_config branch.autoSetupMerge inherit &&
-+ # With branch.autoSetupMerge=inherit, we copy the tracking config
-+ git checkout -b b1 main &&
++ # With --track=inherit, we copy the tracking config from main
++ git checkout --track=inherit -b b1 main &&
+ test_cmp_config origin branch.b1.remote &&
+ test_cmp_config refs/heads/main branch.b1.merge &&
++ # With branch.autoSetupMerge=inherit, we do the same
++ git checkout -b b2 main &&
++ test_cmp_config origin branch.b2.remote &&
++ test_cmp_config refs/heads/main branch.b2.merge &&
+ # But --track overrides this
-+ git checkout --track -b b2 main &&
-+ test_cmp_config . branch.b2.remote &&
-+ test_cmp_config refs/heads/main branch.b2.merge
++ git checkout --track -b b3 main &&
++ test_cmp_config . branch.b3.remote &&
++ test_cmp_config refs/heads/main branch.b3.merge &&
++ # And --track=direct does as well
++ git checkout --track=direct -b b4 main &&
++ test_cmp_config . branch.b4.remote &&
++ test_cmp_config refs/heads/main branch.b4.merge
+'
+
test_done
@@ t/t2060-switch.sh: test_expect_success 'not switching when something is in progr
+ git switch -c foo-no-inherit foo &&
+ test -z "$(git config branch.foo-no-inherit.remote)" &&
+ test -z "$(git config branch.foo-no-inherit.merge)" &&
-+ # with autoSetupMerge=inherit, we copy tracking info from foo
-+ test_config branch.autoSetupMerge inherit &&
-+ git switch -c foo2 foo &&
++ # with --track=inherit, we copy tracking info from foo
++ git switch --track=inherit -c foo2 foo &&
+ test_cmp_config origin branch.foo2.remote &&
+ test_cmp_config refs/heads/foo branch.foo2.merge &&
++ # with autoSetupMerge=inherit, we do the same
++ test_config branch.autoSetupMerge inherit &&
++ git switch -c foo3 foo &&
++ test_cmp_config origin branch.foo3.remote &&
++ test_cmp_config refs/heads/foo branch.foo3.merge &&
++ # with --track, we override autoSetupMerge
++ git switch --track -c foo4 foo &&
++ test_cmp_config . branch.foo4.remote &&
++ test_cmp_config refs/heads/foo branch.foo4.merge &&
++ # and --track=direct does as well
++ git switch --track=direct -c foo5 foo &&
++ test_cmp_config . branch.foo5.remote &&
++ test_cmp_config refs/heads/foo branch.foo5.merge &&
+ # no tracking info to inherit from main
+ git switch -c main2 main &&
+ test -z "$(git config branch.main2.remote)" &&
@@ t/t3200-branch.sh: test_expect_success 'invalid sort parameter in configuration'
)
'
++test_expect_success 'tracking info copied with --track=inherit' '
++ git branch --track=inherit foo2 my1 &&
++ test_cmp_config local branch.foo2.remote &&
++ test_cmp_config refs/heads/main branch.foo2.merge
++'
++
+test_expect_success 'tracking info copied with autoSetupMerge=inherit' '
+ test_unconfig branch.autoSetupMerge &&
+ # default config does not copy tracking info
@@ t/t3200-branch.sh: test_expect_success 'invalid sort parameter in configuration'
+ test -z "$(git config branch.foo-no-inherit.merge)" &&
+ # with autoSetupMerge=inherit, we copy tracking info from my1
+ test_config branch.autoSetupMerge inherit &&
-+ git branch foo2 my1 &&
-+ test_cmp_config local branch.foo2.remote &&
-+ test_cmp_config refs/heads/main branch.foo2.merge &&
++ git branch foo3 my1 &&
++ test_cmp_config local branch.foo3.remote &&
++ test_cmp_config refs/heads/main branch.foo3.merge &&
+ # no tracking info to inherit from main
+ git branch main2 main &&
+ test -z "$(git config branch.main2.remote)" &&
+ test -z "$(git config branch.main2.merge)"
+'
++
++test_expect_success '--track overrides branch.autoSetupMerge' '
++ test_config branch.autoSetupMerge inherit &&
++ git branch --track=direct foo4 my1 &&
++ test_cmp_config . branch.foo4.remote &&
++ test_cmp_config refs/heads/my1 branch.foo4.merge &&
++ git branch --no-track foo5 my1 &&
++ test -z "$(git config branch.foo5.remote)" &&
++ test -z "$(git config branch.foo5.merge)"
++'
+
test_done
Documentation/config/branch.txt | 3 ++-
Documentation/git-branch.txt | 21 ++++++++++++-------
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 37 ++++++++++++++++++++++++++++++++-
branch.h | 3 ++-
builtin/branch.c | 6 ++++--
builtin/checkout.c | 6 ++++--
config.c | 3 +++
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, 192 insertions(+), 16 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -205,24 +205,31 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track::+--track [inherit|direct]:: When creating a new branch, set up `branch.<name>.remote` and- `branch.<name>.merge` configuration entries to mark the- start-point branch as "upstream" from the new branch. This+ `branch.<name>.merge` configuration entries to set "upstream" tracking+ configuration for the new branch. This configuration will tell git to show the relationship between the two branches in `git status` and `git branch -v`. Furthermore, it directs `git pull` without arguments to pull from the upstream when the new branch is checked out. +-This behavior is the default when the start point is a remote-tracking branch.+The exact upstream branch is chosen depending on the optional argument:+`--track` or `--track direct` means to use the start-point branch itself as the+upstream; `--track inherit` means to copy the upstream configuration of the+start-point branch.+++`--track direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+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. --no-track:: Do not set up "upstream" configuration, even if the- branch.autoSetupMerge configuration variable is true.+ branch.autoSetupMerge configuration variable is set. --set-upstream:: As this option had confusing syntax, it is no longer supported.
@@ -155,7 +155,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -152,7 +152,7 @@ should result in deletion of the path). attached to a terminal, regardless of `--quiet`. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. `-c` is implied. See `--track` in linkgit:git-branch[1] for details.
@@ -126,6 +126,39 @@ int install_branch_config(int flag, const char *local, const char *origin, constreturn-1;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+structstrbufkey=STRBUF_INIT;+char*remote;+constchar*bare_ref;++bare_ref=orig_ref;+skip_prefix(orig_ref,"refs/heads/",&bare_ref);++strbuf_addf(&key,"branch.%s.remote",bare_ref);+if(git_config_get_string(key.buf,&remote)){+warning(_("asked to inherit tracking from %s, but could not find %s"),+bare_ref,key.buf);+strbuf_release(&key);+return-1;+}++strbuf_reset(&key);+strbuf_addf(&key,"branch.%s.merge",bare_ref);+if(git_config_get_string(key.buf,&tracking->src)){+warning(_("asked to inherit tracking from %s, but could not find %s"),+bare_ref,key.buf);+strbuf_release(&key);+free(remote);+return-1;+}++tracking->remote=remote;+tracking->matches=1;+strbuf_release(&key);+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT_SET_INT_F(0,"set-upstream",&track,N_("do not use"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),OPT_STRING('u',"set-upstream-to",&new_upstream,N_("upstream"),N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+gitconfigbranch.main.remoteorigin&&+gitconfigbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge+'+ test_done
@@ -107,4 +107,32 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with --track=inherit, we copy tracking info from foo+gitswitch--track=inherit-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# with autoSetupMerge=inherit, we do the same+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo3foo&&+test_cmp_configoriginbranch.foo3.remote&&+test_cmp_configrefs/heads/foobranch.foo3.merge&&+# with --track, we override autoSetupMerge+gitswitch--track-cfoo4foo&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/foobranch.foo4.merge&&+# and --track=direct does as well+gitswitch--track=direct-cfoo5foo&&+test_cmp_config.branch.foo5.remote&&+test_cmp_configrefs/heads/foobranch.foo5.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
@@ -1409,4 +1409,37 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with --track=inherit''+gitbranch--track=inheritfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge+'++test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo3my1&&+test_cmp_configlocalbranch.foo3.remote&&+test_cmp_configrefs/heads/mainbranch.foo3.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'++test_expect_success'--track overrides branch.autoSetupMerge''+test_configbranch.autoSetupMergeinherit&&+gitbranch--track=directfoo4my1&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/my1branch.foo4.merge&&+gitbranch--no-trackfoo5my1&&+test-z"$(gitconfigbranch.foo5.remote)"&&+test-z"$(gitconfigbranch.foo5.merge)"+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
I believe that we can get the branch remote via struct branch. Instead
of reading the config, we could do something along the lines of:
int *explicit;
struct branch *branch = branch_get();
char *remote = remote_for_branch(branch, explicit);
/* Optionally check explicit if we don't want to fall back to
* "origin" */
I'm not sure which is the idiomatic way to get the branch remote, feel
free to correct me.
I believe that we can get the branch remote via struct branch. Instead
of reading the config, we could do something along the lines of:
int *explicit;
struct branch *branch = branch_get();
char *remote = remote_for_branch(branch, explicit);
/* Optionally check explicit if we don't want to fall back to
* "origin" */
I'm not sure which is the idiomatic way to get the branch remote, feel
free to correct me.
Gah, I read and responded to v1 thinking it was v3. Reading v3, it looks
like the feedback is still relevant, so please pretend I responded to
the right email :P
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
There's no method to get *only* that config, but this use-case is why
the "-c" option (copy branch) was added.
I haven't looked at this in any detail, but the seeming lack of mention
of it in the commit message & docs makes me wonder if you missed that
that option could do what you wanted (but granted, it does a lot more,
which maybe you don't want).
But in terms of implementation can't this share more code with the copy
mode? I.e. I'd think that this would just be a limited mode of that,
where we pass some whitelist of specific config to copy over, instead
the current "all the config" with "copy".
And should these options be made to work together somehow? I.e. if you
want to copy branch A to B, but copy tracking info from C?
[...]
-t::
---track::
+--track [inherit|direct]::
When creating a new branch, set up `branch.<name>.remote` and
- `branch.<name>.merge` configuration entries to mark the
- start-point branch as "upstream" from the new branch. This
+ `branch.<name>.merge` configuration entries to set "upstream" tracking
+ configuration for the new branch. This
Setting up ".remote" is what --tracke does, but doesn't it make sense
for such an option to copy over any other config related to that area,
e.g. also .pushRemote, as a user may have edited it since the creation
of the copied-from branch?
Maybe, maybe not. But this & the above comparison with copy makes me
wonder if we'd be better off with some mode similar to the matching
regexes "git config", i.e. you could do a "copy" but only on a list of
matching variables.
Then the --track mode could just be implemented in terms of that, no?
I believe that we can get the branch remote via struct branch. Instead
of reading the config, we could do something along the lines of:
int *explicit;
struct branch *branch = branch_get();
char *remote = remote_for_branch(branch, explicit);
/* Optionally check explicit if we don't want to fall back to
* "origin" */
I'm not sure which is the idiomatic way to get the branch remote, feel
free to correct me.
On 2021.10.18 20:31, Ævar Arnfjörð Bjarmason wrote:
On Sat, Oct 16 2021, Josh Steadmon wrote:
quoted
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
There's no method to get *only* that config, but this use-case is why
the "-c" option (copy branch) was added.
I haven't looked at this in any detail, but the seeming lack of mention
of it in the commit message & docs makes me wonder if you missed that
that option could do what you wanted (but granted, it does a lot more,
which maybe you don't want).
Indeed, I did miss that option. Thank you for the pointer. I am
conflicted about whether or not we want to copy all the branch
configuration. Most of the options do seem useful to copy, but the
existing config values available for `branch.autoSetupMerge` are
strictly about setting up `branch.<name>.remote`, `branch.<name>.merge`,
and `branch.<name>.rebase`. Adding a new value here that additionally
pulls in all the rest of the config may be confusing. Alternatively we
could add an entirely new option, but then its interaction with
`branch.autoSetupMerge` would be confusing as well.
But in terms of implementation can't this share more code with the copy
mode? I.e. I'd think that this would just be a limited mode of that,
where we pass some whitelist of specific config to copy over, instead
the current "all the config" with "copy".
I will look into the copy machinery and see what can be reused in V4.
And should these options be made to work together somehow? I.e. if you
want to copy branch A to B, but copy tracking info from C?
I am skeptical of the benefit here, but I'm certainly willing to hear
arguments in favor.
The motivation for this series is for Git users (who are not necessarily
Git experts) to have a simple config they can tune to make reduce
friction for the use case of having large repositories with many
submodules (see Emily's discussion [1]). The idea is that we have many
people with a workflow where they'd have `submodule.recurse=true` and
`branch.autoSetupMerge=inherit`. When they checkout a new branch in the
superproject, branches would also be checked out in the submodules, and
appropriate tracking information would also be inherited so that they
can later `git push` without having to manually configure tracking for
every submodule.
This would be a very common operation for these users, and should
therefore require as little friction as possible. While I can see use
cases for your "copy A to B but copy tracking from C", it seems to me
that this would be a much less common situation, and is probably going
to be needed by Git experts who are capable of setting this manually
without relying on configs to make it the default behavior.
[1]: https://lore.kernel.org/git/YHofmWcIAidkvJiD@google.com/
quoted
[...]
-t::
---track::
+--track [inherit|direct]::
When creating a new branch, set up `branch.<name>.remote` and
- `branch.<name>.merge` configuration entries to mark the
- start-point branch as "upstream" from the new branch. This
+ `branch.<name>.merge` configuration entries to set "upstream" tracking
+ configuration for the new branch. This
Setting up ".remote" is what --tracke does, but doesn't it make sense
for such an option to copy over any other config related to that area,
e.g. also .pushRemote, as a user may have edited it since the creation
of the copied-from branch?
Yes, .pushRemote and .mergeOptions both seem like they'd be useful to
copy here.
Maybe, maybe not. But this & the above comparison with copy makes me
wonder if we'd be better off with some mode similar to the matching
regexes "git config", i.e. you could do a "copy" but only on a list of
matching variables.
Then the --track mode could just be implemented in terms of that, no?
Using config matching to only copy portions of the branch config seems
overkill to me. IMO it would be better to get agreement for which of the
branch.<name>.* variables to copy, and then use that consistently for
all possible settings of `branch.autoSetupMerge` and
`branch.autoSetupRebase`. If that allows us to reuse the existing copy
machinery, then so much the better.
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
Finally, teach branch.autoSetupMerge a new "inherit" option. When this
is set, "--track=inherit" becomes the default behavior.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
I've addressed Glen's feedback from V3. However, this brings up a new
issue that was not obvious before: "branch.<name>.merge" can be
specified more than once. On the other hand, the existing tracking setup
code supports only a single merge entry. For now I'm defaulting to use
the first merge entry listed in the branch struct, but I'm curious what
people think the best solution would be. This may be another point in
favor of Ævar's suggestion to reuse the copy-branch-config machinery.
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.
Range-diff against v3:
1: b9356d9837 ! 1: 7ad7507f18 branch: add flags and config to inherit tracking
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
+static int inherit_tracking(struct tracking *tracking, const char *orig_ref)
+{
-+ struct strbuf key = STRBUF_INIT;
-+ char *remote;
+ const char *bare_ref;
++ struct branch *branch;
+
+ bare_ref = orig_ref;
+ skip_prefix(orig_ref, "refs/heads/", &bare_ref);
+
-+ strbuf_addf(&key, "branch.%s.remote", bare_ref);
-+ if (git_config_get_string(key.buf, &remote)) {
-+ warning(_("asked to inherit tracking from %s, but could not find %s"),
-+ bare_ref, key.buf);
-+ strbuf_release(&key);
++ 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;
+ }
+
-+ strbuf_reset(&key);
-+ strbuf_addf(&key, "branch.%s.merge", bare_ref);
-+ if (git_config_get_string(key.buf, &tracking->src)) {
-+ warning(_("asked to inherit tracking from %s, but could not find %s"),
-+ bare_ref, key.buf);
-+ strbuf_release(&key);
-+ free(remote);
++ 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 = remote;
++ tracking->remote = xstrdup(branch->remote_name);
++ tracking->src = xstrdup(branch->merge_name[0]);
+ tracking->matches = 1;
-+ strbuf_release(&key);
+ 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 `%s' expects \"direct\" or \"inherit\""),
-+ opt->long_name);
++ return error(_("option `--track' expects \"direct\" or \"inherit\""));
+
+ return 0;
+}
Documentation/config/branch.txt | 3 ++-
Documentation/git-branch.txt | 21 ++++++++++++++-------
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 31 ++++++++++++++++++++++++++++++-
branch.h | 3 ++-
builtin/branch.c | 6 ++++--
builtin/checkout.c | 6 ++++--
config.c | 3 +++
parse-options-cb.c | 14 ++++++++++++++
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, 185 insertions(+), 16 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -205,24 +205,31 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track::+--track [inherit|direct]:: When creating a new branch, set up `branch.<name>.remote` and- `branch.<name>.merge` configuration entries to mark the- start-point branch as "upstream" from the new branch. This+ `branch.<name>.merge` configuration entries to set "upstream" tracking+ configuration for the new branch. This configuration will tell git to show the relationship between the two branches in `git status` and `git branch -v`. Furthermore, it directs `git pull` without arguments to pull from the upstream when the new branch is checked out. +-This behavior is the default when the start point is a remote-tracking branch.+The exact upstream branch is chosen depending on the optional argument:+`--track` or `--track direct` means to use the start-point branch itself as the+upstream; `--track inherit` means to copy the upstream configuration of the+start-point branch.+++`--track direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+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. --no-track:: Do not set up "upstream" configuration, even if the- branch.autoSetupMerge configuration variable is true.+ branch.autoSetupMerge configuration variable is set. --set-upstream:: As this option had confusing syntax, it is no longer supported.
@@ -155,7 +155,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -152,7 +152,7 @@ should result in deletion of the path). attached to a terminal, regardless of `--quiet`. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. `-c` is implied. See `--track` in linkgit:git-branch[1] for details.
@@ -126,6 +126,33 @@ int install_branch_config(int flag, const char *local, const char *origin, constreturn-1;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+constchar*bare_ref;+structbranch*branch;++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);+tracking->src=xstrdup(branch->merge_name[0]);+tracking->matches=1;+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT_SET_INT_F(0,"set-upstream",&track,N_("do not use"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),OPT_STRING('u',"set-upstream-to",&new_upstream,N_("upstream"),N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+gitconfigbranch.main.remoteorigin&&+gitconfigbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge+'+ test_done
@@ -107,4 +107,32 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with --track=inherit, we copy tracking info from foo+gitswitch--track=inherit-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# with autoSetupMerge=inherit, we do the same+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo3foo&&+test_cmp_configoriginbranch.foo3.remote&&+test_cmp_configrefs/heads/foobranch.foo3.merge&&+# with --track, we override autoSetupMerge+gitswitch--track-cfoo4foo&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/foobranch.foo4.merge&&+# and --track=direct does as well+gitswitch--track=direct-cfoo5foo&&+test_cmp_config.branch.foo5.remote&&+test_cmp_configrefs/heads/foobranch.foo5.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
@@ -1409,4 +1409,37 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with --track=inherit''+gitbranch--track=inheritfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge+'++test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo3my1&&+test_cmp_configlocalbranch.foo3.remote&&+test_cmp_configrefs/heads/mainbranch.foo3.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'++test_expect_success'--track overrides branch.autoSetupMerge''+test_configbranch.autoSetupMergeinherit&&+gitbranch--track=directfoo4my1&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/my1branch.foo4.merge&&+gitbranch--no-trackfoo5my1&&+test-z"$(gitconfigbranch.foo5.remote)"&&+test-z"$(gitconfigbranch.foo5.merge)"+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
I've addressed Glen's feedback from V3. However, this brings up a new
issue that was not obvious before: "branch.<name>.merge" can be
specified more than once. On the other hand, the existing tracking setup
code supports only a single merge entry.
Yes, for istance, install_branch_config() uses git_config_set_gently(),
which will override duplicate values.
For now I'm defaulting to use the first merge entry listed in the
branch struct, but I'm curious what people think the best solution
would be.
I can think of at least two possibilities:
The first would be to parse the information into our native data
structures. This is pretty much what you've done in v4, but insteaed of
defaulting to the first merge entry, we would iterate over all of the
possible merge entries and...
quoted hunk
@@ -139,7 +166,9 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, memset(&tracking, 0, sizeof(tracking)); tracking.spec.dst = (char *)orig_ref;- if (for_each_remote(find_tracked_branch, &tracking))+ if (track != BRANCH_TRACK_INHERIT) {+ for_each_remote(find_tracked_branch, &tracking);+ } else if (inherit_tracking(&tracking, orig_ref)) return; if (!tracking.matches)
we get rid of the assumption that we can use a single 'struct tracking'.
when track=BRANCH_TRACK_INHERIT. Of course, this isn't as simple as
calling install_branch_config() repeatedly, because that would override
"branch.<name>.merge" over and over.
This may be another point in favor of Ævar's suggestion to
reuse the copy-branch-config machinery.
This is the second option, which is pretty simple. Inheriting the branch
tracking info is a matter of copying the config, which we already do
when we copy branches in builtin/branch.c:
strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
strbuf_release(&oldref);
strbuf_addf(&newsection, "branch.%s", interpreted_newname);
strbuf_release(&newref);
if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
die(_("Branch is renamed, but update of config-file failed"));
Between these two options, I think the first is a better long-term
solution because I think that parsing the config into our own data
structures is generally less error-prone than operating directly on a
file (e.g. using the data structures was what made this bug obvious to
us in the first place, using repo->config will handle multiple config
files correctly). I don't see '--track=inherit' as being _that_
conceptually similar to copying a branch; I see it as a different mode
of tracking that just so happens to be implementable by copying some
sections in the branch configuration.
But as a practical matter, I don't see any obviously terrible short-term
downsides to just copying the config. It's no less correct than our
branch copying logic and I'm afaid of introducing unintended
consequences by mucking around with install_branch_config().
I've addressed Glen's feedback from V3. However, this brings up a new
issue that was not obvious before: "branch.<name>.merge" can be
specified more than once. On the other hand, the existing tracking setup
code supports only a single merge entry. For now I'm defaulting to use
the first merge entry listed in the branch struct, but I'm curious what
people think the best solution would be. This may be another point in
favor of Ævar's suggestion to reuse the copy-branch-config machinery.
I haven't looked in any detail now at the "should we copy the config?"
questions. Just some quick comments/nits below:
+static int inherit_tracking(struct tracking *tracking, const char *orig_ref)
+{
+ const char *bare_ref;
+ struct branch *branch;
+
+ 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);
Should quote ('%s') the %s in both here.
quoted hunk
+ return -1;
+ }
+
+ tracking->remote = xstrdup(branch->remote_name);
+ tracking->src = xstrdup(branch->merge_name[0]);
+ tracking->matches = 1;
+ return 0;
+}
+
/*
* 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
Style: Dangling braces, can just skip the braces here.
quoted hunk
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT__VERBOSE(&filter.verbose, N_("show hash and subject, give twice for upstream branch")), OPT__QUIET(&quiet, N_("suppress informational messages")),- 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))"),
Hrm, should we say "git help pull" here, on just not reference it at all
and have a linkgit:git-pull[1]?
Or maybe git-branch.txt and git-pull.txt should be including a template?
As we do with Documentation/rev-list-options.txt, then this
cross-reference wouldn't be needed.
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
Looks like an existing issue, but we just document "inherit", not
"INHERIT", "iNhErIt" etc. I.e. should it being strcasecmp()
v.s. strcmp() be documented?
+ return error(_("option `--track' expects \"direct\" or \"inherit\""));
Already commented-on by Junio.
+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.autoSetupMerge inherit &&
+ # With --track=inherit, we copy the tracking config from main
+ git checkout --track=inherit -b b1 main &&
+ test_cmp_config origin branch.b1.remote &&
+ test_cmp_config refs/heads/main branch.b1.merge &&
+ # With branch.autoSetupMerge=inherit, we do the same
+ git checkout -b b2 main &&
+ test_cmp_config origin branch.b2.remote &&
+ test_cmp_config refs/heads/main branch.b2.merge &&
+ # But --track overrides this
+ git checkout --track -b b3 main &&
+ test_cmp_config . branch.b3.remote &&
+ test_cmp_config refs/heads/main branch.b3.merge &&
+ # And --track=direct does as well
+ git checkout --track=direct -b b4 main &&
+ test_cmp_config . branch.b4.remote &&
+ test_cmp_config refs/heads/main branch.b4.merge
+'
+
This is the last test, we can use test_config instead of "git config"
there I think, i.e. it's not setting up config for subseuent tests.
On 2021.11.19 07:47, Ævar Arnfjörð Bjarmason wrote:
On Tue, Nov 16 2021, Josh Steadmon wrote:
quoted
I've addressed Glen's feedback from V3. However, this brings up a new
issue that was not obvious before: "branch.<name>.merge" can be
specified more than once. On the other hand, the existing tracking setup
code supports only a single merge entry. For now I'm defaulting to use
the first merge entry listed in the branch struct, but I'm curious what
people think the best solution would be. This may be another point in
favor of Ævar's suggestion to reuse the copy-branch-config machinery.
I haven't looked in any detail now at the "should we copy the config?"
questions. Just some quick comments/nits below:
Thanks for the comments. They're all fixed in V5, which I'll be sending
out soon.
[snip]
quoted
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT__VERBOSE(&filter.verbose, N_("show hash and subject, give twice for upstream branch")), OPT__QUIET(&quiet, N_("suppress informational messages")),- 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))"),
Hrm, should we say "git help pull" here, on just not reference it at all
and have a linkgit:git-pull[1]?
Or maybe git-branch.txt and git-pull.txt should be including a template?
As we do with Documentation/rev-list-options.txt, then this
cross-reference wouldn't be needed.
Yeah, there's nothing really helpful in git-pull(1) about "--track"
that's easily searchable (i.e. without reading it all straight through),
so I just removed the pointer in the option help string, add added
linkgit:git-pull(1) and linkgit:git-config(1) to git-branch.txt.
I briefly looked at writing a common template for both git-branch.txt
and git-pull.txt but I feel like the git-pull discussion of tracking is
so spread out in that doc that it would require a significant rewrite to
make a common template work.
On 2021.11.19 07:47, Ævar Arnfjörð Bjarmason wrote:
quoted
On Tue, Nov 16 2021, Josh Steadmon wrote:
quoted
I've addressed Glen's feedback from V3. However, this brings up a new
issue that was not obvious before: "branch.<name>.merge" can be
specified more than once. On the other hand, the existing tracking setup
code supports only a single merge entry. For now I'm defaulting to use
the first merge entry listed in the branch struct, but I'm curious what
people think the best solution would be. This may be another point in
favor of Ævar's suggestion to reuse the copy-branch-config machinery.
I haven't looked in any detail now at the "should we copy the config?"
questions. Just some quick comments/nits below:
Thanks for the comments. They're all fixed in V5, which I'll be sending
out soon.
[snip]
Thanks, happy that it helped.
quoted
quoted
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix) OPT__VERBOSE(&filter.verbose, N_("show hash and subject, give twice for upstream branch")), OPT__QUIET(&quiet, N_("suppress informational messages")),- 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))"),
Hrm, should we say "git help pull" here, on just not reference it at all
and have a linkgit:git-pull[1]?
Or maybe git-branch.txt and git-pull.txt should be including a template?
As we do with Documentation/rev-list-options.txt, then this
cross-reference wouldn't be needed.
Yeah, there's nothing really helpful in git-pull(1) about "--track"
that's easily searchable (i.e. without reading it all straight through),
so I just removed the pointer in the option help string, add added
linkgit:git-pull(1) and linkgit:git-config(1) to git-branch.txt.
I briefly looked at writing a common template for both git-branch.txt
and git-pull.txt but I feel like the git-pull discussion of tracking is
so spread out in that doc that it would require a significant rewrite to
make a common template work.
*nod*. I didn't look into if it was easy/doable, just a hint in case
that direction was fruitful. Makes sense.
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
Add a new static variant of install_branch_config() that accepts
multiple remote branch names for tracking. This will be used in an
upcoming commit that enables inheriting the tracking configuration from
a parent branch.
Currently, all callers of install_branch_config() pass only a single
remote. Make install_branch_config() a small wrapper around
install_branch_config_multiple_remotes() so that existing callers do not
need to be changed.
Signed-off-by: Josh Steadmon <redacted>
---
branch.c | 120 ++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 87 insertions(+), 33 deletions(-)
@@ -55,19 +55,24 @@ N_("\n""the remote tracking information by invoking\n""\"git branch --set-upstream-to=%s%s%s\".");-intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,constchar*origin,+structstring_list*remotes){constchar*shortname=NULL;structstrbufkey=STRBUF_INIT;-intrebasing=should_setup_rebase(origin);--if(skip_prefix(remote,"refs/heads/",&shortname)-&&!strcmp(local,shortname)-&&!origin){-warning(_("Not setting branch %s as its own upstream."),-local);-return0;-}+inti,rebasing=should_setup_rebase(origin);++if(remotes->nr<1)+BUG("must provide at least one remote for branch config");++if(!origin)+for(i=0;i<remotes->nr;i++)+if(skip_prefix(remotes->items[i].string,"refs/heads/",&shortname)+&&!strcmp(local,shortname)){+warning(_("Not setting branch %s as its own upstream."),+local);+return0;+}strbuf_addf(&key,"branch.%s.remote",local);if(git_config_set_gently(key.buf,origin?origin:".")<0)
@@ -87,29 +101,62 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);if(flag&BRANCH_CONFIG_VERBOSE){-if(shortname){-if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing."):-_("Branch '%s' set up to track remote branch '%s' from '%s'."),-local,shortname,origin);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local branch '%s' by rebasing."):-_("Branch '%s' set up to track local branch '%s'."),-local,shortname);+intplural=remotes->nr>1;+intall_shortnames=1;+constchar*msg_fmt;+structstrbufref_string=STRBUF_INIT;++for(i=0;i<remotes->nr;i++)+if(skip_prefix(remotes->items[i].string,"refs/heads/",&shortname)){+strbuf_addf(&ref_string,"'%s', ",shortname);+}else{+all_shortnames=0;+strbuf_addf(&ref_string,"'%s', ",remotes->items[i].string);+}+/* The last two characters are an extraneous ", ", so trim those. */+strbuf_setlen(&ref_string,ref_string.len-2);++if(all_shortnames&&origin){+if(rebasing&&plural)+msg_fmt="Branch '%s' set up to track remote branches %s from '%s' by rebasing.";+elseif(rebasing&&!plural)+msg_fmt="Branch '%s' set up to track remote branch %s from '%s' by rebasing.";+elseif(!rebasing&&plural)+msg_fmt="Branch '%s' set up to track remote branches %s from '%s'.";+elseif(!rebasing&&!plural)+msg_fmt="Branch '%s' set up to track remote branch %s from '%s'.";++printf_ln(_(msg_fmt),local,ref_string,origin);}else{-if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote ref '%s' by rebasing."):-_("Branch '%s' set up to track remote ref '%s'."),-local,remote);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local ref '%s' by rebasing."):-_("Branch '%s' set up to track local ref '%s'."),-local,remote);+if(all_shortnames&&!origin&&rebasing&&plural)+msg_fmt="Branch '%s' set up to track local branches %s by rebasing.";+if(all_shortnames&&!origin&&rebasing&&!plural)+msg_fmt="Branch '%s' set up to track local branch %s by rebasing.";+if(all_shortnames&&!origin&&!rebasing&&plural)+msg_fmt="Branch '%s' set up to track local branches %s.";+if(all_shortnames&&!origin&&!rebasing&&!plural)+msg_fmt="Branch '%s' set up to track local branch %s.";+if(!all_shortnames&&origin&&rebasing&&plural)+msg_fmt="Branch '%s' set up to track remote refs %s by rebasing.";+if(!all_shortnames&&origin&&rebasing&&!plural)+msg_fmt="Branch '%s' set up to track remote ref %s by rebasing.";+if(!all_shortnames&&origin&&!rebasing&&plural)+msg_fmt="Branch '%s' set up to track remote refs %s.";+if(!all_shortnames&&origin&&!rebasing&&!plural)+msg_fmt="Branch '%s' set up to track remote ref %s.";+if(!all_shortnames&&!origin&&rebasing&&plural)+msg_fmt="Branch '%s' set up to track local refs %s by rebasing.";+if(!all_shortnames&&!origin&&rebasing&&!plural)+msg_fmt="Branch '%s' set up to track local ref %s by rebasing.";+if(!all_shortnames&&!origin&&!rebasing&&plural)+msg_fmt="Branch '%s' set up to track local refs %s.";+if(!all_shortnames&&!origin&&!rebasing&&!plural)+msg_fmt="Branch '%s' set up to track local ref %s.";++printf_ln(_(msg_fmt),local,ref_string);}++strbuf_release(&ref_string);}return0;
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
Finally, teach branch.autoSetupMerge a new "inherit" option. When this
is set, "--track=inherit" becomes the default behavior.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 +++++++++++-----
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 49 ++++++++++++++++++++++++++++-----
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, 202 insertions(+), 23 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -205,24 +205,34 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track::+--track [inherit|direct]:: When creating a new branch, set up `branch.<name>.remote` and- `branch.<name>.merge` configuration entries to mark the- start-point branch as "upstream" from the new branch. This+ `branch.<name>.merge` configuration entries to set "upstream" tracking+ configuration for the new branch. This configuration will tell git to show the relationship between the two branches in `git status` and `git branch -v`. Furthermore, it directs `git pull` without arguments to pull from the upstream when the new branch is checked out. +-This behavior is the default when the start point is a remote-tracking branch.+The exact upstream branch is chosen depending on the optional argument:+`--track` or `--track direct` means to use the start-point branch itself as the+upstream; `--track inherit` means to copy the upstream configuration of the+start-point branch.+++`--track direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+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- branch.autoSetupMerge configuration variable is true.+ branch.autoSetupMerge configuration variable is set. --set-upstream:: As this option had confusing syntax, it is no longer supported.
@@ -155,7 +155,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -152,7 +152,7 @@ should result in deletion of the path). attached to a terminal, regardless of `--quiet`. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. `-c` is implied. See `--track` in linkgit:git-branch[1] for details.
@@ -180,6 +180,35 @@ int install_branch_config(int flag, const char *local, const char *origin, conststring_list_clear(&remotes,0);}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+constchar*bare_ref;+structbranch*branch;+inti;++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;+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),OPT_STRING('u',"set-upstream-to",&new_upstream,N_("upstream"),N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+test_configbranch.main.remoteorigin&&+test_configbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge+'+ test_done
@@ -107,4 +107,32 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with --track=inherit, we copy tracking info from foo+gitswitch--track=inherit-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# with autoSetupMerge=inherit, we do the same+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo3foo&&+test_cmp_configoriginbranch.foo3.remote&&+test_cmp_configrefs/heads/foobranch.foo3.merge&&+# with --track, we override autoSetupMerge+gitswitch--track-cfoo4foo&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/foobranch.foo4.merge&&+# and --track=direct does as well+gitswitch--track=direct-cfoo5foo&&+test_cmp_config.branch.foo5.remote&&+test_cmp_configrefs/heads/foobranch.foo5.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
@@ -1409,4 +1409,37 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with --track=inherit''+gitbranch--track=inheritfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge+'++test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo3my1&&+test_cmp_configlocalbranch.foo3.remote&&+test_cmp_configrefs/heads/mainbranch.foo3.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'++test_expect_success'--track overrides branch.autoSetupMerge''+test_configbranch.autoSetupMergeinherit&&+gitbranch--track=directfoo4my1&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/my1branch.foo4.merge&&+gitbranch--no-trackfoo5my1&&+test-z"$(gitconfigbranch.foo5.remote)"&&+test-z"$(gitconfigbranch.foo5.merge)"+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test-z"$(gitconfigbranch.foo-no-inherit.remote)"&&+test-z"$(gitconfigbranch.foo-no-inherit.merge)"&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test-z"$(gitconfigbranch.main2.remote)"&&+test-z"$(gitconfigbranch.main2.merge)"+'+ test_done
Add a new static variant of install_branch_config() that accepts
multiple remote branch names for tracking. This will be used in an
upcoming commit that enables inheriting the tracking configuration from
a parent branch.
Currently, all callers of install_branch_config() pass only a single
remote. Make install_branch_config() a small wrapper around
install_branch_config_multiple_remotes() so that existing callers do not
need to be changed.
Signed-off-by: Josh Steadmon <redacted>
---
branch.c | 120 ++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 87 insertions(+), 33 deletions(-)
@@ -55,19 +55,24 @@ N_("\n""the remote tracking information by invoking\n""\"git branch --set-upstream-to=%s%s%s\".");-intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,constchar*origin,+structstring_list*remotes){constchar*shortname=NULL;structstrbufkey=STRBUF_INIT;-intrebasing=should_setup_rebase(origin);--if(skip_prefix(remote,"refs/heads/",&shortname)-&&!strcmp(local,shortname)-&&!origin){-warning(_("Not setting branch %s as its own upstream."),-local);-return0;-}+inti,rebasing=should_setup_rebase(origin);++if(remotes->nr<1)+BUG("must provide at least one remote for branch config");
Since it's unsigned IMO this would be clearer: if (!remotes->nr)
+
+ if (!origin)
+ for (i = 0; i < remotes->nr; i++)
+ if (skip_prefix(remotes->items[i].string, "refs/heads/", &shortname)
For this and others, since you don't use the [i] for anything except
getting the current item using for_each_string_list_item() would be
better.
Partially a nit, partially that I've got another WIP
soon-to-be-submitted topic to fix overflow bugs in that API, and not
having "int i" etc. hardcoded in various places
helps. I.e. for_each_string_list_item() is future-proof.
+ && !strcmp(local, shortname)) {
+ warning(_("Not setting branch %s as its own upstream."),
Better to quote '%s', also s/Not/not/ (lower-case) for all error/warning/die etc.
@@ -75,8 +80,17 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local);- if (git_config_set_gently(key.buf, remote) < 0)+ /*+ * We want to overwrite any existing config with all the branches in+ * "remotes". Override any existing config with the first branch, but if+ * more than one is provided, use CONFIG_REGEX_NONE to preserve what+ * we've written so far.+ */+ if (git_config_set_gently(key.buf, remotes->items[0].string) < 0) goto out_err;+ for (i = 1; i < remotes->nr; i++)+ if (git_config_set_multivar_gently(key.buf, remotes->items[i].string, CONFIG_REGEX_NONE, 0) < 0)+ goto out_err; if (rebasing) { strbuf_reset(&key);
@@ -87,29 +101,62 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) {- if (shortname) {- if (origin)- printf_ln(rebasing ?- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :- _("Branch '%s' set up to track remote branch '%s' from '%s'."),- local, shortname, origin);- else- printf_ln(rebasing ?- _("Branch '%s' set up to track local branch '%s' by rebasing.") :- _("Branch '%s' set up to track local branch '%s'."),- local, shortname);+ int plural = remotes->nr > 1;
This....
+ int all_shortnames = 1;
+ const char *msg_fmt;
+ struct strbuf ref_string = STRBUF_INIT;
+
+ for (i = 0; i < remotes->nr; i++)
+ if (skip_prefix(remotes->items[i].string, "refs/heads/", &shortname)) {
+ strbuf_addf(&ref_string, "'%s', ", shortname);
+ } else {
+ all_shortnames = 0;
+ strbuf_addf(&ref_string, "'%s', ", remotes->items[i].string);
+ }
+ /* The last two characters are an extraneous ", ", so trim those. */
+ strbuf_setlen(&ref_string, ref_string.len - 2);
+
+ if (all_shortnames && origin) {
+ if (rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote branches %s from '%s' by rebasing.";
+ else if (rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote branch %s from '%s' by rebasing.";
+ else if (!rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote branches %s from '%s'.";
+ else if (!rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote branch %s from '%s'.";
...and this is hardcoding plural rules used in English that don't apply
in a lot of other languages...
+
+ printf_ln(_(msg_fmt), local, ref_string, origin);
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ if (all_shortnames && !origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local branches %s by rebasing.";
+ if (all_shortnames && !origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local branch %s by rebasing.";
+ if (all_shortnames && !origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local branches %s.";
+ if (all_shortnames && !origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local branch %s.";
+ if (!all_shortnames && origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote refs %s by rebasing.";
+ if (!all_shortnames && origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote ref %s by rebasing.";
+ if (!all_shortnames && origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote refs %s.";
+ if (!all_shortnames && origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote ref %s.";
+ if (!all_shortnames && !origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local refs %s by rebasing.";
+ if (!all_shortnames && !origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local ref %s by rebasing.";
+ if (!all_shortnames && !origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local refs %s.";
+ if (!all_shortnames && !origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local ref %s.";
...in English you've got one dog, then dogs, so == 1 and >1, but in
various other languages it's:
git grep Plural-Forms -- po
Anyway, this is easily solved, and even with less verbosity, see:
git grep -E -W '\bQ_\('
For examples of how to use the magic of libintl to do this for you.
+ string_list_append(&remotes, remote);
+ return install_branch_config_multiple_remotes(flag, local, origin, &remotes);
+ string_list_clear(&remotes, 0);
+}
+
/*
* 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
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),
But map --track, --track=direct --track=inherit to 3/5 of them. Will it
ever make sense to do the oher 2/5 (I really haven't checked)....
quoted hunk
OPT_STRING('u', "set-upstream-to", &new_upstream, N_("upstream"), N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
I wonder if this interface wouldn't be a lot simpler as:
--track
--track-explicit --track-direct --track-inherit
Both because it'll work better for auto-complete, and we can (and
presumably some will want) just make --track mean whatever configured
--track-THING you want.
in any case, isn't there a NONEG missing here, or is --no-track-direct
etc. handled by OPT_CALLBACK_F() (I forget...).
if (!strcmp(var, "branch.autosetupmerge")) {
- if (value && !strcasecmp(value, "always")) {
+ if (value && !strcmp(value, "always")) {
...This probably makes sense, but it seems like the behavior change of
"let's not take this case-insensitive" should be split up into its own
change...
+ test_must_fail git rev-parse --verify HEAD^ &&
+ git checkout main &&
+ git config branch.autosetupmerge inherit &&
+ git checkout --orphan eta &&
+ test -z "$(git config branch.eta.merge)" &&
+ test -z "$(git config branch.eta.remote)" &&
Better with the test_must_be_empty etc. helpers.
+ test refs/heads/eta = "$(git symbolic-ref HEAD)" &&
@@ -75,8 +80,17 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local);- if (git_config_set_gently(key.buf, remote) < 0)+ /*+ * We want to overwrite any existing config with all the branches in+ * "remotes". Override any existing config with the first branch, but if+ * more than one is provided, use CONFIG_REGEX_NONE to preserve what+ * we've written so far.+ */+ if (git_config_set_gently(key.buf, remotes->items[0].string) < 0) goto out_err;+ for (i = 1; i < remotes->nr; i++)+ if (git_config_set_multivar_gently(key.buf, remotes->items[i].string, CONFIG_REGEX_NONE, 0) < 0)+ goto out_err;
I think that instead of overriding all config with the first value and
then appending every value after that, it'll be more obvious to readers
if we first unset all of the config, then write every value (then the
comment wouldn't have to justify why we make two calls and iteration
starts at 1).
I believe that unsetting all values for a key is supported by
git_config_set_multivar_gently() with value == NULL, i.e.
/*
* unset with value = NULL, not sure how this interacts with
* CONFIG_REGEX_NONE
*/
if (git_config_set_multivar_gently(key.buf, NULL,
CONFIG_REGEX_NONE, 0))
goto out_err;
for_each_string_list_item(item, remotes) {
git_config_set_multivar_gently(key.buf, item, CONFIG_REGEX_NONE, 0);
}
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
@@ -75,8 +80,17 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local);- if (git_config_set_gently(key.buf, remote) < 0)+ /*+ * We want to overwrite any existing config with all the branches in+ * "remotes". Override any existing config with the first branch, but if+ * more than one is provided, use CONFIG_REGEX_NONE to preserve what+ * we've written so far.+ */+ if (git_config_set_gently(key.buf, remotes->items[0].string) < 0) goto out_err;+ for (i = 1; i < remotes->nr; i++)+ if (git_config_set_multivar_gently(key.buf, remotes->items[i].string, CONFIG_REGEX_NONE, 0) < 0)+ goto out_err;
I think that instead of overriding all config with the first value and
then appending every value after that, it'll be more obvious to readers
if we first unset all of the config, then write every value (then the
comment wouldn't have to justify why we make two calls and iteration
starts at 1).
I believe that unsetting all values for a key is supported by
git_config_set_multivar_gently() with value == NULL, i.e.
/*
* unset with value = NULL, not sure how this interacts with
* CONFIG_REGEX_NONE
*/
if (git_config_set_multivar_gently(key.buf, NULL,
CONFIG_REGEX_NONE, 0))
goto out_err;
for_each_string_list_item(item, remotes) {
git_config_set_multivar_gently(key.buf, item, CONFIG_REGEX_NONE, 0);
}
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),
But map --track, --track=direct --track=inherit to 3/5 of them. Will it
ever make sense to do the oher 2/5 (I really haven't checked)....
Reasonable question, but I believe the answer is no, it doesn't make
sense to map all the values:
* BRANCH_TRACK_REMOTE is just a default value as far as I can tell (I
don't think this does anything?)
* BRANCH_TRACK_ALWAYS behaves like BRANCH_TRACK_EXPLICIT but it's only
meant to be set from config files, see 9ed36cfa35 (branch: optionally
setup branch.*.merge from upstream local branches, 2008-02-19). We're
more lenient with _ALWAYS than with _EXPLICIT; e.g. we don't die()
when the upstream doesn't exist.
Even one of the other options doesn't really make that much sense...
* BRANCH_TRACK_OVERRIDE used to be used to implement --set-upstream, but
that's not necessary any more. Now it's used to make create_branch()
*not* create a branch sometimes, but that's going away if I get my
refactor of create_branch()
(https://lore.kernel.org/git/xmqq1r2pcnyw.fsf@gitster.g/T/#u) :)
+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()...
quoted hunk
+
/*
* 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.
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.
I noticed some test failures due to the printf_ln(msg_fmt) region. Since
you are reworking this, the problem might just go away, but I thought I
should mention it just in case.
+ if (all_shortnames && origin) {
+ if (rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote branches %s from '%s' by rebasing.";
+ else if (rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote branch %s from '%s' by rebasing.";
+ else if (!rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote branches %s from '%s'.";
+ else if (!rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote branch %s from '%s'.";
+
+ printf_ln(_(msg_fmt), local, ref_string, origin);
Here
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ if (all_shortnames && !origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local branches %s by rebasing.";
+ if (all_shortnames && !origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local branch %s by rebasing.";
+ if (all_shortnames && !origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local branches %s.";
+ if (all_shortnames && !origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local branch %s.";
+ if (!all_shortnames && origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote refs %s by rebasing.";
+ if (!all_shortnames && origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote ref %s by rebasing.";
+ if (!all_shortnames && origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote refs %s.";
+ if (!all_shortnames && origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote ref %s.";
+ if (!all_shortnames && !origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local refs %s by rebasing.";
+ if (!all_shortnames && !origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local ref %s by rebasing.";
+ if (!all_shortnames && !origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local refs %s.";
+ if (!all_shortnames && !origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local ref %s.";
+
+ printf_ln(_(msg_fmt), local, ref_string);
and here
}
+
+ strbuf_release(&ref_string);
}
We print ref_string, which is a strbuf. This causes t/t3200-branch.sh to
segfault on my mac + clang, but inconsistently! With -O2, it doesn't
always segfault, but the wrong memory is read:
Branch 'my3' set up to track remote branch local from 'Branch '%s' set up to track remote branch %s from '%s'.'.
With -O0, it always segfaults.
You can see this in the osx-clang run in [1], but it looks like the gcc
ones refuse to build.
s/ref_string/ref_string.buf should fix the problem.
[1] https://github.com/chooglen/git/runs/4464134763?check_suite_focus=true
We print ref_string, which is a strbuf. This causes t/t3200-branch.sh to
segfault on my mac + clang, but inconsistently! With -O2, it doesn't
always segfault, but the wrong memory is read:
Branch 'my3' set up to track remote branch local from 'Branch '%s' set up to track remote branch %s from '%s'.'.
I forgot to mention this earlier but in this example, the test *passes*
even though the stderr message is obviously wrong. I don't see any
coverage of the help message in t3200, which is a bit worrying to me.
After this series is done, is it worth adding test coverage of the help
message?
@@ -75,8 +80,17 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local);- if (git_config_set_gently(key.buf, remote) < 0)+ /*+ * We want to overwrite any existing config with all the branches in+ * "remotes". Override any existing config with the first branch, but if+ * more than one is provided, use CONFIG_REGEX_NONE to preserve what+ * we've written so far.+ */+ if (git_config_set_gently(key.buf, remotes->items[0].string) < 0) goto out_err;+ for (i = 1; i < remotes->nr; i++)+ if (git_config_set_multivar_gently(key.buf, remotes->items[i].string, CONFIG_REGEX_NONE, 0) < 0)+ goto out_err;
I think that instead of overriding all config with the first value and
then appending every value after that, it'll be more obvious to readers
if we first unset all of the config, then write every value (then the
comment wouldn't have to justify why we make two calls and iteration
starts at 1).
I believe that unsetting all values for a key is supported by
git_config_set_multivar_gently() with value == NULL, i.e.
/*
* unset with value = NULL, not sure how this interacts with
* CONFIG_REGEX_NONE
*/
if (git_config_set_multivar_gently(key.buf, NULL,
CONFIG_REGEX_NONE, 0))
goto out_err;
for_each_string_list_item(item, remotes) {
git_config_set_multivar_gently(key.buf, item, CONFIG_REGEX_NONE, 0);
}
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
Sounds like we should just have separate advice strings for single vs.
multiple merge configs?
We print ref_string, which is a strbuf. This causes t/t3200-branch.sh to
segfault on my mac + clang, but inconsistently! With -O2, it doesn't
always segfault, but the wrong memory is read:
Branch 'my3' set up to track remote branch local from 'Branch '%s' set up to track remote branch %s from '%s'.'.
I forgot to mention this earlier but in this example, the test *passes*
even though the stderr message is obviously wrong. I don't see any
coverage of the help message in t3200, which is a bit worrying to me.
After this series is done, is it worth adding test coverage of the help
message?
Yeah, I caught this earlier while reworking this section based on Ævar's
review, but thank you for pointing it out. I'm unsure about checking
formatting of message strings in tests; it would certainly have caught
this bug but it seems that more often they're just "change detectors"
rather than good tests. But I could be swayed if you or others feel it's
important.
On 2021.12.07 09:57, Ævar Arnfjörð Bjarmason wrote:
On Mon, Dec 06 2021, Josh Steadmon wrote:
quoted
Add a new static variant of install_branch_config() that accepts
multiple remote branch names for tracking. This will be used in an
upcoming commit that enables inheriting the tracking configuration from
a parent branch.
Currently, all callers of install_branch_config() pass only a single
remote. Make install_branch_config() a small wrapper around
install_branch_config_multiple_remotes() so that existing callers do not
need to be changed.
Signed-off-by: Josh Steadmon <redacted>
---
branch.c | 120 ++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 87 insertions(+), 33 deletions(-)
@@ -55,19 +55,24 @@ N_("\n""the remote tracking information by invoking\n""\"git branch --set-upstream-to=%s%s%s\".");-intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,constchar*origin,+structstring_list*remotes){constchar*shortname=NULL;structstrbufkey=STRBUF_INIT;-intrebasing=should_setup_rebase(origin);--if(skip_prefix(remote,"refs/heads/",&shortname)-&&!strcmp(local,shortname)-&&!origin){-warning(_("Not setting branch %s as its own upstream."),-local);-return0;-}+inti,rebasing=should_setup_rebase(origin);++if(remotes->nr<1)+BUG("must provide at least one remote for branch config");
Since it's unsigned IMO this would be clearer: if (!remotes->nr)
Fixed in v6.
quoted
+
+ if (!origin)
+ for (i = 0; i < remotes->nr; i++)
+ if (skip_prefix(remotes->items[i].string, "refs/heads/", &shortname)
For this and others, since you don't use the [i] for anything except
getting the current item using for_each_string_list_item() would be
better.
Partially a nit, partially that I've got another WIP
soon-to-be-submitted topic to fix overflow bugs in that API, and not
having "int i" etc. hardcoded in various places
helps. I.e. for_each_string_list_item() is future-proof.
Thanks. I somehow missed for_each_string_list_item() when I checked the
header file. Fixed in v6.
quoted
+ && !strcmp(local, shortname)) {
+ warning(_("Not setting branch %s as its own upstream."),
Better to quote '%s', also s/Not/not/ (lower-case) for all error/warning/die etc.
Done (for now) in v6, but this might become moot as I address Junio's
review comments.
@@ -75,8 +80,17 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local);- if (git_config_set_gently(key.buf, remote) < 0)+ /*+ * We want to overwrite any existing config with all the branches in+ * "remotes". Override any existing config with the first branch, but if+ * more than one is provided, use CONFIG_REGEX_NONE to preserve what+ * we've written so far.+ */+ if (git_config_set_gently(key.buf, remotes->items[0].string) < 0) goto out_err;+ for (i = 1; i < remotes->nr; i++)+ if (git_config_set_multivar_gently(key.buf, remotes->items[i].string, CONFIG_REGEX_NONE, 0) < 0)+ goto out_err; if (rebasing) { strbuf_reset(&key);
@@ -87,29 +101,62 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) {- if (shortname) {- if (origin)- printf_ln(rebasing ?- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :- _("Branch '%s' set up to track remote branch '%s' from '%s'."),- local, shortname, origin);- else- printf_ln(rebasing ?- _("Branch '%s' set up to track local branch '%s' by rebasing.") :- _("Branch '%s' set up to track local branch '%s'."),- local, shortname);+ int plural = remotes->nr > 1;
This....
quoted
+ int all_shortnames = 1;
+ const char *msg_fmt;
+ struct strbuf ref_string = STRBUF_INIT;
+
+ for (i = 0; i < remotes->nr; i++)
+ if (skip_prefix(remotes->items[i].string, "refs/heads/", &shortname)) {
+ strbuf_addf(&ref_string, "'%s', ", shortname);
+ } else {
+ all_shortnames = 0;
+ strbuf_addf(&ref_string, "'%s', ", remotes->items[i].string);
+ }
+ /* The last two characters are an extraneous ", ", so trim those. */
+ strbuf_setlen(&ref_string, ref_string.len - 2);
+
+ if (all_shortnames && origin) {
+ if (rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote branches %s from '%s' by rebasing.";
+ else if (rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote branch %s from '%s' by rebasing.";
+ else if (!rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote branches %s from '%s'.";
+ else if (!rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote branch %s from '%s'.";
...and this is hardcoding plural rules used in English that don't apply
in a lot of other languages...
quoted
+
+ printf_ln(_(msg_fmt), local, ref_string, origin);
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ if (all_shortnames && !origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local branches %s by rebasing.";
+ if (all_shortnames && !origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local branch %s by rebasing.";
+ if (all_shortnames && !origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local branches %s.";
+ if (all_shortnames && !origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local branch %s.";
+ if (!all_shortnames && origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote refs %s by rebasing.";
+ if (!all_shortnames && origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote ref %s by rebasing.";
+ if (!all_shortnames && origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track remote refs %s.";
+ if (!all_shortnames && origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track remote ref %s.";
+ if (!all_shortnames && !origin && rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local refs %s by rebasing.";
+ if (!all_shortnames && !origin && rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local ref %s by rebasing.";
+ if (!all_shortnames && !origin && !rebasing && plural)
+ msg_fmt = "Branch '%s' set up to track local refs %s.";
+ if (!all_shortnames && !origin && !rebasing && !plural)
+ msg_fmt = "Branch '%s' set up to track local ref %s.";
...in English you've got one dog, then dogs, so == 1 and >1, but in
various other languages it's:
git grep Plural-Forms -- po
Anyway, this is easily solved, and even with less verbosity, see:
git grep -E -W '\bQ_\('
For examples of how to use the magic of libintl to do this for you.
Thank you for the pointer. I looked specifically for dealing with plural
forms in our docs, but the referenced "Preparing Strings" gettext docs
were not helpful for this. (Although I see now I should have read
further in po/README.md to find the relevant advice). I may send a
separate change to po/README.md to make it easier to find in the future.
+ string_list_append(&remotes, remote);
+ return install_branch_config_multiple_remotes(flag, local, origin, &remotes);
+ string_list_clear(&remotes, 0);
+}
+
/*
* 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
We print ref_string, which is a strbuf. This causes t/t3200-branch.sh to
segfault on my mac + clang, but inconsistently! With -O2, it doesn't
always segfault, but the wrong memory is read:
Branch 'my3' set up to track remote branch local from 'Branch '%s' set up to track remote branch %s from '%s'.'.
I forgot to mention this earlier but in this example, the test *passes*
even though the stderr message is obviously wrong. I don't see any
coverage of the help message in t3200, which is a bit worrying to me.
After this series is done, is it worth adding test coverage of the help
message?
Yeah, I caught this earlier while reworking this section based on Ævar's
review, but thank you for pointing it out. I'm unsure about checking
formatting of message strings in tests; it would certainly have caught
this bug but it seems that more often they're just "change detectors"
rather than good tests.
I agree, although such a test might still be beneficial on the whole if
the change detector is easy to update.
But I could be swayed if you or others feel it's important.
Because this is rather "change detector"y, I don't think it's important
either, but I'm also open to being convinced.
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
Sounds like we should just have separate advice strings for single vs.
multiple merge configs?
That sounds like a good idea if it's not too much work. Otherwise, a
NEEDSWORK is still acceptable to me (but that said, I'm not an authority
on this matter).
On 2021.12.07 09:57, Ævar Arnfjörð Bjarmason wrote:
quoted
On Mon, Dec 06 2021, Josh Steadmon wrote:
...in English you've got one dog, then dogs, so == 1 and >1, but in
various other languages it's:
git grep Plural-Forms -- po
Anyway, this is easily solved, and even with less verbosity, see:
git grep -E -W '\bQ_\('
For examples of how to use the magic of libintl to do this for you.
Thank you for the pointer. I looked specifically for dealing with plural
forms in our docs, but the referenced "Preparing Strings" gettext docs
were not helpful for this. (Although I see now I should have read
further in po/README.md to find the relevant advice). I may send a
separate change to po/README.md to make it easier to find in the future.
Thanks, that would be really helpful.
quoted
quoted
+ string_list_append(&remotes, remote);
+ return install_branch_config_multiple_remotes(flag, local, origin, &remotes);
+ string_list_clear(&remotes, 0);
+}
+
/*
* 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
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
Sounds like we should just have separate advice strings for single vs.
multiple merge configs?
That sounds like a good idea if it's not too much work. Otherwise, a
NEEDSWORK is still acceptable to me (but that said, I'm not an authority
on this matter).
We haven't used Q_() with advise() yet, but there's no reason not to:
advise(Q_("fix your branch by doing xyz",
"fix your branches by doing xyz",
branches_nr));
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
Sounds like we should just have separate advice strings for single vs.
multiple merge configs?
That sounds like a good idea if it's not too much work. Otherwise, a
NEEDSWORK is still acceptable to me (but that said, I'm not an authority
on this matter).
We haven't used Q_() with advise() yet, but there's no reason not to:
advise(Q_("fix your branch by doing xyz",
"fix your branches by doing xyz",
branches_nr));
Neat, that should do it in most cases. I think this one is a little
trickier because the plural advice messages requires constructing a
list, e.g.
Singular:
"\n"
"After fixing the error cause you may try to fix up\n"
"the remote tracking information by invoking\n"
"\"git branch --set-upstream-to=%s%s%s\"."
Plural:
"\n"
"After fixing the error cause you may try to fix up\n"
"the remote tracking information by invoking\n"
"\"git config --add my_new_remote remote_name\"
"\"git config --add my_new_upstream1 upstream_name1\"
"\"git config --add my_new_upstream2 upstream_name2\"
But perhaps this is not too hard since you've already included examples
of how to format lists of strings [1]
[1] https://lore.kernel.org/git/211207.86mtlcpyu4.gmgdl@evledraar.gmail.com
When there is more than one item in remotes->items, this advice is
_technically_ incorrect because --set-upstream-to only takes a single
upstream branch. I think that supporting multiple upstreams in
--set-upstream-to is a fairly niche use case and is out of scope of this
series, so let's not pursue that option.
Another option would be to replace the mention of --set-upstream-to with
"git config add", but that's unfriendly to the >90% of the user
population that doesn't want multiple merge entries.
If we leave the advice as-is, even though it is misleading, a user who
is sophisticated enough to set up multiple merge entries should also
know that --set-upstream-to won't solve their problems, and would
probably be able to fix their problems by mucking around with
.git/config or git config.
So I think it is ok to not change the advice and to only mention the
first merge item. However, it might be worth marking this as NEEDSWORK
so that subsequent readers of this file understand that this advice is
overly-simplistic and might be worth fixing.
Sounds like we should just have separate advice strings for single vs.
multiple merge configs?
That sounds like a good idea if it's not too much work. Otherwise, a
NEEDSWORK is still acceptable to me (but that said, I'm not an authority
on this matter).
We haven't used Q_() with advise() yet, but there's no reason not to:
advise(Q_("fix your branch by doing xyz",
"fix your branches by doing xyz",
branches_nr));
Neat, that should do it in most cases. I think this one is a little
trickier because the plural advice messages requires constructing a
list, e.g.
Singular:
"\n"
"After fixing the error cause you may try to fix up\n"
"the remote tracking information by invoking\n"
"\"git branch --set-upstream-to=%s%s%s\"."
Plural:
"\n"
"After fixing the error cause you may try to fix up\n"
"the remote tracking information by invoking\n"
"\"git config --add my_new_remote remote_name\"
"\"git config --add my_new_upstream1 upstream_name1\"
"\"git config --add my_new_upstream2 upstream_name2\"
But perhaps this is not too hard since you've already included examples
of how to format lists of strings [1]
[1] https://lore.kernel.org/git/211207.86mtlcpyu4.gmgdl@evledraar.gmail.com
You don't need to use the plural facility for that sort of
message.
Plurals in translated messages are specifically for the case where you
need to compose a sentence like:
I had %d glasses of water with breakfast
But it's not needed for a message that can be rehrased as a heading
followed by a list of 1 or more items, e.g.:
Things I've consumed in liquid form during breakfast this week:
- Water
- Tea
- Coffe
If that list stopped at "Water" it would be OK. Every language has some
way of referring to items on a list in general terms, without knowing if
that list is composed of only one item, two etc.
+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()...
quoted
+
/*
* 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.
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.
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),
But map --track, --track=direct --track=inherit to 3/5 of them. Will it
ever make sense to do the oher 2/5 (I really haven't checked)....
Reasonable question, but I believe the answer is no, it doesn't make
sense to map all the values:
* BRANCH_TRACK_REMOTE is just a default value as far as I can tell (I
don't think this does anything?)
* BRANCH_TRACK_ALWAYS behaves like BRANCH_TRACK_EXPLICIT but it's only
meant to be set from config files, see 9ed36cfa35 (branch: optionally
setup branch.*.merge from upstream local branches, 2008-02-19). We're
more lenient with _ALWAYS than with _EXPLICIT; e.g. we don't die()
when the upstream doesn't exist.
Even one of the other options doesn't really make that much sense...
* BRANCH_TRACK_OVERRIDE used to be used to implement --set-upstream, but
that's not necessary any more. Now it's used to make create_branch()
*not* create a branch sometimes, but that's going away if I get my
refactor of create_branch()
(https://lore.kernel.org/git/xmqq1r2pcnyw.fsf@gitster.g/T/#u) :)
Agreed, thank you for stating things better than I could have :)
On 2021.12.07 10:08, Ævar Arnfjörð Bjarmason wrote:
On Mon, Dec 06 2021, Josh Steadmon wrote:
quoted
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),
But map --track, --track=direct --track=inherit to 3/5 of them. Will it
ever make sense to do the oher 2/5 (I really haven't checked)....
quoted
OPT_STRING('u', "set-upstream-to", &new_upstream, N_("upstream"), N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
I wonder if this interface wouldn't be a lot simpler as:
--track
--track-explicit --track-direct --track-inherit
Both because it'll work better for auto-complete, and we can (and
presumably some will want) just make --track mean whatever configured
--track-THING you want.
If I understand you correctly, I disagree here. I think you're saying
that if you always (or usually) want a specific tracking mode, you have
to both set the config appropriately and remember to pass `--track` on
the command line? Seems simpler to just let the config take precedence
in the absence of a flag. But I think I may have misunderstood you.
in any case, isn't there a NONEG missing here, or is --no-track-direct
etc. handled by OPT_CALLBACK_F() (I forget...).
Yeah, --no-track is correctly handled. It sets BRANCH_TRACK_NEVER, so
that you can override a branch.autosetupmerge=always config if
necessary.
quoted
if (!strcmp(var, "branch.autosetupmerge")) {
- if (value && !strcasecmp(value, "always")) {
+ if (value && !strcmp(value, "always")) {
...This probably makes sense, but it seems like the behavior change of
"let's not take this case-insensitive" should be split up into its own
change...
Done in V6.
quoted
+ test_must_fail git rev-parse --verify HEAD^ &&
+ git checkout main &&
+ git config branch.autosetupmerge inherit &&
+ git checkout --orphan eta &&
+ test -z "$(git config branch.eta.merge)" &&
+ test -z "$(git config branch.eta.remote)" &&
Better with the test_must_be_empty etc. helpers.
quoted
+ test refs/heads/eta = "$(git symbolic-ref HEAD)" &&
Add a new static variant of install_branch_config() that accepts
multiple remote branch names for tracking. This will be used in an
upcoming commit that enables inheriting the tracking configuration from
a parent branch.
Currently, all callers of install_branch_config() pass only a single
remote. Make install_branch_config() a small wrapper around
install_branch_config_multiple_remotes() so that existing callers do not
need to be changed.
Signed-off-by: Josh Steadmon <redacted>
---
branch.c | 135 ++++++++++++++++++++++++++++++++--------------
t/t3200-branch.sh | 6 +--
2 files changed, 99 insertions(+), 42 deletions(-)
@@ -49,25 +49,41 @@ static int should_setup_rebase(const char *origin)return0;}-staticconstchartracking_advice[]=-N_("\n"-"After fixing the error cause you may try to fix up\n"-"the remote tracking information by invoking\n"-"\"git branch --set-upstream-to=%s%s%s\".");--intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+/**+*Installupstreamtrackingconfigurationforabranch;specifically,add+*`branch.<name>.remote`and`branch.<name>.merge`entries.+*+*`flag`containsintegerflagsforoptions;currentlyonly+*BRANCH_CONFIG_VERBOSEischecked.+*+*`local`isthenameofthebranchwhoseconfigurationwe'reinstalling.+*+*`origin`isthenameoftheremoteowningtheupstreambranches.NULLmeans+*theupstreambranchesarelocaltothisrepo.+*+*`remotes`isalistofrefsthatareupstreamoflocal+*/+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,+constchar*origin,structstring_list*remotes){constchar*shortname=NULL;structstrbufkey=STRBUF_INIT;+structstring_list_item*item;intrebasing=should_setup_rebase(origin);-if(skip_prefix(remote,"refs/heads/",&shortname)-&&!strcmp(local,shortname)-&&!origin){-warning(_("Not setting branch %s as its own upstream."),-local);-return0;-}+if(!remotes->nr)+BUG("must provide at least one remote for branch config");+if(rebasing&&remotes->nr>1)+die(_("cannot inherit upstream tracking configuration when rebasing is requested"));++if(!origin)+for_each_string_list_item(item,remotes)+if(skip_prefix(item->string,"refs/heads/",&shortname)+&&!strcmp(local,shortname)){+warning(_("not setting branch '%s' as its own upstream."),+local);+return0;+}strbuf_addf(&key,"branch.%s.remote",local);if(git_config_set_gently(key.buf,origin?origin:".")<0)
@@ -87,29 +112,42 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);if(flag&BRANCH_CONFIG_VERBOSE){-if(shortname){+constchar*name;+structstrbufref_string=STRBUF_INIT;++for_each_string_list_item(item,remotes){+name=item->string;+skip_prefix(name,"refs/heads/",&name);+strbuf_addf(&ref_string," %s\n",name);+}++if(remotes->nr==1){+structstrbufrefname=STRBUF_INIT;+if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing."):-_("Branch '%s' set up to track remote branch '%s' from '%s'."),-local,shortname,origin);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local branch '%s' by rebasing."):-_("Branch '%s' set up to track local branch '%s'."),-local,shortname);+strbuf_addf(&refname,"%s/",origin);+strbuf_addstr(&refname,remotes->items[0].string);++/*+*Rebasingisonlyallowedinthecaseofasingle+*upstreambranch.+*/+printf_ln(rebasing?+_("branch '%s' set up to track '%s' by rebasing."):+_("branch '%s' set up to track '%s'."),+local,refname.buf);++strbuf_release(&refname);+}elseif(origin){+printf_ln(_("branch '%s' set up to track from '%s':"),+local,origin);+printf("%s",ref_string.buf);}else{-if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote ref '%s' by rebasing."):-_("Branch '%s' set up to track remote ref '%s'."),-local,remote);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local ref '%s' by rebasing."):-_("Branch '%s' set up to track local ref '%s'."),-local,remote);+printf_ln(_("branch '%s' set up to track:"),local);+printf("%s",ref_string.buf);}++strbuf_release(&ref_string);}return0;
@@ -118,14 +156,33 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);error(_("Unable to write upstream branch configuration"));-advise(_(tracking_advice),-origin?origin:"",-origin?"/":"",-shortname?shortname:remote);+advise(_("\nAfter fixing the error cause you may try to fix up\n"+"the remote tracking information by invoking:"));+if(remotes->nr==1)+advise(" git branch --set-upstream-to=%s%s%s",+origin?origin:"",+origin?"/":"",+remotes->items[0].string);+else+for_each_string_list_item(item,remotes)+advise(" git config --add branch.\"%s\".merge %s",+local,item->string);return-1;}+intinstall_branch_config(intflag,constchar*local,constchar*origin,+constchar*remote)+{+intret;+structstring_listremotes=STRING_LIST_INIT_DUP;++string_list_append(&remotes,remote);+ret=install_branch_config_multiple_remotes(flag,local,origin,&remotes);+string_list_clear(&remotes,0);+returnret;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -950,15 +950,15 @@ test_expect_success 'disabled option --set-upstream fails' 'test_must_failgitbranch--set-upstreamorigin/main'-test_expect_success'--set-upstream-to notices an error to set branch as own upstream''+test_expect_success'--set-upstream-to notices an error to set branch as own upstream'"gitbranch--set-upstream-torefs/heads/my13my132>actual&&cat>expect<<-\EOF&&-warning:Notsettingbranchmy13asitsownupstream.+warning:notsettingbranch'my13'asitsownupstream.EOFtest_expect_code1gitconfigbranch.my13.remote&&test_expect_code1gitconfigbranch.my13.merge&&test_cmpexpectactual-'+"# Keep this test last, as it changes the current branch cat>expect<<EOF
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
Finally, teach branch.autoSetupMerge a new "inherit" option. When this
is set, "--track=inherit" becomes the default behavior.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 +++++++++++-----
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 49 ++++++++++++++++++++++++++++-----
branch.h | 3 +-
builtin/branch.c | 6 ++--
builtin/checkout.c | 6 ++--
config.c | 3 ++
parse-options-cb.c | 16 +++++++++++
parse-options.h | 2 ++
t/t2017-checkout-orphan.sh | 11 +++++++-
t/t2027-checkout-track.sh | 23 ++++++++++++++++
t/t2060-switch.sh | 28 +++++++++++++++++++
t/t3200-branch.sh | 33 ++++++++++++++++++++++
t/t7201-co.sh | 17 ++++++++++++
16 files changed, 205 insertions(+), 23 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -205,24 +205,34 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track::+--track [inherit|direct]:: When creating a new branch, set up `branch.<name>.remote` and- `branch.<name>.merge` configuration entries to mark the- start-point branch as "upstream" from the new branch. This+ `branch.<name>.merge` configuration entries to set "upstream" tracking+ configuration for the new branch. This configuration will tell git to show the relationship between the two branches in `git status` and `git branch -v`. Furthermore, it directs `git pull` without arguments to pull from the upstream when the new branch is checked out. +-This behavior is the default when the start point is a remote-tracking branch.+The exact upstream branch is chosen depending on the optional argument:+`--track` or `--track direct` means to use the start-point branch itself as the+upstream; `--track inherit` means to copy the upstream configuration of the+start-point branch.+++`--track direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+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- branch.autoSetupMerge configuration variable is true.+ branch.autoSetupMerge configuration variable is set. --set-upstream:: As this option had confusing syntax, it is no longer supported.
@@ -155,7 +155,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -152,7 +152,7 @@ should result in deletion of the path). attached to a terminal, regardless of `--quiet`. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. `-c` is implied. See `--track` in linkgit:git-branch[1] for details.
@@ -183,6 +183,34 @@ int install_branch_config(int flag, const char *local, const char *origin,returnret;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+constchar*bare_ref;+structbranch*branch;+inti;++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]);+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),OPT_STRING('u',"set-upstream-to",&new_upstream,N_("upstream"),N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+test_configbranch.main.remoteorigin&&+test_configbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge+'+ test_done
@@ -107,4 +107,32 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with --track=inherit, we copy tracking info from foo+gitswitch--track=inherit-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# with autoSetupMerge=inherit, we do the same+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo3foo&&+test_cmp_configoriginbranch.foo3.remote&&+test_cmp_configrefs/heads/foobranch.foo3.merge&&+# with --track, we override autoSetupMerge+gitswitch--track-cfoo4foo&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/foobranch.foo4.merge&&+# and --track=direct does as well+gitswitch--track=direct-cfoo5foo&&+test_cmp_config.branch.foo5.remote&&+test_cmp_configrefs/heads/foobranch.foo5.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
@@ -1409,4 +1409,37 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with --track=inherit''+gitbranch--track=inheritfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge+'++test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo3my1&&+test_cmp_configlocalbranch.foo3.remote&&+test_cmp_configrefs/heads/mainbranch.foo3.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'++test_expect_success'--track overrides branch.autoSetupMerge''+test_configbranch.autoSetupMergeinherit&&+gitbranch--track=directfoo4my1&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/my1branch.foo4.merge&&+gitbranch--no-trackfoo5my1&&+test_cmp_config""--default""branch.foo5.remote&&+test_cmp_config""--default""branch.foo5.merge+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
Although we only documented that branch.autosetupmerge would accept
"always" as a value, the actual implementation would accept any
combination of upper- or lower-case. Fix this to be consistent with
documentation and with other values of this config variable.
Signed-off-by: Josh Steadmon <redacted>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
I meant to expand on this but forgot before sending the series. I
removed as many distinctions as possible, as most can still be inferred
from context. For example, previously the output specifically called out
branches vs. tags, but this is obvious in the name itself: "some-branch"
vs. "refs/tags/some-tag" for example. Likewise, we don't need to specify
whether refs are remote or local: "some-remote/some-branch" vs.
"a-local-branch" should be understandable without us spelling it out.
Of course, if people feel like I've over-simplified here, I'm happy to
revert this change.
From: Junio C Hamano <hidden> Date: 2021-12-16 00:02:44
Josh Steadmon [off-list ref] writes:
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
* Note that rebasing is not compatible with inheriting multiple upstream
branches.
* Moved the change to case-sensitivity for branch.autosetupmerge to its
own commit.
* Improve advice on failed tracking setup when multiple branches are
involved.
* Make better use of string_list API.
* Make better use of config API.
* More straight-forward use of the `struct tracking` API.
* Numerous style fixes.
I've queued this, and rebased Glen's "branch --recurse-submodules"
on top, and parked both of them near the tip of 'seen'. I do not
have much confidence in the conflict resolution needed during the
rebasing or the other branch or merges into 'seen', and I would
appreciate it if you two can take a look to sanity check the result.
Thanks.
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
* Note that rebasing is not compatible with inheriting multiple upstream
branches.
* Moved the change to case-sensitivity for branch.autosetupmerge to its
own commit.
* Improve advice on failed tracking setup when multiple branches are
involved.
* Make better use of string_list API.
* Make better use of config API.
* More straight-forward use of the `struct tracking` API.
* Numerous style fixes.
I've queued this, and rebased Glen's "branch --recurse-submodules"
on top, and parked both of them near the tip of 'seen'. I do not
have much confidence in the conflict resolution needed during the
rebasing or the other branch or merges into 'seen', and I would
appreciate it if you two can take a look to sanity check the result.
Thanks.
I've just sent out a new version [1] which is rebased on top of Josh's
v6. Please use that version instead :)
I did not rebase this on top of 'seen' though; I'll take a look and see
if there's anything of concern.
[1] https://lore.kernel.org/git/20211216003213.99135-1-chooglen@google.com/,
@@ -49,25 +49,41 @@ static int should_setup_rebase(const char *origin)return0;}-staticconstchartracking_advice[]=-N_("\n"-"After fixing the error cause you may try to fix up\n"-"the remote tracking information by invoking\n"-"\"git branch --set-upstream-to=%s%s%s\".");--intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+/**+*Installupstreamtrackingconfigurationforabranch;specifically,add+*`branch.<name>.remote`and`branch.<name>.merge`entries.+*+*`flag`containsintegerflagsforoptions;currentlyonly+*BRANCH_CONFIG_VERBOSEischecked.+*+*`local`isthenameofthebranchwhoseconfigurationwe'reinstalling.+*+*`origin`isthenameoftheremoteowningtheupstreambranches.NULLmeans+*theupstreambranchesarelocaltothisrepo.+*+*`remotes`isalistofrefsthatareupstreamoflocal+*/+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,+constchar*origin,structstring_list*remotes)
Very helpful description. I got slightly confused when I first reviewed
this, so having the comments will help future readers a lot.
{
const char *shortname = NULL;
struct strbuf key = STRBUF_INIT;
+ struct string_list_item *item;
int rebasing = should_setup_rebase(origin);
- if (skip_prefix(remote, "refs/heads/", &shortname)
- && !strcmp(local, shortname)
- && !origin) {
- warning(_("Not setting branch %s as its own upstream."),
- local);
- return 0;
- }
+ if (!remotes->nr)
+ BUG("must provide at least one remote for branch config");
+ if (rebasing && remotes->nr > 1)
+ die(_("cannot inherit upstream tracking configuration when rebasing is requested"));
Nit: if we're being pedantic, we cannot inherit upstream tracking
configuration when rebasing is requested with multiple upstream
branches.
But this message is already very niche and loaded with specifics, so
adding "...with multiple upstream branches" might just be more
confusing, so this is a nit.
quoted hunk
@@ -75,8 +91,17 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_reset(&key); strbuf_addf(&key, "branch.%s.merge", local);- if (git_config_set_gently(key.buf, remote) < 0)+ /*+ * We want to overwrite any existing config with all the branches in+ * "remotes". Override any existing config, then write our branches. If+ * more than one is provided, use CONFIG_REGEX_NONE to preserve what+ * we've written so far.+ */+ if (git_config_set_gently(key.buf, NULL) < 0) goto out_err;+ for_each_string_list_item(item, remotes)+ if (git_config_set_multivar_gently(key.buf, item->string, CONFIG_REGEX_NONE, 0) < 0)+ goto out_err;
We get to use for_each_string_item() now, nice.
quoted hunk
@@ -87,29 +112,42 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) {- if (shortname) {+ const char *name;+ struct strbuf ref_string = STRBUF_INIT;++ for_each_string_list_item(item, remotes) {+ name = item->string;+ skip_prefix(name, "refs/heads/", &name);+ strbuf_addf(&ref_string, " %s\n", name);+ }++ if (remotes->nr == 1) {+ struct strbuf refname = STRBUF_INIT;+ if (origin)- printf_ln(rebasing ?- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :- _("Branch '%s' set up to track remote branch '%s' from '%s'."),- local, shortname, origin);- else- printf_ln(rebasing ?- _("Branch '%s' set up to track local branch '%s' by rebasing.") :- _("Branch '%s' set up to track local branch '%s'."),- local, shortname);+ strbuf_addf(&refname, "%s/", origin);+ strbuf_addstr(&refname, remotes->items[0].string);++ /*+ * Rebasing is only allowed in the case of a single+ * upstream branch.+ */+ printf_ln(rebasing ?+ _("branch '%s' set up to track '%s' by rebasing.") :+ _("branch '%s' set up to track '%s'."),+ local, refname.buf);++ strbuf_release(&refname);+ } else if (origin) {+ printf_ln(_("branch '%s' set up to track from '%s':"),+ local, origin);+ printf("%s", ref_string.buf);
It's not clear to me why the hint contains the word 'from' when it is a
remote ref...
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ printf_ln(_("branch '%s' set up to track:"), local);
+ printf("%s", ref_string.buf);
but does not have the word 'from' when it is a local ref. As far as I
can tell, this is the only difference between remote and local refs, and
adding the word 'from' does not seem like a good enough reason to add an
'if' condition. Maybe I missed something here?
This motivates my answer to the question you asked in [1]:
I removed as many distinctions as possible, as most can still be
inferred from context. [...] Likewise, we don't need to specify whether
refs are remote or local: "some-remote/some-branch" vs.
"a-local-branch" should be understandable without us spelling it out.
I agree that there is adequate context, so I would be ok with the
simplification if there was corresponding code simplification e.g.
dropping "if (origin)". But in its current form, I don't think there is
good enough reason to simplify the message.
Of course, IIUC, this is as simple as dropping 'from' in the "if
(origin)" case.
quoted hunk
@@ -118,14 +156,33 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); error(_("Unable to write upstream branch configuration"));- advise(_(tracking_advice),- origin ? origin : "",- origin ? "/" : "",- shortname ? shortname : remote);+ advise(_("\nAfter fixing the error cause you may try to fix up\n"+ "the remote tracking information by invoking:"));+ if (remotes->nr == 1)+ advise(" git branch --set-upstream-to=%s%s%s",+ origin ? origin : "",+ origin ? "/" : "",+ remotes->items[0].string);+ else+ for_each_string_list_item(item, remotes)+ advise(" git config --add branch.\"%s\".merge %s",+ local, item->string);
So, in the BRANCH_TRACK_{ALWAYS,EXPLICIT,OVERRIDE} cases, we append
orig_ref because we expect orig_ref to be a local ref that the caller
wants to track. This is not the case with BRANCH_TRACK_INHERIT, where we
want to inherit the configuration and we no longer care about orig_ref.
This is correct, though it's more unobvious than what I originally
envisioned when I commented on [1]. As a small nit, it might benefit
from a clarifying comment, but this is fine as it is :)
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+test_configbranch.main.remoteorigin&&+test_configbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge
Nit: in both cases, the expected result is that branch.b*.merge is
"refs/heads/main". so the difference between --track=direct and
--track=inherit would be more obvious if main tracked something other
than origin/main.
As an side, the comments in the tests make it really readable :)
Overall this patch looks good.
[1] https://lore.kernel.org/git/kl6lfsr3c3j7.fsf@chooglen-macbookpro.roam.corp.google.com
{
const char *shortname = NULL;
struct strbuf key = STRBUF_INIT;
+ struct string_list_item *item;
int rebasing = should_setup_rebase(origin);
- if (skip_prefix(remote, "refs/heads/", &shortname)
- && !strcmp(local, shortname)
- && !origin) {
- warning(_("Not setting branch %s as its own upstream."),
- local);
- return 0;
- }
+ if (!remotes->nr)
+ BUG("must provide at least one remote for branch config");
+ if (rebasing && remotes->nr > 1)
+ die(_("cannot inherit upstream tracking configuration when rebasing is requested"));
Nit: if we're being pedantic, we cannot inherit upstream tracking
configuration when rebasing is requested with multiple upstream
branches.
But this message is already very niche and loaded with specifics, so
adding "...with multiple upstream branches" might just be more
confusing, so this is a nit.
I think it's worthwhile to be precise. Thanks for pointing this out.
Fixed in V7.
quoted
@@ -87,29 +112,42 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) {- if (shortname) {+ const char *name;+ struct strbuf ref_string = STRBUF_INIT;++ for_each_string_list_item(item, remotes) {+ name = item->string;+ skip_prefix(name, "refs/heads/", &name);+ strbuf_addf(&ref_string, " %s\n", name);+ }++ if (remotes->nr == 1) {+ struct strbuf refname = STRBUF_INIT;+ if (origin)- printf_ln(rebasing ?- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :- _("Branch '%s' set up to track remote branch '%s' from '%s'."),- local, shortname, origin);- else- printf_ln(rebasing ?- _("Branch '%s' set up to track local branch '%s' by rebasing.") :- _("Branch '%s' set up to track local branch '%s'."),- local, shortname);+ strbuf_addf(&refname, "%s/", origin);+ strbuf_addstr(&refname, remotes->items[0].string);++ /*+ * Rebasing is only allowed in the case of a single+ * upstream branch.+ */+ printf_ln(rebasing ?+ _("branch '%s' set up to track '%s' by rebasing.") :+ _("branch '%s' set up to track '%s'."),+ local, refname.buf);++ strbuf_release(&refname);+ } else if (origin) {+ printf_ln(_("branch '%s' set up to track from '%s':"),+ local, origin);+ printf("%s", ref_string.buf);
It's not clear to me why the hint contains the word 'from' when it is a
remote ref...
Because in the multiple-branch case, we don't prepend the origin to each
ref, so we need to let users know which remote the refs are coming from.
quoted
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ printf_ln(_("branch '%s' set up to track:"), local);
+ printf("%s", ref_string.buf);
but does not have the word 'from' when it is a local ref. As far as I
can tell, this is the only difference between remote and local refs, and
adding the word 'from' does not seem like a good enough reason to add an
'if' condition. Maybe I missed something here?
This motivates my answer to the question you asked in [1]:
I removed as many distinctions as possible, as most can still be
inferred from context. [...] Likewise, we don't need to specify whether
refs are remote or local: "some-remote/some-branch" vs.
"a-local-branch" should be understandable without us spelling it out.
I agree that there is adequate context, so I would be ok with the
simplification if there was corresponding code simplification e.g.
dropping "if (origin)". But in its current form, I don't think there is
good enough reason to simplify the message.
I think the proper point of comparison is not the original code, but the
code from V5 where we try to preserve the same level of detail in output
as the original code. If we are committed to both having multiple
remotes and keeping similar styles of output as the original
implementation, then something like the massive conditional in V5 is
unavoidable.
Of course, IIUC, this is as simple as dropping 'from' in the "if
(origin)" case.
quoted
@@ -118,14 +156,33 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); error(_("Unable to write upstream branch configuration"));- advise(_(tracking_advice),- origin ? origin : "",- origin ? "/" : "",- shortname ? shortname : remote);+ advise(_("\nAfter fixing the error cause you may try to fix up\n"+ "the remote tracking information by invoking:"));+ if (remotes->nr == 1)+ advise(" git branch --set-upstream-to=%s%s%s",+ origin ? origin : "",+ origin ? "/" : "",+ remotes->items[0].string);+ else+ for_each_string_list_item(item, remotes)+ advise(" git config --add branch.\"%s\".merge %s",+ local, item->string);
So, in the BRANCH_TRACK_{ALWAYS,EXPLICIT,OVERRIDE} cases, we append
orig_ref because we expect orig_ref to be a local ref that the caller
wants to track. This is not the case with BRANCH_TRACK_INHERIT, where we
want to inherit the configuration and we no longer care about orig_ref.
This is correct, though it's more unobvious than what I originally
envisioned when I commented on [1]. As a small nit, it might benefit
from a clarifying comment, but this is fine as it is :)
Actually, you're right, the `track != BRANCH_TRACK_INHERIT` condition is
superfluous. The only way we could have BRANCH_TRACK_INHERIT and
tracking.srcs->nr < 1 at the same time would be if there were no
"branch.*.merge" entries in the config to inherit, but if that were true
then we would have had returned from this function earlier.
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+test_configbranch.main.remoteorigin&&+test_configbranch.main.mergerefs/heads/main&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/mainbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/mainbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge
Nit: in both cases, the expected result is that branch.b*.merge is
"refs/heads/main". so the difference between --track=direct and
--track=inherit would be more obvious if main tracked something other
than origin/main.
Changes since V6:
* Strip the refs/heads/ prefix in the verbose output when we have only a
single upstream branch.
* Improve the fatal error message to note that rebasing is only
incompatible with multiple upstream refs.
* Also note that `branch.<name>.remote` should be set in the manual
recovery advice.
* Simplify the logic in setup_tracking() when no tracking sources match.
* Make the difference in test cases in t2027 more obvious.
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
* Note that rebasing is not compatible with inheriting multiple upstream
branches.
* Moved the change to case-sensitivity for branch.autosetupmerge to its
own commit.
* Improve advice on failed tracking setup when multiple branches are
involved.
* Make better use of string_list API.
* Make better use of config API.
* More straight-forward use of the `struct tracking` API.
* Numerous style fixes.
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 (3):
branch: accept multiple upstream branches for tracking
branch: add flags and config to inherit tracking
config: require lowercase for branch.*.autosetupmerge
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 ++--
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 189 ++++++++++++++++++++++++--------
branch.h | 3 +-
builtin/branch.c | 6 +-
builtin/checkout.c | 6 +-
config.c | 5 +-
parse-options-cb.c | 16 +++
parse-options.h | 2 +
t/t2017-checkout-orphan.sh | 11 +-
t/t2027-checkout-track.sh | 23 ++++
t/t2060-switch.sh | 28 +++++
t/t3200-branch.sh | 39 ++++++-
t/t7201-co.sh | 17 +++
16 files changed, 310 insertions(+), 66 deletions(-)
Range-diff against v6:
1: 43d6f83fed ! 1: 9152367ba9 branch: accept multiple upstream branches for tracking
@@ branch.c: static int should_setup_rebase(const char *origin)
+ if (!remotes->nr)
+ BUG("must provide at least one remote for branch config");
+ if (rebasing && remotes->nr > 1)
-+ die(_("cannot inherit upstream tracking configuration when rebasing is requested"));
++ die(_("cannot inherit upstream tracking configuration of "
++ "multiple refs when rebasing is requested"));
+
+ if (!origin)
+ for_each_string_list_item(item, remotes)
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
- _("Branch '%s' set up to track local branch '%s'."),
- local, shortname);
+ strbuf_addf(&refname, "%s/", origin);
-+ strbuf_addstr(&refname, remotes->items[0].string);
++ skip_prefix(remotes->items[0].string, "refs/heads/", &name);
++ strbuf_addstr(&refname, name);
+
+ /*
+ * Rebasing is only allowed in the case of a single
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
+ origin ? origin : "",
+ origin ? "/" : "",
+ remotes->items[0].string);
-+ else
++ else {
++ advise(" git config --add branch.\"%s\".remote %s",
++ local, origin ? origin : ".");
+ for_each_string_list_item(item, remotes)
+ advise(" git config --add branch.\"%s\".merge %s",
+ local, item->string);
++ }
return -1;
}
2: 57e57e6e6a ! 2: afeb84539e branch: add flags and config to inherit tracking
@@ branch.c: static void setup_tracking(const char *new_ref, const char *orig_ref,
- if (install_branch_config(config_flags, new_ref, tracking.remote,
- tracking.src ? tracking.src : orig_ref) < 0)
-+ if (tracking.srcs->nr < 1 && track != BRANCH_TRACK_INHERIT)
++ 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)
++ if (install_branch_config_multiple_remotes(config_flags, new_ref,
++ tracking.remote, tracking.srcs) < 0)
exit(-1);
- free(tracking.src);
@@ 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
+ test_config branch.main.remote origin &&
-+ test_config branch.main.merge refs/heads/main &&
++ test_config branch.main.merge refs/heads/some-branch &&
+ test_config branch.autoSetupMerge inherit &&
+ # With --track=inherit, we copy the tracking config from main
+ git checkout --track=inherit -b b1 main &&
+ test_cmp_config origin branch.b1.remote &&
-+ test_cmp_config refs/heads/main branch.b1.merge &&
++ test_cmp_config refs/heads/some-branch branch.b1.merge &&
+ # With branch.autoSetupMerge=inherit, we do the same
+ git checkout -b b2 main &&
+ test_cmp_config origin branch.b2.remote &&
-+ test_cmp_config refs/heads/main branch.b2.merge &&
++ test_cmp_config refs/heads/some-branch branch.b2.merge &&
+ # But --track overrides this
+ git checkout --track -b b3 main &&
+ test_cmp_config . branch.b3.remote &&
3: f79d27dc24 = 3: a818a6561b config: require lowercase for branch.*.autosetupmerge
base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee
--
2.34.1.173.g76aa8bc2d0-goog
Add a new static variant of install_branch_config() that accepts
multiple remote branch names for tracking. This will be used in an
upcoming commit that enables inheriting the tracking configuration from
a parent branch.
Currently, all callers of install_branch_config() pass only a single
remote. Make install_branch_config() a small wrapper around
install_branch_config_multiple_remotes() so that existing callers do not
need to be changed.
Signed-off-by: Josh Steadmon <redacted>
---
branch.c | 140 +++++++++++++++++++++++++++++++++-------------
t/t3200-branch.sh | 6 +-
2 files changed, 104 insertions(+), 42 deletions(-)
@@ -49,25 +49,42 @@ static int should_setup_rebase(const char *origin)return0;}-staticconstchartracking_advice[]=-N_("\n"-"After fixing the error cause you may try to fix up\n"-"the remote tracking information by invoking\n"-"\"git branch --set-upstream-to=%s%s%s\".");--intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+/**+*Installupstreamtrackingconfigurationforabranch;specifically,add+*`branch.<name>.remote`and`branch.<name>.merge`entries.+*+*`flag`containsintegerflagsforoptions;currentlyonly+*BRANCH_CONFIG_VERBOSEischecked.+*+*`local`isthenameofthebranchwhoseconfigurationwe'reinstalling.+*+*`origin`isthenameoftheremoteowningtheupstreambranches.NULLmeans+*theupstreambranchesarelocaltothisrepo.+*+*`remotes`isalistofrefsthatareupstreamoflocal+*/+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,+constchar*origin,structstring_list*remotes){constchar*shortname=NULL;structstrbufkey=STRBUF_INIT;+structstring_list_item*item;intrebasing=should_setup_rebase(origin);-if(skip_prefix(remote,"refs/heads/",&shortname)-&&!strcmp(local,shortname)-&&!origin){-warning(_("Not setting branch %s as its own upstream."),-local);-return0;-}+if(!remotes->nr)+BUG("must provide at least one remote for branch config");+if(rebasing&&remotes->nr>1)+die(_("cannot inherit upstream tracking configuration of "+"multiple refs when rebasing is requested"));++if(!origin)+for_each_string_list_item(item,remotes)+if(skip_prefix(item->string,"refs/heads/",&shortname)+&&!strcmp(local,shortname)){+warning(_("not setting branch '%s' as its own upstream."),+local);+return0;+}strbuf_addf(&key,"branch.%s.remote",local);if(git_config_set_gently(key.buf,origin?origin:".")<0)
@@ -87,29 +113,43 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);if(flag&BRANCH_CONFIG_VERBOSE){-if(shortname){+constchar*name;+structstrbufref_string=STRBUF_INIT;++for_each_string_list_item(item,remotes){+name=item->string;+skip_prefix(name,"refs/heads/",&name);+strbuf_addf(&ref_string," %s\n",name);+}++if(remotes->nr==1){+structstrbufrefname=STRBUF_INIT;+if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing."):-_("Branch '%s' set up to track remote branch '%s' from '%s'."),-local,shortname,origin);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local branch '%s' by rebasing."):-_("Branch '%s' set up to track local branch '%s'."),-local,shortname);+strbuf_addf(&refname,"%s/",origin);+skip_prefix(remotes->items[0].string,"refs/heads/",&name);+strbuf_addstr(&refname,name);++/*+*Rebasingisonlyallowedinthecaseofasingle+*upstreambranch.+*/+printf_ln(rebasing?+_("branch '%s' set up to track '%s' by rebasing."):+_("branch '%s' set up to track '%s'."),+local,refname.buf);++strbuf_release(&refname);+}elseif(origin){+printf_ln(_("branch '%s' set up to track from '%s':"),+local,origin);+printf("%s",ref_string.buf);}else{-if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote ref '%s' by rebasing."):-_("Branch '%s' set up to track remote ref '%s'."),-local,remote);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local ref '%s' by rebasing."):-_("Branch '%s' set up to track local ref '%s'."),-local,remote);+printf_ln(_("branch '%s' set up to track:"),local);+printf("%s",ref_string.buf);}++strbuf_release(&ref_string);}return0;
@@ -118,14 +158,36 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);error(_("Unable to write upstream branch configuration"));-advise(_(tracking_advice),-origin?origin:"",-origin?"/":"",-shortname?shortname:remote);+advise(_("\nAfter fixing the error cause you may try to fix up\n"+"the remote tracking information by invoking:"));+if(remotes->nr==1)+advise(" git branch --set-upstream-to=%s%s%s",+origin?origin:"",+origin?"/":"",+remotes->items[0].string);+else{+advise(" git config --add branch.\"%s\".remote %s",+local,origin?origin:".");+for_each_string_list_item(item,remotes)+advise(" git config --add branch.\"%s\".merge %s",+local,item->string);+}return-1;}+intinstall_branch_config(intflag,constchar*local,constchar*origin,+constchar*remote)+{+intret;+structstring_listremotes=STRING_LIST_INIT_DUP;++string_list_append(&remotes,remote);+ret=install_branch_config_multiple_remotes(flag,local,origin,&remotes);+string_list_clear(&remotes,0);+returnret;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -950,15 +950,15 @@ test_expect_success 'disabled option --set-upstream fails' 'test_must_failgitbranch--set-upstreamorigin/main'-test_expect_success'--set-upstream-to notices an error to set branch as own upstream''+test_expect_success'--set-upstream-to notices an error to set branch as own upstream'"gitbranch--set-upstream-torefs/heads/my13my132>actual&&cat>expect<<-\EOF&&-warning:Notsettingbranchmy13asitsownupstream.+warning:notsettingbranch'my13'asitsownupstream.EOFtest_expect_code1gitconfigbranch.my13.remote&&test_expect_code1gitconfigbranch.my13.merge&&test_cmpexpectactual-'+"# Keep this test last, as it changes the current branch cat>expect<<EOF
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
Finally, teach branch.autoSetupMerge a new "inherit" option. When this
is set, "--track=inherit" becomes the default behavior.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 +++++++++++-----
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 49 ++++++++++++++++++++++++++++-----
branch.h | 3 +-
builtin/branch.c | 6 ++--
builtin/checkout.c | 6 ++--
config.c | 3 ++
parse-options-cb.c | 16 +++++++++++
parse-options.h | 2 ++
t/t2017-checkout-orphan.sh | 11 +++++++-
t/t2027-checkout-track.sh | 23 ++++++++++++++++
t/t2060-switch.sh | 28 +++++++++++++++++++
t/t3200-branch.sh | 33 ++++++++++++++++++++++
t/t7201-co.sh | 17 ++++++++++++
16 files changed, 205 insertions(+), 23 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -205,24 +205,34 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track::+--track [inherit|direct]:: When creating a new branch, set up `branch.<name>.remote` and- `branch.<name>.merge` configuration entries to mark the- start-point branch as "upstream" from the new branch. This+ `branch.<name>.merge` configuration entries to set "upstream" tracking+ configuration for the new branch. This configuration will tell git to show the relationship between the two branches in `git status` and `git branch -v`. Furthermore, it directs `git pull` without arguments to pull from the upstream when the new branch is checked out. +-This behavior is the default when the start point is a remote-tracking branch.+The exact upstream branch is chosen depending on the optional argument:+`--track` or `--track direct` means to use the start-point branch itself as the+upstream; `--track inherit` means to copy the upstream configuration of the+start-point branch.+++`--track direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+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- branch.autoSetupMerge configuration variable is true.+ branch.autoSetupMerge configuration variable is set. --set-upstream:: As this option had confusing syntax, it is no longer supported.
@@ -155,7 +155,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -152,7 +152,7 @@ should result in deletion of the path). attached to a terminal, regardless of `--quiet`. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. `-c` is implied. See `--track` in linkgit:git-branch[1] for details.
@@ -188,6 +188,34 @@ int install_branch_config(int flag, const char *local, const char *origin,returnret;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+constchar*bare_ref;+structbranch*branch;+inti;++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]);+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),OPT_STRING('u',"set-upstream-to",&new_upstream,N_("upstream"),N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+test_configbranch.main.remoteorigin&&+test_configbranch.main.mergerefs/heads/some-branch&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/some-branchbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/some-branchbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge+'+ test_done
@@ -107,4 +107,32 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with --track=inherit, we copy tracking info from foo+gitswitch--track=inherit-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# with autoSetupMerge=inherit, we do the same+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo3foo&&+test_cmp_configoriginbranch.foo3.remote&&+test_cmp_configrefs/heads/foobranch.foo3.merge&&+# with --track, we override autoSetupMerge+gitswitch--track-cfoo4foo&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/foobranch.foo4.merge&&+# and --track=direct does as well+gitswitch--track=direct-cfoo5foo&&+test_cmp_config.branch.foo5.remote&&+test_cmp_configrefs/heads/foobranch.foo5.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
@@ -1409,4 +1409,37 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with --track=inherit''+gitbranch--track=inheritfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge+'++test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo3my1&&+test_cmp_configlocalbranch.foo3.remote&&+test_cmp_configrefs/heads/mainbranch.foo3.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'++test_expect_success'--track overrides branch.autoSetupMerge''+test_configbranch.autoSetupMergeinherit&&+gitbranch--track=directfoo4my1&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/my1branch.foo4.merge&&+gitbranch--no-trackfoo5my1&&+test_cmp_config""--default""branch.foo5.remote&&+test_cmp_config""--default""branch.foo5.merge+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
Although we only documented that branch.*.autosetupmerge would accept
"always" as a value, the actual implementation would accept any
combination of upper- or lower-case. Fix this to be consistent with
documentation and with other values of this config variable.
Signed-off-by: Josh Steadmon <redacted>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -87,29 +112,42 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) {- if (shortname) {+ const char *name;+ struct strbuf ref_string = STRBUF_INIT;++ for_each_string_list_item(item, remotes) {+ name = item->string;+ skip_prefix(name, "refs/heads/", &name);+ strbuf_addf(&ref_string, " %s\n", name);+ }++ if (remotes->nr == 1) {+ struct strbuf refname = STRBUF_INIT;+ if (origin)- printf_ln(rebasing ?- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :- _("Branch '%s' set up to track remote branch '%s' from '%s'."),- local, shortname, origin);- else- printf_ln(rebasing ?- _("Branch '%s' set up to track local branch '%s' by rebasing.") :- _("Branch '%s' set up to track local branch '%s'."),- local, shortname);+ strbuf_addf(&refname, "%s/", origin);+ strbuf_addstr(&refname, remotes->items[0].string);++ /*+ * Rebasing is only allowed in the case of a single+ * upstream branch.+ */+ printf_ln(rebasing ?+ _("branch '%s' set up to track '%s' by rebasing.") :+ _("branch '%s' set up to track '%s'."),+ local, refname.buf);++ strbuf_release(&refname);+ } else if (origin) {+ printf_ln(_("branch '%s' set up to track from '%s':"),+ local, origin);+ printf("%s", ref_string.buf);
It's not clear to me why the hint contains the word 'from' when it is a
remote ref...
Because in the multiple-branch case, we don't prepend the origin to each
ref, so we need to let users know which remote the refs are coming from.
I see. So if I'm reading this correctly, the error message in the remote
case would read something like:
branch 'main' set up to track from 'origin':
main
topic1
topic2
Is there any reason why we couldn't append the origin to the ref to make
it consistent? I think this could be as simple as:
for_each_string_list_item(item, remotes) {
name = item->string;
skip_prefix(name, "refs/heads/", &name);
if (origin)
+ strbuf_addf(&ref_string, "%s/", origin);
strbuf_addf(&ref_string, " %s\n", name);
}
and the resulting list could look like:
branch 'main' set up to track from 'origin':
origin/main
origin/topic1
origin/topic2
This looks repetitive, but I suggest this because, as I understand it,
we are omitting the "{local,remote} ref" phrase based on conventions
around ref names, like "origin/main" is probably a remote ref and not an
oddly named local ref. However, when we print the list like so,
branch 'main' set up to track from 'origin':
main
topic1
topic2
we now expect the user to understand that 'main', 'topic1' and 'topic2'
to implicitly have 'origin/' prepended to them. This behavior seems
inconsistent to me; I'd anticipate most users responding "Wait, I was
supposed to be tracking 'origin' branches right? Why am I looking at
local branches?". Some users would be able to recover because they can
figure out what we mean, but others might just give up.
Prepending 'origin/' would get rid of this problem altogether, and it
would let us drop the 'from'.
quoted
quoted
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ printf_ln(_("branch '%s' set up to track:"), local);
+ printf("%s", ref_string.buf);
but does not have the word 'from' when it is a local ref. As far as I
can tell, this is the only difference between remote and local refs, and
adding the word 'from' does not seem like a good enough reason to add an
'if' condition. Maybe I missed something here?
This motivates my answer to the question you asked in [1]:
I removed as many distinctions as possible, as most can still be
inferred from context. [...] Likewise, we don't need to specify whether
refs are remote or local: "some-remote/some-branch" vs.
"a-local-branch" should be understandable without us spelling it out.
I agree that there is adequate context, so I would be ok with the
simplification if there was corresponding code simplification e.g.
dropping "if (origin)". But in its current form, I don't think there is
good enough reason to simplify the message.
I think the proper point of comparison is not the original code, but the
code from V5 where we try to preserve the same level of detail in output
as the original code. If we are committed to both having multiple
remotes and keeping similar styles of output as the original
implementation, then something like the massive conditional in V5 is
unavoidable.
I see. So for instance, post-simplification you have:
printf_ln(rebasing ?
_("branch '%s' set up to track '%s' by rebasing.") :
_("branch '%s' set up to track '%s'."),
local, refname.buf);
if you preserve the same amount of detail as before, you'd have to
distinguish between local/remote, which doubles the number of cases to
4, which is why the conditional v5 is so complicated.
That said, I think that it's already much simpler than v5 because you've
split the singular and plural cases. I wonder if you have considered
building the final string purely from format strings, like:
char *message_format = _("branch %s set up to track %s%s%s%s");
char *ref_type_clause = origin ? " remote ref " : " local ref ";
char *rebasing_clause = rebasing ? " by rebasing." : ".";
char *branch_names = "<branch names>";
printf_ln(message_format, local, ref_type_clause, branch_names, rebasing_clause);
This sounds potentially unfriendly to i18n, but it would make the
conditional simpler. What do you think?
Changes since V6:
* Strip the refs/heads/ prefix in the verbose output when we have only a
single upstream branch.
* Improve the fatal error message to note that rebasing is only
incompatible with multiple upstream refs.
* Also note that `branch.<name>.remote` should be set in the manual
recovery advice.
* Simplify the logic in setup_tracking() when no tracking sources match.
* Make the difference in test cases in t2027 more obvious.
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
* Note that rebasing is not compatible with inheriting multiple upstream
branches.
* Moved the change to case-sensitivity for branch.autosetupmerge to its
own commit.
* Improve advice on failed tracking setup when multiple branches are
involved.
* Make better use of string_list API.
* Make better use of config API.
* More straight-forward use of the `struct tracking` API.
* Numerous style fixes.
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 (3):
branch: accept multiple upstream branches for tracking
branch: add flags and config to inherit tracking
config: require lowercase for branch.*.autosetupmerge
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 ++--
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 189 ++++++++++++++++++++++++--------
branch.h | 3 +-
builtin/branch.c | 6 +-
builtin/checkout.c | 6 +-
config.c | 5 +-
parse-options-cb.c | 16 +++
parse-options.h | 2 +
t/t2017-checkout-orphan.sh | 11 +-
t/t2027-checkout-track.sh | 23 ++++
t/t2060-switch.sh | 28 +++++
t/t3200-branch.sh | 39 ++++++-
t/t7201-co.sh | 17 +++
16 files changed, 310 insertions(+), 66 deletions(-)
Range-diff against v6:
1: 43d6f83fed ! 1: 9152367ba9 branch: accept multiple upstream branches for tracking
@@ branch.c: static int should_setup_rebase(const char *origin)
+ if (!remotes->nr)
+ BUG("must provide at least one remote for branch config");
+ if (rebasing && remotes->nr > 1)
-+ die(_("cannot inherit upstream tracking configuration when rebasing is requested"));
++ die(_("cannot inherit upstream tracking configuration of "
++ "multiple refs when rebasing is requested"));
+
+ if (!origin)
+ for_each_string_list_item(item, remotes)
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
- _("Branch '%s' set up to track local branch '%s'."),
- local, shortname);
+ strbuf_addf(&refname, "%s/", origin);
-+ strbuf_addstr(&refname, remotes->items[0].string);
++ skip_prefix(remotes->items[0].string, "refs/heads/", &name);
++ strbuf_addstr(&refname, name);
+
+ /*
+ * Rebasing is only allowed in the case of a single
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
+ origin ? origin : "",
+ origin ? "/" : "",
+ remotes->items[0].string);
-+ else
++ else {
++ advise(" git config --add branch.\"%s\".remote %s",
++ local, origin ? origin : ".");
+ for_each_string_list_item(item, remotes)
+ advise(" git config --add branch.\"%s\".merge %s",
+ local, item->string);
++ }
return -1;
}
2: 57e57e6e6a ! 2: afeb84539e branch: add flags and config to inherit tracking
@@ branch.c: static void setup_tracking(const char *new_ref, const char *orig_ref,
- if (install_branch_config(config_flags, new_ref, tracking.remote,
- tracking.src ? tracking.src : orig_ref) < 0)
-+ if (tracking.srcs->nr < 1 && track != BRANCH_TRACK_INHERIT)
++ 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)
++ if (install_branch_config_multiple_remotes(config_flags, new_ref,
++ tracking.remote, tracking.srcs) < 0)
exit(-1);
- free(tracking.src);
@@ 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
+ test_config branch.main.remote origin &&
-+ test_config branch.main.merge refs/heads/main &&
++ test_config branch.main.merge refs/heads/some-branch &&
+ test_config branch.autoSetupMerge inherit &&
+ # With --track=inherit, we copy the tracking config from main
+ git checkout --track=inherit -b b1 main &&
+ test_cmp_config origin branch.b1.remote &&
-+ test_cmp_config refs/heads/main branch.b1.merge &&
++ test_cmp_config refs/heads/some-branch branch.b1.merge &&
+ # With branch.autoSetupMerge=inherit, we do the same
+ git checkout -b b2 main &&
+ test_cmp_config origin branch.b2.remote &&
-+ test_cmp_config refs/heads/main branch.b2.merge &&
++ test_cmp_config refs/heads/some-branch branch.b2.merge &&
+ # But --track overrides this
+ git checkout --track -b b3 main &&
+ test_cmp_config . branch.b3.remote &&
3: f79d27dc24 = 3: a818a6561b config: require lowercase for branch.*.autosetupmerge
base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee
--
2.34.1.173.g76aa8bc2d0-goog
Thanks! As noted in v6, I don't have strong opinions about omitting
'local/remote' from the help message, but I suspect others might. It
would be nice to get more opinions here.
v7 looks pretty good, it addresses all of the feedback on v6.
Unfortunately, I spotted some things _after_ you had already sent out v7
(my bad). That feedback is in [1].
We can ignore my feedback on 'simplifying' the conditional with a
format string. The more I think about it, the more impossible i18n
seems.
This just leaves the feedback on the message when inheriting from
multiple remote-tracking branches, i.e. prepending 'origin/' to the refs
in:
branch 'main' set up to track from 'origin':
main
topic1
topic2
[1] https://lore.kernel.org/git/kl6lzgovyvt7.fsf@chooglen-macbookpro.roam.corp.google.com
@@ -87,29 +112,42 @@ int install_branch_config(int flag, const char *local, const char *origin, const strbuf_release(&key); if (flag & BRANCH_CONFIG_VERBOSE) {- if (shortname) {+ const char *name;+ struct strbuf ref_string = STRBUF_INIT;++ for_each_string_list_item(item, remotes) {+ name = item->string;+ skip_prefix(name, "refs/heads/", &name);+ strbuf_addf(&ref_string, " %s\n", name);+ }++ if (remotes->nr == 1) {+ struct strbuf refname = STRBUF_INIT;+ if (origin)- printf_ln(rebasing ?- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :- _("Branch '%s' set up to track remote branch '%s' from '%s'."),- local, shortname, origin);- else- printf_ln(rebasing ?- _("Branch '%s' set up to track local branch '%s' by rebasing.") :- _("Branch '%s' set up to track local branch '%s'."),- local, shortname);+ strbuf_addf(&refname, "%s/", origin);+ strbuf_addstr(&refname, remotes->items[0].string);++ /*+ * Rebasing is only allowed in the case of a single+ * upstream branch.+ */+ printf_ln(rebasing ?+ _("branch '%s' set up to track '%s' by rebasing.") :+ _("branch '%s' set up to track '%s'."),+ local, refname.buf);++ strbuf_release(&refname);+ } else if (origin) {+ printf_ln(_("branch '%s' set up to track from '%s':"),+ local, origin);+ printf("%s", ref_string.buf);
It's not clear to me why the hint contains the word 'from' when it is a
remote ref...
Because in the multiple-branch case, we don't prepend the origin to each
ref, so we need to let users know which remote the refs are coming from.
I see. So if I'm reading this correctly, the error message in the remote
case would read something like:
branch 'main' set up to track from 'origin':
main
topic1
topic2
Is there any reason why we couldn't append the origin to the ref to make
it consistent? I think this could be as simple as:
for_each_string_list_item(item, remotes) {
name = item->string;
skip_prefix(name, "refs/heads/", &name);
if (origin)
+ strbuf_addf(&ref_string, "%s/", origin);
strbuf_addf(&ref_string, " %s\n", name);
}
and the resulting list could look like:
branch 'main' set up to track from 'origin':
origin/main
origin/topic1
origin/topic2
This looks repetitive, but I suggest this because, as I understand it,
we are omitting the "{local,remote} ref" phrase based on conventions
around ref names, like "origin/main" is probably a remote ref and not an
oddly named local ref. However, when we print the list like so,
branch 'main' set up to track from 'origin':
main
topic1
topic2
we now expect the user to understand that 'main', 'topic1' and 'topic2'
to implicitly have 'origin/' prepended to them. This behavior seems
inconsistent to me; I'd anticipate most users responding "Wait, I was
supposed to be tracking 'origin' branches right? Why am I looking at
local branches?". Some users would be able to recover because they can
figure out what we mean, but others might just give up.
Prepending 'origin/' would get rid of this problem altogether, and it
would let us drop the 'from'.
Yeah, I think that's better. Fixed in V7, thanks.
quoted
quoted
quoted
} else {
- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote ref '%s' by rebasing.") :
- _("Branch '%s' set up to track remote ref '%s'."),
- local, remote);
- else
- printf_ln(rebasing ?
- _("Branch '%s' set up to track local ref '%s' by rebasing.") :
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ printf_ln(_("branch '%s' set up to track:"), local);
+ printf("%s", ref_string.buf);
but does not have the word 'from' when it is a local ref. As far as I
can tell, this is the only difference between remote and local refs, and
adding the word 'from' does not seem like a good enough reason to add an
'if' condition. Maybe I missed something here?
This motivates my answer to the question you asked in [1]:
I removed as many distinctions as possible, as most can still be
inferred from context. [...] Likewise, we don't need to specify whether
refs are remote or local: "some-remote/some-branch" vs.
"a-local-branch" should be understandable without us spelling it out.
I agree that there is adequate context, so I would be ok with the
simplification if there was corresponding code simplification e.g.
dropping "if (origin)". But in its current form, I don't think there is
good enough reason to simplify the message.
I think the proper point of comparison is not the original code, but the
code from V5 where we try to preserve the same level of detail in output
as the original code. If we are committed to both having multiple
remotes and keeping similar styles of output as the original
implementation, then something like the massive conditional in V5 is
unavoidable.
I see. So for instance, post-simplification you have:
printf_ln(rebasing ?
_("branch '%s' set up to track '%s' by rebasing.") :
_("branch '%s' set up to track '%s'."),
local, refname.buf);
if you preserve the same amount of detail as before, you'd have to
distinguish between local/remote, which doubles the number of cases to
4, which is why the conditional v5 is so complicated.
That said, I think that it's already much simpler than v5 because you've
split the singular and plural cases. I wonder if you have considered
building the final string purely from format strings, like:
char *message_format = _("branch %s set up to track %s%s%s%s");
char *ref_type_clause = origin ? " remote ref " : " local ref ";
char *rebasing_clause = rebasing ? " by rebasing." : ".";
char *branch_names = "<branch names>";
printf_ln(message_format, local, ref_type_clause, branch_names, rebasing_clause);
This sounds potentially unfriendly to i18n, but it would make the
conditional simpler. What do you think?
Yeah, the translation-unfriendliness is why I avoided this approach.
Changes since V7:
* Further simplify verbose output by adding an "<origin>/" prefix for
remote-tracking upstream refs.
* Add a comment explaining the self-tracking check & early exit.
Changes since V6:
* Strip the refs/heads/ prefix in the verbose output when we have only a
single upstream branch.
* Improve the fatal error message to note that rebasing is only
incompatible with multiple upstream refs.
* Also note that `branch.<name>.remote` should be set in the manual
recovery advice.
* Simplify the logic in setup_tracking() when no tracking sources match.
* Make the difference in test cases in t2027 more obvious.
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
* Note that rebasing is not compatible with inheriting multiple upstream
branches.
* Moved the change to case-sensitivity for branch.autosetupmerge to its
own commit.
* Improve advice on failed tracking setup when multiple branches are
involved.
* Make better use of string_list API.
* Make better use of config API.
* More straight-forward use of the `struct tracking` API.
* Numerous style fixes.
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 (3):
branch: accept multiple upstream branches for tracking
branch: add flags and config to inherit tracking
config: require lowercase for branch.*.autosetupmerge
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 ++--
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 192 ++++++++++++++++++++++++--------
branch.h | 3 +-
builtin/branch.c | 6 +-
builtin/checkout.c | 6 +-
config.c | 5 +-
parse-options-cb.c | 16 +++
parse-options.h | 2 +
t/t2017-checkout-orphan.sh | 11 +-
t/t2027-checkout-track.sh | 23 ++++
t/t2060-switch.sh | 28 +++++
t/t3200-branch.sh | 39 ++++++-
t/t7201-co.sh | 17 +++
16 files changed, 312 insertions(+), 67 deletions(-)
Range-diff against v7:
1: 9152367ba9 ! 1: a5265e1c7f branch: accept multiple upstream branches for tracking
@@ branch.c: static int should_setup_rebase(const char *origin)
+ die(_("cannot inherit upstream tracking configuration of "
+ "multiple refs when rebasing is requested"));
+
++ /*
++ * If the new branch is trying to track itself, something has gone
++ * wrong. Warn the user and don't proceed any further.
++ */
+ if (!origin)
+ for_each_string_list_item(item, remotes)
+ if (skip_prefix(item->string, "refs/heads/", &shortname)
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
if (flag & BRANCH_CONFIG_VERBOSE) {
- if (shortname) {
-+ const char *name;
-+ struct strbuf ref_string = STRBUF_INIT;
-+
-+ for_each_string_list_item(item, remotes) {
-+ name = item->string;
-+ skip_prefix(name, "refs/heads/", &name);
-+ strbuf_addf(&ref_string, " %s\n", name);
-+ }
-+
-+ if (remotes->nr == 1) {
-+ struct strbuf refname = STRBUF_INIT;
-+
- if (origin)
+- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :
- _("Branch '%s' set up to track remote branch '%s' from '%s'."),
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
- _("Branch '%s' set up to track local branch '%s' by rebasing.") :
- _("Branch '%s' set up to track local branch '%s'."),
- local, shortname);
-+ strbuf_addf(&refname, "%s/", origin);
-+ skip_prefix(remotes->items[0].string, "refs/heads/", &name);
-+ strbuf_addstr(&refname, name);
++ struct strbuf tmp_ref_name = STRBUF_INIT;
++ struct string_list friendly_ref_names = STRING_LIST_INIT_DUP;
+
++ for_each_string_list_item(item, remotes) {
++ shortname = item->string;
++ skip_prefix(shortname, "refs/heads/", &shortname);
++ if (origin) {
++ strbuf_addf(&tmp_ref_name, "%s/%s",
++ origin, shortname);
++ string_list_append_nodup(
++ &friendly_ref_names,
++ strbuf_detach(&tmp_ref_name, NULL));
++ } else {
++ string_list_append(
++ &friendly_ref_names, shortname);
++ }
++ }
++
++ if (remotes->nr == 1) {
+ /*
+ * Rebasing is only allowed in the case of a single
+ * upstream branch.
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
+ printf_ln(rebasing ?
+ _("branch '%s' set up to track '%s' by rebasing.") :
+ _("branch '%s' set up to track '%s'."),
-+ local, refname.buf);
-+
-+ strbuf_release(&refname);
-+ } else if (origin) {
-+ printf_ln(_("branch '%s' set up to track from '%s':"),
-+ local, origin);
-+ printf("%s", ref_string.buf);
++ local, friendly_ref_names.items[0].string);
} else {
- if (origin)
- printf_ln(rebasing ?
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ printf_ln(_("branch '%s' set up to track:"), local);
-+ printf("%s", ref_string.buf);
++ for_each_string_list_item(item, &friendly_ref_names)
++ printf_ln(" %s", item->string);
}
+
-+ strbuf_release(&ref_string);
++ string_list_clear(&friendly_ref_names, 0);
}
return 0;
2: afeb84539e = 2: dcba40e2c4 branch: add flags and config to inherit tracking
3: a818a6561b = 3: ae7d27b4be config: require lowercase for branch.*.autosetupmerge
base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee
--
2.34.1.307.g9b7440fafd-goog
Add a new static variant of install_branch_config() that accepts
multiple remote branch names for tracking. This will be used in an
upcoming commit that enables inheriting the tracking configuration from
a parent branch.
Currently, all callers of install_branch_config() pass only a single
remote. Make install_branch_config() a small wrapper around
install_branch_config_multiple_remotes() so that existing callers do not
need to be changed.
Signed-off-by: Josh Steadmon <redacted>
---
branch.c | 143 +++++++++++++++++++++++++++++++++-------------
t/t3200-branch.sh | 6 +-
2 files changed, 106 insertions(+), 43 deletions(-)
@@ -49,25 +49,46 @@ static int should_setup_rebase(const char *origin)return0;}-staticconstchartracking_advice[]=-N_("\n"-"After fixing the error cause you may try to fix up\n"-"the remote tracking information by invoking\n"-"\"git branch --set-upstream-to=%s%s%s\".");--intinstall_branch_config(intflag,constchar*local,constchar*origin,constchar*remote)+/**+*Installupstreamtrackingconfigurationforabranch;specifically,add+*`branch.<name>.remote`and`branch.<name>.merge`entries.+*+*`flag`containsintegerflagsforoptions;currentlyonly+*BRANCH_CONFIG_VERBOSEischecked.+*+*`local`isthenameofthebranchwhoseconfigurationwe'reinstalling.+*+*`origin`isthenameoftheremoteowningtheupstreambranches.NULLmeans+*theupstreambranchesarelocaltothisrepo.+*+*`remotes`isalistofrefsthatareupstreamoflocal+*/+staticintinstall_branch_config_multiple_remotes(intflag,constchar*local,+constchar*origin,structstring_list*remotes){constchar*shortname=NULL;structstrbufkey=STRBUF_INIT;+structstring_list_item*item;intrebasing=should_setup_rebase(origin);-if(skip_prefix(remote,"refs/heads/",&shortname)-&&!strcmp(local,shortname)-&&!origin){-warning(_("Not setting branch %s as its own upstream."),-local);-return0;-}+if(!remotes->nr)+BUG("must provide at least one remote for branch config");+if(rebasing&&remotes->nr>1)+die(_("cannot inherit upstream tracking configuration of "+"multiple refs when rebasing is requested"));++/*+*Ifthenewbranchistryingtotrackitself,somethinghasgone+*wrong.Warntheuseranddon'tproceedanyfurther.+*/+if(!origin)+for_each_string_list_item(item,remotes)+if(skip_prefix(item->string,"refs/heads/",&shortname)+&&!strcmp(local,shortname)){+warning(_("not setting branch '%s' as its own upstream."),+local);+return0;+}strbuf_addf(&key,"branch.%s.remote",local);if(git_config_set_gently(key.buf,origin?origin:".")<0)
@@ -87,29 +117,40 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);if(flag&BRANCH_CONFIG_VERBOSE){-if(shortname){-if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing."):-_("Branch '%s' set up to track remote branch '%s' from '%s'."),-local,shortname,origin);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local branch '%s' by rebasing."):-_("Branch '%s' set up to track local branch '%s'."),-local,shortname);+structstrbuftmp_ref_name=STRBUF_INIT;+structstring_listfriendly_ref_names=STRING_LIST_INIT_DUP;++for_each_string_list_item(item,remotes){+shortname=item->string;+skip_prefix(shortname,"refs/heads/",&shortname);+if(origin){+strbuf_addf(&tmp_ref_name,"%s/%s",+origin,shortname);+string_list_append_nodup(+&friendly_ref_names,+strbuf_detach(&tmp_ref_name,NULL));+}else{+string_list_append(+&friendly_ref_names,shortname);+}+}++if(remotes->nr==1){+/*+*Rebasingisonlyallowedinthecaseofasingle+*upstreambranch.+*/+printf_ln(rebasing?+_("branch '%s' set up to track '%s' by rebasing."):+_("branch '%s' set up to track '%s'."),+local,friendly_ref_names.items[0].string);}else{-if(origin)-printf_ln(rebasing?-_("Branch '%s' set up to track remote ref '%s' by rebasing."):-_("Branch '%s' set up to track remote ref '%s'."),-local,remote);-else-printf_ln(rebasing?-_("Branch '%s' set up to track local ref '%s' by rebasing."):-_("Branch '%s' set up to track local ref '%s'."),-local,remote);+printf_ln(_("branch '%s' set up to track:"),local);+for_each_string_list_item(item,&friendly_ref_names)+printf_ln(" %s",item->string);}++string_list_clear(&friendly_ref_names,0);}return0;
@@ -118,14 +159,36 @@ int install_branch_config(int flag, const char *local, const char *origin, conststrbuf_release(&key);error(_("Unable to write upstream branch configuration"));-advise(_(tracking_advice),-origin?origin:"",-origin?"/":"",-shortname?shortname:remote);+advise(_("\nAfter fixing the error cause you may try to fix up\n"+"the remote tracking information by invoking:"));+if(remotes->nr==1)+advise(" git branch --set-upstream-to=%s%s%s",+origin?origin:"",+origin?"/":"",+remotes->items[0].string);+else{+advise(" git config --add branch.\"%s\".remote %s",+local,origin?origin:".");+for_each_string_list_item(item,remotes)+advise(" git config --add branch.\"%s\".merge %s",+local,item->string);+}return-1;}+intinstall_branch_config(intflag,constchar*local,constchar*origin,+constchar*remote)+{+intret;+structstring_listremotes=STRING_LIST_INIT_DUP;++string_list_append(&remotes,remote);+ret=install_branch_config_multiple_remotes(flag,local,origin,&remotes);+string_list_clear(&remotes,0);+returnret;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -950,15 +950,15 @@ test_expect_success 'disabled option --set-upstream fails' 'test_must_failgitbranch--set-upstreamorigin/main'-test_expect_success'--set-upstream-to notices an error to set branch as own upstream''+test_expect_success'--set-upstream-to notices an error to set branch as own upstream'"gitbranch--set-upstream-torefs/heads/my13my132>actual&&cat>expect<<-\EOF&&-warning:Notsettingbranchmy13asitsownupstream.+warning:notsettingbranch'my13'asitsownupstream.EOFtest_expect_code1gitconfigbranch.my13.remote&&test_expect_code1gitconfigbranch.my13.merge&&test_cmpexpectactual-'+"# Keep this test last, as it changes the current branch cat>expect<<EOF
It can be helpful when creating a new branch to use the existing
tracking configuration from the branch point. However, there is
currently not a method to automatically do so.
Teach git-{branch,checkout,switch} an "inherit" argument to the
"--track" option. When this is set, creating a new branch will cause the
tracking configuration to default to the configuration of the branch
point, if set.
For example, if branch "main" tracks "origin/main", and we run
`git checkout --track=inherit -b feature main`, then branch "feature"
will track "origin/main". Thus, `git status` will show us how far
ahead/behind we are from origin, and `git pull` will pull from origin.
This is particularly useful when creating branches across many
submodules, such as with `git submodule foreach ...` (or if running with
a patch such as [1], which we use at $job), as it avoids having to
manually set tracking info for each submodule.
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
Finally, teach branch.autoSetupMerge a new "inherit" option. When this
is set, "--track=inherit" becomes the default behavior.
[1]: https://lore.kernel.org/git/20180927221603.148025-1-sbeller@google.com/
Signed-off-by: Josh Steadmon <redacted>
---
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 +++++++++++-----
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 49 ++++++++++++++++++++++++++++-----
branch.h | 3 +-
builtin/branch.c | 6 ++--
builtin/checkout.c | 6 ++--
config.c | 3 ++
parse-options-cb.c | 16 +++++++++++
parse-options.h | 2 ++
t/t2017-checkout-orphan.sh | 11 +++++++-
t/t2027-checkout-track.sh | 23 ++++++++++++++++
t/t2060-switch.sh | 28 +++++++++++++++++++
t/t3200-branch.sh | 33 ++++++++++++++++++++++
t/t7201-co.sh | 17 ++++++++++++
16 files changed, 205 insertions(+), 23 deletions(-)
@@ -7,7 +7,8 @@ branch.autoSetupMerge:: automatic setup is done; `true` -- automatic setup is done when the starting point is a remote-tracking branch; `always` -- automatic setup is done when the starting point is either a- local branch or remote-tracking+ local branch or remote-tracking branch; `inherit` -- if the starting point+ has a tracking configuration, it is copied to the new branch. This option defaults to true. branch.autoSetupRebase::
@@ -205,24 +205,34 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track::+--track [inherit|direct]:: When creating a new branch, set up `branch.<name>.remote` and- `branch.<name>.merge` configuration entries to mark the- start-point branch as "upstream" from the new branch. This+ `branch.<name>.merge` configuration entries to set "upstream" tracking+ configuration for the new branch. This configuration will tell git to show the relationship between the two branches in `git status` and `git branch -v`. Furthermore, it directs `git pull` without arguments to pull from the upstream when the new branch is checked out. +-This behavior is the default when the start point is a remote-tracking branch.+The exact upstream branch is chosen depending on the optional argument:+`--track` or `--track direct` means to use the start-point branch itself as the+upstream; `--track inherit` means to copy the upstream configuration of the+start-point branch.+++`--track direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the-start-point is either a local or remote-tracking branch.+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- branch.autoSetupMerge configuration variable is true.+ branch.autoSetupMerge configuration variable is set. --set-upstream:: As this option had confusing syntax, it is no longer supported.
@@ -155,7 +155,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -152,7 +152,7 @@ should result in deletion of the path). attached to a terminal, regardless of `--quiet`. -t::---track::+--track [direct|inherit]:: When creating a new branch, set up "upstream" configuration. `-c` is implied. See `--track` in linkgit:git-branch[1] for details.
@@ -189,6 +189,34 @@ int install_branch_config(int flag, const char *local, const char *origin,returnret;}+staticintinherit_tracking(structtracking*tracking,constchar*orig_ref)+{+constchar*bare_ref;+structbranch*branch;+inti;++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]);+return0;+}+/**Thisiscalledwhennew_refisbranchedoffoforig_ref,andtries*toinferthesettingsforbranch.<new_ref>.{remote,merge}fromthe
@@ -632,8 +632,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)OPT__VERBOSE(&filter.verbose,N_("show hash and subject, give twice for upstream branch")),OPT__QUIET(&quiet,N_("suppress informational messages")),-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 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"),BRANCH_TRACK_OVERRIDE,PARSE_OPT_HIDDEN),OPT_STRING('u',"set-upstream-to",&new_upstream,N_("upstream"),N_("change the upstream info")),
@@ -1532,8 +1532,10 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_SET_INT('t',"track",&opts->track,N_("set upstream info for new branch"),-BRANCH_TRACK_EXPLICIT),+OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",+N_("set up tracking mode (see git-pull(1))"),+PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),OPT_STRING(0,"orphan",&opts->new_orphan_branch,N_("new-branch"),N_("new unparented branch")),
@@ -24,4 +24,27 @@ test_expect_success 'checkout --track -b rejects an extra path argument' 'test_i18ngrep"cannot be used with updating paths"err'+test_expect_success'checkout --track -b overrides autoSetupMerge=inherit''+# Set up tracking config on main+test_configbranch.main.remoteorigin&&+test_configbranch.main.mergerefs/heads/some-branch&&+test_configbranch.autoSetupMergeinherit&&+# With --track=inherit, we copy the tracking config from main+gitcheckout--track=inherit-bb1main&&+test_cmp_configoriginbranch.b1.remote&&+test_cmp_configrefs/heads/some-branchbranch.b1.merge&&+# With branch.autoSetupMerge=inherit, we do the same+gitcheckout-bb2main&&+test_cmp_configoriginbranch.b2.remote&&+test_cmp_configrefs/heads/some-branchbranch.b2.merge&&+# But --track overrides this+gitcheckout--track-bb3main&&+test_cmp_config.branch.b3.remote&&+test_cmp_configrefs/heads/mainbranch.b3.merge&&+# And --track=direct does as well+gitcheckout--track=direct-bb4main&&+test_cmp_config.branch.b4.remote&&+test_cmp_configrefs/heads/mainbranch.b4.merge+'+ test_done
@@ -107,4 +107,32 @@ test_expect_success 'not switching when something is in progress' 'test_must_failgitswitch-d@^'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+# default config does not copy tracking info+gitswitch-cfoo-no-inheritfoo&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with --track=inherit, we copy tracking info from foo+gitswitch--track=inherit-cfoo2foo&&+test_cmp_configoriginbranch.foo2.remote&&+test_cmp_configrefs/heads/foobranch.foo2.merge&&+# with autoSetupMerge=inherit, we do the same+test_configbranch.autoSetupMergeinherit&&+gitswitch-cfoo3foo&&+test_cmp_configoriginbranch.foo3.remote&&+test_cmp_configrefs/heads/foobranch.foo3.merge&&+# with --track, we override autoSetupMerge+gitswitch--track-cfoo4foo&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/foobranch.foo4.merge&&+# and --track=direct does as well+gitswitch--track=direct-cfoo5foo&&+test_cmp_config.branch.foo5.remote&&+test_cmp_configrefs/heads/foobranch.foo5.merge&&+# no tracking info to inherit from main+gitswitch-cmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
@@ -1409,4 +1409,37 @@ test_expect_success 'invalid sort parameter in configuration' ')'+test_expect_success'tracking info copied with --track=inherit''+gitbranch--track=inheritfoo2my1&&+test_cmp_configlocalbranch.foo2.remote&&+test_cmp_configrefs/heads/mainbranch.foo2.merge+'++test_expect_success'tracking info copied with autoSetupMerge=inherit''+test_unconfigbranch.autoSetupMerge&&+# default config does not copy tracking info+gitbranchfoo-no-inheritmy1&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from my1+test_configbranch.autoSetupMergeinherit&&+gitbranchfoo3my1&&+test_cmp_configlocalbranch.foo3.remote&&+test_cmp_configrefs/heads/mainbranch.foo3.merge&&+# no tracking info to inherit from main+gitbranchmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'++test_expect_success'--track overrides branch.autoSetupMerge''+test_configbranch.autoSetupMergeinherit&&+gitbranch--track=directfoo4my1&&+test_cmp_config.branch.foo4.remote&&+test_cmp_configrefs/heads/my1branch.foo4.merge&&+gitbranch--no-trackfoo5my1&&+test_cmp_config""--default""branch.foo5.remote&&+test_cmp_config""--default""branch.foo5.merge+'+ test_done
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
Although we only documented that branch.*.autosetupmerge would accept
"always" as a value, the actual implementation would accept any
combination of upper- or lower-case. Fix this to be consistent with
documentation and with other values of this config variable.
Signed-off-by: Josh Steadmon <redacted>
---
config.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Changes since V7:
* Further simplify verbose output by adding an "<origin>/" prefix for
remote-tracking upstream refs.
* Add a comment explaining the self-tracking check & early exit.
Changes since V6:
* Strip the refs/heads/ prefix in the verbose output when we have only a
single upstream branch.
* Improve the fatal error message to note that rebasing is only
incompatible with multiple upstream refs.
* Also note that `branch.<name>.remote` should be set in the manual
recovery advice.
* Simplify the logic in setup_tracking() when no tracking sources match.
* Make the difference in test cases in t2027 more obvious.
Changes since V5:
* Greatly simplified BRANCH_CONFIG_VERBOSE output to not require nearly
so many conditionals.
* Note that rebasing is not compatible with inheriting multiple upstream
branches.
* Moved the change to case-sensitivity for branch.autosetupmerge to its
own commit.
* Improve advice on failed tracking setup when multiple branches are
involved.
* Make better use of string_list API.
* Make better use of config API.
* More straight-forward use of the `struct tracking` API.
* Numerous style fixes.
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 (3):
branch: accept multiple upstream branches for tracking
branch: add flags and config to inherit tracking
config: require lowercase for branch.*.autosetupmerge
Documentation/config/branch.txt | 3 +-
Documentation/git-branch.txt | 24 ++--
Documentation/git-checkout.txt | 2 +-
Documentation/git-switch.txt | 2 +-
branch.c | 192 ++++++++++++++++++++++++--------
branch.h | 3 +-
builtin/branch.c | 6 +-
builtin/checkout.c | 6 +-
config.c | 5 +-
parse-options-cb.c | 16 +++
parse-options.h | 2 +
t/t2017-checkout-orphan.sh | 11 +-
t/t2027-checkout-track.sh | 23 ++++
t/t2060-switch.sh | 28 +++++
t/t3200-branch.sh | 39 ++++++-
t/t7201-co.sh | 17 +++
16 files changed, 312 insertions(+), 67 deletions(-)
Range-diff against v7:
1: 9152367ba9 ! 1: a5265e1c7f branch: accept multiple upstream branches for tracking
@@ branch.c: static int should_setup_rebase(const char *origin)
+ die(_("cannot inherit upstream tracking configuration of "
+ "multiple refs when rebasing is requested"));
+
++ /*
++ * If the new branch is trying to track itself, something has gone
++ * wrong. Warn the user and don't proceed any further.
++ */
+ if (!origin)
+ for_each_string_list_item(item, remotes)
+ if (skip_prefix(item->string, "refs/heads/", &shortname)
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
if (flag & BRANCH_CONFIG_VERBOSE) {
- if (shortname) {
-+ const char *name;
-+ struct strbuf ref_string = STRBUF_INIT;
-+
-+ for_each_string_list_item(item, remotes) {
-+ name = item->string;
-+ skip_prefix(name, "refs/heads/", &name);
-+ strbuf_addf(&ref_string, " %s\n", name);
-+ }
-+
-+ if (remotes->nr == 1) {
-+ struct strbuf refname = STRBUF_INIT;
-+
- if (origin)
+- if (origin)
- printf_ln(rebasing ?
- _("Branch '%s' set up to track remote branch '%s' from '%s' by rebasing.") :
- _("Branch '%s' set up to track remote branch '%s' from '%s'."),
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
- _("Branch '%s' set up to track local branch '%s' by rebasing.") :
- _("Branch '%s' set up to track local branch '%s'."),
- local, shortname);
-+ strbuf_addf(&refname, "%s/", origin);
-+ skip_prefix(remotes->items[0].string, "refs/heads/", &name);
-+ strbuf_addstr(&refname, name);
++ struct strbuf tmp_ref_name = STRBUF_INIT;
++ struct string_list friendly_ref_names = STRING_LIST_INIT_DUP;
+
++ for_each_string_list_item(item, remotes) {
++ shortname = item->string;
++ skip_prefix(shortname, "refs/heads/", &shortname);
++ if (origin) {
++ strbuf_addf(&tmp_ref_name, "%s/%s",
++ origin, shortname);
++ string_list_append_nodup(
++ &friendly_ref_names,
++ strbuf_detach(&tmp_ref_name, NULL));
++ } else {
++ string_list_append(
++ &friendly_ref_names, shortname);
++ }
++ }
++
++ if (remotes->nr == 1) {
+ /*
+ * Rebasing is only allowed in the case of a single
+ * upstream branch.
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
+ printf_ln(rebasing ?
+ _("branch '%s' set up to track '%s' by rebasing.") :
+ _("branch '%s' set up to track '%s'."),
-+ local, refname.buf);
-+
-+ strbuf_release(&refname);
-+ } else if (origin) {
-+ printf_ln(_("branch '%s' set up to track from '%s':"),
-+ local, origin);
-+ printf("%s", ref_string.buf);
++ local, friendly_ref_names.items[0].string);
} else {
- if (origin)
- printf_ln(rebasing ?
@@ branch.c: int install_branch_config(int flag, const char *local, const char *ori
- _("Branch '%s' set up to track local ref '%s'."),
- local, remote);
+ printf_ln(_("branch '%s' set up to track:"), local);
-+ printf("%s", ref_string.buf);
++ for_each_string_list_item(item, &friendly_ref_names)
++ printf_ln(" %s", item->string);
}
+
-+ strbuf_release(&ref_string);
++ string_list_clear(&friendly_ref_names, 0);
}
return 0;
2: afeb84539e = 2: dcba40e2c4 branch: add flags and config to inherit tracking
3: a818a6561b = 3: ae7d27b4be config: require lowercase for branch.*.autosetupmerge
base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee
--
2.34.1.307.g9b7440fafd-goog
These changes look good to me. I'll leave some suggestions on the
patches, but those are optional.
I still think it would be nice to get more thoughts on the help message
changes, but that's Junio's call to make :)
Reviewed-by: Glen Choo <redacted>
@@ -657,4 +657,21 @@ test_expect_success 'custom merge driver with checkout -m' 'test_cmpexpectarm'+test_expect_success'tracking info copied with autoSetupMerge=inherit''+gitreset--hardmain&&+# default config does not copy tracking info+gitcheckout-bfoo-no-inheritkoala/bear&&+test_cmp_config""--default""branch.foo-no-inherit.remote&&+test_cmp_config""--default""branch.foo-no-inherit.merge&&+# with autoSetupMerge=inherit, we copy tracking info from koala/bear+test_configbranch.autoSetupMergeinherit&&+gitcheckout-bfookoala/bear&&+test_cmp_configoriginbranch.foo.remote&&+test_cmp_configrefs/heads/koala/bearbranch.foo.merge&&+# no tracking info to inherit from main+gitcheckout-bmain2main&&+test_cmp_config""--default""branch.main2.remote&&+test_cmp_config""--default""branch.main2.merge+'+ test_done
--
2.34.1.307.g9b7440fafd-goog
As a suggestion, I don't think we have tests for multiple branch.*.merge
entries and it would be nice to have some (though I don't think it's
absolutely essential).
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
[...]
-'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
+'git branch' [--track [direct|inherit] | --no-track] [-f] <branchname> [<start-point>]
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"),
- BRANCH_TRACK_EXPLICIT),
+ OPT_CALLBACK_F('t', "track", &opts->track, "direct|inherit",
... but these are not. I.e. we'll emit:
-t, --track[=direct|inherit]
set branch tracking configuration
I.e. implying that the valid uses are --track, --track=direct, and
--trackinherit.
It looks like the problem is (ab)use of PARSE_OPT_OPTARG, i.e. it was
never meant for an enumeration of possible values, but for something
like N_("mode"). It could be made to support that, but it would require
some light patching of the releant bits of parse-options.c.
The PARSE_OPT_LITERAL_ARGHELP should also be dropped if it's fixed to
use a string like "mode".
As Ævar pointed out in [1], the use of PARSE_OPT_LITERAL_ARGHELP with a
list of allowed parameters is not recommended. Both git-branch and
git-checkout were changed in d311566 (branch: add flags and config to
inherit tracking, 2021-12-20) to use this discouraged combination for
their --track flags.
Fix this by removing PARSE_OPT_LITERAL_ARGHELP, and changing the arghelp
to simply be "mode". Users may discover allowed values in the manual
pages.
[1]: https://lore.kernel.org/git/220111.86a6g3yqf9.gmgdl@evledraar.gmail.com/
Signed-off-by: Josh Steadmon <redacted>
---
builtin/branch.c | 4 ++--
builtin/checkout.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
@@ -1549,9 +1549,9 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",-N_("set up tracking mode (see git-pull(1))"),-PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+OPT_CALLBACK_F('t',"track",&opts->track,N_("mode"),+N_("set branch tracking configuration"),+PARSE_OPT_OPTARG,parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),
From: René Scharfe <hidden> Date: 2022-01-19 10:20:17
Am 11.01.22 um 02:57 schrieb Ævar Arnfjörð Bjarmason:
On Mon, Dec 20 2021, Josh Steadmon wrote:
quoted
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
[...]
-'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
+'git branch' [--track [direct|inherit] | --no-track] [-f] <branchname> [<start-point>]
The usage info here is correct...
Actually it isn't, because optional arguments need the equal sign. I.e.
this works as expected:
git branch --track=direct branch
But this here interprets "direct" as a branch name (and branch as a
start point):
git branch --track direct branch
The usage string could start with:
'git branch' [--track | --track=direct | --track=inherit | --no-track]
... or the less repetitive:
'git branch' [--track[=(direct|inherit)] | --no-track]
Options with required arguments accept either whitespace or an equal
sign between option name and arg. With PARSE_OPT_OPTARG we cannot
accept whitespace because we cannot decide whether the next thing after
the option name is an argument or the next parameter.
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"),
- BRANCH_TRACK_EXPLICIT),
+ OPT_CALLBACK_F('t', "track", &opts->track, "direct|inherit",
... but these are not. I.e. we'll emit:
-t, --track[=direct|inherit]
set branch tracking configuration
I.e. implying that the valid uses are --track, --track=direct, and
--trackinherit.
Well spotted. It should be specified as "(direct|inherit)" (i.e. with
parens).
It looks like the problem is (ab)use of PARSE_OPT_OPTARG, i.e. it was
never meant for an enumeration of possible values, but for something
like N_("mode"). It could be made to support that, but it would require
some light patching of the releant bits of parse-options.c.
Could you please elaborate that point? AFAIU PARSE_OPT_OPTARG just
requires arguments to be attached with equal signs and there is no
limitation regarding the number of possible argument values.
The PARSE_OPT_LITERAL_ARGHELP should also be dropped if it's fixed to
use a string like "mode".
That's true. And it's also enabled automatically if the argument help
string contains any of the following characters: ()<>[]|. So basically
it's never needed explicitly..
Am 11.01.22 um 02:57 schrieb Ævar Arnfjörð Bjarmason:
quoted
On Mon, Dec 20 2021, Josh Steadmon wrote:
quoted
Since we've added an argument to "--track", also add "--track=direct" as
another way to explicitly get the original "--track" behavior ("--track"
without an argument still works as well).
[...]
-'git branch' [--track | --no-track] [-f] <branchname> [<start-point>]
+'git branch' [--track [direct|inherit] | --no-track] [-f] <branchname> [<start-point>]
The usage info here is correct...
Actually it isn't, because optional arguments need the equal sign. I.e.
this works as expected:
git branch --track=direct branch
But this here interprets "direct" as a branch name (and branch as a
start point):
git branch --track direct branch
The usage string could start with:
'git branch' [--track | --track=direct | --track=inherit | --no-track]
... or the less repetitive:
'git branch' [--track[=(direct|inherit)] | --no-track]
Options with required arguments accept either whitespace or an equal
sign between option name and arg. With PARSE_OPT_OPTARG we cannot
accept whitespace because we cannot decide whether the next thing after
the option name is an argument or the next parameter.
Well spotted. Your downthread patch LGTM (with the small nit I noted
that having an optbug() check for this would be even better).
struct option options[] = {
OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")),
- OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"),
- BRANCH_TRACK_EXPLICIT),
+ OPT_CALLBACK_F('t', "track", &opts->track, "direct|inherit",
... but these are not. I.e. we'll emit:
-t, --track[=direct|inherit]
set branch tracking configuration
I.e. implying that the valid uses are --track, --track=direct, and
--trackinherit.
Well spotted. It should be specified as "(direct|inherit)" (i.e. with
parens).
*nod*
quoted
It looks like the problem is (ab)use of PARSE_OPT_OPTARG, i.e. it was
never meant for an enumeration of possible values, but for something
like N_("mode"). It could be made to support that, but it would require
some light patching of the releant bits of parse-options.c.
Could you please elaborate that point? AFAIU PARSE_OPT_OPTARG just
requires arguments to be attached with equal signs and there is no
limitation regarding the number of possible argument values.
I'd skimmed the code & -h generation, but see on a second reading that I
was just wrong about this.
I.e. I thought it would always misformat alternate args, but as your
downthread patch shows where we'll now for "git am -h" emit e.g.:
--show-current-patch[=(diff|raw)]
The output is now correct (and was before, we were just giving the flag
rudendantly).
quoted
The PARSE_OPT_LITERAL_ARGHELP should also be dropped if it's fixed to
use a string like "mode".
That's true. And it's also enabled automatically if the argument help
string contains any of the following characters: ()<>[]|. So basically
it's never needed explicitly..
As Ævar pointed out in [1], the use of PARSE_OPT_LITERAL_ARGHELP with a
list of allowed parameters is not recommended. Both git-branch and
git-checkout were changed in d311566 (branch: add flags and config to
inherit tracking, 2021-12-20) to use this discouraged combination for
their --track flags.
Fix this by removing PARSE_OPT_LITERAL_ARGHELP, and changing the arghelp
to simply be "mode". Users may discover allowed values in the manual
pages.
[1]: https://lore.kernel.org/git/220111.86a6g3yqf9.gmgdl@evledraar.gmail.com/
Signed-off-by: Josh Steadmon <redacted>
---
builtin/branch.c | 4 ++--
builtin/checkout.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
@@ -1549,9 +1549,9 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_CALLBACK_F('t',"track",&opts->track,"direct|inherit",-N_("set up tracking mode (see git-pull(1))"),-PARSE_OPT_OPTARG|PARSE_OPT_LITERAL_ARGHELP,+OPT_CALLBACK_F('t',"track",&opts->track,N_("mode"),+N_("set branch tracking configuration"),+PARSE_OPT_OPTARG,parse_opt_tracking_mode),OPT__FORCE(&opts->force,N_("force checkout (throw away local modifications)"),PARSE_OPT_NOCOMPLETE),
Additional comment: I think this change is correct, as noted in my
just-sent
https://lore.kernel.org/git/220120.864k5ymx55.gmgdl@evledraar.gmail.com/;
it would be nice but not necessary to follow-up with an optbug() test as
noted in
https://lore.kernel.org/git/220119.867davokff.gmgdl@evledraar.gmail.com/
though.
But to not merely repeat myself, I also saw that we're emitting the
wrong output from usage_argh() in cases of !PARSE_OPT_NOARG. I.e. we
need this fix too:
diff --git a/parse-options.c b/parse-options.c
index a8283037be9..2be1eabd84e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -915,8 +915,10 @@ static int usage_argh(const struct option *opts, FILE *outfile)
s = literal ? "[=%s]" : "[=<%s>]";
else
s = literal ? "[%s]" : "[<%s>]";
- else
+ else if (opts->flags & PARSE_OPT_NOARG)
s = literal ? " %s" : " <%s>";
+ else
+ s = literal ? "[=]%s" : "[=]<%s>";
return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
}
With that we'll now emit:
$ ./git add -h 2>&1|grep chmod
--chmod[=](+|-)x override the executable bit of the listed files
Which is correct, as we accept both of:
git add --chmod +x
git add --chmod=+x
But not:
$ git add --chmod
error: parse-options.c:58: option `chmod' requires a value
But the usage output stated that the "=" was mandatory before.
From: René Scharfe <hidden> Date: 2022-01-20 12:36:14
Document that the accepted variants of the --track option are --track,
--track=direct, and --track=inherit. The equal sign in the latter two
cannot be replaced with whitespace; in general optional arguments need
to be attached firmly to their option.
Put "direct" consistently before "inherit", if only for the reasons
that the former is the default, explained first in the documentation,
and comes before the latter alphabetically.
Mention both modes in the short help so that readers don't have to look
them up in the full documentation. They are literal strings and thus
untranslatable. PARSE_OPT_LITERAL_ARGHELP is inferred due to the pipe
and parenthesis characters, so we don't have to provide that flag
explicitly.
Mention that -t has the same effect as --track and --track=direct.
There is no way to specify inherit mode using the short option, because
short options generally don't accept optional arguments.
Signed-off-by: René Scharfe <redacted>
---
Documentation/git-branch.txt | 12 ++++++------
Documentation/git-checkout.txt | 2 +-
builtin/branch.c | 2 +-
builtin/checkout.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
@@ -206,7 +206,7 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track [inherit|direct]::+--track[=(direct|inherit)]:: When creating a new branch, set up `branch.<name>.remote` and `branch.<name>.merge` configuration entries to set "upstream" tracking configuration for the new branch. This
@@ -216,11 +216,11 @@ This option is only applicable in non-verbose mode. upstream when the new branch is checked out. + The exact upstream branch is chosen depending on the optional argument:-`--track` or `--track direct` means to use the start-point branch itself as the-upstream; `--track inherit` means to copy the upstream configuration of the-start-point branch.+`-t`, `--track`, or `--track=direct` means to use the start-point branch+itself as the upstream; `--track=inherit` means to copy the upstream+configuration of the start-point branch. +-`--track direct` is the default when the start point is a remote-tracking branch.+`--track=direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the
@@ -156,7 +156,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track [direct|inherit]::+--track[=(direct|inherit)]:: When creating a new branch, set up "upstream" configuration. See "--track" in linkgit:git-branch[1] for details. +
@@ -1549,7 +1549,7 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_CALLBACK_F('t',"track",&opts->track,N_("mode"),+OPT_CALLBACK_F('t',"track",&opts->track,"(direct|inherit)",N_("set branch tracking configuration"),PARSE_OPT_OPTARG,parse_opt_tracking_mode),--
Document that the accepted variants of the --track option are --track,
--track=direct, and --track=inherit. The equal sign in the latter two
cannot be replaced with whitespace; in general optional arguments need
to be attached firmly to their option.
Put "direct" consistently before "inherit", if only for the reasons
that the former is the default, explained first in the documentation,
and comes before the latter alphabetically.
Mention both modes in the short help so that readers don't have to look
them up in the full documentation. They are literal strings and thus
untranslatable. PARSE_OPT_LITERAL_ARGHELP is inferred due to the pipe
and parenthesis characters, so we don't have to provide that flag
explicitly.
Mention that -t has the same effect as --track and --track=direct.
There is no way to specify inherit mode using the short option, because
short options generally don't accept optional arguments.
Signed-off-by: René Scharfe <redacted>
---
Documentation/git-branch.txt | 12 ++++++------
Documentation/git-checkout.txt | 2 +-
builtin/branch.c | 2 +-
builtin/checkout.c | 2 +-
4 files changed, 9 insertions(+), 9 deletions(-)
@@ -206,7 +206,7 @@ This option is only applicable in non-verbose mode. Display the full sha1s in the output listing rather than abbreviating them. -t::---track [inherit|direct]::+--track[=(direct|inherit)]:: When creating a new branch, set up `branch.<name>.remote` and `branch.<name>.merge` configuration entries to set "upstream" tracking configuration for the new branch. This
@@ -216,11 +216,11 @@ This option is only applicable in non-verbose mode. upstream when the new branch is checked out. + The exact upstream branch is chosen depending on the optional argument:-`--track` or `--track direct` means to use the start-point branch itself as the-upstream; `--track inherit` means to copy the upstream configuration of the-start-point branch.+`-t`, `--track`, or `--track=direct` means to use the start-point branch+itself as the upstream; `--track=inherit` means to copy the upstream+configuration of the start-point branch. +-`--track direct` is the default when the start point is a remote-tracking branch.+`--track=direct` is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the
@@ -156,7 +156,7 @@ of it"). linkgit:git-branch[1] for details. -t::---track [direct|inherit]::+--track[=(direct|inherit)]::
These changes (and the below) all look good to me. Thanks for fixing
this.
When creating a new branch, set up "upstream" configuration. See
"--track" in linkgit:git-branch[1] for details.
As a side-note this "--track" reference is incorrect, and has been since
d3115660b4c (branch: add flags and config to inherit tracking,
2021-12-20), i.e. it should now mention "--track[=(direct|inherit)]".
But as we're not explicitly cross-linking anything here with the
relevant syntax I think leaving it as-is is fine, the user would also
find it with a substring search.
From: Junio C Hamano <hidden> Date: 2022-01-20 19:08:43
René Scharfe [off-list ref] writes:
Document that the accepted variants of the --track option are --track,
--track=direct, and --track=inherit. The equal sign in the latter two
cannot be replaced with whitespace; in general optional arguments need
to be attached firmly to their option.
Put "direct" consistently before "inherit", if only for the reasons
that the former is the default, explained first in the documentation,
and comes before the latter alphabetically.
;-) I see too many good reasons to modestly say "if only for" ;-)
@@ -216,11 +216,11 @@ This option is only applicable in non-verbose mode. upstream when the new branch is checked out. + The exact upstream branch is chosen depending on the optional argument:-`--track` or `--track direct` means to use the start-point branch itself as the-upstream; `--track inherit` means to copy the upstream configuration of the-start-point branch.+`-t`, `--track`, or `--track=direct` means to use the start-point branch+itself as the upstream; `--track=inherit` means to copy the upstream+configuration of the start-point branch.
When "-x" and "--long-x" both do the same thing, we list both in the
heading but omit "-x" from the text, but in this case I fully agree
with the updated text as "-t" and "--track[=...]" work a bit
differently and there is no way to say "we want inherit" with "-t".
-`--track direct` is the default when the start point is a remote-tracking branch.
+`--track=direct` is the default when the start point is a remote-tracking branch.
@@ -1549,7 +1549,7 @@ static struct option *add_common_switch_branch_options({structoptionoptions[]={OPT_BOOL('d',"detach",&opts->force_detach,N_("detach HEAD at named commit")),-OPT_CALLBACK_F('t',"track",&opts->track,N_("mode"),+OPT_CALLBACK_F('t',"track",&opts->track,"(direct|inherit)",N_("set branch tracking configuration"),PARSE_OPT_OPTARG,parse_opt_tracking_mode),