{"thread":{"id":"57632","subject":"[PATCH 1/4] branch: support more tracking modes when recursing","startedAt":"2022-03-29T20:01:26Z","lastAt":"2022-04-01T20:14:34Z","messageCount":21,"participants":["Glen Choo via GitGitGadget","Junio C Hamano","Ævar Arnfjörð Bjarmason","Glen Choo"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"452601","messageId":"0b682c173c8cfa7f49ba17b2d71049ac702ec747.1648584079.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.git.1648584079.gitgitgadget@gmail.com","subject":"[PATCH 1/4] branch: support more tracking modes when recursing","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-29T20:01:16Z","receivedAt":"2022-03-29T20:01:26Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\n\"git branch --recurse-submodules\" does not propagate \"--track=inherit\"\nor \"--no-track\" to submodules, which causes submodule branches to use\nthe wrong tracking mode [1]. To fix this, pass the correct options to\nthe \"submodule--helper create-branch\" child process and test for it.\n\nWhile we are refactoring the same code, replace \"--track\" with the\nsynonymous, but more consistent-looking \"--track=direct\" option\n(introduced at the same time as \"--track=inherit\", d3115660b4 (branch:\nadd flags and config to inherit tracking, 2021-12-20)).\n\n[1] This bug is partially a timing issue: \"branch --recurse-submodules\"\n was introduced around the same time as \"--track=inherit\", and even\n though I rebased \"branch --recurse-submodules\" on top of that, I had\n neglected to support the new tracking mode. Omitting \"--no-track\"\n was just a plain old mistake, though.\n\nSigned-off-by: Glen Choo \n---\n branch.c | 29 +++++++++++++++++++++++++---\n builtin/submodule--helper.c | 7 ++++---\n t/t3207-branch-submodule.sh | 38 ++++++++++++++++++++++++++++++++++++-\n 3 files changed, 67 insertions(+), 7 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 6b31df539a5..7377b9f451a 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -233,6 +233,9 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n \tstruct string_list tracking_srcs = STRING_LIST_INIT_DUP;\n \tint config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;\n \n+\tif (!track)\n+\t\tBUG(\"asked to set up tracking, but tracking is disallowed\");\n+\n \tmemset(&tracking, 0, sizeof(tracking));\n \ttracking.spec.dst = (char *)orig_ref;\n \ttracking.srcs = &tracking_srcs;\n@@ -534,8 +537,27 @@ static int submodule_create_branch(struct repository *r,\n \t\tstrvec_push(&child.args, \"--quiet\");\n \tif (reflog)\n \t\tstrvec_push(&child.args, \"--create-reflog\");\n-\tif (track == BRANCH_TRACK_ALWAYS || track == BRANCH_TRACK_EXPLICIT)\n-\t\tstrvec_push(&child.args, \"--track\");\n+\n+\tswitch (track) {\n+\tcase BRANCH_TRACK_NEVER:\n+\t\tstrvec_push(&child.args, \"--no-track\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_ALWAYS:\n+\tcase BRANCH_TRACK_EXPLICIT:\n+\t\tstrvec_push(&child.args, \"--track=direct\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_OVERRIDE:\n+\t\tBUG(\"BRANCH_TRACK_OVERRIDE cannot be used when creating a branch.\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_INHERIT:\n+\t\tstrvec_push(&child.args, \"--track=inherit\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_UNSPECIFIED:\n+\t\t/* Default for \"git checkout\". No need to pass --track. */\n+\tcase BRANCH_TRACK_REMOTE:\n+\t\t/* Default for \"git branch\". No need to pass --track. */\n+\t\tbreak;\n+\t}\n \n \tstrvec_pushl(&child.args, name, start_oid, tracking_name, NULL);\n \n@@ -614,7 +636,8 @@ void create_branches_recursively(struct repository *r, const char *name,\n \t * tedious to determine whether or not tracking was set up in the\n \t * superproject.\n \t */\n-\tsetup_tracking(name, tracking_name, track, quiet);\n+\tif (track)\n+\t\tsetup_tracking(name, tracking_name, track, quiet);\n \n \tfor (i = 0; i < submodule_entry_list.entry_nr; i++) {\n \t\tif (submodule_create_branch(\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 5301612d24b..081c8267c33 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3021,9 +3021,10 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT__FORCE(&force, N_(\"force creation\"), 0),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog,\n \t\t\t N_(\"create the branch's reflog\")),\n-\t\tOPT_SET_INT('t', \"track\", &track,\n-\t\t\t N_(\"set up tracking mode (see git-pull(1))\"),\n-\t\t\t BRANCH_TRACK_EXPLICIT),\n+\t\tOPT_CALLBACK_F('t', \"track\", &track, \"(direct|inherit)\",\n+\t\t\tN_(\"set branch tracking configuration\"),\n+\t\t\tPARSE_OPT_OPTARG,\n+\t\t\tparse_opt_tracking_mode),\n \t\tOPT__DRY_RUN(&dry_run,\n \t\t\t N_(\"show whether the branch would be created\")),\n \t\tOPT_END()\ndiff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh\nindex 0d93f7516c9..cfde6b237f5 100755\n--- a/t/t3207-branch-submodule.sh\n+++ b/t/t3207-branch-submodule.sh\n@@ -260,7 +260,7 @@ test_expect_success 'should get fatal error upon branch creation when submodule\n \t)\n '\n \n-test_expect_success 'should set up tracking of remote-tracking branches' '\n+test_expect_success 'should set up tracking of remote-tracking branches by default' '\n \ttest_when_finished \"reset_remote_test\" &&\n \t(\n \t\tcd super-clone &&\n@@ -289,4 +289,40 @@ test_expect_success 'should not fail when unable to set up tracking in submodule\n \t)\n '\n \n+test_expect_success '--track=inherit should set up tracking correctly' '\n+\ttest_when_finished \"reset_remote_test\" &&\n+\t(\n+\t\tcd super-clone &&\n+\t\tgit branch --recurse-submodules branch-a origin/branch-a &&\n+\t\t# Set this manually instead of using branch --set-upstream-to\n+\t\t# to circumvent the \"nonexistent upstream\" check.\n+\t\tgit -C sub config branch.branch-a.remote origin &&\n+\t\tgit -C sub config branch.branch-a.merge refs/heads/sub-branch-a &&\n+\t\tgit -C sub/sub-sub config branch.branch-a.remote other &&\n+\t\tgit -C sub/sub-sub config branch.branch-a.merge refs/heads/sub-sub-branch-a &&\n+\n+\t\tgit branch --recurse-submodules --track=inherit branch-b branch-a &&\n+\t\ttest_cmp_config origin branch.branch-b.remote &&\n+\t\ttest_cmp_config refs/heads/branch-a branch.branch-b.merge &&\n+\t\ttest_cmp_config -C sub origin branch.branch-b.remote &&\n+\t\ttest_cmp_config -C sub refs/heads/sub-branch-a branch.branch-b.merge &&\n+\t\ttest_cmp_config -C sub/sub-sub other branch.branch-b.remote &&\n+\t\ttest_cmp_config -C sub/sub-sub refs/heads/sub-sub-branch-a branch.branch-b.merge\n+\t)\n+'\n+\n+test_expect_success '--no-track should not set up tracking' '\n+\ttest_when_finished \"reset_remote_test\" &&\n+\t(\n+\t\tcd super-clone &&\n+\t\tgit branch --recurse-submodules --no-track branch-a origin/branch-a &&\n+\t\ttest_cmp_config \"\" --default \"\" branch.branch-a.remote &&\n+\t\ttest_cmp_config \"\" --default \"\" branch.branch-a.merge &&\n+\t\ttest_cmp_config -C sub \"\" --default \"\" branch.branch-a.remote &&\n+\t\ttest_cmp_config -C sub \"\" --default \"\" branch.branch-a.merge &&\n+\t\ttest_cmp_config -C sub/sub-sub \"\" --default \"\" branch.branch-a.remote &&\n+\t\ttest_cmp_config -C sub/sub-sub \"\" --default \"\" branch.branch-a.merge\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"452602","messageId":"pull.1190.git.1648584079.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":null,"subject":"[PATCH 0/4] branch --recurse-submodules: Bug fixes and clean ups","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-29T20:01:15Z","receivedAt":"2022-03-29T20:01:28Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"This a catch-all series that address some bugs discovered while testing \"git\nbranch --recurse-submodules\" (patches 1-2), as well as some clean ups that\ncame up when \"branch --recurse-submodules\" was first proposed (patches 3-4).\n\n== Patch organization\n\nPatches 1-2 are bugfixes, 3-4 are clean ups.\n\n * Patch 1 fixes a bug where \"git branch --recurse-submodules\" would not\n propagate the \"--track\" option if its value was \"--no-track\" or\n \"--track=inherit\".\n\n * Patch 2 fixes a bug where \"git branch --recurse-submodules\" would give\n advice before telling the user what the problem is (instead of the other\n way around).\n\n * Patch 3 fixes some old inconsistencies when \"git branch\n\n--set-upstream-to\" gives advice and when it doesn't.\n\n * Patch 4 replaces exit(-1) with exit(1).\n\n== Outstanding concerns\n\nPatch 4: exit(1) was suggested in\nhttps://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com. I'm\nnot sure if we have a strong convention around exit codes and I'm using \"1\"\nas \"unspecified error. Perhaps others could chime in.\n\nGlen Choo (4):\n branch: support more tracking modes when recursing\n branch: give submodule updating advice before exit\n branch --set-upstream-to: be consistent when advising\n branch: remove negative exit code\n\n branch.c | 42 ++++++++++++++++++++++++++++++-------\n builtin/submodule--helper.c | 7 ++++---\n t/t3207-branch-submodule.sh | 38 ++++++++++++++++++++++++++++++++-\n 3 files changed, 75 insertions(+), 12 deletions(-)\n\n\nbase-commit: abf474a5dd901f28013c52155411a48fd4c09922\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1190%2Fchooglen%2Fbranch%2Frecursive-fixes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1190/chooglen/branch/recursive-fixes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1190\n-- \ngitgitgadget\n"},{"id":"452603","messageId":"f21d283e5ade542b73cb36178c3c33008e2c67df.1648584079.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.git.1648584079.gitgitgadget@gmail.com","subject":"[PATCH 2/4] branch: give submodule updating advice before exit","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-29T20:01:17Z","receivedAt":"2022-03-29T20:01:32Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\nFix a bug where \"hint:\" was printed _before_ \"fatal:\" (instead of the\nother way around).\n\nSigned-off-by: Glen Choo \n---\n branch.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 7377b9f451a..133e6047bc6 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -607,11 +607,13 @@ void create_branches_recursively(struct repository *r, const char *name,\n \t */\n \tfor (i = 0; i < submodule_entry_list.entry_nr; i++) {\n \t\tif (submodule_entry_list.entries[i].repo == NULL) {\n+\t\t\tint code = die_message(\n+\t\t\t\t_(\"submodule '%s': unable to find submodule\"),\n+\t\t\t\tsubmodule_entry_list.entries[i].submodule->name);\n \t\t\tif (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))\n \t\t\t\tadvise(_(\"You may try updating the submodules using 'git checkout %s && git submodule update --init'\"),\n \t\t\t\t start_commitish);\n-\t\t\tdie(_(\"submodule '%s': unable to find submodule\"),\n-\t\t\t submodule_entry_list.entries[i].submodule->name);\n+\t\t\texit(code);\n \t\t}\n \n \t\tif (submodule_create_branch(\n-- \ngitgitgadget\n\n"},{"id":"452604","messageId":"8e6ea3478b376b24835f3a3fef7fca39bed0afec.1648584079.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.git.1648584079.gitgitgadget@gmail.com","subject":"[PATCH 3/4] branch --set-upstream-to: be consistent when advising","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-29T20:01:18Z","receivedAt":"2022-03-29T20:01:34Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\n\"git branch --set-upstream-to\" behaves differently when advice is\nenabled/disabled:\n\n| | error prefix | exit code |\n|-----------------+--------------+-----------|\n| advice enabled | error: | 1 |\n| advice disabled | fatal: | 128 |\n\nMake both cases consistent by using die_message() when advice is\nenabled (this was first proposed in [1]).\n\n[1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com\n\nSigned-off-by: Glen Choo \n---\n branch.c | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 133e6047bc6..4a8796489c7 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -389,9 +389,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n \tif (get_oid_mb(start_name, &oid)) {\n \t\tif (explicit_tracking) {\n \t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n-\t\t\t\terror(_(upstream_missing), start_name);\n+\t\t\t\tint code = die_message(_(upstream_missing),\n+\t\t\t\t\t\t start_name);\n \t\t\t\tadvise(_(upstream_advice));\n-\t\t\t\texit(1);\n+\t\t\t\texit(code);\n \t\t\t}\n \t\t\tdie(_(upstream_missing), start_name);\n \t\t}\n-- \ngitgitgadget\n\n"},{"id":"452605","messageId":"fb2b472d9ae66bcdcde6a8417486172a47c71258.1648584079.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.git.1648584079.gitgitgadget@gmail.com","subject":"[PATCH 4/4] branch: remove negative exit code","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-29T20:01:19Z","receivedAt":"2022-03-29T20:01:41Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\nReplace an instance of \"exit(-1)\" with \"exit(1)\". We don't use negative\nexit codes - they are misleading because Unix machines will coerce them\nto 8-bit unsigned values, losing the sign.\n\nSigned-off-by: Glen Choo \n---\n branch.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/branch.c b/branch.c\nindex 4a8796489c7..eb231b950bb 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -263,7 +263,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n \t\tstring_list_append(tracking.srcs, orig_ref);\n \tif (install_branch_config_multiple_remotes(config_flags, new_ref,\n \t\t\t\ttracking.remote, tracking.srcs) < 0)\n-\t\texit(-1);\n+\t\texit(1);\n \n cleanup:\n \tstring_list_clear(&tracking_srcs, 0);\n-- \ngitgitgadget\n"},{"id":"452703","messageId":"xmqqee2j6t5l.fsf@gitster.g","threadId":"57632","inReplyTo":"0b682c173c8cfa7f49ba17b2d71049ac702ec747.1648584079.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/4] branch: support more tracking modes when recursing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-30T21:13:10Z","receivedAt":"2022-03-30T21:13:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Glen Choo via GitGitGadget\" writes:\n\n> diff --git a/branch.c b/branch.c\n> index 6b31df539a5..7377b9f451a 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -233,6 +233,9 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n> \tstruct string_list tracking_srcs = STRING_LIST_INIT_DUP;\n> \tint config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;\n> \n> +\tif (!track)\n> +\t\tBUG(\"asked to set up tracking, but tracking is disallowed\");\n\nI am wondering if this wants to be\n\n\tif (track == BRANCH_TRACK_NEVER)\n\ninstead. Do we elsewhere rely on the fact that NEVER is assigned 0?\n\n> @@ -534,8 +537,27 @@ static int submodule_create_branch(struct repository *r,\n> \t\tstrvec_push(&child.args, \"--quiet\");\n> \tif (reflog)\n> \t\tstrvec_push(&child.args, \"--create-reflog\");\n> -\tif (track == BRANCH_TRACK_ALWAYS || track == BRANCH_TRACK_EXPLICIT)\n> -\t\tstrvec_push(&child.args, \"--track\");\n> +\n> +\tswitch (track) {\n> +\tcase BRANCH_TRACK_NEVER:\n> +\t\tstrvec_push(&child.args, \"--no-track\");\n> +\t\tbreak;\n> +\tcase BRANCH_TRACK_ALWAYS:\n> +\tcase BRANCH_TRACK_EXPLICIT:\n> +\t\tstrvec_push(&child.args, \"--track=direct\");\n> +\t\tbreak;\n> +\tcase BRANCH_TRACK_OVERRIDE:\n> +\t\tBUG(\"BRANCH_TRACK_OVERRIDE cannot be used when creating a branch.\");\n> +\t\tbreak;\n> +\tcase BRANCH_TRACK_INHERIT:\n> +\t\tstrvec_push(&child.args, \"--track=inherit\");\n> +\t\tbreak;\n\nOK.\n\n> +\tcase BRANCH_TRACK_UNSPECIFIED:\n> +\t\t/* Default for \"git checkout\". No need to pass --track. */\n> +\tcase BRANCH_TRACK_REMOTE:\n> +\t\t/* Default for \"git branch\". No need to pass --track. */\n> +\t\tbreak;\n\nIs that \"no need to pass\", or \"no need to, and it will be detrimental to, pass\"?\n\nIOW, if we are relying on the command spawned via start_command()\ninterface to read and honor the configured default for themselves,\nthen passing explicit --track=whatever from this caller will be not\njust necessary but is wrong, right? I am worried about \"No need to\"\ntempting \"helpful\" developers into doing unnecessary things, just to\nbe more explicit, for example. \n\n> @@ -614,7 +636,8 @@ void create_branches_recursively(struct repository *r, const char *name,\n> \t * tedious to determine whether or not tracking was set up in the\n> \t * superproject.\n> \t */\n> -\tsetup_tracking(name, tracking_name, track, quiet);\n> +\tif (track)\n> +\t\tsetup_tracking(name, tracking_name, track, quiet);\n\nHere we do rely on the fact that NEVER has the value of 0. If there\nare other instances of code elsewhere that does so, then this one\nand the other one at the top of this message are both fine.\n\nGiven that we started simple and then gradually added more features,\nI would not be surprised if the older code written back when there\nwere only 0 (no track) and 1 (track) assumed 0 means no. There is\none in create_branch() where we do\n\n\tif (real_ref && track)\n\t\tsetup_tracking(ref.buf + 11, real_ref, track, quiet);\n\nwhich also relies on the fact that NEVER is 0.\n\n> -\t\tOPT_SET_INT('t', \"track\", &track,\n> -\t\t\t N_(\"set up tracking mode (see git-pull(1))\"),\n> -\t\t\t BRANCH_TRACK_EXPLICIT),\n> +\t\tOPT_CALLBACK_F('t', \"track\", &track, \"(direct|inherit)\",\n> +\t\t\tN_(\"set branch tracking configuration\"),\n> +\t\t\tPARSE_OPT_OPTARG,\n> +\t\t\tparse_opt_tracking_mode),\n\nHmph, this is quite curious. How did the whole thing even worked\nwithout this?\n\nAh, OK, this is in submodule--helper.c and tracking specification in\nthe top-level were OK. Just that we forgot to correctly pass it\ndown when calling down to submodules. Makes sense.\n\nThanks.\n"},{"id":"452705","messageId":"xmqqa6d76t2c.fsf@gitster.g","threadId":"57632","inReplyTo":"f21d283e5ade542b73cb36178c3c33008e2c67df.1648584079.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/4] branch: give submodule updating advice before exit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-30T21:15:07Z","receivedAt":"2022-03-30T21:15:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Glen Choo via GitGitGadget\" writes:\n\n> From: Glen Choo \n>\n> Fix a bug where \"hint:\" was printed _before_ \"fatal:\" (instead of the\n> other way around).\n>\n> Signed-off-by: Glen Choo \n> ---\n> branch.c | 6 ++++--\n> 1 file changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/branch.c b/branch.c\n> index 7377b9f451a..133e6047bc6 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -607,11 +607,13 @@ void create_branches_recursively(struct repository *r, const char *name,\n> \t */\n> \tfor (i = 0; i < submodule_entry_list.entry_nr; i++) {\n> \t\tif (submodule_entry_list.entries[i].repo == NULL) {\n> +\t\t\tint code = die_message(\n> +\t\t\t\t_(\"submodule '%s': unable to find submodule\"),\n> +\t\t\t\tsubmodule_entry_list.entries[i].submodule->name);\n> \t\t\tif (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))\n> \t\t\t\tadvise(_(\"You may try updating the submodules using 'git checkout %s && git submodule update --init'\"),\n> \t\t\t\t start_commitish);\n> -\t\t\tdie(_(\"submodule '%s': unable to find submodule\"),\n> -\t\t\t submodule_entry_list.entries[i].submodule->name);\n> +\t\t\texit(code);\n> \t\t}\n> \n> \t\tif (submodule_create_branch(\n\nGreat.\n\nThanks.\n"},{"id":"452715","messageId":"220331.867d8bt6i6.gmgdl@evledraar.gmail.com","threadId":"57632","inReplyTo":"8e6ea3478b376b24835f3a3fef7fca39bed0afec.1648584079.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] branch --set-upstream-to: be consistent when advising","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-03-30T22:28:17Z","receivedAt":"2022-03-30T22:33:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Mar 29 2022, Glen Choo via GitGitGadget wrote:\n\n> From: Glen Choo \n>\n> \"git branch --set-upstream-to\" behaves differently when advice is\n> enabled/disabled:\n>\n> | | error prefix | exit code |\n> |-----------------+--------------+-----------|\n> | advice enabled | error: | 1 |\n> | advice disabled | fatal: | 128 |\n>\n> Make both cases consistent by using die_message() when advice is\n> enabled (this was first proposed in [1]).\n>\n> [1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com\n\nThanks for following up on this :)\n\n> Signed-off-by: Glen Choo \n> ---\n> branch.c | 5 +++--\n> 1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/branch.c b/branch.c\n> index 133e6047bc6..4a8796489c7 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -389,9 +389,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n> \tif (get_oid_mb(start_name, &oid)) {\n> \t\tif (explicit_tracking) {\n> \t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n> -\t\t\t\terror(_(upstream_missing), start_name);\n> +\t\t\t\tint code = die_message(_(upstream_missing),\n> +\t\t\t\t\t\t start_name);\n> \t\t\t\tadvise(_(upstream_advice));\n> -\t\t\t\texit(1);\n> +\t\t\t\texit(code);\n> \t\t\t}\n> \t\t\tdie(_(upstream_missing), start_name);\n> \t\t}\n\nThis is really close to being much better, i.e. we can now just do this\n(this is on top of your branch):\n\t\n\tdiff --git a/branch.c b/branch.c\n\tindex eb231b950bb..5b648cb27ed 100644\n\t--- a/branch.c\n\t+++ b/branch.c\n\t@@ -342,8 +342,6 @@ static int validate_remote_tracking_branch(char *ref)\n\t \n\t static const char upstream_not_branch[] =\n\t N_(\"cannot set up tracking information; starting point '%s' is not a branch\");\n\t-static const char upstream_missing[] =\n\t-N_(\"the requested upstream branch '%s' does not exist\");\n\t static const char upstream_advice[] =\n\t N_(\"\\n\"\n\t \"If you are planning on basing your work on an upstream\\n\"\n\t@@ -388,13 +386,11 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n\t \treal_ref = NULL;\n\t \tif (get_oid_mb(start_name, &oid)) {\n\t \t\tif (explicit_tracking) {\n\t-\t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n\t-\t\t\t\tint code = die_message(_(upstream_missing),\n\t-\t\t\t\t\t\t start_name);\n\t-\t\t\t\tadvise(_(upstream_advice));\n\t-\t\t\t\texit(code);\n\t-\t\t\t}\n\t-\t\t\tdie(_(upstream_missing), start_name);\n\t+\t\t\tint code = die_message(_(\"the requested upstream branch '%s' does not exist\"),\n\t+\t\t\t\t\t start_name);\n\t+\t\t\tadvise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,\n\t+\t\t\t\t\t _(upstream_advice));\n\t+\t\t\texit(code);\n\t \t\t}\n\t \t\tdie(_(\"not a valid object name: '%s'\"), start_name);\n\t \t}\n\t\nI.e. the only reason we needed to mention upstream_missing multiple\ntimes is because we didn't have something like die_message() before, now\nwe can just skip that other \"die\" entirely.\n\nThe advise_if_enabled() might be worthwhile to change while at it, maybe\nnot.\n\nBut also useful, is that we can now simply inline the \"upstream_missing\"\nstring, which will give us type checks for the printf format. The reason\nwe had a variable before was also because of the lack of die_message()>\n\nI notice that we can do likewise with the advice itself, and with\n\"upstream_not_branch\" if we either make that a \"goto\", or add a trivial\nhelper function.\n"},{"id":"452722","messageId":"kl6l4k3ft3y3.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"57632","inReplyTo":"xmqqee2j6t5l.fsf@gitster.g","subject":"Re: [PATCH 1/4] branch: support more tracking modes when recursing","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-30T23:29:08Z","receivedAt":"2022-03-30T23:29:23Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Junio C Hamano writes:\n\n> \"Glen Choo via GitGitGadget\" writes:\n>\n>> diff --git a/branch.c b/branch.c\n>> index 6b31df539a5..7377b9f451a 100644\n>> --- a/branch.c\n>> +++ b/branch.c\n>> @@ -233,6 +233,9 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n>> \tstruct string_list tracking_srcs = STRING_LIST_INIT_DUP;\n>> \tint config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;\n>> \n>> +\tif (!track)\n>> +\t\tBUG(\"asked to set up tracking, but tracking is disallowed\");\n>\n> I am wondering if this wants to be\n>\n> \tif (track == BRANCH_TRACK_NEVER)\n>\n> instead. Do we elsewhere rely on the fact that NEVER is assigned 0?\n\nAs you discussed here...\n\n> Given that we started simple and then gradually added more features,\n> I would not be surprised if the older code written back when there\n> were only 0 (no track) and 1 (track) assumed 0 means no. There is\n> one in create_branch() where we do\n>\n> \tif (real_ref && track)\n> \t\tsetup_tracking(ref.buf + 11, real_ref, track, quiet);\n>\n> which also relies on the fact that NEVER is 0.\n\nWe know the answer is \"yes there is older code that relies on NEVER\nbeing 0\". I believe this is the only instance though, which means this\npatch comprises the majority of instances of \"if (!track)\", so we can\nchange it if you prefer. The older code is pretty old after all - the\nenum was introduced in 9ed36cfa35 (branch: optionally setup\nbranch.*.merge from upstream local branches, 2008-02-19).\n\n>> +\tcase BRANCH_TRACK_UNSPECIFIED:\n>> +\t\t/* Default for \"git checkout\". No need to pass --track. */\n>> +\tcase BRANCH_TRACK_REMOTE:\n>> +\t\t/* Default for \"git branch\". No need to pass --track. */\n>> +\t\tbreak;\n>\n> Is that \"no need to pass\", or \"no need to, and it will be detrimental to, pass\"?\n>\n> IOW, if we are relying on the command spawned via start_command()\n> interface to read and honor the configured default for themselves,\n> then passing explicit --track=whatever from this caller will be not\n> just necessary but is wrong, right? I am worried about \"No need to\"\n> tempting \"helpful\" developers into doing unnecessary things, just to\n> be more explicit, for example. \n\nHm, interesting, I hadn't considered that temptation. This is the\nlatter, i.e. it is not correct to pass --track. I'll reword it for\nclarity, something like \"Should not pass --track\".\n\n>> -\t\tOPT_SET_INT('t', \"track\", &track,\n>> -\t\t\t N_(\"set up tracking mode (see git-pull(1))\"),\n>> -\t\t\t BRANCH_TRACK_EXPLICIT),\n>> +\t\tOPT_CALLBACK_F('t', \"track\", &track, \"(direct|inherit)\",\n>> +\t\t\tN_(\"set branch tracking configuration\"),\n>> +\t\t\tPARSE_OPT_OPTARG,\n>> +\t\t\tparse_opt_tracking_mode),\n>\n> Hmph, this is quite curious. How did the whole thing even worked\n> without this?\n>\n> Ah, OK, this is in submodule--helper.c and tracking specification in\n> the top-level were OK. Just that we forgot to correctly pass it\n> down when calling down to submodules. Makes sense.\n\nYes, that's correct. This was missed because I only added tests for\n--track and the default case (and didn't add tests for --track=inherit\nor --no-track.\n"},{"id":"452806","messageId":"kl6l1qyit67d.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"57632","inReplyTo":"220331.867d8bt6i6.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 3/4] branch --set-upstream-to: be consistent when advising","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-31T16:52:38Z","receivedAt":"2022-03-31T16:52:44Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Tue, Mar 29 2022, Glen Choo via GitGitGadget wrote:\n>\n>> From: Glen Choo \n>>\n>> \"git branch --set-upstream-to\" behaves differently when advice is\n>> enabled/disabled:\n>>\n>> | | error prefix | exit code |\n>> |-----------------+--------------+-----------|\n>> | advice enabled | error: | 1 |\n>> | advice disabled | fatal: | 128 |\n>>\n>> Make both cases consistent by using die_message() when advice is\n>> enabled (this was first proposed in [1]).\n>>\n>> [1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com\n>\n> Thanks for following up on this :)\n\n:)\n\n>> Signed-off-by: Glen Choo \n>> ---\n>> branch.c | 5 +++--\n>> 1 file changed, 3 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/branch.c b/branch.c\n>> index 133e6047bc6..4a8796489c7 100644\n>> --- a/branch.c\n>> +++ b/branch.c\n>> @@ -389,9 +389,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n>> \tif (get_oid_mb(start_name, &oid)) {\n>> \t\tif (explicit_tracking) {\n>> \t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n>> -\t\t\t\terror(_(upstream_missing), start_name);\n>> +\t\t\t\tint code = die_message(_(upstream_missing),\n>> +\t\t\t\t\t\t start_name);\n>> \t\t\t\tadvise(_(upstream_advice));\n>> -\t\t\t\texit(1);\n>> +\t\t\t\texit(code);\n>> \t\t\t}\n>> \t\t\tdie(_(upstream_missing), start_name);\n>> \t\t}\n>\n> This is really close to being much better, i.e. we can now just do this\n> (this is on top of your branch):\n> \t\n> \tdiff --git a/branch.c b/branch.c\n> \tindex eb231b950bb..5b648cb27ed 100644\n> \t--- a/branch.c\n> \t+++ b/branch.c\n> \t@@ -342,8 +342,6 @@ static int validate_remote_tracking_branch(char *ref)\n> \t \n> \t static const char upstream_not_branch[] =\n> \t N_(\"cannot set up tracking information; starting point '%s' is not a branch\");\n> \t-static const char upstream_missing[] =\n> \t-N_(\"the requested upstream branch '%s' does not exist\");\n> \t static const char upstream_advice[] =\n> \t N_(\"\\n\"\n> \t \"If you are planning on basing your work on an upstream\\n\"\n> \t@@ -388,13 +386,11 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n> \t \treal_ref = NULL;\n> \t \tif (get_oid_mb(start_name, &oid)) {\n> \t \t\tif (explicit_tracking) {\n> \t-\t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n> \t-\t\t\t\tint code = die_message(_(upstream_missing),\n> \t-\t\t\t\t\t\t start_name);\n> \t-\t\t\t\tadvise(_(upstream_advice));\n> \t-\t\t\t\texit(code);\n> \t-\t\t\t}\n> \t-\t\t\tdie(_(upstream_missing), start_name);\n> \t+\t\t\tint code = die_message(_(\"the requested upstream branch '%s' does not exist\"),\n> \t+\t\t\t\t\t start_name);\n> \t+\t\t\tadvise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,\n> \t+\t\t\t\t\t _(upstream_advice));\n> \t+\t\t\texit(code);\n> \t \t\t}\n> \t \t\tdie(_(\"not a valid object name: '%s'\"), start_name);\n> \t \t}\n> \t\n> I.e. the only reason we needed to mention upstream_missing multiple\n> times is because we didn't have something like die_message() before, now\n> we can just skip that other \"die\" entirely.\n\nOh, good point. Yeah I like this better, I'll do that.\n\n> The advise_if_enabled() might be worthwhile to change while at it, maybe\n> not.\n\nI think it's worthwhile; this does exactly what we want. I would have\nused it if I had known it existed.\n\n>\n> But also useful, is that we can now simply inline the \"upstream_missing\"\n> string, which will give us type checks for the printf format. The reason\n> we had a variable before was also because of the lack of die_message()>\n>\n> I notice that we can do likewise with the advice itself, and with\n> \"upstream_not_branch\" if we either make that a \"goto\", or add a trivial\n> helper function.\n\nInteresting, I hadn't considered type checking. So in general we prefer\nto inline the strings and not use variables? I'll keep that in mind.\n"},{"id":"452814","messageId":"pull.1190.v2.git.1648752601.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.git.1648584079.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] branch --recurse-submodules: Bug fixes and clean ups","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-31T18:49:57Z","receivedAt":"2022-03-31T18:50:08Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Thanks for the feedback, all. This version incorporates most of the\nsuggestions (which were pretty small anyway).\n\n== Patch organization\n\nPatches 1-2 are bugfixes, 3-4 are clean ups.\n\nPatch 1 fixes a bug where \"git branch --recurse-submodules\" would not\npropagate the \"--track\" option if its value was \"--no-track\" or\n\"--track=inherit\".\n\nPatch 2 fixes a bug where \"git branch --recurse-submodules\" would give\nadvice before telling the user what the problem is (instead of the other way\naround).\n\nPatch 3 fixes some old inconsistencies when \"git branch --set-upstream-to\"\ngives advice and when it doesn't.\n\nPatch 4 replaces exit(-1) with exit(1).\n\n== Changes\n\nSince v1:\n\n * Patch 1: reword the --track comments to be prescriptive\n * Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to\n inline the advice string to save reviewers the trouble of proofreading\n (and the format string has no placeholders anyway, so I don't think we'd\n get much benefit out of typechecking). We can inline it in another\n series.\n\nGlen Choo (4):\n branch: support more tracking modes when recursing\n branch: give submodule updating advice before exit\n branch --set-upstream-to: be consistent when advising\n branch: remove negative exit code\n\n branch.c | 47 +++++++++++++++++++++++++++----------\n builtin/submodule--helper.c | 7 +++---\n t/t3207-branch-submodule.sh | 38 +++++++++++++++++++++++++++++-\n 3 files changed, 76 insertions(+), 16 deletions(-)\n\n\nbase-commit: abf474a5dd901f28013c52155411a48fd4c09922\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1190%2Fchooglen%2Fbranch%2Frecursive-fixes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1190/chooglen/branch/recursive-fixes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1190\n\nRange-diff vs v1:\n\n 1: 0b682c173c8 ! 1: 5e96a09199f branch: support more tracking modes when recursing\n @@ branch.c: static int submodule_create_branch(struct repository *r,\n +\t\tstrvec_push(&child.args, \"--track=inherit\");\n +\t\tbreak;\n +\tcase BRANCH_TRACK_UNSPECIFIED:\n -+\t\t/* Default for \"git checkout\". No need to pass --track. */\n ++\t\t/* Default for \"git checkout\". Do not pass --track. */\n +\tcase BRANCH_TRACK_REMOTE:\n -+\t\t/* Default for \"git branch\". No need to pass --track. */\n ++\t\t/* Default for \"git branch\". Do not pass --track. */\n +\t\tbreak;\n +\t}\n \n 2: f21d283e5ad = 2: 74b839bfc4e branch: give submodule updating advice before exit\n 3: 8e6ea3478b3 ! 3: 64d9b8e0f44 branch --set-upstream-to: be consistent when advising\n @@ Commit message\n \n [1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com\n \n + Helped-by: Ævar Arnfjörð Bjarmason \n Signed-off-by: Glen Choo \n \n ## branch.c ##\n @@ branch.c: static void dwim_branch_start(struct repository *r, const char *start_name,\n + \treal_ref = NULL;\n \tif (get_oid_mb(start_name, &oid)) {\n \t\tif (explicit_tracking) {\n - \t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n +-\t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n -\t\t\t\terror(_(upstream_missing), start_name);\n -+\t\t\t\tint code = die_message(_(upstream_missing),\n -+\t\t\t\t\t\t start_name);\n - \t\t\t\tadvise(_(upstream_advice));\n +-\t\t\t\tadvise(_(upstream_advice));\n -\t\t\t\texit(1);\n -+\t\t\t\texit(code);\n - \t\t\t}\n - \t\t\tdie(_(upstream_missing), start_name);\n +-\t\t\t}\n +-\t\t\tdie(_(upstream_missing), start_name);\n ++\t\t\tint code = die_message(_(upstream_missing), start_name);\n ++\t\t\tadvise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,\n ++\t\t\t\t\t _(upstream_advice));\n ++\t\t\texit(code);\n \t\t}\n + \t\tdie(_(\"not a valid object name: '%s'\"), start_name);\n + \t}\n 4: fb2b472d9ae = 4: 306858373cc branch: remove negative exit code\n\n-- \ngitgitgadget\n"},{"id":"452815","messageId":"5e96a09199fab0be35d26eab8e73d7cd803032c8.1648752601.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.v2.git.1648752601.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] branch: support more tracking modes when recursing","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-31T18:49:58Z","receivedAt":"2022-03-31T18:50:12Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\n\"git branch --recurse-submodules\" does not propagate \"--track=inherit\"\nor \"--no-track\" to submodules, which causes submodule branches to use\nthe wrong tracking mode [1]. To fix this, pass the correct options to\nthe \"submodule--helper create-branch\" child process and test for it.\n\nWhile we are refactoring the same code, replace \"--track\" with the\nsynonymous, but more consistent-looking \"--track=direct\" option\n(introduced at the same time as \"--track=inherit\", d3115660b4 (branch:\nadd flags and config to inherit tracking, 2021-12-20)).\n\n[1] This bug is partially a timing issue: \"branch --recurse-submodules\"\n was introduced around the same time as \"--track=inherit\", and even\n though I rebased \"branch --recurse-submodules\" on top of that, I had\n neglected to support the new tracking mode. Omitting \"--no-track\"\n was just a plain old mistake, though.\n\nSigned-off-by: Glen Choo \n---\n branch.c | 29 +++++++++++++++++++++++++---\n builtin/submodule--helper.c | 7 ++++---\n t/t3207-branch-submodule.sh | 38 ++++++++++++++++++++++++++++++++++++-\n 3 files changed, 67 insertions(+), 7 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 6b31df539a5..aac24591d22 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -233,6 +233,9 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n \tstruct string_list tracking_srcs = STRING_LIST_INIT_DUP;\n \tint config_flags = quiet ? 0 : BRANCH_CONFIG_VERBOSE;\n \n+\tif (!track)\n+\t\tBUG(\"asked to set up tracking, but tracking is disallowed\");\n+\n \tmemset(&tracking, 0, sizeof(tracking));\n \ttracking.spec.dst = (char *)orig_ref;\n \ttracking.srcs = &tracking_srcs;\n@@ -534,8 +537,27 @@ static int submodule_create_branch(struct repository *r,\n \t\tstrvec_push(&child.args, \"--quiet\");\n \tif (reflog)\n \t\tstrvec_push(&child.args, \"--create-reflog\");\n-\tif (track == BRANCH_TRACK_ALWAYS || track == BRANCH_TRACK_EXPLICIT)\n-\t\tstrvec_push(&child.args, \"--track\");\n+\n+\tswitch (track) {\n+\tcase BRANCH_TRACK_NEVER:\n+\t\tstrvec_push(&child.args, \"--no-track\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_ALWAYS:\n+\tcase BRANCH_TRACK_EXPLICIT:\n+\t\tstrvec_push(&child.args, \"--track=direct\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_OVERRIDE:\n+\t\tBUG(\"BRANCH_TRACK_OVERRIDE cannot be used when creating a branch.\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_INHERIT:\n+\t\tstrvec_push(&child.args, \"--track=inherit\");\n+\t\tbreak;\n+\tcase BRANCH_TRACK_UNSPECIFIED:\n+\t\t/* Default for \"git checkout\". Do not pass --track. */\n+\tcase BRANCH_TRACK_REMOTE:\n+\t\t/* Default for \"git branch\". Do not pass --track. */\n+\t\tbreak;\n+\t}\n \n \tstrvec_pushl(&child.args, name, start_oid, tracking_name, NULL);\n \n@@ -614,7 +636,8 @@ void create_branches_recursively(struct repository *r, const char *name,\n \t * tedious to determine whether or not tracking was set up in the\n \t * superproject.\n \t */\n-\tsetup_tracking(name, tracking_name, track, quiet);\n+\tif (track)\n+\t\tsetup_tracking(name, tracking_name, track, quiet);\n \n \tfor (i = 0; i < submodule_entry_list.entry_nr; i++) {\n \t\tif (submodule_create_branch(\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 5301612d24b..081c8267c33 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3021,9 +3021,10 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT__FORCE(&force, N_(\"force creation\"), 0),\n \t\tOPT_BOOL(0, \"create-reflog\", &reflog,\n \t\t\t N_(\"create the branch's reflog\")),\n-\t\tOPT_SET_INT('t', \"track\", &track,\n-\t\t\t N_(\"set up tracking mode (see git-pull(1))\"),\n-\t\t\t BRANCH_TRACK_EXPLICIT),\n+\t\tOPT_CALLBACK_F('t', \"track\", &track, \"(direct|inherit)\",\n+\t\t\tN_(\"set branch tracking configuration\"),\n+\t\t\tPARSE_OPT_OPTARG,\n+\t\t\tparse_opt_tracking_mode),\n \t\tOPT__DRY_RUN(&dry_run,\n \t\t\t N_(\"show whether the branch would be created\")),\n \t\tOPT_END()\ndiff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh\nindex 0d93f7516c9..cfde6b237f5 100755\n--- a/t/t3207-branch-submodule.sh\n+++ b/t/t3207-branch-submodule.sh\n@@ -260,7 +260,7 @@ test_expect_success 'should get fatal error upon branch creation when submodule\n \t)\n '\n \n-test_expect_success 'should set up tracking of remote-tracking branches' '\n+test_expect_success 'should set up tracking of remote-tracking branches by default' '\n \ttest_when_finished \"reset_remote_test\" &&\n \t(\n \t\tcd super-clone &&\n@@ -289,4 +289,40 @@ test_expect_success 'should not fail when unable to set up tracking in submodule\n \t)\n '\n \n+test_expect_success '--track=inherit should set up tracking correctly' '\n+\ttest_when_finished \"reset_remote_test\" &&\n+\t(\n+\t\tcd super-clone &&\n+\t\tgit branch --recurse-submodules branch-a origin/branch-a &&\n+\t\t# Set this manually instead of using branch --set-upstream-to\n+\t\t# to circumvent the \"nonexistent upstream\" check.\n+\t\tgit -C sub config branch.branch-a.remote origin &&\n+\t\tgit -C sub config branch.branch-a.merge refs/heads/sub-branch-a &&\n+\t\tgit -C sub/sub-sub config branch.branch-a.remote other &&\n+\t\tgit -C sub/sub-sub config branch.branch-a.merge refs/heads/sub-sub-branch-a &&\n+\n+\t\tgit branch --recurse-submodules --track=inherit branch-b branch-a &&\n+\t\ttest_cmp_config origin branch.branch-b.remote &&\n+\t\ttest_cmp_config refs/heads/branch-a branch.branch-b.merge &&\n+\t\ttest_cmp_config -C sub origin branch.branch-b.remote &&\n+\t\ttest_cmp_config -C sub refs/heads/sub-branch-a branch.branch-b.merge &&\n+\t\ttest_cmp_config -C sub/sub-sub other branch.branch-b.remote &&\n+\t\ttest_cmp_config -C sub/sub-sub refs/heads/sub-sub-branch-a branch.branch-b.merge\n+\t)\n+'\n+\n+test_expect_success '--no-track should not set up tracking' '\n+\ttest_when_finished \"reset_remote_test\" &&\n+\t(\n+\t\tcd super-clone &&\n+\t\tgit branch --recurse-submodules --no-track branch-a origin/branch-a &&\n+\t\ttest_cmp_config \"\" --default \"\" branch.branch-a.remote &&\n+\t\ttest_cmp_config \"\" --default \"\" branch.branch-a.merge &&\n+\t\ttest_cmp_config -C sub \"\" --default \"\" branch.branch-a.remote &&\n+\t\ttest_cmp_config -C sub \"\" --default \"\" branch.branch-a.merge &&\n+\t\ttest_cmp_config -C sub/sub-sub \"\" --default \"\" branch.branch-a.remote &&\n+\t\ttest_cmp_config -C sub/sub-sub \"\" --default \"\" branch.branch-a.merge\n+\t)\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"452816","messageId":"74b839bfc4e9503f8b2c62aeab44ba5c07f09f0c.1648752601.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.v2.git.1648752601.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] branch: give submodule updating advice before exit","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-31T18:49:59Z","receivedAt":"2022-03-31T18:50:16Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\nFix a bug where \"hint:\" was printed _before_ \"fatal:\" (instead of the\nother way around).\n\nSigned-off-by: Glen Choo \n---\n branch.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex aac24591d22..1252b17b980 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -607,11 +607,13 @@ void create_branches_recursively(struct repository *r, const char *name,\n \t */\n \tfor (i = 0; i < submodule_entry_list.entry_nr; i++) {\n \t\tif (submodule_entry_list.entries[i].repo == NULL) {\n+\t\t\tint code = die_message(\n+\t\t\t\t_(\"submodule '%s': unable to find submodule\"),\n+\t\t\t\tsubmodule_entry_list.entries[i].submodule->name);\n \t\t\tif (advice_enabled(ADVICE_SUBMODULES_NOT_UPDATED))\n \t\t\t\tadvise(_(\"You may try updating the submodules using 'git checkout %s && git submodule update --init'\"),\n \t\t\t\t start_commitish);\n-\t\t\tdie(_(\"submodule '%s': unable to find submodule\"),\n-\t\t\t submodule_entry_list.entries[i].submodule->name);\n+\t\t\texit(code);\n \t\t}\n \n \t\tif (submodule_create_branch(\n-- \ngitgitgadget\n\n"},{"id":"452817","messageId":"64d9b8e0f444a7191c83a69b943c95631270ff5b.1648752601.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.v2.git.1648752601.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] branch --set-upstream-to: be consistent when advising","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-31T18:50:00Z","receivedAt":"2022-03-31T18:50:18Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\n\"git branch --set-upstream-to\" behaves differently when advice is\nenabled/disabled:\n\n| | error prefix | exit code |\n|-----------------+--------------+-----------|\n| advice enabled | error: | 1 |\n| advice disabled | fatal: | 128 |\n\nMake both cases consistent by using die_message() when advice is\nenabled (this was first proposed in [1]).\n\n[1] https://lore.kernel.org/git/211210.86ee6ldwlc.gmgdl@evledraar.gmail.com\n\nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Glen Choo \n---\n branch.c | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 1252b17b980..ca7f0c3adf8 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -388,12 +388,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n \treal_ref = NULL;\n \tif (get_oid_mb(start_name, &oid)) {\n \t\tif (explicit_tracking) {\n-\t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n-\t\t\t\terror(_(upstream_missing), start_name);\n-\t\t\t\tadvise(_(upstream_advice));\n-\t\t\t\texit(1);\n-\t\t\t}\n-\t\t\tdie(_(upstream_missing), start_name);\n+\t\t\tint code = die_message(_(upstream_missing), start_name);\n+\t\t\tadvise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,\n+\t\t\t\t\t _(upstream_advice));\n+\t\t\texit(code);\n \t\t}\n \t\tdie(_(\"not a valid object name: '%s'\"), start_name);\n \t}\n-- \ngitgitgadget\n\n"},{"id":"452818","messageId":"306858373ccda2da80fe13fe3113ed793db56e91.1648752601.git.gitgitgadget@gmail.com","threadId":"57632","inReplyTo":"pull.1190.v2.git.1648752601.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] branch: remove negative exit code","fromName":"Glen Choo via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-03-31T18:50:01Z","receivedAt":"2022-03-31T18:50:28Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Glen Choo \n\nReplace an instance of \"exit(-1)\" with \"exit(1)\". We don't use negative\nexit codes - they are misleading because Unix machines will coerce them\nto 8-bit unsigned values, losing the sign.\n\nSigned-off-by: Glen Choo \n---\n branch.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/branch.c b/branch.c\nindex ca7f0c3adf8..581afd634da 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -263,7 +263,7 @@ static void setup_tracking(const char *new_ref, const char *orig_ref,\n \t\tstring_list_append(tracking.srcs, orig_ref);\n \tif (install_branch_config_multiple_remotes(config_flags, new_ref,\n \t\t\t\ttracking.remote, tracking.srcs) < 0)\n-\t\texit(-1);\n+\t\texit(1);\n \n cleanup:\n \tstring_list_clear(&tracking_srcs, 0);\n-- \ngitgitgadget\n"},{"id":"452845","messageId":"xmqq7d89zqys.fsf@gitster.g","threadId":"57632","inReplyTo":"pull.1190.v2.git.1648752601.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/4] branch --recurse-submodules: Bug fixes and clean ups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-31T22:39:55Z","receivedAt":"2022-03-31T22:40:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Glen Choo via GitGitGadget\" writes:\n\n> Thanks for the feedback, all. This version incorporates most of the\n> suggestions (which were pretty small anyway).\n>\n> == Patch organization\n>\n> Patches 1-2 are bugfixes, 3-4 are clean ups.\n>\n> Patch 1 fixes a bug where \"git branch --recurse-submodules\" would not\n> propagate the \"--track\" option if its value was \"--no-track\" or\n> \"--track=inherit\".\n>\n> Patch 2 fixes a bug where \"git branch --recurse-submodules\" would give\n> advice before telling the user what the problem is (instead of the other way\n> around).\n>\n> Patch 3 fixes some old inconsistencies when \"git branch --set-upstream-to\"\n> gives advice and when it doesn't.\n>\n> Patch 4 replaces exit(-1) with exit(1).\n>\n> == Changes\n>\n> Since v1:\n>\n> * Patch 1: reword the --track comments to be prescriptive\n> * Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to\n> inline the advice string to save reviewers the trouble of proofreading\n> (and the format string has no placeholders anyway, so I don't think we'd\n> get much benefit out of typechecking). We can inline it in another\n> series.\n\nThanks, but sorry that I've already merged the previous round. Let\nme turn them into incrementals.\n\n"},{"id":"452846","messageId":"20220331224118.3014407-1-gitster@pobox.com","threadId":"57632","inReplyTo":"xmqq7d89zqys.fsf@gitster.g","subject":"[PATCH 1/2] branch: rework comments for future developers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-31T22:41:17Z","receivedAt":"2022-03-31T22:41:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Glen Choo \n\nFor two cases in which we do not explicitly pass --track=\noption down to the submodule--helper subprocess, we have comments\nthat say \"we do not have to pass --track\", but in fact we not just\ndo not have to, but it would be incorrect to pass any --track option\nto the subprocess (instead, the correct behaviour is to let the\nsubprocess figure out what is the appropriate tracking mode to use).\n\nSigned-off-by: Glen Choo \n---\n branch.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex ed6f993aa6..8ee9f43539 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -549,9 +549,9 @@ static int submodule_create_branch(struct repository *r,\n \t\tstrvec_push(&child.args, \"--track=inherit\");\n \t\tbreak;\n \tcase BRANCH_TRACK_UNSPECIFIED:\n-\t\t/* Default for \"git checkout\". No need to pass --track. */\n+\t\t/* Default for \"git checkout\". Do not pass --track. */\n \tcase BRANCH_TRACK_REMOTE:\n-\t\t/* Default for \"git branch\". No need to pass --track. */\n+\t\t/* Default for \"git branch\". Do not pass --track. */\n \t\tbreak;\n \t}\n \n-- \n2.35.1-917-g7c4048624a\n\n"},{"id":"452847","messageId":"20220331224118.3014407-2-gitster@pobox.com","threadId":"57632","inReplyTo":"20220331224118.3014407-1-gitster@pobox.com","subject":"[PATCH 2/2] branch.c: simplify advice-and-die sequence","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-03-31T22:41:18Z","receivedAt":"2022-03-31T22:41:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: Glen Choo \n\nIn the dwim_branch_start(), when we cannot find an appropriate\nupstream, we will die with the same message anyway, whether we\nissue an advice message.\n\nFlip the code around a bit and simplify the flow using\nadvise_if_enabled() function.\n\nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Glen Choo \n---\n branch.c | 11 ++++-------\n 1 file changed, 4 insertions(+), 7 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 8ee9f43539..b673143cbe 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -383,13 +383,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n \treal_ref = NULL;\n \tif (get_oid_mb(start_name, &oid)) {\n \t\tif (explicit_tracking) {\n-\t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n-\t\t\t\tint code = die_message(_(upstream_missing),\n-\t\t\t\t\t\t start_name);\n-\t\t\t\tadvise(_(upstream_advice));\n-\t\t\t\texit(code);\n-\t\t\t}\n-\t\t\tdie(_(upstream_missing), start_name);\n+\t\t\tint code = die_message(_(upstream_missing), start_name);\n+\t\t\tadvise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,\n+\t\t\t\t\t _(upstream_advice));\n+\t\t\texit(code);\n \t\t}\n \t\tdie(_(\"Not a valid object name: '%s'.\"), start_name);\n \t}\n-- \n2.35.1-917-g7c4048624a\n\n"},{"id":"452861","messageId":"220401.86lewpnlie.gmgdl@evledraar.gmail.com","threadId":"57632","inReplyTo":"20220331224118.3014407-2-gitster@pobox.com","subject":"Re: [PATCH 2/2] branch.c: simplify advice-and-die sequence","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-04-01T10:03:03Z","receivedAt":"2022-04-01T10:30:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Mar 31 2022, Junio C Hamano wrote:\n\n> From: Glen Choo \n>\n> In the dwim_branch_start(), when we cannot find an appropriate\n> upstream, we will die with the same message anyway, whether we\n> issue an advice message.\n>\n> Flip the code around a bit and simplify the flow using\n> advise_if_enabled() function.\n>\n> Helped-by: Ævar Arnfjörð Bjarmason \n> Signed-off-by: Glen Choo \n> ---\n> branch.c | 11 ++++-------\n> 1 file changed, 4 insertions(+), 7 deletions(-)\n>\n> diff --git a/branch.c b/branch.c\n> index 8ee9f43539..b673143cbe 100644\n> --- a/branch.c\n> +++ b/branch.c\n> @@ -383,13 +383,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n> \treal_ref = NULL;\n> \tif (get_oid_mb(start_name, &oid)) {\n> \t\tif (explicit_tracking) {\n> -\t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n> -\t\t\t\tint code = die_message(_(upstream_missing),\n> -\t\t\t\t\t\t start_name);\n> -\t\t\t\tadvise(_(upstream_advice));\n> -\t\t\t\texit(code);\n> -\t\t\t}\n> -\t\t\tdie(_(upstream_missing), start_name);\n> +\t\t\tint code = die_message(_(upstream_missing), start_name);\n> +\t\t\tadvise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,\n> +\t\t\t\t\t _(upstream_advice));\n> +\t\t\texit(code);\n> \t\t}\n> \t\tdie(_(\"Not a valid object name: '%s'.\"), start_name);\n> \t}\n\nLooks good, in case you missed it I noted that we could also get extra\nhelp from the compiler by folding the \"upstream_missing\" variable inline\nas an argument to die_message(), likewise for upstream_advice.\n\nI.e. the latter already had only one user, but the former was used in\ntwo places (now one).\n\nThat was mainly for getting more compiler aid with format checking,\nalthough the readibility of avoiding the indirection is arguably a good\nreason to do it.\n\nBut I see from testing just now that I was (mostly) wrong about\nthat. I.e. if we do:\n\t\n\tdiff --git a/branch.c b/branch.c\n\tindex 6b31df539a5..8a704c3ff53 100644\n\t--- a/branch.c\n\t+++ b/branch.c\n\t@@ -339,8 +339,7 @@ static int validate_remote_tracking_branch(char *ref)\n\t \n\t static const char upstream_not_branch[] =\n\t N_(\"cannot set up tracking information; starting point '%s' is not a branch\");\n\t-static const char upstream_missing[] =\n\t-N_(\"the requested upstream branch '%s' does not exist\");\n\t+static const char upstream_missing[] = \"blah %s blah %s\";\n\t static const char upstream_advice[] =\n\t N_(\"\\n\"\n\t \"If you are planning on basing your work on an upstream\\n\n\nBoth my local gcc and clang will warn on it. What I was recalling is\nthat if you try these variants:\n\t\n\tdiff --git a/branch.c b/branch.c\n\tindex 6b31df539a5..eea50605c8c 100644\n\t--- a/branch.c\n\t+++ b/branch.c\n\t@@ -339,8 +339,6 @@ static int validate_remote_tracking_branch(char *ref)\n\t \n\t static const char upstream_not_branch[] =\n\t N_(\"cannot set up tracking information; starting point '%s' is not a branch\");\n\t-static const char upstream_missing[] =\n\t-N_(\"the requested upstream branch '%s' does not exist\");\n\t static const char upstream_advice[] =\n\t N_(\"\\n\"\n\t \"If you are planning on basing your work on an upstream\\n\"\n\t@@ -384,6 +382,10 @@ static void dwim_branch_start(struct repository *r, const char *start_name,\n\t \n\t \treal_ref = NULL;\n\t \tif (get_oid_mb(start_name, &oid)) {\n\t+\t\tchar *upstream_missing = \"blah %s blah %s\";\n\t+\t\tconst char *upstream_missing = \"blah %s blah %s\";\n\t+\t\tconst char *const upstream_missing = \"blah %s blah %s\";\n\t+\t\tstatic const char upstream_missing[] = \"blah %s blah %s\";\n\t \t\tif (explicit_tracking) {\n\t \t\t\tif (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {\n\t \t\t\t\terror(_(upstream_missing), start_name);\n\nMy clang version starts warning on the 3rd one (const char *const), but\nit takes until the 4th one (static const char ... []) until GCC will\nwarn on it.\n\nWe've had bugs in the past where we passed some variants of that to a\nformat-expecting function.\n\nSo I think that's a good argument for wider use of \"const char *const\"\nin some cases over \"const char *\" if possible. E.g. if I do this:\n\t\n\tdiff --git a/sequencer.c b/sequencer.c\n\tindex a1bb39383db..cd8f59c2a2d 100644\n\t--- a/sequencer.c\n\t+++ b/sequencer.c\n\t@@ -3035,7 +3035,6 @@ static int create_seq_dir(struct repository *r)\n\t {\n\t \tenum replay_action action;\n\t \tconst char *in_progress_error = NULL;\n\t-\tconst char *in_progress_advice = NULL;\n\t \tunsigned int advise_skip =\n\t \t\trefs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\") ||\n\t \t\trefs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\");\n\t@@ -3044,19 +3043,19 @@ static int create_seq_dir(struct repository *r)\n\t \t\tswitch (action) {\n\t \t\tcase REPLAY_REVERT:\n\t \t\t\tin_progress_error = _(\"revert is already in progress\");\n\t-\t\t\tin_progress_advice =\n\t-\t\t\t_(\"try \\\"git revert (--continue | %s--abort | --quit)\\\"\");\n\t \t\t\tbreak;\n\t \t\tcase REPLAY_PICK:\n\t \t\t\tin_progress_error = _(\"cherry-pick is already in progress\");\n\t-\t\t\tin_progress_advice =\n\t-\t\t\t_(\"try \\\"git cherry-pick (--continue | %s--abort | --quit)\\\"\");\n\t \t\t\tbreak;\n\t \t\tdefault:\n\t \t\t\tBUG(\"unexpected action in create_seq_dir\");\n\t \t\t}\n\t \t}\n\t \tif (in_progress_error) {\n\t+\t\tconst char *in_progress_advice = action == REPLAY_REVERT\n\t+\t\t\t? _(\"try \\\"git cherry-pick (--continue | %s--abort | --quit)\\\" %s\")\n\t+\t\t\t: _(\"try \\\"git revert (--continue | %s--abort | --quit)\\\" %s\");\n\t+\n\t \t\terror(\"%s\", in_progress_error);\n\t \t\tif (advice_enabled(ADVICE_SEQUENCER_IN_USE))\n\t \t\t\tadvise(in_progress_advice,\n\nNeither clang nor gcc will detect that I snuck in an extra %s into the\nmessage, which would be a runtime error, but make that \"const char\n*const\" and clang will spot it, but gcc won't.\n\nThis was with Debian clang version 13.0.1-3+b2 & gcc (GCC) 12.0.1\n20220120 (experimental). I also tested this last one on gcc (Debian\n10.2.1-6) 10.2.1 20210110. YMMV.\n"},{"id":"452908","messageId":"kl6lk0c8spt4.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"57632","inReplyTo":"xmqq7d89zqys.fsf@gitster.g","subject":"Re: [PATCH v2 0/4] branch --recurse-submodules: Bug fixes and clean ups","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-04-01T16:59:03Z","receivedAt":"2022-04-01T16:59:08Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Junio C Hamano writes:\n\n> \"Glen Choo via GitGitGadget\" writes:\n>\n>> Thanks for the feedback, all. This version incorporates most of the\n>> suggestions (which were pretty small anyway).\n>>\n>> == Patch organization\n>>\n>> Patches 1-2 are bugfixes, 3-4 are clean ups.\n>>\n>> Patch 1 fixes a bug where \"git branch --recurse-submodules\" would not\n>> propagate the \"--track\" option if its value was \"--no-track\" or\n>> \"--track=inherit\".\n>>\n>> Patch 2 fixes a bug where \"git branch --recurse-submodules\" would give\n>> advice before telling the user what the problem is (instead of the other way\n>> around).\n>>\n>> Patch 3 fixes some old inconsistencies when \"git branch --set-upstream-to\"\n>> gives advice and when it doesn't.\n>>\n>> Patch 4 replaces exit(-1) with exit(1).\n>>\n>> == Changes\n>>\n>> Since v1:\n>>\n>> * Patch 1: reword the --track comments to be prescriptive\n>> * Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to\n>> inline the advice string to save reviewers the trouble of proofreading\n>> (and the format string has no placeholders anyway, so I don't think we'd\n>> get much benefit out of typechecking). We can inline it in another\n>> series.\n>\n> Thanks, but sorry that I've already merged the previous round. Let\n> me turn them into incrementals.\n\nAh whoops, sorry I didn't send them out sooner.\n\nThanks for that :)\n"},{"id":"452919","messageId":"xmqqh77cv9wc.fsf@gitster.g","threadId":"57632","inReplyTo":"kl6lk0c8spt4.fsf@chooglen-macbookpro.roam.corp.google.com","subject":"Re: [PATCH v2 0/4] branch --recurse-submodules: Bug fixes and clean ups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-04-01T20:14:27Z","receivedAt":"2022-04-01T20:14:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Glen Choo writes:\n\n>> Thanks, but sorry that I've already merged the previous round. Let\n>> me turn them into incrementals.\n>\n> Ah whoops, sorry I didn't send them out sooner.\n\nNo, it was me this time. I have prepared 'next' long in advance so\nI could have redone it if I delayed pushing the result out, but in\nany case, we are working asynchronously and these things are bound\nto happen.\n\n> Thanks for that :)\n\nThanks for working on the topic.\n"}]}