[v2,3/7] submodule--helper clone: create named branch

Commit Message

Glen Choo Oct. 20, 2022, 8:20 p.m. UTC

From: Glen Choo <[e-mail]>

When submodule branching is enabled (i.e. "submodule.propagateBranches =
true"), submodules are expected to have the same set of branches as the
superproject. To support this behavior in newly cloned submodules, teach
"git submodule--helper clone" to:

- clone with the "--detach" flag (so that the submodule doesn't create a
  branch corresponding to the remote's HEAD)
- create a branch when using the --branch and --branch-oid flags

The --branch and --branch-oid flags are only allowed when submodule
branching is enabled, otherwise the named branch might conflict with the
branch from the submodule remote's HEAD.

These flags will be used by `git submodule update` in a later commit.
"git submodule add" (which also invokes "git submodule--helper clone")
should also do something similar when submodule branching is enabled,
but this is left for a later series.

Arguably, "--detach" should also be the default for
"submodule.propagateBranches=false" since it doesn't make sense to
create a submodule branch when the submodule is always expected to be in
detached HEAD. But, to be conservative, this commit does not change the
behavior of "submodule.propagateBranches=false".

Signed-off-by: Glen Choo <[e-mail]>
---
 builtin/submodule--helper.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Comments

Jonathan Tan Oct. 25, 2022, 6 p.m. UTC | #1

"Glen Choo via GitGitGadget" <[e-mail]> writes:
> Signed-off-by: Glen Choo <[e-mail]>

Add a note in the commit message that this will be used and tested in a 
subsequent patch. 

> -	if (argc || !clone_data.url || !clone_data.path || !*(clone_data.path))
> +	if (argc || !clone_data.url || !clone_data.path || !*(clone_data.path)
> +	    || (!!clone_data.branch != !!clone_data.branch_oid))
>  		usage_with_options(git_submodule_helper_usage,
>  				   module_clone_options);

I know that this is just internal code, but could we have a better 
diagnostic? You can leave the existing check alone, and then do the 
!!clone_data.branch != !!clone_data.branch_oid with a BUG() if the 
result is not what you expect.

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 0b4acb442b2..1ce3458a29c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1503,6 +1503,8 @@  struct module_clone_data {
 	const char *name;
 	const char *url;
 	const char *depth;
+	const char *branch;
+	const char *branch_oid;
 	struct list_objects_filter_options *filter_options;
 	unsigned int quiet: 1;
 	unsigned int progress: 1;
@@ -1692,6 +1694,8 @@  static int clone_submodule(const struct module_clone_data *clone_data,
 			strvec_push(&cp.args, clone_data->single_branch ?
 				    "--single-branch" :
 				    "--no-single-branch");
+		if (the_repository->settings.submodule_propagate_branches)
+			strvec_push(&cp.args, "--detach");
 
 		strvec_push(&cp.args, "--");
 		strvec_push(&cp.args, clone_data->url);
@@ -1704,6 +1708,21 @@  static int clone_submodule(const struct module_clone_data *clone_data,
 		if(run_command(&cp))
 			die(_("clone of '%s' into submodule path '%s' failed"),
 			    clone_data->url, clone_data_path);
+
+		if (clone_data->branch) {
+			struct child_process branch_cp = CHILD_PROCESS_INIT;
+
+			branch_cp.git_cmd = 1;
+			prepare_other_repo_env(&branch_cp.env, sm_gitdir);
+
+			strvec_pushl(&branch_cp.args, "branch",
+				     clone_data->branch, clone_data->branch_oid,
+				     NULL);
+
+			if (run_command(&branch_cp))
+				die(_("could not create branch '%s' in submodule path '%s'"),
+				    clone_data->branch, clone_data_path);
+		}
 	} else {
 		char *path;
 
@@ -1778,6 +1797,12 @@  static int module_clone(int argc, const char **argv, const char *prefix)
 			   N_("disallow cloning into non-empty directory")),
 		OPT_BOOL(0, "single-branch", &clone_data.single_branch,
 			 N_("clone only one branch, HEAD or --branch")),
+		OPT_STRING(0, "branch", &clone_data.branch,
+			   N_("string"),
+			   N_("name of branch to be created")),
+		OPT_STRING(0, "branch-oid", &clone_data.branch_oid,
+			   N_("object-id"),
+			   N_("commit id for new branch")),
 		OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
 		OPT_END()
 	};
@@ -1785,12 +1810,14 @@  static int module_clone(int argc, const char **argv, const char *prefix)
 		N_("git submodule--helper clone [--prefix=<path>] [--quiet] "
 		   "[--reference <repository>] [--name <name>] [--depth <depth>] "
 		   "[--single-branch] [--filter <filter-spec>] "
+		   "[--branch <branch> --branch-oid <oid>]"
 		   "--url <url> --path <path>"),
 		NULL
 	};
 
 	argc = parse_options(argc, argv, prefix, module_clone_options,
 			     git_submodule_helper_usage, 0);
+	prepare_repo_settings(the_repository);
 
 	clone_data.dissociate = !!dissociate;
 	clone_data.quiet = !!quiet;
@@ -1798,9 +1825,13 @@  static int module_clone(int argc, const char **argv, const char *prefix)
 	clone_data.require_init = !!require_init;
 	clone_data.filter_options = &filter_options;
 
-	if (argc || !clone_data.url || !clone_data.path || !*(clone_data.path))
+	if (argc || !clone_data.url || !clone_data.path || !*(clone_data.path)
+	    || (!!clone_data.branch != !!clone_data.branch_oid))
 		usage_with_options(git_submodule_helper_usage,
 				   module_clone_options);
+	if ((clone_data.branch &&
+	     !the_repository->settings.submodule_propagate_branches))
+		BUG("--branch is only expected with submodule.propagateBranches");
 
 	clone_submodule(&clone_data, &reference);
 	list_objects_filter_release(&filter_options);