diff mbox series

[v3,8/8] clone, submodule update: create and check out branches

Message ID 7cdd6c4184da2d3109498589167f10ecf972edc9.1666988096.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series clone, submodule update: check out submodule branches | expand

Commit Message

Glen Choo Oct. 28, 2022, 8:14 p.m. UTC
From: Glen Choo <chooglen@google.com>

Teach "git submodule update" to:

- create the branch with the same name as the current superproject
  branch when cloning a submodule
- check out that branch (instead of the commit OID) when updating
  the submodule worktree

when submodule branching is enabled (submodule.propagateBranches = true)
on the superproject and a branch is checked out. "git clone
--recurse-submodules" also learns this trick because it is implemented
with "git submodule update --recursive".

This approach of checking out the branch will not result in a dirty
worktree for freshly cloned submodules because we can ensure that the
submodule branch points to the superproject gitlink. In other cases, it
does not work as well, but we can handle them incrementally:

- "git pull --recurse-submodules" merges the superproject tree,
  (changing the gitlink without updating the submodule branches), and
  runs "git submodule update" to update the worktrees, so it is almost
  guaranteed to result in a dirty worktree.

  The implementation of "git pull --recurse-submodules" is likely to
  change drastically as submodule.propagateBranches work progresses
  (e.g. "git merge" learns to recurse in to submodules), and we may be
  able to replace the "git submodule update" invocation, or teach it new
  tricks that make the update behave well.

- The user might make changes to the submodule branch without committing
  them back to superproject. This is primarily affects "git checkout
  --recurse-submodules", since that is the primary way of switching away
  from a branch and leaving behind WIP (as opposed to "git submodule
  update", which is run post-checkout).

  In a future series, "git checkout --recurse-submodules" will learn to
  consider submodule branches. We can introduce appropriate guardrails
  then, e.g. requiring that the superproject working tree is not dirty
  before switching away.

Signed-off-by: Glen Choo <chooglen@google.com>
---
 builtin/submodule--helper.c |  37 ++++++++-
 t/t5617-clone-submodules.sh |  38 +++++++++
 t/t7406-submodule-update.sh | 156 ++++++++++++++++++++++++++++++++++++
 3 files changed, 227 insertions(+), 4 deletions(-)

Comments

Philippe Blain Nov. 8, 2022, 1:53 p.m. UTC | #1
Hi Glen,

Le 2022-10-28 à 16:14, Glen Choo via GitGitGadget a écrit :
> From: Glen Choo <chooglen@google.com>
> 
> Teach "git submodule update" to:
> 
> - create the branch with the same name as the current superproject
>   branch when cloning a submodule
> - check out that branch (instead of the commit OID) when updating
>   the submodule worktree
> 
> when submodule branching is enabled (submodule.propagateBranches = true)
> on the superproject and a branch is checked out. "git clone
> --recurse-submodules" also learns this trick because it is implemented
> with "git submodule update --recursive".
> 
> This approach of checking out the branch will not result in a dirty
> worktree for freshly cloned submodules because we can ensure that the
> submodule branch points to the superproject gitlink. In other cases, it
> does not work as well, but we can handle them incrementally:
> 
> - "git pull --recurse-submodules" merges the superproject tree,
>   (changing the gitlink without updating the submodule branches), and
>   runs "git submodule update" to update the worktrees, so it is almost
>   guaranteed to result in a dirty worktree.

OK, here you mean that 'git -c submodule.propagateBranches=true submodule update'
would just checkout the submodule branch (or do nothing if it's already checked out),
but that branch would most likely not be up to date with the newly updated gitlink
in the superproject, resulting in a dirty worktree.

> 
>   The implementation of "git pull --recurse-submodules" is likely to
>   change drastically as submodule.propagateBranches work progresses
>   (e.g. "git merge" learns to recurse in to submodules), and we may be
>   able to replace the "git submodule update" invocation, or teach it new
>   tricks that make the update behave well.
> 
> - The user might make changes to the submodule branch without committing
>   them back to superproject. This is primarily affects "git checkout
>   --recurse-submodules", since that is the primary way of switching away
>   from a branch and leaving behind WIP (as opposed to "git submodule
>   update", which is run post-checkout).
> 
>   In a future series, "git checkout --recurse-submodules" will learn to
>   consider submodule branches. We can introduce appropriate guardrails
>   then, e.g. requiring that the superproject working tree is not dirty
>   before switching away.
> 
> Signed-off-by: Glen Choo <chooglen@google.com>
> ---
>  builtin/submodule--helper.c |  37 ++++++++-
>  t/t5617-clone-submodules.sh |  38 +++++++++
>  t/t7406-submodule-update.sh | 156 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 227 insertions(+), 4 deletions(-)
> 

--8<--

> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index f094e3d7f36..b749d35f784 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh

--8<--

> +# Test the behavior of an already-cloned submodule.
> +# NEEDSWORK When updating with branches, we always use the branch instead of the
> +# gitlink's OID. This results in some imperfect behavior:
> +#
> +# - If the gitlink's OID disagrees with the branch OID, updating with branches
> +#   may result in a dirty worktree
> +# - If the branch does not exist, the update fails.
> +#
> +# We will reevaluate when "git checkout --recurse-submodules" supports branches
> +# For now, just test for this imperfect behavior.

OK. Maybe it would be a good idea to explicitely flag which tests below have
an imperfect behaviour ? This way it's easier to spot where the NEEDSWORK applies...

--8<--

This following test show the imperfect behaviour:

> +test_expect_success 'branches - correct branch checked out, OIDs disagree' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch --recurse-submodules new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	git -C branch-super-cloned/sub1 checkout new-branch &&
> +	test_commit -C branch-super-cloned/sub1 new-commit &&
> +	git -C branch-super-cloned submodule update &&
> +
> +	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
> +	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
> +	test_clean_submodule ! branch-super-cloned sub1
> +'

--8<--

This one also:

> +
> +test_expect_success 'branches - other branch checked out, correct branch exists, OIDs disagree' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch --recurse-submodules new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	git -C branch-super-cloned/sub1 checkout new-branch &&
> +	test_commit -C branch-super-cloned/sub1 new-commit &&
> +	git -C branch-super-cloned/sub1 checkout main &&
> +	git -C branch-super-cloned submodule update &&
> +
> +	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
> +	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
> +	test_clean_submodule ! branch-super-cloned sub1
> +'
> +

This one I'm not sure about: with this series 'git submodule update' does not create
submodule branches at other times than at clone time, so I think this behviour would not change.
I think this one is really an edge case where the user mixes submodule branching on/off.

> +test_expect_success 'branches - other branch checked out, correct branch does not exist' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	test_must_fail git -C branch-super-cloned submodule update
> +'
> +
>  test_done
Jonathan Tan Nov. 15, 2022, 6:15 p.m. UTC | #2
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> +# Test the behavior of an already-cloned submodule.
> +# NEEDSWORK When updating with branches, we always use the branch instead of the
> +# gitlink's OID. This results in some imperfect behavior:
> +#
> +# - If the gitlink's OID disagrees with the branch OID, updating with branches
> +#   may result in a dirty worktree
> +# - If the branch does not exist, the update fails.
> +#
> +# We will reevaluate when "git checkout --recurse-submodules" supports branches
> +# For now, just test for this imperfect behavior.

I think the rationale for this behavior is as follows:

We want a world in which submodules have branches and Git commands use them
wherever possible. There are a few options for "git submodule update" when the
superproject has a branch checked out:

1. Checkout the branch, ignoring OID (as in this patch).
2. Checkout the branch, erroring out if the OID is wrong.
3. 1 + creating the branch if it does not exist.
4. 2 + creating the branch if it does not exist.
5. Always forcibly create the branch at the gitlink's OID and then checking
   it out.

At this point in the discussion, for a low-level command like "git submodule
update", doing as little as possible makes sense to me, which is 1.

But since we do not automatically create the branch if it does not exist, this
means that we have to do it when we clone the submodule. Our options are:

A. Create only the branch that is checked out in the superproject (as in this
   patch).
B. Create all branches that are present in the superproject.
C. Go back on our previous decision, switching to 3.

My instinct is that we want to maintain, as much as possible, the invariant
that for each branch in the superproject, if the branch tip has a gitlink
pointing to a submodule, that submodule has a branch of the same name. And I
think that this invariant can only be maintained by "git submodule update" if
we use B or C.

> +test_expect_success 'branches - other branch checked out, correct branch exists, OIDs disagree' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch --recurse-submodules new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	git -C branch-super-cloned/sub1 checkout new-branch &&
> +	test_commit -C branch-super-cloned/sub1 new-commit &&
> +	git -C branch-super-cloned/sub1 checkout main &&
> +	git -C branch-super-cloned submodule update &&
> +
> +	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
> +	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
> +	test_clean_submodule ! branch-super-cloned sub1
> +'
> +
> +test_expect_success 'branches - other branch checked out, correct branch does not exist' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	test_must_fail git -C branch-super-cloned submodule update

Can we also check what error message is being printed?
Glen Choo Nov. 22, 2022, 6:44 p.m. UTC | #3
Thanks for the thoughtful response, Jonathan :)

Jonathan Tan <jonathantanmy@google.com> writes:

> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> +# Test the behavior of an already-cloned submodule.
>> +# NEEDSWORK When updating with branches, we always use the branch instead of the
>> +# gitlink's OID. This results in some imperfect behavior:
>> +#
>> +# - If the gitlink's OID disagrees with the branch OID, updating with branches
>> +#   may result in a dirty worktree
>> +# - If the branch does not exist, the update fails.
>> +#
>> +# We will reevaluate when "git checkout --recurse-submodules" supports branches
>> +# For now, just test for this imperfect behavior.
>
> I think the rationale for this behavior is as follows:
>
> We want a world in which submodules have branches and Git commands use them
> wherever possible. There are a few options for "git submodule update" when the
> superproject has a branch checked out:
>
> 1. Checkout the branch, ignoring OID (as in this patch).
> 2. Checkout the branch, erroring out if the OID is wrong.
> 3. 1 + creating the branch if it does not exist.
> 4. 2 + creating the branch if it does not exist.
> 5. Always forcibly create the branch at the gitlink's OID and then checking
>    it out.
>
> At this point in the discussion, for a low-level command like "git submodule
> update", doing as little as possible makes sense to me, which is 1.
>
> But since we do not automatically create the branch if it does not exist, this
> means that we have to do it when we clone the submodule. Our options are:
>
> A. Create only the branch that is checked out in the superproject (as in this
>    patch).
> B. Create all branches that are present in the superproject.
> C. Go back on our previous decision, switching to 3.
>
> My instinct is that we want to maintain, as much as possible, the invariant
> that for each branch in the superproject, if the branch tip has a gitlink
> pointing to a submodule, that submodule has a branch of the same name. And I
> think that this invariant can only be maintained by "git submodule update" if
> we use B or C.

I think C is good to have in this series, though for slightly different
reasons.

I agree that the invariant should be preserved when we check out
branches both in the initial clone and in subsequent checkouts. However,
I don't think that we necessarily need to have all superproject branches
after the initial clone. Even if the submodule only has a single
superproject branch, that's enough to have an ephemeral clone for
writing small changes. We could defer the "all superproject branches"
problem til after we worry about subsequent checkouts (i.e. "git
checkout" with branches).

We can handle "initial clone" and "subsequent checkout" as smaller, more
digestible series as long as the work for "initial clone" doesn't get in
the way of "subsequent checkout". My plan (as of v2) was:

- For the intial clone, create only the checked out superproject branch
  at clone time and check it out (aka A)
- For subsequent checkouts, check out the superproject branch, creating
  it if it does not exist (aka C)

But it doesn't make sense to mix both A _and_ C, since C would already
give us the same result as A, so it probably makes sense to go straight
to C in this series (i.e. only for the initial clone, not subsequent
checkouts). I'll do that in v3.

I prefer C in the long run, since both A and B require that the list of
submodule branches never get out of sync with the superproject, which is
hard to enforce, e.g.:

- The user could create a branch in the superproject without recursing
  in to submodules.
- The user could delete the branch in the submodule.
- (Worst yet) The process that creates branches in the submodule _after_
  creating the branch in the superproject could exit unexpectedly (e.g.
  SIGINT). There is no atomic way to create branches in both repos.

We could create a command that would repair broken branch states ("git
submodule repair"?), but C can self-repair, which avoids this problem
entirely.
Jonathan Tan Nov. 23, 2022, 1:33 a.m. UTC | #4
Glen Choo <chooglen@google.com> writes:
> > 1. Checkout the branch, ignoring OID (as in this patch).
> > 2. Checkout the branch, erroring out if the OID is wrong.
> > 3. 1 + creating the branch if it does not exist.
> > 4. 2 + creating the branch if it does not exist.
> > 5. Always forcibly create the branch at the gitlink's OID and then checking
> >    it out.

[snip]

> > A. Create only the branch that is checked out in the superproject (as in this
> >    patch).
> > B. Create all branches that are present in the superproject.
> > C. Go back on our previous decision, switching to 3.

[snip]

> But it doesn't make sense to mix both A _and_ C, since C would already
> give us the same result as A, so it probably makes sense to go straight
> to C in this series (i.e. only for the initial clone, not subsequent
> checkouts). I'll do that in v3.
> 
> I prefer C in the long run, since both A and B require that the list of
> submodule branches never get out of sync with the superproject, which is
> hard to enforce, e.g.:
 
I discussed this with Glen in-office and Glen pointed out that A is actually
not necessarily redundant with respect to C, since a "git submodule add" may
clone a submodule, but it would not run "git submodule update" (so 3 + C would
mean that no branch is created in the submodule, which is not what we want). So
we still need A.

As for 1 vs. 3, we will still need 3 in the future for the reasons I described
in my previous e-mail, but I think that that can be done incrementally. My
concern is to avoid doing something in a patch set that we will later need to
undo; I think that we are indeed avoiding it here (we're doing A but we will
still need it in the future, so there is no undoing of A needed).

So overall, after this discussion, this patch set looks good to me, except for
the minor points that I have commented on in my previous emails.
Junio C Hamano Nov. 23, 2022, 4 a.m. UTC | #5
Jonathan Tan <jonathantanmy@google.com> writes:

> ... My
> concern is to avoid doing something in a patch set that we will later need to
> undo; I think that we are indeed avoiding it here (we're doing A but we will
> still need it in the future, so there is no undoing of A needed).
>
> So overall, after this discussion, this patch set looks good to me, except for
> the minor points that I have commented on in my previous emails.

Thanks for a summary.
diff mbox series

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ef76a111c7f..767a0c81cde 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1917,6 +1917,7 @@  static void submodule_update_clone_release(struct submodule_update_clone *suc)
 struct update_data {
 	const char *prefix;
 	char *displaypath;
+	const char *super_branch;
 	enum submodule_update_type update_default;
 	struct string_list references;
 	struct submodule_update_strategy update_strategy;
@@ -2091,6 +2092,11 @@  static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
 		strvec_push(&child->args, suc->update_data->single_branch ?
 					      "--single-branch" :
 					      "--no-single-branch");
+	if (ud->super_branch) {
+		strvec_pushf(&child->args, "--branch=%s", ud->super_branch);
+		strvec_pushf(&child->args, "--branch-oid=%s",
+			     oid_to_hex(&ce->oid));
+	}
 
 cleanup:
 	free(displaypath);
@@ -2254,9 +2260,14 @@  static int fetch_in_submodule(const char *module_path, int depth, int quiet,
 static int run_update_command(const struct update_data *ud, int subforce)
 {
 	struct child_process cp = CHILD_PROCESS_INIT;
-	const char *update_target = oid_to_hex(&ud->oid);;
+	const char *update_target;
 	int ret;
 
+	if (ud->update_strategy.type == SM_UPDATE_CHECKOUT && ud->super_branch)
+		update_target = ud->super_branch;
+	else
+		update_target = oid_to_hex(&ud->oid);
+
 	switch (ud->update_strategy.type) {
 	case SM_UPDATE_CHECKOUT:
 		cp.git_cmd = 1;
@@ -2523,6 +2534,7 @@  static int update_submodule(struct update_data *update_data)
 	int submodule_up_to_date;
 	int ret;
 	struct object_id suboid;
+	const char *submodule_head = NULL;
 
 	ret = determine_submodule_update_strategy(the_repository,
 						  update_data->just_cloned,
@@ -2533,7 +2545,8 @@  static int update_submodule(struct update_data *update_data)
 		return ret;
 
 	if (!update_data->just_cloned &&
-	    resolve_gitlink_ref(update_data->sm_path, "HEAD", &suboid, NULL))
+	    resolve_gitlink_ref(update_data->sm_path, "HEAD", &suboid,
+				&submodule_head))
 		return die_message(_("Unable to find current revision in submodule path '%s'"),
 				   update_data->displaypath);
 
@@ -2568,8 +2581,17 @@  static int update_submodule(struct update_data *update_data)
 		free(remote_ref);
 	}
 
-	submodule_up_to_date = !update_data->just_cloned &&
-		oideq(&update_data->oid, &suboid);
+	if (update_data->just_cloned)
+		submodule_up_to_date = 0;
+	else if (update_data->super_branch)
+		/* Check that the submodule's HEAD points to super_branch. */
+		submodule_up_to_date =
+			skip_prefix(submodule_head, "refs/heads/",
+				    &submodule_head) &&
+			!strcmp(update_data->super_branch, submodule_head);
+	else
+		submodule_up_to_date = oideq(&update_data->oid, &suboid);
+
 	if (!submodule_up_to_date || update_data->force) {
 		ret = run_update_procedure(update_data);
 		if (ret)
@@ -2603,6 +2625,12 @@  static int update_submodules(struct update_data *update_data)
 	int i, ret = 0;
 	struct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;
 
+	if (the_repository->settings.submodule_propagate_branches) {
+		struct branch *current_branch = branch_get(NULL);
+		if (current_branch)
+			update_data->super_branch = current_branch->name;
+	}
+
 	suc.update_data = update_data;
 	run_processes_parallel_tr2(suc.update_data->max_jobs, update_clone_get_next_task,
 				   update_clone_start_failure,
@@ -2718,6 +2746,7 @@  static int module_update(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, module_update_options,
 			     git_submodule_helper_usage, 0);
+	prepare_repo_settings(the_repository);
 
 	if (opt.require_init)
 		opt.init = 1;
diff --git a/t/t5617-clone-submodules.sh b/t/t5617-clone-submodules.sh
index c43a5b26fab..43f9b52bd44 100755
--- a/t/t5617-clone-submodules.sh
+++ b/t/t5617-clone-submodules.sh
@@ -13,10 +13,17 @@  test_expect_success 'setup' '
 	git config --global protocol.file.allow always &&
 	git checkout -b main &&
 	test_commit commit1 &&
+	mkdir subsub &&
+	(
+		cd subsub &&
+		git init &&
+		test_commit subsubcommit1
+	) &&
 	mkdir sub &&
 	(
 		cd sub &&
 		git init &&
+		git submodule add "file://$pwd/subsub" subsub &&
 		test_commit subcommit1 &&
 		git tag sub_when_added_to_super &&
 		git branch other
@@ -107,4 +114,35 @@  test_expect_success '--no-also-filter-submodules overrides clone.filterSubmodule
 	test_cmp_config -C super_clone3/sub false --default false remote.origin.promisor
 '
 
+test_expect_success 'submodule.propagateBranches checks out branches at correct commits' '
+	test_when_finished "git checkout main" &&
+
+	git checkout -b checked-out &&
+	git -C sub checkout -b not-in-clone &&
+	git -C subsub checkout -b not-in-clone &&
+	git clone --recurse-submodules \
+		--branch checked-out \
+		-c submodule.propagateBranches=true \
+		"file://$pwd/." super_clone4 &&
+
+	# Assert that each repo is pointing to "checked-out"
+	for REPO in "super_clone4" "super_clone4/sub" "super_clone4/sub/subsub"
+	do
+	    HEAD_BRANCH=$(git -C $REPO symbolic-ref HEAD) &&
+	    test $HEAD_BRANCH = "refs/heads/checked-out" || return 1
+	done &&
+
+	# Assert that the submodule branches are pointing to the right revs
+	EXPECT_SUB_OID="$(git -C super_clone4 rev-parse :sub)" &&
+	ACTUAL_SUB_OID="$(git -C super_clone4/sub rev-parse refs/heads/checked-out)" &&
+	test $EXPECT_SUB_OID = $ACTUAL_SUB_OID &&
+	EXPECT_SUBSUB_OID="$(git -C super_clone4/sub rev-parse :subsub)" &&
+	ACTUAL_SUBSUB_OID="$(git -C super_clone4/sub/subsub rev-parse refs/heads/checked-out)" &&
+	test $EXPECT_SUBSUB_OID = $ACTUAL_SUBSUB_OID &&
+
+	# Assert that the submodules do not have branches from their upstream
+	test_must_fail git -C super_clone4/sub rev-parse not-in-clone &&
+	test_must_fail git -C super_clone4/sub/subsub rev-parse not-in-clone
+'
+
 test_done
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index f094e3d7f36..b749d35f784 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1179,4 +1179,160 @@  test_expect_success 'submodule update --recursive skip submodules with strategy=
 	test_cmp expect.err actual.err
 '
 
+test_expect_success 'setup superproject with submodule.propagateBranches' '
+	git init sub1 &&
+	test_commit -C sub1 "sub1" &&
+	git init branch-super &&
+	git -C branch-super submodule add ../sub1 sub1 &&
+	git -C branch-super commit -m "super" &&
+
+	# Clone into a clean repo that we can cp around
+	git clone --recurse-submodules \
+		-c submodule.propagateBranches=true \
+		branch-super branch-super-clean &&
+	git -C branch-super-clean config submodule.propagateBranches true &&
+
+	# sub2 will not be in the clone. We will fetch the containing
+	# superproject commit and clone sub2 with "git submodule update".
+	git init sub2 &&
+	test_commit -C sub2 "sub2" &&
+	git -C branch-super submodule add ../sub2 sub2 &&
+	git -C branch-super commit -m "add sub2"
+'
+
+test_clean_submodule ()
+{
+	local negate super_dir sub_dir expect_oid actual_oid &&
+	if test "$1" = "!"
+	then
+		negate=t
+		shift
+	fi
+	super_dir="$1" &&
+	sub_dir="$2" &&
+	expect_oid="$(git -C "$super_dir" rev-parse ":$sub_dir")" &&
+	actual_oid="$(git -C "$super_dir/$sub_dir" rev-parse HEAD)" &&
+	if test -n "$negate"
+	then
+		! test "$expect_oid" = "$actual_oid"
+	else
+		test "$expect_oid" = "$actual_oid"
+	fi
+}
+
+# Test the behavior of a newly cloned submodule
+test_expect_success 'branches - newly-cloned submodule, detached HEAD' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned fetch origin main &&
+	git -C branch-super-cloned checkout FETCH_HEAD &&
+	git -C branch-super-cloned/sub1 checkout --detach &&
+	git -C branch-super-cloned submodule update &&
+
+	# sub1 and sub2 should be in detached HEAD
+	git -C branch-super-cloned/sub1 rev-parse --verify HEAD &&
+	test_must_fail git -C branch-super-cloned/sub1 symbolic-ref HEAD &&
+	test_clean_submodule branch-super-cloned sub1 &&
+	git -C branch-super-cloned/sub2 rev-parse --verify HEAD &&
+	test_must_fail git -C branch-super-cloned/sub2 symbolic-ref HEAD &&
+	test_clean_submodule branch-super-cloned sub2
+'
+
+test_expect_success 'branches - newly-cloned submodule, branch checked out' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned fetch origin main &&
+	git -C branch-super-cloned checkout FETCH_HEAD &&
+	git -C branch-super-cloned branch new-branch &&
+	git -C branch-super-cloned checkout new-branch &&
+	git -C branch-super-cloned/sub1 branch new-branch &&
+	git -C branch-super-cloned submodule update &&
+
+	# Ignore sub1, we will test it later.
+	# sub2 should check out the branch
+	HEAD_BRANCH2=$(git -C branch-super-cloned/sub2 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH2 = "refs/heads/new-branch" &&
+	test_clean_submodule branch-super-cloned sub2
+'
+
+# Test the behavior of an already-cloned submodule.
+# NEEDSWORK When updating with branches, we always use the branch instead of the
+# gitlink's OID. This results in some imperfect behavior:
+#
+# - If the gitlink's OID disagrees with the branch OID, updating with branches
+#   may result in a dirty worktree
+# - If the branch does not exist, the update fails.
+#
+# We will reevaluate when "git checkout --recurse-submodules" supports branches
+# For now, just test for this imperfect behavior.
+test_expect_success 'branches - correct branch checked out, OIDs agree' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned branch --recurse-submodules new-branch &&
+	git -C branch-super-cloned checkout new-branch &&
+	git -C branch-super-cloned/sub1 checkout new-branch &&
+	git -C branch-super-cloned submodule update &&
+
+	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
+	test_clean_submodule branch-super-cloned sub1
+'
+
+test_expect_success 'branches - correct branch checked out, OIDs disagree' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned branch --recurse-submodules new-branch &&
+	git -C branch-super-cloned checkout new-branch &&
+	git -C branch-super-cloned/sub1 checkout new-branch &&
+	test_commit -C branch-super-cloned/sub1 new-commit &&
+	git -C branch-super-cloned submodule update &&
+
+	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
+	test_clean_submodule ! branch-super-cloned sub1
+'
+
+test_expect_success 'branches - other branch checked out, correct branch exists, OIDs agree' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned branch --recurse-submodules new-branch &&
+	git -C branch-super-cloned checkout new-branch &&
+	git -C branch-super-cloned/sub1 checkout main &&
+	git -C branch-super-cloned submodule update &&
+
+	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
+	test_clean_submodule branch-super-cloned sub1
+'
+
+test_expect_success 'branches - other branch checked out, correct branch exists, OIDs disagree' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned branch --recurse-submodules new-branch &&
+	git -C branch-super-cloned checkout new-branch &&
+	git -C branch-super-cloned/sub1 checkout new-branch &&
+	test_commit -C branch-super-cloned/sub1 new-commit &&
+	git -C branch-super-cloned/sub1 checkout main &&
+	git -C branch-super-cloned submodule update &&
+
+	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
+	test_clean_submodule ! branch-super-cloned sub1
+'
+
+test_expect_success 'branches - other branch checked out, correct branch does not exist' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned branch new-branch &&
+	git -C branch-super-cloned checkout new-branch &&
+	test_must_fail git -C branch-super-cloned submodule update
+'
+
 test_done