| Message ID | 7ad7507f183332cb2b5fdf2eb76fbbc9dd7199ef.1637085915.git.steadmon@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] branch: add flags and config to inherit tracking | expand |
Josh Steadmon <steadmon@google.com> writes: > 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... > @@ -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().
Josh Steadmon <steadmon@google.com> writes: > 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. Or we can extend "existing tracking setup code" to support multiple merge sources. How does the "git pull" machinery react to them, by the way? I think the original intention is to support pulling multiple branches from the (single) remote configured for the branch with a single invocation of "git pull", creating an octopus merge, but does it still work, or nobody uses such a crazy curiosity anymore and it was once broken and left in non-working state ever since? What I am dreaming here is if we can safely ignore all but one of them, taking the usual "last-one-wins" rule, after some transition period. > +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 `--track' expects \"direct\" or \"inherit\"")); According to recent discussion in another thread, error(_("option '--%s` expects '%s' or '%s'"), "track", "direct", "inherit"); would be more translater friendly, as these three words are not subject to translation? I am not sure if it is really worth it, though.
On Tue, Nov 16 2021, Josh Steadmon wrote: > 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. > + 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 > @@ -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; Style: Dangling braces, can just skip the braces here. > @@ -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. > + 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")), > diff --git a/builtin/checkout.c b/builtin/checkout.c > index b5d477919a..45dab414ea 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -1532,8 +1532,10 @@ 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")), > diff --git a/config.c b/config.c > index cb4a8058bf..4bd5a18faf 100644 > --- a/config.c > +++ b/config.c > @@ -1580,6 +1580,9 @@ static int git_default_branch_config(const char *var, const char *value) > if (value && !strcasecmp(value, "always")) { > git_branch_track = BRANCH_TRACK_ALWAYS; > return 0; > + } else if (value && !strcasecmp(value, "inherit")) { > + git_branch_track = BRANCH_TRACK_INHERIT; > + return 0; > } 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: > > > 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] > > @@ -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.18 14:29, Junio C Hamano wrote: > Josh Steadmon <steadmon@google.com> writes: > > > 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. > > Or we can extend "existing tracking setup code" to support multiple > merge sources. > > How does the "git pull" machinery react to them, by the way? I > think the original intention is to support pulling multiple branches > from the (single) remote configured for the branch with a single > invocation of "git pull", creating an octopus merge, but does it > still work, or nobody uses such a crazy curiosity anymore and it was > once broken and left in non-working state ever since? What I am > dreaming here is if we can safely ignore all but one of them, taking > the usual "last-one-wins" rule, after some transition period. It does still work and creates an octopus merge. Tested as follows: $ git clone https://github.com/gitster/git $ cd git $ git config pull.rebase false $ git checkout -b test-branch v2.30.0 $ cat >>.git/config <<EOF [branch "test-branch"] remote = origin merge = refs/heads/master merge = refs/heads/ab/ambiguous-object-name merge = refs/heads/js/scalar EOF $ git pull Fast-forwarding to: 4ef9e1ce4ac4ad79f99f5c5712146254b4cca530 Trying simple merge with 352f8e8fcba4726340c946200149e6285c514fc0 Trying simple merge with abe6bb3905392d5eb6b01fa6e54d7e784e0522aa Simple merge did not work, trying automatic merge. Auto-merging Makefile Auto-merging contrib/buildsystems/CMakeLists.txt Merge made by the 'octopus' strategy. [...] $ git cat-file commit HEAD tree 78bd62c62edfc6e51beb956e548b92b97210ddd4 parent 4ef9e1ce4ac4ad79f99f5c5712146254b4cca530 parent 352f8e8fcba4726340c946200149e6285c514fc0 parent abe6bb3905392d5eb6b01fa6e54d7e784e0522aa author Josh Steadmon <josh@steadmon.net> 1638309707 -0800 committer Josh Steadmon <josh@steadmon.net> 1638309707 -0800 Merge branches 'ab/ambiguous-object-name', 'js/scalar' and 'master' of https://github.com/gitster/git into test-branch > > +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 `--track' expects \"direct\" or \"inherit\"")); > > According to recent discussion in another thread, > > error(_("option '--%s` expects '%s' or '%s'"), > "track", "direct", "inherit"); > > would be more translater friendly, as these three words are not > subject to translation? I am not sure if it is really worth it, > though. I don't really feel strongly either way, so I switched to the more translatable version. I was originally following some other examples in the file, where it seemed that most strings that would require more than one field expansion were just hardcoded instead.
On Tue, Nov 30 2021, Josh Steadmon wrote: > On 2021.11.19 07:47, Ævar Arnfjörð Bjarmason wrote: >> >> On Tue, Nov 16 2021, Josh Steadmon wrote: >> >> > 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. >> > @@ -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.
diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt index cc5f3249fc..55f7522e12 100644 --- a/Documentation/config/branch.txt +++ b/Documentation/config/branch.txt @@ -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:: diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 94dc9a54f2..e484a3803a 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -16,7 +16,7 @@ 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> @@ -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. diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index b1a6fe4499..a48e1ab62f 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -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. + diff --git a/Documentation/git-switch.txt b/Documentation/git-switch.txt index 5c438cd505..96dc036ea5 100644 --- a/Documentation/git-switch.txt +++ b/Documentation/git-switch.txt @@ -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. diff --git a/branch.c b/branch.c index 7a88a4861e..f018e440a6 100644 --- a/branch.c +++ b/branch.c @@ -126,6 +126,33 @@ int install_branch_config(int flag, const char *local, const char *origin, const return -1; } +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); + 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 @@ -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) diff --git a/branch.h b/branch.h index df0be61506..6484bda8a2 100644 --- a/branch.h +++ b/branch.h @@ -10,7 +10,8 @@ enum branch_track { BRANCH_TRACK_REMOTE, BRANCH_TRACK_ALWAYS, BRANCH_TRACK_EXPLICIT, - BRANCH_TRACK_OVERRIDE + BRANCH_TRACK_OVERRIDE, + BRANCH_TRACK_INHERIT }; extern enum branch_track git_branch_track; diff --git a/builtin/branch.c b/builtin/branch.c index b23b1d1752..15f70fe3fa 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -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")), diff --git a/builtin/checkout.c b/builtin/checkout.c index b5d477919a..45dab414ea 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1532,8 +1532,10 @@ 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")), diff --git a/config.c b/config.c index cb4a8058bf..4bd5a18faf 100644 --- a/config.c +++ b/config.c @@ -1580,6 +1580,9 @@ static int git_default_branch_config(const char *var, const char *value) if (value && !strcasecmp(value, "always")) { git_branch_track = BRANCH_TRACK_ALWAYS; return 0; + } else if (value && !strcasecmp(value, "inherit")) { + git_branch_track = BRANCH_TRACK_INHERIT; + return 0; } git_branch_track = git_config_bool(var, value); return 0; diff --git a/parse-options-cb.c b/parse-options-cb.c index 3c811e1e4a..91261d8c41 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -1,5 +1,6 @@ #include "git-compat-util.h" #include "parse-options.h" +#include "branch.h" #include "cache.h" #include "commit.h" #include "color.h" @@ -293,3 +294,16 @@ 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 `--track' expects \"direct\" or \"inherit\"")); + + return 0; +} diff --git a/parse-options.h b/parse-options.h index a845a9d952..f35dbfdd5a 100644 --- a/parse-options.h +++ b/parse-options.h @@ -303,6 +303,8 @@ 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)) diff --git a/t/t2017-checkout-orphan.sh b/t/t2017-checkout-orphan.sh index 88d6992a5e..31fb64c5be 100755 --- a/t/t2017-checkout-orphan.sh +++ b/t/t2017-checkout-orphan.sh @@ -64,6 +64,13 @@ 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^ ' diff --git a/t/t2027-checkout-track.sh b/t/t2027-checkout-track.sh index 4453741b96..e87d77f629 100755 --- a/t/t2027-checkout-track.sh +++ b/t/t2027-checkout-track.sh @@ -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 + 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 +' + test_done diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh index 9bc6a3aa5c..76e9d12e36 100755 --- a/t/t2060-switch.sh +++ b/t/t2060-switch.sh @@ -107,4 +107,32 @@ 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 --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)" && + test -z "$(git config branch.main2.merge)" +' + test_done diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index cc4b10236e..bc547b08e1 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -1409,4 +1409,37 @@ 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 + 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 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 diff --git a/t/t7201-co.sh b/t/t7201-co.sh index 7f6e23a4bb..ae9f8d02c2 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -657,4 +657,21 @@ 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
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 <steadmon@google.com> --- 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(-) base-commit: 6c40894d2466d4e7fddc047a05116aa9d14712ee