From 75388bf5b47678c95f24b58007d2b37d744bf0f7 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 29 Mar 2022 20:01:16 +0000 Subject: branch: support more tracking modes when recursing "git branch --recurse-submodules" does not propagate "--track=inherit" or "--no-track" to submodules, which causes submodule branches to use the wrong tracking mode [1]. To fix this, pass the correct options to the "submodule--helper create-branch" child process and test for it. While we are refactoring the same code, replace "--track" with the synonymous, but more consistent-looking "--track=direct" option (introduced at the same time as "--track=inherit", d3115660b4 (branch: add flags and config to inherit tracking, 2021-12-20)). [1] This bug is partially a timing issue: "branch --recurse-submodules" was introduced around the same time as "--track=inherit", and even though I rebased "branch --recurse-submodules" on top of that, I had neglected to support the new tracking mode. Omitting "--no-track" was just a plain old mistake, though. Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- branch.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 47251669e1..608db0fc91 100644 --- a/branch.c +++ b/branch.c @@ -233,6 +233,9 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, struct string_list tracking_srcs = STRING_LIST_INIT_DUP; int config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE; + if (!track) + BUG("asked to set up tracking, but tracking is disallowed"); + memset(&tracking, 0, sizeof(tracking)); tracking.spec.dst = (char *)orig_ref; tracking.srcs = &tracking_srcs; @@ -529,8 +532,27 @@ static int submodule_create_branch(struct repository *r, strvec_push(&child.args, "--quiet"); if (reflog) strvec_push(&child.args, "--create-reflog"); - if (track == BRANCH_TRACK_ALWAYS || track == BRANCH_TRACK_EXPLICIT) - strvec_push(&child.args, "--track"); + + switch (track) { + case BRANCH_TRACK_NEVER: + strvec_push(&child.args, "--no-track"); + break; + case BRANCH_TRACK_ALWAYS: + case BRANCH_TRACK_EXPLICIT: + strvec_push(&child.args, "--track=direct"); + break; + case BRANCH_TRACK_OVERRIDE: + BUG("BRANCH_TRACK_OVERRIDE cannot be used when creating a branch."); + break; + case BRANCH_TRACK_INHERIT: + strvec_push(&child.args, "--track=inherit"); + break; + case BRANCH_TRACK_UNSPECIFIED: + /* Default for "git checkout". No need to pass --track. */ + case BRANCH_TRACK_REMOTE: + /* Default for "git branch". No need to pass --track. */ + break; + } strvec_pushl(&child.args, name, start_oid, tracking_name, NULL); @@ -609,7 +631,8 @@ void create_branches_recursively(struct repository *r, const char *name, * tedious to determine whether or not tracking was set up in the * superproject. */ - setup_tracking(name, tracking_name, track, quiet); + if (track) + setup_tracking(name, tracking_name, track, quiet); for (i = 0; i < submodule_entry_list.entry_nr; i++) { if (submodule_create_branch( -- cgit v1.2.3 From cfbda6ba6b33e903df58f96fdb2ee9314097ff2f Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 29 Mar 2022 20:01:17 +0000 Subject: branch: give submodule updating advice before exit Fix a bug where "hint:" was printed _before_ "fatal:" (instead of the other way around). Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- branch.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 608db0fc91..50459c432c 100644 --- a/branch.c +++ b/branch.c @@ -602,11 +602,13 @@ void create_branches_recursively(struct repository *r, const char *name, */ for (i = 0; i < submodule_entry_list.entry_nr; i++) { if (submodule_entry_list.entries[i].repo == NULL) { + int code = die_message( + _("submodule '%s': unable to find submodule"), + submodule_entry_list.entries[i].submodule->name); if (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED)) advise(_("You may try updating the submodules using 'git checkout %s && git submodule update --init'"), start_commitish); - die(_("submodule '%s': unable to find submodule"), - submodule_entry_list.entries[i].submodule->name); + exit(code); } if (submodule_create_branch( -- cgit v1.2.3 From ac59c742de5f548ed07735fb212cc87129383bcd Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 29 Mar 2022 20:01:18 +0000 Subject: branch --set-upstream-to: be consistent when advising "git branch --set-upstream-to" behaves differently when advice is enabled/disabled: | | error prefix | exit code | |-----------------+--------------+-----------| | advice enabled | error: | 1 | | advice disabled | fatal: | 128 | Make both cases consistent by using die_message() when advice is enabled (this was first proposed in [1]). [1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- branch.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 50459c432c..add6a37b79 100644 --- a/branch.c +++ b/branch.c @@ -384,9 +384,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name, if (get_oid_mb(start_name, &oid)) { if (explicit_tracking) { if (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) { - error(_(upstream_missing), start_name); + int code = die_message(_(upstream_missing), + start_name); advise(_(upstream_advice)); - exit(1); + exit(code); } die(_(upstream_missing), start_name); } -- cgit v1.2.3 From 5391e94813418dfb5ccc4c2f1d8518995b4f3ca5 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Tue, 29 Mar 2022 20:01:19 +0000 Subject: branch: remove negative exit code Replace an instance of "exit(-1)" with "exit(1)". We don't use negative exit codes - they are misleading because Unix machines will coerce them to 8-bit unsigned values, losing the sign. Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- branch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index add6a37b79..ed6f993aa6 100644 --- a/branch.c +++ b/branch.c @@ -263,7 +263,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref, string_list_append(tracking.srcs, orig_ref); if (install_branch_config_multiple_remotes(config_flags, new_ref, tracking.remote, tracking.srcs) < 0) - exit(-1); + exit(1); cleanup: string_list_clear(&tracking_srcs, 0); -- cgit v1.2.3 From 1f888282e2914283890f61000a7589d32b4132bc Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Thu, 31 Mar 2022 15:41:17 -0700 Subject: branch: rework comments for future developers For two cases in which we do not explicitly pass --track= option down to the submodule--helper subprocess, we have comments that say "we do not have to pass --track", but in fact we not just do not have to, but it would be incorrect to pass any --track option to the subprocess (instead, the correct behaviour is to let the subprocess figure out what is the appropriate tracking mode to use). Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- branch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index ed6f993aa6..8ee9f43539 100644 --- a/branch.c +++ b/branch.c @@ -549,9 +549,9 @@ static int submodule_create_branch(struct repository *r, strvec_push(&child.args, "--track=inherit"); break; case BRANCH_TRACK_UNSPECIFIED: - /* Default for "git checkout". No need to pass --track. */ + /* Default for "git checkout". Do not pass --track. */ case BRANCH_TRACK_REMOTE: - /* Default for "git branch". No need to pass --track. */ + /* Default for "git branch". Do not pass --track. */ break; } -- cgit v1.2.3 From 6696601241d27cf7b2834b92788a73a6f4af2e89 Mon Sep 17 00:00:00 2001 From: Glen Choo Date: Thu, 31 Mar 2022 15:41:18 -0700 Subject: branch.c: simplify advice-and-die sequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the dwim_branch_start(), when we cannot find an appropriate upstream, we will die with the same message anyway, whether we issue an advice message. Flip the code around a bit and simplify the flow using advise_if_enabled() function. Helped-by: Ævar Arnfjörð Bjarmason Signed-off-by: Glen Choo Signed-off-by: Junio C Hamano --- branch.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'branch.c') diff --git a/branch.c b/branch.c index 8ee9f43539..b673143cbe 100644 --- a/branch.c +++ b/branch.c @@ -383,13 +383,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name, real_ref = NULL; if (get_oid_mb(start_name, &oid)) { if (explicit_tracking) { - if (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) { - int code = die_message(_(upstream_missing), - start_name); - advise(_(upstream_advice)); - exit(code); - } - die(_(upstream_missing), start_name); + int code = die_message(_(upstream_missing), start_name); + advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE, + _(upstream_advice)); + exit(code); } die(_("Not a valid object name: '%s'."), start_name); } -- cgit v1.2.3