git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Philippe Blain <levraiphilippeblain@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	Glen Choo <chooglen@google.com>, Glen Choo <chooglen@google.com>
Subject: [PATCH v2 7/7] clone, submodule update: create and check out branches
Date: Thu, 20 Oct 2022 20:20:38 +0000	[thread overview]
Message-ID: <3f98b0d17397dbce85aa87a9591981c790f8f7a2.1666297239.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1321.v2.git.git.1666297238.gitgitgadget@gmail.com>

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

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

  parent reply	other threads:[~2022-10-20 20:21 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-29 20:54 [PATCH 0/6] clone, submodule update: check out submodule branches Glen Choo via GitGitGadget
2022-08-29 20:54 ` [PATCH 1/6] clone: teach --detach option Glen Choo via GitGitGadget
2022-08-30  4:02   ` Philippe Blain
2022-08-29 20:54 ` [PATCH 2/6] repo-settings: add submodule_propagate_branches Glen Choo via GitGitGadget
2022-08-30  4:02   ` Philippe Blain
2022-08-29 20:54 ` [PATCH 3/6] t5617: drop references to remote-tracking branches Glen Choo via GitGitGadget
2022-08-30  4:03   ` Philippe Blain
2022-08-29 20:54 ` [PATCH 4/6] submodule: return target of submodule symref Glen Choo via GitGitGadget
2022-09-01 20:01   ` Jonathan Tan
2022-09-01 20:46     ` Glen Choo
2022-08-29 20:54 ` [PATCH 5/6] submodule--helper: refactor up-to-date criterion Glen Choo via GitGitGadget
2022-08-29 20:54 ` [PATCH 6/6] clone, submodule update: check out branches Glen Choo via GitGitGadget
2022-08-30  4:03   ` Philippe Blain
2022-08-30 22:54     ` Glen Choo
2022-09-01 20:00   ` Jonathan Tan
2022-10-20 20:20 ` [PATCH v2 0/7] clone, submodule update: check out submodule branches Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 1/7] clone: teach --detach option Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 2/7] repo-settings: add submodule_propagate_branches Glen Choo via GitGitGadget
2022-10-25 18:03     ` Jonathan Tan
2022-10-20 20:20   ` [PATCH v2 3/7] submodule--helper clone: create named branch Glen Choo via GitGitGadget
2022-10-25 18:00     ` Jonathan Tan
2022-10-20 20:20   ` [PATCH v2 4/7] t5617: drop references to remote-tracking branches Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 5/7] submodule: return target of submodule symref Glen Choo via GitGitGadget
2022-10-20 20:20   ` [PATCH v2 6/7] submodule update: refactor update targets Glen Choo via GitGitGadget
2022-10-20 20:20   ` Glen Choo via GitGitGadget [this message]
2022-10-25 17:56     ` [PATCH v2 7/7] clone, submodule update: create and check out branches Jonathan Tan
2022-10-25 21:49       ` Glen Choo
2022-10-20 22:40   ` [PATCH v2 0/7] clone, submodule update: check out submodule branches Junio C Hamano
2022-10-20 23:53     ` Glen Choo
2022-10-21  0:01       ` Junio C Hamano
2022-10-28 20:14   ` [PATCH v3 0/8] " Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 1/8] clone: teach --detach option Glen Choo via GitGitGadget
2022-10-28 21:40       ` Junio C Hamano
2022-10-28 21:54         ` Junio C Hamano
2022-10-28 22:55           ` Glen Choo
2022-10-30 18:14             ` Taylor Blau
2022-10-31 17:07               ` Glen Choo
2022-11-08 13:32       ` Philippe Blain
2022-10-28 20:14     ` [PATCH v3 2/8] repo-settings: add submodule_propagate_branches Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 3/8] submodule--helper clone: create named branch Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 4/8] t5617: drop references to remote-tracking branches Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 5/8] submodule: return target of submodule symref Glen Choo via GitGitGadget
2022-10-28 21:49       ` Junio C Hamano
2022-10-28 23:11         ` Glen Choo
2022-10-28 20:14     ` [PATCH v3 6/8] submodule update: refactor update targets Glen Choo via GitGitGadget
2022-10-28 20:14     ` [PATCH v3 7/8] submodule--helper: remove update_data.suboid Glen Choo via GitGitGadget
2022-11-14 23:45       ` Jonathan Tan
2022-10-28 20:14     ` [PATCH v3 8/8] clone, submodule update: create and check out branches Glen Choo via GitGitGadget
2022-11-08 13:53       ` Philippe Blain
2022-11-15 18:15       ` Jonathan Tan
2022-11-22 18:44         ` Glen Choo
2022-11-23  1:33           ` Jonathan Tan
2022-11-23  4:00             ` Junio C Hamano
2022-10-30 18:19     ` [PATCH v3 0/8] clone, submodule update: check out submodule branches Taylor Blau
2022-11-08 14:23     ` Philippe Blain
2022-11-08 20:43       ` Glen Choo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3f98b0d17397dbce85aa87a9591981c790f8f7a2.1666297239.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --cc=levraiphilippeblain@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).