Volume XXII, number 279Tuesday, October 6, 2026Latest message 12 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patch, 2 partsbranch: fix --recurse-submodules with a nameless start point

3 messages between Aug 21, 2026 and Aug 21, 2026, from Volodymyr Vriukalo.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Volodymyr VriukaloAug 21, 2026, 23:01 UTC on lore
vv/branch-recurse-no-start-ref

"git branch" with submodule.propagateBranches enabled mishandled a start point that names no ref, such as a raw object id. In a repository with no submodules it aborted with a BUG after having already moved the branch; with submodules it failed earlier, when the helper it invokes rejected a truncated argument list. Both are the same missing tracking name, which has been corrected.

I hit the first of these while scripting a branch rewrite -- the sort that moves a branch to a commit no ref points at yet, which for such a script is the ordinary case rather than the exception:

    BUG: refspec.c:442: refspec_find_match: need either src or dst
    Aborted (core dumped)

The ref had already been updated. So the abort is loud and the damage is quiet, which is the wrong way round: an exit status of 134 on an operation that in fact completed will fool any caller that checks it, and one whose error path rolls back will helpfully undo a successful update.

The exposure is bounded. submodule.propagateBranches is documented as experimental, and reproducing it needs all four of that setting, submodule.recurse, a configured remote, and a start point that is not a ref name; drop any one and the command succeeds. Within those bounds it is not exotic -- a script that moves a branch by object id hits it on the first attempt, the commit it wants having no ref on it yet, which is rather the point of moving a branch. It reproduces identically on 2.54.0, 2.55.0 and master, which is why this is based on maint.

Both patches guard at the call site rather than inside the callee. That follows create_branch(), which already declines the same value with "if (real_ref && track)", and it leaves setup_tracking()'s own convention intact: it opens by BUG()ing on a caller that should not have called it, so absorbing a NULL quietly would contradict that four lines later. Neither patch changes anything for a start point that does name a ref.

This series was written with LLM assistance, recorded as an "Assisted-by: An LLM." trailer on both patches. I used it for the whole change rather than as step-by-step guidance, because at this size the distinction is irrelevant: two guards and two tests, short enough to read in full. I have read the surrounding code and checked every claim in the commit messages against the source myself.

Two things I would welcome direction on.

dwim_and_setup_tracking() carries the same unguarded call. It survives only because its single caller passes BRANCH_TRACK_OVERRIDE, under which dwim_branch_start() dies rather than returning NULL -- safe by an argument its caller happens to pass, not by anything in the function itself. Nothing enforces that, so a future caller could reintroduce the same abort. I left it alone to keep the series to the bug I actually hit, but I am happy to add a third patch.

The first test uses no submodule, because the bug does not need one. I put it in t3207 since create_branches_recursively() is only reached under submodule.propagateBranches, but t3200 is a perfectly defensible home and I will move it on request.

The series merges cleanly into next and seen; both merge results build and pass t3200 and t3207, and the full suite passes on the topic itself.

---
Volodymyr Vriukalo (2):
      branch: do not track a start point with no ref
      branch: allow recursion with no tracking name
 branch.c                    | 12 ++++++++++--
 builtin/submodule--helper.c |  7 ++++---
 t/t3207-branch-submodule.sh | 30 ++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 5 deletions(-)
---
base-commit: e9019fcafe0040228b8631c30f97ae1adb61bcdc
change-id: 20260822-vv-branch-recurse-no-start-ref-31fac1e34eab

Best regards, -- Volodymyr Vriukalo <0@zitro.id>

Volodymyr VriukaloAug 21, 2026, 23:01 UTC in reply to Volodymyr Vriukalo on lore

[PATCH 1/2] branch: do not track a start point with no ref

Forcing a branch to a commit that no ref points at aborts when both
  `submodule.recurse` and `submodule.propagateBranches` are set and
  the repository has a remote configured:
    BUG: refspec.c:442: refspec_find_match: need either src or dst
    Aborted (core dumped)
`create_branches_recursively()` resolves the start point through
  `dwim_branch_start()`, which leaves `branch_point` NULL when the
  start point names no ref -- an object id, or a revision expression
  such as `HEAD~0`.  That NULL becomes `tracking_name`, and the
  `setup_tracking()` call below it is guarded on `track` alone.
  `setup_tracking()` assigns it to `tracking.spec.dst` without
  checking, then hands the spec to `for_each_remote()`, so
  `refspec_find_match()` receives a query with neither src nor dst
  and trips its assertion.
`for_each_remote()` never reaches that callback where no remote is
  configured, which is why the abort needs one.
961b130d20 (branch: add --recurse-submodules option for branch
  creation, 2022-01-28) added the call with no guard at all.
75388bf5b4 (branch: support more tracking modes when recursing,
  2022-03-29) added the guard on `track`.
Updating the branch happens before the abort, so the command does
  what was asked and then exits 134.
Callers that check the exit status therefore see a failure that did
  not happen, and one that rolls back on failure would undo a
  successful update.
`create_branch()` already declines this: it calls `setup_tracking()`
  under `if (real_ref && track)`, leaving tracking unset when the
  start point resolved to no ref.
Make the recursive path agree.
Checking for NULL inside `setup_tracking()` would also silence the
  abort, but it would put the decision in the callee for one caller
  that has the answer already, and leave the two creation paths
  disagreeing about when tracking is set up.
Reproducing it needs all four of:
  - `submodule.recurse=true`
  - `submodule.propagateBranches=true`
  - a configured remote
  - a start point that is not a ref name
Submodules take no part, so the new test builds a repository with
  neither a submodule nor a `.gitmodules`, where `propagateBranches`
  is set and has nothing to propagate to.
Assisted-by: An LLM.
Signed-off-by: Volodymyr Vriukalo <0@zitro.id>
---
 branch.c                    |  2 +-
 t/t3207-branch-submodule.sh | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
Show changes to 2 files +18 −1

branch.c, t/t3207-branch-submodule.sh

diff --git a/branch.c b/branch.c
index 243db7d0fc..182fc4a3dd 100644
--- a/branch.c
+++ b/branch.c
@@ -806,7 +806,7 @@ void create_branches_recursively(struct repository *r, const char *name,
 	 * tedious to determine whether or not tracking was set up in the
 	 * superproject.
 	 */
-	if (track)
+	if (tracking_name && track)
 		setup_tracking(name, tracking_name, track, quiet);
 
 	for (i = 0; i < submodule_entry_list.entry_nr; i++) {
diff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh
index fe72b24716..54f7caeb2f 100755
--- a/t/t3207-branch-submodule.sh
+++ b/t/t3207-branch-submodule.sh
@@ -98,6 +98,23 @@ test_expect_success 'should respect submodule.recurse when creating branches' '
 	)
 '
 
+test_expect_success 'should move a branch to a start point that names no ref' '
+	test_when_finished "rm -rf no-submodules" &&
+	git init no-submodules &&
+	(
+		cd no-submodules &&
+		test_commit one &&
+		test_commit two &&
+		git remote add origin . &&
+		git config submodule.propagateBranches true &&
+		git config submodule.recurse true &&
+		git branch branch-a HEAD~1 &&
+		oid=$(git rev-parse HEAD) &&
+		git branch -f branch-a "$oid" &&
+		test_cmp_rev HEAD branch-a
+	)
+'
+
 test_expect_success 'should ignore submodule.recurse when not creating branches' '
 	test_when_finished "reset_test" &&
 	(
-- 
2.55.0.2.g927b4b9963
Volodymyr VriukaloAug 21, 2026, 23:01 UTC in reply to Volodymyr Vriukalo on lore

[PATCH 2/2] branch: allow recursion with no tracking name

Creating a branch across submodules from a commit that no ref points
  at fails, with the helper's usage text reprinted as an error:
    submodule 'sub': usage: git submodule--helper create-branch [...]
    fatal: submodule 'sub': cannot create branch 'branch-a'
`submodule_create_branch()` runs the helper in a child process because
  `install_branch_config_multiple_remotes()` cannot write config into a
  submodule, and passes the branch name, the start oid and the tracking
  name as three positionals.
`dwim_branch_start()` leaves the tracking name NULL where the start
  point named no ref, and `strvec_pushl()` stops at the first NULL, so
  the child receives two positionals.
`module_create_branch()` requires exactly three and prints its usage.
Make the third positional optional, since a start point that named no
  ref has no tracking name to give and the recursion has nothing to
  track in the submodule either.
Push it separately in the caller too: relying on `strvec_pushl()` to
  stop early leaves the argument dropped by accident rather than by
  intent, and a reader has to know where the terminator falls to see
  that it can go missing at all.
961b130d20 (branch: add --recurse-submodules option for branch
  creation, 2022-01-28) introduced both sides.
This is the same NULL tracking name as the previous patch, reached one
  step earlier: the dry-run pass over the submodules runs before the
  superproject's own `setup_tracking()` call, so with a submodule
  present this failure hides the abort that patch removes.
The new test therefore needs that patch under it.
Assisted-by: An LLM.
Signed-off-by: Volodymyr Vriukalo <0@zitro.id>
---
 branch.c                    | 10 +++++++++-
 builtin/submodule--helper.c |  7 ++++---
 t/t3207-branch-submodule.sh | 13 +++++++++++++
 3 files changed, 26 insertions(+), 4 deletions(-)
Show changes to 3 files +26 −4

branch.c, builtin/submodule--helper.c, t/t3207-branch-submodule.sh

diff --git a/branch.c b/branch.c
index 182fc4a3dd..2dab1f1e35 100644
--- a/branch.c
+++ b/branch.c
@@ -726,7 +726,15 @@ static int submodule_create_branch(struct repository *r,
 		break;
 	}
 
-	strvec_pushl(&child.args, name, start_oid, tracking_name, NULL);
+	/*
+	 * The tracking name is absent when the start point named no ref.
+	 * Push it separately: strvec_pushl() stops at the first NULL, so
+	 * passing it inline would drop the argument by accident rather
+	 * than by intent.
+	 */
+	strvec_pushl(&child.args, name, start_oid, NULL);
+	if (tracking_name)
+		strvec_push(&child.args, tracking_name);
 
 	if ((ret = start_command(&child)))
 		return ret;
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1cc82a134d..6895216712 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -3335,7 +3335,7 @@ static int module_create_branch(int argc, const char **argv, const char *prefix,
 		OPT_END()
 	};
 	const char *const usage[] = {
-		N_("git submodule--helper create-branch [-f|--force] [--create-reflog] [-q|--quiet] [-t|--track] [-n|--dry-run] <name> <start-oid> <start-name>"),
+		N_("git submodule--helper create-branch [-f|--force] [--create-reflog] [-q|--quiet] [-t|--track] [-n|--dry-run] <name> <start-oid> [<start-name>]"),
 		NULL
 	};
 	struct repo_config_values *cfg = repo_config_values(the_repository);
@@ -3344,13 +3344,14 @@ static int module_create_branch(int argc, const char **argv, const char *prefix,
 	track = cfg->branch_track;
 	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
-	if (argc != 3)
+	if (argc < 2 || argc > 3)
 		usage_with_options(usage, options);
 
 	if (!quiet && !dry_run)
 		printf_ln(_("creating branch '%s'"), argv[0]);
 
-	create_branches_recursively(the_repository, argv[0], argv[1], argv[2],
+	create_branches_recursively(the_repository, argv[0], argv[1],
+				    argc > 2 ? argv[2] : NULL,
 				    force, reflog, quiet, track, dry_run);
 	return 0;
 }
diff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh
index 54f7caeb2f..c56cea31cb 100755
--- a/t/t3207-branch-submodule.sh
+++ b/t/t3207-branch-submodule.sh
@@ -115,6 +115,19 @@ test_expect_success 'should move a branch to a start point that names no ref' '
 	)
 '
 
+test_expect_success 'should recurse into submodules from a start point that names no ref' '
+	test_when_finished "reset_test" &&
+	(
+		cd super &&
+		oid=$(git rev-parse HEAD) &&
+		git branch --recurse-submodules branch-a "$oid" &&
+		git rev-parse branch-a &&
+		git -C sub rev-parse branch-a &&
+		git -C sub/sub-sub rev-parse branch-a &&
+		git -C second/sub rev-parse branch-a
+	)
+'
+
 test_expect_success 'should ignore submodule.recurse when not creating branches' '
 	test_when_finished "reset_test" &&
 	(
-- 
2.55.0.2.g927b4b9963

Back to recent threads

[PATCH 0/2] branch: fix --recurse-submodules with a nameless start point | The Git List