{"thread":{"id":"66206","subject":"[PATCH 0/2] branch: fix --recurse-submodules with a nameless start point","startedAt":"2026-08-21T23:02:19Z","lastAt":"2026-08-21T23:02:43Z","messageCount":3,"participants":["Volodymyr Vriukalo"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"551041","messageId":"20260822-vv-branch-recurse-no-start-ref-v1-0-46dc140acaa8@zitro.id","threadId":"66206","inReplyTo":null,"subject":"[PATCH 0/2] branch: fix --recurse-submodules with a nameless start point","fromName":"Volodymyr Vriukalo","fromEmail":"0@zitro.id","sentAt":"2026-08-21T23:01:40Z","receivedAt":"2026-08-21T23:02:19Z","isPatch":true,"body":"vv/branch-recurse-no-start-ref\n\n\"git branch\" with submodule.propagateBranches enabled mishandled a\nstart point that names no ref, such as a raw object id.  In a\nrepository with no submodules it aborted with a BUG after having\nalready moved the branch; with submodules it failed earlier, when the\nhelper it invokes rejected a truncated argument list.  Both are the\nsame missing tracking name, which has been corrected.\n\nI hit the first of these while scripting a branch rewrite -- the sort\nthat moves a branch to a commit no ref points at yet, which for such a\nscript is the ordinary case rather than the exception:\n\n    BUG: refspec.c:442: refspec_find_match: need either src or dst\n    Aborted (core dumped)\n\nThe ref had already been updated.  So the abort is loud and the damage\nis quiet, which is the wrong way round: an exit status of 134 on an\noperation that in fact completed will fool any caller that checks it,\nand one whose error path rolls back will helpfully undo a successful\nupdate.\n\nThe exposure is bounded.  submodule.propagateBranches is documented as\nexperimental, and reproducing it needs all four of that setting,\nsubmodule.recurse, a configured remote, and a start point that is not a\nref name; drop any one and the command succeeds.  Within those bounds\nit is not exotic -- a script that moves a branch by object id hits it\non the first attempt, the commit it wants having no ref on it yet,\nwhich is rather the point of moving a branch.  It reproduces\nidentically on 2.54.0, 2.55.0 and master, which is why this is based on\nmaint.\n\nBoth patches guard at the call site rather than inside the callee.\nThat follows create_branch(), which already declines the same value\nwith \"if (real_ref && track)\", and it leaves setup_tracking()'s own\nconvention intact: it opens by BUG()ing on a caller that should not\nhave called it, so absorbing a NULL quietly would contradict that four\nlines later.  Neither patch changes anything for a start point that\ndoes name a ref.\n\nThis series was written with LLM assistance, recorded as an\n\"Assisted-by: An LLM.\" trailer on both patches.  I used it for the\nwhole change rather than as step-by-step guidance, because at this\nsize the distinction is irrelevant: two guards and two tests, short\nenough to read in full.  I have read the surrounding code and checked\nevery claim in the commit messages against the source myself.\n\nTwo things I would welcome direction on.\n\ndwim_and_setup_tracking() carries the same unguarded call.  It survives\nonly because its single caller passes BRANCH_TRACK_OVERRIDE, under\nwhich dwim_branch_start() dies rather than returning NULL -- safe by an\nargument its caller happens to pass, not by anything in the function\nitself.  Nothing enforces that, so a future caller could reintroduce\nthe same abort.  I left it alone to keep the series to the bug I\nactually hit, but I am happy to add a third patch.\n\nThe first test uses no submodule, because the bug does not need one.  I\nput it in t3207 since create_branches_recursively() is only reached\nunder submodule.propagateBranches, but t3200 is a perfectly defensible\nhome and I will move it on request.\n\nThe series merges cleanly into next and seen; both merge results build\nand pass t3200 and t3207, and the full suite passes on the topic\nitself.\n\n---\nVolodymyr Vriukalo (2):\n      branch: do not track a start point with no ref\n      branch: allow recursion with no tracking name\n\n branch.c                    | 12 ++++++++++--\n builtin/submodule--helper.c |  7 ++++---\n t/t3207-branch-submodule.sh | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 44 insertions(+), 5 deletions(-)\n---\nbase-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc\nchange-id: 20260822-vv-branch-recurse-no-start-ref-31fac1e34eab\n\nBest regards,\n--  \nVolodymyr Vriukalo <0@zitro.id>\n\n"},{"id":"551042","messageId":"20260822-vv-branch-recurse-no-start-ref-v1-1-46dc140acaa8@zitro.id","threadId":"66206","inReplyTo":"20260822-vv-branch-recurse-no-start-ref-v1-0-46dc140acaa8@zitro.id","subject":"[PATCH 1/2] branch: do not track a start point with no ref","fromName":"Volodymyr Vriukalo","fromEmail":"0@zitro.id","sentAt":"2026-08-21T23:01:41Z","receivedAt":"2026-08-21T23:02:31Z","isPatch":true,"body":"Forcing a branch to a commit that no ref points at aborts when both\n  `submodule.recurse` and `submodule.propagateBranches` are set and\n  the repository has a remote configured:\n\n    BUG: refspec.c:442: refspec_find_match: need either src or dst\n    Aborted (core dumped)\n\n`create_branches_recursively()` resolves the start point through\n  `dwim_branch_start()`, which leaves `branch_point` NULL when the\n  start point names no ref -- an object id, or a revision expression\n  such as `HEAD~0`.  That NULL becomes `tracking_name`, and the\n  `setup_tracking()` call below it is guarded on `track` alone.\n  `setup_tracking()` assigns it to `tracking.spec.dst` without\n  checking, then hands the spec to `for_each_remote()`, so\n  `refspec_find_match()` receives a query with neither src nor dst\n  and trips its assertion.\n`for_each_remote()` never reaches that callback where no remote is\n  configured, which is why the abort needs one.\n\n961b130d20 (branch: add --recurse-submodules option for branch\n  creation, 2022-01-28) added the call with no guard at all.\n75388bf5b4 (branch: support more tracking modes when recursing,\n  2022-03-29) added the guard on `track`.\n\nUpdating the branch happens before the abort, so the command does\n  what was asked and then exits 134.\nCallers that check the exit status therefore see a failure that did\n  not happen, and one that rolls back on failure would undo a\n  successful update.\n\n`create_branch()` already declines this: it calls `setup_tracking()`\n  under `if (real_ref && track)`, leaving tracking unset when the\n  start point resolved to no ref.\nMake the recursive path agree.\nChecking for NULL inside `setup_tracking()` would also silence the\n  abort, but it would put the decision in the callee for one caller\n  that has the answer already, and leave the two creation paths\n  disagreeing about when tracking is set up.\n\nReproducing it needs all four of:\n\n  - `submodule.recurse=true`\n  - `submodule.propagateBranches=true`\n  - a configured remote\n  - a start point that is not a ref name\n\nSubmodules take no part, so the new test builds a repository with\n  neither a submodule nor a `.gitmodules`, where `propagateBranches`\n  is set and has nothing to propagate to.\n\nAssisted-by: An LLM.\nSigned-off-by: Volodymyr Vriukalo <0@zitro.id>\n---\n branch.c                    |  2 +-\n t/t3207-branch-submodule.sh | 17 +++++++++++++++++\n 2 files changed, 18 insertions(+), 1 deletion(-)\n\ndiff --git a/branch.c b/branch.c\nindex 243db7d0fc..182fc4a3dd 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -806,7 +806,7 @@ 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-\tif (track)\n+\tif (tracking_name && track)\n \t\tsetup_tracking(name, tracking_name, track, quiet);\n \n \tfor (i = 0; i < submodule_entry_list.entry_nr; i++) {\ndiff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh\nindex fe72b24716..54f7caeb2f 100755\n--- a/t/t3207-branch-submodule.sh\n+++ b/t/t3207-branch-submodule.sh\n@@ -98,6 +98,23 @@ test_expect_success 'should respect submodule.recurse when creating branches' '\n \t)\n '\n \n+test_expect_success 'should move a branch to a start point that names no ref' '\n+\ttest_when_finished \"rm -rf no-submodules\" &&\n+\tgit init no-submodules &&\n+\t(\n+\t\tcd no-submodules &&\n+\t\ttest_commit one &&\n+\t\ttest_commit two &&\n+\t\tgit remote add origin . &&\n+\t\tgit config submodule.propagateBranches true &&\n+\t\tgit config submodule.recurse true &&\n+\t\tgit branch branch-a HEAD~1 &&\n+\t\toid=$(git rev-parse HEAD) &&\n+\t\tgit branch -f branch-a \"$oid\" &&\n+\t\ttest_cmp_rev HEAD branch-a\n+\t)\n+'\n+\n test_expect_success 'should ignore submodule.recurse when not creating branches' '\n \ttest_when_finished \"reset_test\" &&\n \t(\n\n-- \n2.55.0.2.g927b4b9963\n\n"},{"id":"551043","messageId":"20260822-vv-branch-recurse-no-start-ref-v1-2-46dc140acaa8@zitro.id","threadId":"66206","inReplyTo":"20260822-vv-branch-recurse-no-start-ref-v1-0-46dc140acaa8@zitro.id","subject":"[PATCH 2/2] branch: allow recursion with no tracking name","fromName":"Volodymyr Vriukalo","fromEmail":"0@zitro.id","sentAt":"2026-08-21T23:01:42Z","receivedAt":"2026-08-21T23:02:43Z","isPatch":true,"body":"Creating a branch across submodules from a commit that no ref points\n  at fails, with the helper's usage text reprinted as an error:\n\n    submodule 'sub': usage: git submodule--helper create-branch [...]\n    fatal: submodule 'sub': cannot create branch 'branch-a'\n\n`submodule_create_branch()` runs the helper in a child process because\n  `install_branch_config_multiple_remotes()` cannot write config into a\n  submodule, and passes the branch name, the start oid and the tracking\n  name as three positionals.\n`dwim_branch_start()` leaves the tracking name NULL where the start\n  point named no ref, and `strvec_pushl()` stops at the first NULL, so\n  the child receives two positionals.\n`module_create_branch()` requires exactly three and prints its usage.\n\nMake the third positional optional, since a start point that named no\n  ref has no tracking name to give and the recursion has nothing to\n  track in the submodule either.\nPush it separately in the caller too: relying on `strvec_pushl()` to\n  stop early leaves the argument dropped by accident rather than by\n  intent, and a reader has to know where the terminator falls to see\n  that it can go missing at all.\n\n961b130d20 (branch: add --recurse-submodules option for branch\n  creation, 2022-01-28) introduced both sides.\n\nThis is the same NULL tracking name as the previous patch, reached one\n  step earlier: the dry-run pass over the submodules runs before the\n  superproject's own `setup_tracking()` call, so with a submodule\n  present this failure hides the abort that patch removes.\nThe new test therefore needs that patch under it.\n\nAssisted-by: An LLM.\nSigned-off-by: Volodymyr Vriukalo <0@zitro.id>\n---\n branch.c                    | 10 +++++++++-\n builtin/submodule--helper.c |  7 ++++---\n t/t3207-branch-submodule.sh | 13 +++++++++++++\n 3 files changed, 26 insertions(+), 4 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 182fc4a3dd..2dab1f1e35 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -726,7 +726,15 @@ static int submodule_create_branch(struct repository *r,\n \t\tbreak;\n \t}\n \n-\tstrvec_pushl(&child.args, name, start_oid, tracking_name, NULL);\n+\t/*\n+\t * The tracking name is absent when the start point named no ref.\n+\t * Push it separately: strvec_pushl() stops at the first NULL, so\n+\t * passing it inline would drop the argument by accident rather\n+\t * than by intent.\n+\t */\n+\tstrvec_pushl(&child.args, name, start_oid, NULL);\n+\tif (tracking_name)\n+\t\tstrvec_push(&child.args, tracking_name);\n \n \tif ((ret = start_command(&child)))\n \t\treturn ret;\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 1cc82a134d..6895216712 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -3335,7 +3335,7 @@ static int module_create_branch(int argc, const char **argv, const char *prefix,\n \t\tOPT_END()\n \t};\n \tconst char *const usage[] = {\n-\t\tN_(\"git submodule--helper create-branch [-f|--force] [--create-reflog] [-q|--quiet] [-t|--track] [-n|--dry-run] <name> <start-oid> <start-name>\"),\n+\t\tN_(\"git submodule--helper create-branch [-f|--force] [--create-reflog] [-q|--quiet] [-t|--track] [-n|--dry-run] <name> <start-oid> [<start-name>]\"),\n \t\tNULL\n \t};\n \tstruct repo_config_values *cfg = repo_config_values(the_repository);\n@@ -3344,13 +3344,14 @@ static int module_create_branch(int argc, const char **argv, const char *prefix,\n \ttrack = cfg->branch_track;\n \targc = parse_options(argc, argv, prefix, options, usage, 0);\n \n-\tif (argc != 3)\n+\tif (argc < 2 || argc > 3)\n \t\tusage_with_options(usage, options);\n \n \tif (!quiet && !dry_run)\n \t\tprintf_ln(_(\"creating branch '%s'\"), argv[0]);\n \n-\tcreate_branches_recursively(the_repository, argv[0], argv[1], argv[2],\n+\tcreate_branches_recursively(the_repository, argv[0], argv[1],\n+\t\t\t\t    argc > 2 ? argv[2] : NULL,\n \t\t\t\t    force, reflog, quiet, track, dry_run);\n \treturn 0;\n }\ndiff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh\nindex 54f7caeb2f..c56cea31cb 100755\n--- a/t/t3207-branch-submodule.sh\n+++ b/t/t3207-branch-submodule.sh\n@@ -115,6 +115,19 @@ test_expect_success 'should move a branch to a start point that names no ref' '\n \t)\n '\n \n+test_expect_success 'should recurse into submodules from a start point that names no ref' '\n+\ttest_when_finished \"reset_test\" &&\n+\t(\n+\t\tcd super &&\n+\t\toid=$(git rev-parse HEAD) &&\n+\t\tgit branch --recurse-submodules branch-a \"$oid\" &&\n+\t\tgit rev-parse branch-a &&\n+\t\tgit -C sub rev-parse branch-a &&\n+\t\tgit -C sub/sub-sub rev-parse branch-a &&\n+\t\tgit -C second/sub rev-parse branch-a\n+\t)\n+'\n+\n test_expect_success 'should ignore submodule.recurse when not creating branches' '\n \ttest_when_finished \"reset_test\" &&\n \t(\n\n-- \n2.55.0.2.g927b4b9963\n\n"}]}