diff mbox series

[v2,7/7] clone, submodule update: create and check out branches

Message ID 3f98b0d17397dbce85aa87a9591981c790f8f7a2.1666297239.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series clone, submodule update: check out submodule branches | expand

Commit Message

Glen Choo Oct. 20, 2022, 8:20 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 (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 | 32 +++++++++++++++++--
 t/t5617-clone-submodules.sh | 34 ++++++++++++++++++++
 t/t7406-submodule-update.sh | 64 +++++++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+), 3 deletions(-)

Comments

Jonathan Tan Oct. 25, 2022, 5:56 p.m. UTC | #1
"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Teach "git submodule update" to:
> 
> - create the branch with the same name as the current superproject
>   branch when cloning a submodule

Ah, this is when "git submodule update" needs to clone a submodule (as 
opposed to doing something with a submodule that is already cloned). 

> - check out that branch (instead of the commit OID) when updating
>   the submodule worktree

So whenever we run "git submodule update" and a submodule already has a 
branch of the same name as what's currently checked out in the 
superproject, we check out the submodule's branch, ignoring the gitlink 
in the superproject? 

> when submodule branching is enabled (submodule.propagateBranches = true)
> on the superproject and a branch is checked out. 

OK.

> "git clone
> --recurse-submodules" also learns this trick because it is implemented
> with "git submodule update --recursive".

Is this sentence redundant now that you're specifically calling out the 
cloning part above?

> 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. 

Makes sense.

> In other cases, it
> does not work as well, but we can handle them incrementally:
> 
> - "git pull --recurse-submodules" merges the superproject tree (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.

Is this because when we "git pull", the superproject likely now has a 
different gitlink, but the branch in the submodule hasn't changed? 

>   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).

Makes sense - so in summary, there are (at least) two ways of the 
superproject's gitlink and the submodule's branch can go out of sync: 
the user changing the submodule branch (here) or the user changing the 
superproject's gitlink (above). 
 
>   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>

[snip]

> @@ -2521,6 +2532,7 @@ static int update_submodule(struct update_data *update_data)
>  {
>  	int submodule_up_to_date;
>  	int ret;
> +	const char *submodule_head = "HEAD";

I think it's clearer if this is initialized to NULL. I don't think the 
submodule head is detached in all code paths when this is HEAD.  

>  	ret = determine_submodule_update_strategy(the_repository,
>  						  update_data->just_cloned,
> @@ -2533,7 +2545,7 @@ static int update_submodule(struct update_data *update_data)
>  	if (update_data->just_cloned)
>  		oidcpy(&update_data->suboid, null_oid());
>  	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD",
> -				     &update_data->suboid, NULL))
> +				     &update_data->suboid, &submodule_head))
>  		return die_message(_("Unable to find current revision in submodule path '%s'"),
>  				   update_data->displaypath);
>  
> @@ -2568,7 +2580,14 @@ static int update_submodule(struct update_data *update_data)
>  		free(remote_ref);
>  	}
>  
> -	submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
> +	if (!update_data->super_branch)
> +		submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
> +	else if (skip_prefix(submodule_head, "refs/heads/", &submodule_head))
> +		submodule_up_to_date = !strcmp(update_data->super_branch, submodule_head);
> +	/* submodule_branch is "HEAD"; the submodule is in detached HEAD */
> +	else
> +		submodule_up_to_date = 0;

I think there needs to be a better comment here. It doesn't matter that 
the submodule is in detached HEAD; what matters is that the submodule 
doesn't have the superproject's branch checked out. So maybe something 
like: 

  if (update_data->super_branch) {
    /* (format this appropriately) We also need to check that the 
       submodule's HEAD points to super_branch. */ 
    const char *submodule_head;
    submodule_up_to_date = skip_prefix(...) && !strcmp(...)
  } else {
    submodule_up_to_date = oideq(...)
  }

> diff --git a/t/t5617-clone-submodules.sh b/t/t5617-clone-submodules.sh

[snip]

> @@ -107,4 +114,31 @@ 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' '
> +	git -C sub checkout -b not-main &&
> +	git -C subsub checkout -b not-main &&
> +	git clone --recurse-submodules \
> +		-c submodule.propagateBranches=true \
> +		"file://$pwd/." super_clone4 &&
> +
> +	# Assert that each repo is pointing to "main"
> +	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/main" || return 1
> +	done &&

As I said in my earlier review [1], could we use a branch name that is 
not  "main"? That way, we also check that the clone *creates* the 
branches,  not just reuses something already there. 

[1] https://lore.kernel.org/git/20220901200047.515294-1-jonathantanmy@google.com/

> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh

[snip]

> +test_expect_success 'submodule.propagateBranches - detached HEAD' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned checkout --detach &&
> +	git -C branch-super-cloned pull origin main &&

If the behavior of "pull" is going to change soon (as stated in the 
commit message), can we avoid using it in tests here? 

> +	git -C branch-super-cloned submodule update &&
> +
> +	# sub2 should be in detached HEAD
> +	git -C branch-super-cloned/sub2 rev-parse --verify HEAD &&
> +	test_must_fail git -C branch-super-cloned/sub2 symbolic-ref HEAD
> +'
> +
> +test_expect_success 'submodule.propagateBranches - branch checked out' '
> +	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 --recurse-submodules new-branch &&
> +	git -C branch-super-cloned pull origin 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" &&
> +	HEAD_BRANCH2=$(git -C branch-super-cloned/sub2 symbolic-ref HEAD) &&
> +	test $HEAD_BRANCH2 = "refs/heads/new-branch"

I'm not sure of the behavior of "pull" here, so I didn't look too  
closely into what these tests test, but I think that you should cover 
these situations: 
 - submodule has correct OID and correct branch checked out
 - submodule has correct OID, but wrong branch checked out, and correct 
   branch doesn't exist 
 - submodule has correct OID, but wrong branch checked out, and correct 
   branch exists at correct OID 
 - submodule has correct OID, but wrong branch checked out, and correct 
   branch exists at wrong OID 
 - submodule has incorrect OID but correct branch checked out (if this 
   currently doesn't work, maybe add a test_must_fail with a NEEDSWORK) 

There are other combinations but I think these are the most important 
ones. 

Also, if there is a situation where you ignore the superproject's 
gitlink in favor of the submodule's branch, you should check the OIDs 
in all cases too, not just what the HEAD symbolic ref points to.
Glen Choo Oct. 25, 2022, 9:49 p.m. UTC | #2
Jonathan Tan <jonathantanmy@google.com> writes:

> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> 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".
>
> Is this sentence redundant now that you're specifically calling out the 
> cloning part above?

Not quite. The "trick" I'm referring to is "clone, create the
named branch and check it out", which is implemented by "git submodule
update" (and its constituent subprocesses, e.g. "git submodule clone"),
so it is pertinent that "git clone" is implemented by "git submodule
update".

>> In other cases, it
>> does not work as well, but we can handle them incrementally:
>> 
>> - "git pull --recurse-submodules" merges the superproject tree (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.
>
> Is this because when we "git pull", the superproject likely now has a 
> different gitlink, but the branch in the submodule hasn't changed? 

Yes. That sounds like a clarification I should make in v3.

>>   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).
>
> Makes sense - so in summary, there are (at least) two ways of the 
> superproject's gitlink and the submodule's branch can go out of sync: 
> the user changing the submodule branch (here) or the user changing the 
> superproject's gitlink (above). 

Yes. I didn't mention modifying the superproject's gitlink (outside of
"git pull"), but I imagine that it would use the same guardrail I
mentioned here. I could add mention of it if that seems useful.

>  
>>   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>
>
> [snip]
>
>> @@ -2521,6 +2532,7 @@ static int update_submodule(struct update_data *update_data)
>>  {
>>  	int submodule_up_to_date;
>>  	int ret;
>> +	const char *submodule_head = "HEAD";
>
> I think it's clearer if this is initialized to NULL. I don't think the 
> submodule head is detached in all code paths when this is HEAD.  

Ok, I'll initialize to NULL since refs_resolve_ref_unsafe() can return
NULL. I'm quite convinced that it only returns NULL in the error case
though (and we need to handle that explicitly), otherwise detached HEAD
always results in HEAD.

>
>>  	ret = determine_submodule_update_strategy(the_repository,
>>  						  update_data->just_cloned,
>> @@ -2533,7 +2545,7 @@ static int update_submodule(struct update_data *update_data)
>>  	if (update_data->just_cloned)
>>  		oidcpy(&update_data->suboid, null_oid());
>>  	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD",
>> -				     &update_data->suboid, NULL))
>> +				     &update_data->suboid, &submodule_head))
>>  		return die_message(_("Unable to find current revision in submodule path '%s'"),
>>  				   update_data->displaypath);
>>  
>> @@ -2568,7 +2580,14 @@ static int update_submodule(struct update_data *update_data)
>>  		free(remote_ref);
>>  	}
>>  
>> -	submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
>> +	if (!update_data->super_branch)
>> +		submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
>> +	else if (skip_prefix(submodule_head, "refs/heads/", &submodule_head))
>> +		submodule_up_to_date = !strcmp(update_data->super_branch, submodule_head);
>> +	/* submodule_branch is "HEAD"; the submodule is in detached HEAD */
>> +	else
>> +		submodule_up_to_date = 0;
>
> I think there needs to be a better comment here. It doesn't matter that 
> the submodule is in detached HEAD; what matters is that the submodule 
> doesn't have the superproject's branch checked out. So maybe something 
> like: 
>
>   if (update_data->super_branch) {
>     /* (format this appropriately) We also need to check that the 
>        submodule's HEAD points to super_branch. */ 
>     const char *submodule_head;
>     submodule_up_to_date = skip_prefix(...) && !strcmp(...)
>   } else {
>     submodule_up_to_date = oideq(...)
>   }
>

I see, thanks for the suggestion.

>> diff --git a/t/t5617-clone-submodules.sh b/t/t5617-clone-submodules.sh
>
> [snip]
>
>> @@ -107,4 +114,31 @@ 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' '
>> +	git -C sub checkout -b not-main &&
>> +	git -C subsub checkout -b not-main &&
>> +	git clone --recurse-submodules \
>> +		-c submodule.propagateBranches=true \
>> +		"file://$pwd/." super_clone4 &&
>> +
>> +	# Assert that each repo is pointing to "main"
>> +	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/main" || return 1
>> +	done &&
>
> As I said in my earlier review [1], could we use a branch name that is 
> not  "main"? That way, we also check that the clone *creates* the 
> branches,  not just reuses something already there. 
>
> [1] https://lore.kernel.org/git/20220901200047.515294-1-jonathantanmy@google.com/

Ah I missed that, thanks.

>
>> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
>
> [snip]
>
>> +test_expect_success 'submodule.propagateBranches - detached HEAD' '
>> +	test_when_finished "rm -fr branch-super-cloned" &&
>> +	cp -r branch-super-clean branch-super-cloned &&
>> +
>> +	git -C branch-super-cloned checkout --detach &&
>> +	git -C branch-super-cloned pull origin main &&
>
> If the behavior of "pull" is going to change soon (as stated in the 
> commit message), can we avoid using it in tests here? 

Here I've omitted "--recurse-submodules", so this is a shorthand for
"update the superproject's HEAD to the remote's main". This behavior
isn't going to change, but perhaps I shouldn't rely on such subtleties
in test code (where readability is paramount).

>
>> +	git -C branch-super-cloned submodule update &&
>> +
>> +	# sub2 should be in detached HEAD
>> +	git -C branch-super-cloned/sub2 rev-parse --verify HEAD &&
>> +	test_must_fail git -C branch-super-cloned/sub2 symbolic-ref HEAD
>> +'
>> +
>> +test_expect_success 'submodule.propagateBranches - branch checked out' '
>> +	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 --recurse-submodules new-branch &&
>> +	git -C branch-super-cloned pull origin 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" &&
>> +	HEAD_BRANCH2=$(git -C branch-super-cloned/sub2 symbolic-ref HEAD) &&
>> +	test $HEAD_BRANCH2 = "refs/heads/new-branch"
>
> I'm not sure of the behavior of "pull" here, so I didn't look too  
> closely into what these tests test, but I think that you should cover 
> these situations: 
>  - submodule has correct OID and correct branch checked out
>  - submodule has correct OID, but wrong branch checked out, and correct 
>    branch doesn't exist 
>  - submodule has correct OID, but wrong branch checked out, and correct 
>    branch exists at correct OID 
>  - submodule has correct OID, but wrong branch checked out, and correct 
>    branch exists at wrong OID 
>  - submodule has incorrect OID but correct branch checked out (if this 
>    currently doesn't work, maybe add a test_must_fail with a NEEDSWORK) 
>
> There are other combinations but I think these are the most important 
> ones. 
>
> Also, if there is a situation where you ignore the superproject's 
> gitlink in favor of the submodule's branch, you should check the OIDs 
> in all cases too, not just what the HEAD symbolic ref points to. 

Makes sense. Thanks for the thorough review :)
diff mbox series

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 990adeb2e19..4576ba22544 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1915,6 +1915,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 object_id suboid;
 	struct string_list references;
@@ -2090,6 +2091,10 @@  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);
@@ -2253,9 +2258,15 @@  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;
@@ -2521,6 +2532,7 @@  static int update_submodule(struct update_data *update_data)
 {
 	int submodule_up_to_date;
 	int ret;
+	const char *submodule_head = "HEAD";
 
 	ret = determine_submodule_update_strategy(the_repository,
 						  update_data->just_cloned,
@@ -2533,7 +2545,7 @@  static int update_submodule(struct update_data *update_data)
 	if (update_data->just_cloned)
 		oidcpy(&update_data->suboid, null_oid());
 	else if (resolve_gitlink_ref(update_data->sm_path, "HEAD",
-				     &update_data->suboid, NULL))
+				     &update_data->suboid, &submodule_head))
 		return die_message(_("Unable to find current revision in submodule path '%s'"),
 				   update_data->displaypath);
 
@@ -2568,7 +2580,14 @@  static int update_submodule(struct update_data *update_data)
 		free(remote_ref);
 	}
 
-	submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
+	if (!update_data->super_branch)
+		submodule_up_to_date = oideq(&update_data->oid, &update_data->suboid);
+	else if (skip_prefix(submodule_head, "refs/heads/", &submodule_head))
+		submodule_up_to_date = !strcmp(update_data->super_branch, submodule_head);
+	/* submodule_branch is "HEAD"; the submodule is in detached HEAD */
+	else
+		submodule_up_to_date = 0;
+
 	if (!submodule_up_to_date || update_data->force) {
 		ret = run_update_procedure(update_data);
 		if (ret)
@@ -2603,6 +2622,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 +2743,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..51593376ce4 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,31 @@  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' '
+	git -C sub checkout -b not-main &&
+	git -C subsub checkout -b not-main &&
+	git clone --recurse-submodules \
+		-c submodule.propagateBranches=true \
+		"file://$pwd/." super_clone4 &&
+
+	# Assert that each repo is pointing to "main"
+	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/main" || 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/main)" &&
+	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/main)" &&
+	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-main &&
+	test_must_fail git -C super_clone4/sub/subsub rev-parse not-main
+'
+
 test_done
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index f094e3d7f36..54aa8c5cb54 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1179,4 +1179,68 @@  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 &&
+
+	# Create an upstream submodule not in the clone
+	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_expect_success 'submodule.propagateBranches - detached HEAD' '
+	test_when_finished "rm -fr branch-super-cloned" &&
+	cp -r branch-super-clean branch-super-cloned &&
+
+	git -C branch-super-cloned checkout --detach &&
+	git -C branch-super-cloned pull origin main &&
+	git -C branch-super-cloned submodule update &&
+
+	# sub2 should be in detached HEAD
+	git -C branch-super-cloned/sub2 rev-parse --verify HEAD &&
+	test_must_fail git -C branch-super-cloned/sub2 symbolic-ref HEAD
+'
+
+test_expect_success 'submodule.propagateBranches - branch checked out' '
+	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 --recurse-submodules new-branch &&
+	git -C branch-super-cloned pull origin 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" &&
+	HEAD_BRANCH2=$(git -C branch-super-cloned/sub2 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH2 = "refs/heads/new-branch"
+'
+
+test_expect_success 'submodule.propagateBranches - other branch checked out' '
+	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 --recurse-submodules new-branch &&
+	git -C branch-super-cloned/sub1 checkout -b other-branch &&
+	git -C branch-super-cloned pull origin 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" &&
+	HEAD_BRANCH2=$(git -C branch-super-cloned/sub2 symbolic-ref HEAD) &&
+	test $HEAD_BRANCH2 = "refs/heads/new-branch"
+'
+
 test_done