threads / discuss / 44534

[PATCHv2 1/4] submodule: use absolute path for computing relative path connecting

Subject: [PATCHv2 1/4] submodule: use absolute path for computing relative path connecting

## tl;dr

14 messages between Nov 22, 2016 and Nov 30, 2016.

replies: 13people: 3as markdown or json

Stefan Beller· Nov 22, 2016, 19:22 UTC · lore

[PATCHv2 0/4] `submodule embedgitdirs` [was: Introduce `submodule interngitdirs`]

v2:
* fixed commit message for patch:
 "submodule: use absolute path for computing relative path connecting"
* a new patch "submodule helper: support super prefix"
* redid the final patch with more tests and fixing bugs along the way
* "test-lib-functions.sh: teach test_commit -C <dir>" unchanged
diff to v1 below.
Stefan Beller (4):
  submodule: use absolute path for computing relative path connecting
  submodule helper: support super prefix
  test-lib-functions.sh: teach test_commit -C <dir>
  submodule: add embed-git-dir function
 Documentation/git-submodule.txt   | 14 +++++++
 builtin/submodule--helper.c       | 62 +++++++++++++++++++++++++------
 git-submodule.sh                  |  7 +++-
 git.c                             |  2 +-
 submodule.c                       | 77 +++++++++++++++++++++++++++++++++++---
 submodule.h                       |  2 +
 t/t7412-submodule-embedgitdirs.sh | 78 +++++++++++++++++++++++++++++++++++++++
 t/test-lib-functions.sh           | 20 +++++++---
 8 files changed, 239 insertions(+), 23 deletions(-)
 create mode 100755 t/t7412-submodule-embedgitdirs.sh

v1: The discussion of the submodule checkout series revealed to me that a command is needed to move the git directory from the submodules working tree to be embedded into the superprojects git directory.

So I wrote the code to intern the submodules git dir into the superproject, but whilst writing the code I realized this could be valueable for our use in testing too. So I exposed it via the submodule--helper. But as the submodule helper ought to be just an internal API, we could also offer it via the proper submodule command.

The command as it is has little value to the end user for now, but breaking it out of the submodule checkout series hopefully makes review easier.

Thanks, Stefan

diff to v1:
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 80d55350eb..34791cfc65 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,7 +22,8 @@ SYNOPSIS
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
-'git submodule' [--quiet] interngitdirs [--] [<path>...]
+'git submodule' [--quiet] embedgitdirs [--] [<path>...]
+
 
 DESCRIPTION
 -----------
@@ -245,18 +246,18 @@ sync::
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and sync any nested submodules within.
 
-interngitdirs::
+embedgitdirs::
 	Move the git directory of submodules into its superprojects
 	`$GIT_DIR/modules` path and then connect the git directory and
 	its working directory by setting the `core.worktree` and adding
 	a .git file pointing to the git directory interned into the
 	superproject.
 +
-	A repository that was cloned independently and later added
-	as a submodule or old setups have the submodules git directory
-	inside the submodule instead of the
+A repository that was cloned independently and later added as a submodule or
+old setups have the submodules git directory inside the submodule instead of
+embedded into the superprojects git directory.
 +
-	This command is recursive by default.
+This command is recursive by default.
 
 OPTIONS
 -------
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 256f8e9439..75cdbf45b8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,22 +1076,25 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
 	return 0;
 }
 
-static int intern_git_dir(int argc, const char **argv, const char *prefix)
+static int embed_git_dir(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	struct pathspec pathspec;
 	struct module_list list = MODULE_LIST_INIT;
 
-	struct option intern_gitdir_options[] = {
+	struct option embed_gitdir_options[] = {
+		OPT_STRING(0, "prefix", &prefix,
+			   N_("path"),
+			   N_("path into the working tree")),
 		OPT_END()
 	};
 
 	const char *const git_submodule_helper_usage[] = {
-		N_("git submodule--helper intern-git-dir [<path>...]"),
+		N_("git submodule--helper embed-git-dir [<path>...]"),
 		NULL
 	};
 
-	argc = parse_options(argc, argv, prefix, intern_gitdir_options,
+	argc = parse_options(argc, argv, prefix, embed_gitdir_options,
 			     git_submodule_helper_usage, 0);
 
 	gitmodules_config();
@@ -1101,27 +1104,30 @@ static int intern_git_dir(int argc, const char **argv, const char *prefix)
 		return 1;
 
 	for (i = 0; i < list.nr; i++)
-		migrate_submodule_gitdir(list.entries[i]->name);
+		migrate_submodule_gitdir(prefix, list.entries[i]->name, 1);
 
 	return 0;
 }
 
+#define SUPPORT_SUPER_PREFIX (1<<0)
+
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
+	int option;
 };
 
 static struct cmd_struct commands[] = {
-	{"list", module_list},
-	{"name", module_name},
-	{"clone", module_clone},
-	{"update-clone", update_clone},
-	{"relative-path", resolve_relative_path},
-	{"resolve-relative-url", resolve_relative_url},
-	{"resolve-relative-url-test", resolve_relative_url_test},
-	{"init", module_init},
-	{"remote-branch", resolve_remote_submodule_branch},
-	{"intern-git-dir", intern_git_dir}
+	{"list", module_list, 0},
+	{"name", module_name, 0},
+	{"clone", module_clone, 0},
+	{"update-clone", update_clone, 0},
+	{"relative-path", resolve_relative_path, 0},
+	{"resolve-relative-url", resolve_relative_url, 0},
+	{"resolve-relative-url-test", resolve_relative_url_test, 0},
+	{"init", module_init, 0},
+	{"remote-branch", resolve_remote_submodule_branch, 0},
+	{"embed-git-dirs", embed_git_dir, SUPPORT_SUPER_PREFIX}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
@@ -1131,9 +1137,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
 		die(_("submodule--helper subcommand must be "
 		      "called with a subcommand"));
 
-	for (i = 0; i < ARRAY_SIZE(commands); i++)
-		if (!strcmp(argv[1], commands[i].cmd))
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		if (!strcmp(argv[1], commands[i].cmd)) {
+			if (get_super_prefix() &&
+			    !(commands[i].option & SUPPORT_SUPER_PREFIX))
+				die("%s doesn't support --super-prefix",
+				    commands[i].cmd);
 			return commands[i].fn(argc - 1, argv + 1, prefix);
+		}
+	}
 
 	die(_("'%s' is not a valid submodule--helper "
 	      "subcommand"), argv[1]);
diff --git a/git-submodule.sh b/git-submodule.sh
index 747e934df2..b7e124f340 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,9 +1131,9 @@ cmd_sync()
 	done
 }
 
-cmd_interngitdirs()
+cmd_embedgitdirs()
 {
-	git submodule--helper intern-git-dir "$@"
+	git submodule--helper embed-git-dirs --prefix "$wt_prefix" "$@"
 }
 
 # This loop parses the command line arguments to find the
@@ -1145,7 +1145,7 @@ cmd_interngitdirs()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync | interngitdirs)
+	add | foreach | init | deinit | update | status | summary | sync | embedgitdirs)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/git.c b/git.c
index efa1059fe0..98dcf6c518 100644
--- a/git.c
+++ b/git.c
@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
-	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP },
+	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
 	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 	{ "tag", cmd_tag, RUN_SETUP },
 	{ "unpack-file", cmd_unpack_file, RUN_SETUP },
diff --git a/submodule.c b/submodule.c
index 99befdba85..d330b567a3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1269,22 +1269,12 @@ void prepare_submodule_repo_env(struct argv_array *out)
  * having its git directory within the working tree to the git dir nested
  * in its superprojects git dir under modules/.
  */
-void migrate_submodule_gitdir(const char *path)
+void migrate_submodule_gitdir(const char *prefix, const char *path,
+			      int recursive)
 {
 	char *old_git_dir;
 	const char *new_git_dir;
 	const struct submodule *sub;
-	struct child_process cp = CHILD_PROCESS_INIT;
-
-	cp.git_cmd = 1;
-	cp.no_stdin = 1;
-	cp.dir = path;
-	argv_array_pushl(&cp.args, "submodule", "foreach", "--recursive",
-			"git", "submodule--helper" "intern-git-dir", NULL);
-
-	if (run_command(&cp))
-		die(_("Could not migrate git directory in submodule '%s'"),
-		    path);
 
 	old_git_dir = xstrfmt("%s/.git", path);
 	if (read_gitfile(old_git_dir))
@@ -1295,14 +1285,46 @@ void migrate_submodule_gitdir(const char *path)
 	if (!sub)
 		die(_("Could not lookup name for submodule '%s'"),
 		      path);
+
 	new_git_dir = git_common_path("modules/%s", sub->name);
-	mkdir_in_gitdir(".git/modules");
+	if (safe_create_leading_directories_const(new_git_dir) < 0)
+		die(_("could not create directory '%s'"), new_git_dir);
+
+	if (!prefix)
+		prefix = get_super_prefix();
+	printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
+		prefix ? prefix : "", path,
+		real_path(old_git_dir), new_git_dir);
 
 	if (rename(old_git_dir, new_git_dir) < 0)
 		die_errno(_("Could not migrate git directory from '%s' to '%s'"),
 			old_git_dir, new_git_dir);
 
 	connect_work_tree_and_git_dir(path, new_git_dir);
+
 out:
+	if (recursive) {
+		struct child_process cp = CHILD_PROCESS_INIT;
+		struct strbuf sb = STRBUF_INIT;
+
+		if (get_super_prefix())
+			strbuf_addstr(&sb, get_super_prefix());
+		strbuf_addstr(&sb, path);
+		strbuf_addch(&sb, '/');
+
+		cp.dir = path;
+		cp.git_cmd = 1;
+		cp.no_stdin = 1;
+		argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+					    "submodule--helper",
+					   "embed-git-dirs", NULL);
+		prepare_submodule_repo_env(&cp.env_array);
+		if (run_command(&cp))
+			die(_("Could not migrate git directory in submodule '%s'"),
+			    path);
+
+		strbuf_release(&sb);
+	}
+
 	free(old_git_dir);
 }
diff --git a/submodule.h b/submodule.h
index 859026ecfa..e5975d1f3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,5 +75,6 @@ int parallel_submodules(void);
  */
 void prepare_submodule_repo_env(struct argv_array *out);
 
-extern void migrate_submodule_gitdir(const char *path);
+extern void migrate_submodule_gitdir(const char *prefix,
+				     const char *path, int recursive);
 #endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
new file mode 100755
index 0000000000..3153dbc069
--- /dev/null
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='Test submodule embedgitdirs
+
+This test verifies that `git submodue embedgitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+	git init sub1 &&
+	test_commit -C sub1 first &&
+	git submodule add ./sub1 &&
+	test_tick &&
+	git commit -m superproject
+'
+
+test_expect_success 'embed the git dir' '
+	>expect.1 &&
+	>expect.2 &&
+	>actual.1 &&
+	>actual.2 &&
+	git status >expect.1 &&
+	git -C sub1 rev-parse HEAD >expect.2 &&
+	git submodule embedgitdirs &&
+	git fsck &&
+	test -f sub1/.git &&
+	test -d .git/modules/sub1 &&
+	git status >actual.1 &&
+	git -C sub1 rev-parse HEAD >actual.2 &&
+	test_cmp expect.1 actual.1 &&
+	test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup nested submodule' '
+	git init sub1/nested &&
+	test_commit -C sub1/nested first_nested &&
+	git -C sub1 submodule add ./nested &&
+	test_tick &&
+	git -C sub1 commit -m "add nested" &&
+	git add sub1 &&
+	git commit -m "sub1 to include nested submodule"
+'
+
+test_expect_success 'embed the git dir in a nested submodule' '
+	git status >expect.1 &&
+	git -C sub1/nested rev-parse HEAD >expect.2 &&
+	git submodule embedgitdirs &&
+	test -f sub1/nested/.git &&
+	test -d .git/modules/sub1/modules/nested &&
+	git status >actual.1 &&
+	git -C sub1/nested rev-parse HEAD >actual.2 &&
+	test_cmp expect.1 actual.1 &&
+	test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+	git init sub2 &&
+	test_commit -C sub2 first &&
+	git add sub2 &&
+	git commit -m superproject
+'
+
+test_expect_success 'embedding the git dir fails for incomplete submodules' '
+	git status >expect.1 &&
+	git -C sub2 rev-parse HEAD >expect.2 &&
+	test_must_fail git submodule embedgitdirs &&
+	git -C sub2 fsck &&
+	test -d sub2/.git &&
+	git status >actual &&
+	git -C sub2 rev-parse HEAD >actual.2 &&
+	test_cmp expect.1 actual.1 &&
+	test_cmp expect.2 actual.2
+'
+
+test_done
+
diff --git a/t/t7412-submodule-interngitdirs.sh b/t/t7412-submodule-interngitdirs.sh
deleted file mode 100755
index 709359155d..0000000000
--- a/t/t7412-submodule-interngitdirs.sh
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/bin/sh
-
-test_description='Test submodule interngitdirs
-
-This test verifies that `git submodue interngitdirs` moves a submodules git
-directory into the superproject.
-'
-
-. ./test-lib.sh
-
-test_expect_success 'setup a real submodule' '
-	git init sub1 &&
-	test_commit -C sub1 first &&
-	git submodule add ./sub1 &&
-	test_tick &&
-	git commit -m superproject
-'
-
-test_expect_success 'intern the git dir' '
-	git submodule interngitdirs &&
-	test -f sub1/.git &&
-	test -d .git/modules/sub1 &&
-	# check that we did not break the repository:
-	git status
-'
-
-test_expect_success 'setup a gitlink with missing .gitmodules entry' '
-	git init sub2 &&
-	test_commit -C sub2 first &&
-	git add sub2 &&
-	git commit -m superproject
-'
-
-test_expect_success 'intern the git dir fails for incomplete submodules' '
-	test_must_fail git submodule interngitdirs &&
-	# check that we did not break the repository:
-	git status
-'
-
-test_done
-- 
2.11.0.rc2.4.g3396b6f.dirty
Stefan Beller· Nov 22, 2016, 19:22 UTC · re: Stefan Beller · lore

The current caller of connect_work_tree_and_git_dir passes an absolute path for the `git_dir` parameter. In the future patch we will also pass in relative path for `git_dir`. Extend the functionality of connect_work_tree_and_git_dir to take relative paths for parameters.

We could work around this in the future patch by computing the absolute path for the git_dir in the calling site, however accepting relative paths for either parameter makes the API for this function much harder to misuse.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 submodule.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/submodule.c b/submodule.c
index 6f7d883de9..66c5ce5a24 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1227,23 +1227,25 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)
 {
 	struct strbuf file_name = STRBUF_INIT;
 	struct strbuf rel_path = STRBUF_INIT;
-	const char *real_work_tree = xstrdup(real_path(work_tree));
+	char *real_git_dir = xstrdup(real_path(git_dir));
+	char *real_work_tree = xstrdup(real_path(work_tree));
 
 	/* Update gitfile */
 	strbuf_addf(&file_name, "%s/.git", work_tree);
 	write_file(file_name.buf, "gitdir: %s",
-		   relative_path(git_dir, real_work_tree, &rel_path));
+		   relative_path(real_git_dir, real_work_tree, &rel_path));
 
 	/* Update core.worktree setting */
 	strbuf_reset(&file_name);
-	strbuf_addf(&file_name, "%s/config", git_dir);
+	strbuf_addf(&file_name, "%s/config", real_git_dir);
 	git_config_set_in_file(file_name.buf, "core.worktree",
-			       relative_path(real_work_tree, git_dir,
+			       relative_path(real_work_tree, real_git_dir,
 					     &rel_path));
 
 	strbuf_release(&file_name);
 	strbuf_release(&rel_path);
-	free((void *)real_work_tree);
+	free(real_work_tree);
+	free(real_git_dir);
 }
 
 int parallel_submodules(void)
-- 
2.11.0.rc2.4.g3396b6f.dirty
Stefan Beller· Nov 22, 2016, 19:22 UTC · re: Stefan Beller · lore

[PATCHv2 3/4] test-lib-functions.sh: teach test_commit -C <dir>

Specifically when setting up submodule tests, it comes in handy if we can create commits in repositories that are not at the root of the tested trash dir. Add "-C <dir>" similar to gits -C parameter that will perform the operation in the given directory.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 t/test-lib-functions.sh | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index fdaeb3a96b..579e812506 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -157,16 +157,21 @@ debug () {
 	 GIT_TEST_GDB=1 "$@"
 }
 
-# Call test_commit with the arguments "<message> [<file> [<contents> [<tag>]]]"
+# Call test_commit with the arguments
+# [-C <directory>] <message> [<file> [<contents> [<tag>]]]"
 #
 # This will commit a file with the given contents and the given commit
 # message, and tag the resulting commit with the given tag name.
 #
 # <file>, <contents>, and <tag> all default to <message>.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
 
 test_commit () {
 	notick= &&
 	signoff= &&
+	indir= &&
 	while test $# != 0
 	do
 		case "$1" in
@@ -176,21 +181,26 @@ test_commit () {
 		--signoff)
 			signoff="$1"
 			;;
+		-C)
+			indir="$2"
+			shift
+			;;
 		*)
 			break
 			;;
 		esac
 		shift
 	done &&
+	indir=${indir:+"$indir"/} &&
 	file=${2:-"$1.t"} &&
-	echo "${3-$1}" > "$file" &&
-	git add "$file" &&
+	echo "${3-$1}" > "$indir$file" &&
+	git ${indir:+ -C "$indir"} add "$file" &&
 	if test -z "$notick"
 	then
 		test_tick
 	fi &&
-	git commit $signoff -m "$1" &&
-	git tag "${4:-$1}"
+	git ${indir:+ -C "$indir"} commit $signoff -m "$1" &&
+	git ${indir:+ -C "$indir"} tag "${4:-$1}"
 }
 
 # Call test_merge with the arguments "<message> <commit>", where <commit>
-- 
2.11.0.rc2.4.g3396b6f.dirty
Stefan Beller· Nov 22, 2016, 19:22 UTC · re: Stefan Beller · lore

[PATCHv2 2/4] submodule helper: support super prefix

Just like main commands in Git, the submodule helper needs access to the superproject prefix. Enable this in the git.c but have its own fuse in the helper code by having a flag to turn on the super prefix.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 builtin/submodule--helper.c | 31 ++++++++++++++++++++-----------
 git.c                       |  2 +-
 2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4beeda5f9f..806e29ce4e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,21 +1076,24 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
 	return 0;
 }
 
+#define SUPPORT_SUPER_PREFIX (1<<0)
+
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
+	int option;
 };
 
 static struct cmd_struct commands[] = {
-	{"list", module_list},
-	{"name", module_name},
-	{"clone", module_clone},
-	{"update-clone", update_clone},
-	{"relative-path", resolve_relative_path},
-	{"resolve-relative-url", resolve_relative_url},
-	{"resolve-relative-url-test", resolve_relative_url_test},
-	{"init", module_init},
-	{"remote-branch", resolve_remote_submodule_branch}
+	{"list", module_list, 0},
+	{"name", module_name, 0},
+	{"clone", module_clone, 0},
+	{"update-clone", update_clone, 0},
+	{"relative-path", resolve_relative_path, 0},
+	{"resolve-relative-url", resolve_relative_url, 0},
+	{"resolve-relative-url-test", resolve_relative_url_test, 0},
+	{"init", module_init, 0},
+	{"remote-branch", resolve_remote_submodule_branch, 0}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
@@ -1100,9 +1103,15 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
 		die(_("submodule--helper subcommand must be "
 		      "called with a subcommand"));
 
-	for (i = 0; i < ARRAY_SIZE(commands); i++)
-		if (!strcmp(argv[1], commands[i].cmd))
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		if (!strcmp(argv[1], commands[i].cmd)) {
+			if (get_super_prefix() &&
+			    !(commands[i].option & SUPPORT_SUPER_PREFIX))
+				die("%s doesn't support --super-prefix",
+				    commands[i].cmd);
 			return commands[i].fn(argc - 1, argv + 1, prefix);
+		}
+	}
 
 	die(_("'%s' is not a valid submodule--helper "
 	      "subcommand"), argv[1]);
diff --git a/git.c b/git.c
index efa1059fe0..98dcf6c518 100644
--- a/git.c
+++ b/git.c
@@ -493,7 +493,7 @@ static struct cmd_struct commands[] = {
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
-	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP },
+	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
 	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 	{ "tag", cmd_tag, RUN_SETUP },
 	{ "unpack-file", cmd_unpack_file, RUN_SETUP },
-- 
2.11.0.rc2.4.g3396b6f.dirty
Stefan Beller· Nov 22, 2016, 19:22 UTC · re: Stefan Beller · lore

[PATCHv2 4/4] submodule: add embed-git-dir function

When a submodule has its git dir inside the working dir, the submodule support for checkout that we plan to add in a later patch will fail.

Add functionality to migrate the git directory to be embedded into the superprojects git directory.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 Documentation/git-submodule.txt   | 14 +++++++
 builtin/submodule--helper.c       | 33 ++++++++++++++++-
 git-submodule.sh                  |  7 +++-
 submodule.c                       | 65 ++++++++++++++++++++++++++++++++
 submodule.h                       |  2 +
 t/t7412-submodule-embedgitdirs.sh | 78 +++++++++++++++++++++++++++++++++++++++
 6 files changed, 197 insertions(+), 2 deletions(-)
 create mode 100755 t/t7412-submodule-embedgitdirs.sh
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index d841573475..34791cfc65 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -22,6 +22,7 @@ SYNOPSIS
 	      [commit] [--] [<path>...]
 'git submodule' [--quiet] foreach [--recursive] <command>
 'git submodule' [--quiet] sync [--recursive] [--] [<path>...]
+'git submodule' [--quiet] embedgitdirs [--] [<path>...]
 
 
 DESCRIPTION
@@ -245,6 +246,19 @@ sync::
 If `--recursive` is specified, this command will recurse into the
 registered submodules, and sync any nested submodules within.
 
+embedgitdirs::
+	Move the git directory of submodules into its superprojects
+	`$GIT_DIR/modules` path and then connect the git directory and
+	its working directory by setting the `core.worktree` and adding
+	a .git file pointing to the git directory interned into the
+	superproject.
++
+A repository that was cloned independently and later added as a submodule or
+old setups have the submodules git directory inside the submodule instead of
+embedded into the superprojects git directory.
++
+This command is recursive by default.
+
 OPTIONS
 -------
 -q::
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 806e29ce4e..e94dd68a0e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1076,6 +1076,36 @@ static int resolve_remote_submodule_branch(int argc, const char **argv,
 	return 0;
 }
 
+static int embed_git_dir(int argc, const char **argv, const char *prefix)
+{
+	int i;
+	struct pathspec pathspec;
+	struct module_list list = MODULE_LIST_INIT;
+
+	struct option embed_gitdir_options[] = {
+		OPT_END()
+	};
+
+	const char *const git_submodule_helper_usage[] = {
+		N_("git submodule--helper embed-git-dir [<path>...]"),
+		NULL
+	};
+
+	argc = parse_options(argc, argv, prefix, embed_gitdir_options,
+			     git_submodule_helper_usage, 0);
+
+	gitmodules_config();
+	git_config(submodule_config, NULL);
+
+	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
+		return 1;
+
+	for (i = 0; i < list.nr; i++)
+		migrate_submodule_gitdir(prefix, list.entries[i]->name, 1);
+
+	return 0;
+}
+
 #define SUPPORT_SUPER_PREFIX (1<<0)
 
 struct cmd_struct {
@@ -1093,7 +1123,8 @@ static struct cmd_struct commands[] = {
 	{"resolve-relative-url", resolve_relative_url, 0},
 	{"resolve-relative-url-test", resolve_relative_url_test, 0},
 	{"init", module_init, 0},
-	{"remote-branch", resolve_remote_submodule_branch, 0}
+	{"remote-branch", resolve_remote_submodule_branch, 0},
+	{"embed-git-dirs", embed_git_dir, SUPPORT_SUPER_PREFIX}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index a024a135d6..2178248287 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1131,6 +1131,11 @@ cmd_sync()
 	done
 }
 
+cmd_embedgitdirs()
+{
+	git submodule--helper --prefix "$wt_prefix" embed-git-dirs "$@"
+}
+
 # This loop parses the command line arguments to find the
 # subcommand name to dispatch.  Parsing of the subcommand specific
 # options are primarily done by the subcommand implementations.
@@ -1140,7 +1145,7 @@ cmd_sync()
 while test $# != 0 && test -z "$command"
 do
 	case "$1" in
-	add | foreach | init | deinit | update | status | summary | sync)
+	add | foreach | init | deinit | update | status | summary | sync | embedgitdirs)
 		command=$1
 		;;
 	-q|--quiet)
diff --git a/submodule.c b/submodule.c
index 66c5ce5a24..d330b567a3 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1263,3 +1263,68 @@ void prepare_submodule_repo_env(struct argv_array *out)
 	}
 	argv_array_push(out, "GIT_DIR=.git");
 }
+
+/*
+ * Migrate the given submodule (and all its submodules recursively) from
+ * having its git directory within the working tree to the git dir nested
+ * in its superprojects git dir under modules/.
+ */
+void migrate_submodule_gitdir(const char *prefix, const char *path,
+			      int recursive)
+{
+	char *old_git_dir;
+	const char *new_git_dir;
+	const struct submodule *sub;
+
+	old_git_dir = xstrfmt("%s/.git", path);
+	if (read_gitfile(old_git_dir))
+		/* If it is an actual gitfile, it doesn't need migration. */
+		goto out;
+
+	sub = submodule_from_path(null_sha1, path);
+	if (!sub)
+		die(_("Could not lookup name for submodule '%s'"),
+		      path);
+
+	new_git_dir = git_common_path("modules/%s", sub->name);
+	if (safe_create_leading_directories_const(new_git_dir) < 0)
+		die(_("could not create directory '%s'"), new_git_dir);
+
+	if (!prefix)
+		prefix = get_super_prefix();
+	printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
+		prefix ? prefix : "", path,
+		real_path(old_git_dir), new_git_dir);
+
+	if (rename(old_git_dir, new_git_dir) < 0)
+		die_errno(_("Could not migrate git directory from '%s' to '%s'"),
+			old_git_dir, new_git_dir);
+
+	connect_work_tree_and_git_dir(path, new_git_dir);
+
+out:
+	if (recursive) {
+		struct child_process cp = CHILD_PROCESS_INIT;
+		struct strbuf sb = STRBUF_INIT;
+
+		if (get_super_prefix())
+			strbuf_addstr(&sb, get_super_prefix());
+		strbuf_addstr(&sb, path);
+		strbuf_addch(&sb, '/');
+
+		cp.dir = path;
+		cp.git_cmd = 1;
+		cp.no_stdin = 1;
+		argv_array_pushl(&cp.args, "--super-prefix", sb.buf,
+					    "submodule--helper",
+					   "embed-git-dirs", NULL);
+		prepare_submodule_repo_env(&cp.env_array);
+		if (run_command(&cp))
+			die(_("Could not migrate git directory in submodule '%s'"),
+			    path);
+
+		strbuf_release(&sb);
+	}
+
+	free(old_git_dir);
+}
diff --git a/submodule.h b/submodule.h
index d9e197a948..e5975d1f3d 100644
--- a/submodule.h
+++ b/submodule.h
@@ -75,4 +75,6 @@ int parallel_submodules(void);
  */
 void prepare_submodule_repo_env(struct argv_array *out);
 
+extern void migrate_submodule_gitdir(const char *prefix,
+				     const char *path, int recursive);
 #endif
diff --git a/t/t7412-submodule-embedgitdirs.sh b/t/t7412-submodule-embedgitdirs.sh
new file mode 100755
index 0000000000..3153dbc069
--- /dev/null
+++ b/t/t7412-submodule-embedgitdirs.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+
+test_description='Test submodule embedgitdirs
+
+This test verifies that `git submodue embedgitdirs` moves a submodules git
+directory into the superproject.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup a real submodule' '
+	git init sub1 &&
+	test_commit -C sub1 first &&
+	git submodule add ./sub1 &&
+	test_tick &&
+	git commit -m superproject
+'
+
+test_expect_success 'embed the git dir' '
+	>expect.1 &&
+	>expect.2 &&
+	>actual.1 &&
+	>actual.2 &&
+	git status >expect.1 &&
+	git -C sub1 rev-parse HEAD >expect.2 &&
+	git submodule embedgitdirs &&
+	git fsck &&
+	test -f sub1/.git &&
+	test -d .git/modules/sub1 &&
+	git status >actual.1 &&
+	git -C sub1 rev-parse HEAD >actual.2 &&
+	test_cmp expect.1 actual.1 &&
+	test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup nested submodule' '
+	git init sub1/nested &&
+	test_commit -C sub1/nested first_nested &&
+	git -C sub1 submodule add ./nested &&
+	test_tick &&
+	git -C sub1 commit -m "add nested" &&
+	git add sub1 &&
+	git commit -m "sub1 to include nested submodule"
+'
+
+test_expect_success 'embed the git dir in a nested submodule' '
+	git status >expect.1 &&
+	git -C sub1/nested rev-parse HEAD >expect.2 &&
+	git submodule embedgitdirs &&
+	test -f sub1/nested/.git &&
+	test -d .git/modules/sub1/modules/nested &&
+	git status >actual.1 &&
+	git -C sub1/nested rev-parse HEAD >actual.2 &&
+	test_cmp expect.1 actual.1 &&
+	test_cmp expect.2 actual.2
+'
+
+test_expect_success 'setup a gitlink with missing .gitmodules entry' '
+	git init sub2 &&
+	test_commit -C sub2 first &&
+	git add sub2 &&
+	git commit -m superproject
+'
+
+test_expect_success 'embedding the git dir fails for incomplete submodules' '
+	git status >expect.1 &&
+	git -C sub2 rev-parse HEAD >expect.2 &&
+	test_must_fail git submodule embedgitdirs &&
+	git -C sub2 fsck &&
+	test -d sub2/.git &&
+	git status >actual &&
+	git -C sub2 rev-parse HEAD >actual.2 &&
+	test_cmp expect.1 actual.1 &&
+	test_cmp expect.2 actual.2
+'
+
+test_done
+
-- 
2.11.0.rc2.4.g3396b6f.dirty
Stefan Beller· Nov 22, 2016, 19:33 UTC · re: Stefan Beller · lore

[PATCH] SQUASH

Signed-off-by: Stefan Beller <sbeller@google.com>
---
On Tue, Nov 22, 2016 at 11:22 AM, Stefan Beller <sbeller@google.com> wrote:
Show 6 quoted lines
> When a submodule has its git dir inside the working dir, the submodule
> support for checkout that we plan to add in a later patch will fail.
>
> Add functionality to migrate the git directory to be embedded
> into the superprojects git directory.
>
I spoke too early, this needs to be squashed. :(
Stefan
 builtin/submodule--helper.c | 3 +++
 git-submodule.sh            | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e94dd68a0e..75cdbf45b8 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1083,6 +1083,9 @@ static int embed_git_dir(int argc, const char **argv, const char *prefix)
 	struct module_list list = MODULE_LIST_INIT;
 
 	struct option embed_gitdir_options[] = {
+		OPT_STRING(0, "prefix", &prefix,
+			   N_("path"),
+			   N_("path into the working tree")),
 		OPT_END()
 	};
 
diff --git a/git-submodule.sh b/git-submodule.sh
index 2178248287..b7e124f340 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1133,7 +1133,7 @@ cmd_sync()
 
 cmd_embedgitdirs()
 {
-	git submodule--helper --prefix "$wt_prefix" embed-git-dirs "$@"
+	git submodule--helper embed-git-dirs --prefix "$wt_prefix" "$@"
 }
 
 # This loop parses the command line arguments to find the
-- 
2.11.0.rc2.4.g3396b6f.dirty
Duy Nguyen· Nov 30, 2016, 11:14 UTC · re: Stefan Beller · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
Show 7 quoted lines
> +/*
> + * Migrate the given submodule (and all its submodules recursively) from
> + * having its git directory within the working tree to the git dir nested
> + * in its superprojects git dir under modules/.
> + */
> +void migrate_submodule_gitdir(const char *prefix, const char *path,
> +                             int recursive)

Submodules and worktrees seem to have many things in common. The first one is this. "git worktree move" on a worktree that contains submodules .git also benefits from something like this [1]. I suggest you move this function to some neutral place and maybe rename it to relocate_gitdir() or something.

It probably should take a bit flag instead of "recursive" here. One thing I would need is the ability to tell this function "I have moved all these .git dirs already (because I move whole worktree in one operation), here are the old and new locations of them, fix them up!". In other words, no rename() could be optionally skipped.

[1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u
Show 16 quoted lines
> +{
> +       char *old_git_dir;
> +       const char *new_git_dir;
> +       const struct submodule *sub;
> +
> +       old_git_dir = xstrfmt("%s/.git", path);
> +       if (read_gitfile(old_git_dir))
> +               /* If it is an actual gitfile, it doesn't need migration. */
> +               goto out;
> +
> +       sub = submodule_from_path(null_sha1, path);
> +       if (!sub)
> +               die(_("Could not lookup name for submodule '%s'"),
> +                     path);
> +
> +       new_git_dir = git_common_path("modules/%s", sub->name);

Why doesn't git_path() work here? This would make "modules" shared between worktrees, even though it's not normally. That inconsistency could cause trouble.

Show 14 quoted lines
> +       if (safe_create_leading_directories_const(new_git_dir) < 0)
> +               die(_("could not create directory '%s'"), new_git_dir);
> +
> +       if (!prefix)
> +               prefix = get_super_prefix();
> +       printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
> +               prefix ? prefix : "", path,
> +               real_path(old_git_dir), new_git_dir);
> +
> +       if (rename(old_git_dir, new_git_dir) < 0)
> +               die_errno(_("Could not migrate git directory from '%s' to '%s'"),
> +                       old_git_dir, new_git_dir);
> +
> +       connect_work_tree_and_git_dir(path, new_git_dir);

Another thing in common is, both submodules and worktrees use some form of textual symlinks. You need to fix up some here. But if this submodule has multiple worktreee, there may be some "symlinks" in .git/worktrees which would need fixing up as well.

You don't have to do the fix up thing right away, but I think we should at least make sure we leave no dangling links behind (by die()ing early if we find a .git dir we can't handle yet)

-- 
Duy
Stefan Beller· Nov 30, 2016, 18:04 UTC · re: Duy Nguyen · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

On Wed, Nov 30, 2016 at 3:14 AM, Duy Nguyen <pclouds@gmail.com> wrote:
Show 10 quoted lines
> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
>> +/*
>> + * Migrate the given submodule (and all its submodules recursively) from
>> + * having its git directory within the working tree to the git dir nested
>> + * in its superprojects git dir under modules/.
>> + */
>> +void migrate_submodule_gitdir(const char *prefix, const char *path,
>> +                             int recursive)
>
> Submodules and worktrees seem to have many things in common.
Yes. :)
> The first
> one is this. "git worktree move" on a worktree that contains
> submodules .git also benefits from something like this [1].

That patch is a sensible approach. :) (By checking all files to not be submodules a worktree would not run into problems like a127331cd81, mv: allow moving nested submodules)

> I suggest
> you move this function to some neutral place and maybe rename it to
> relocate_gitdir() or something.

ok tell me where this neutral place is found? (I'd prefer to not clobber it into cache.h *the* most neutral place in git) Maybe dir.{c,h} ?

Show 6 quoted lines
>
> It probably should take a bit flag instead of "recursive" here. One
> thing I would need is the ability to tell this function "I have moved
> all these .git dirs already (because I move whole worktree in one
> operation), here are the old and new locations of them, fix them up!".
> In other words, no rename() could be optionally skipped.

In the non-main working trees you'd also have a .git file linking to the actual git dir and you'd only have to fix them up instead of moving.

Show 23 quoted lines
>
> [1] https://public-inbox.org/git/20161128094319.16176-11-pclouds@gmail.com/T/#u
>
>> +{
>> +       char *old_git_dir;
>> +       const char *new_git_dir;
>> +       const struct submodule *sub;
>> +
>> +       old_git_dir = xstrfmt("%s/.git", path);
>> +       if (read_gitfile(old_git_dir))
>> +               /* If it is an actual gitfile, it doesn't need migration. */
>> +               goto out;
>> +
>> +       sub = submodule_from_path(null_sha1, path);
>> +       if (!sub)
>> +               die(_("Could not lookup name for submodule '%s'"),
>> +                     path);
>> +
>> +       new_git_dir = git_common_path("modules/%s", sub->name);
>
> Why doesn't git_path() work here? This would make "modules" shared
> between worktrees, even though it's not normally. That inconsistency
> could cause trouble.

I thought that was a long term goal? (I actually think about reviving the series you sent out a few weeks ago to make worktree and submodules work well together)

So for that we'd want to have at least the object store shared across all worktrees.

Show 20 quoted lines
>
>> +       if (safe_create_leading_directories_const(new_git_dir) < 0)
>> +               die(_("could not create directory '%s'"), new_git_dir);
>> +
>> +       if (!prefix)
>> +               prefix = get_super_prefix();
>> +       printf("Migrating git directory of %s%s from\n'%s' to\n'%s'\n",
>> +               prefix ? prefix : "", path,
>> +               real_path(old_git_dir), new_git_dir);
>> +
>> +       if (rename(old_git_dir, new_git_dir) < 0)
>> +               die_errno(_("Could not migrate git directory from '%s' to '%s'"),
>> +                       old_git_dir, new_git_dir);
>> +
>> +       connect_work_tree_and_git_dir(path, new_git_dir);
>
> Another thing in common is, both submodules and worktrees use some
> form of textual symlinks. You need to fix up some here. But if this
> submodule has multiple worktreee, there may be some "symlinks" in
> .git/worktrees which would need fixing up as well.

We could signal that via one of the flag bits? (e.g. FIXUP_WORKTREE_SYMLINKS )

Show 6 quoted lines
>
> You don't have to do the fix up thing right away, but I think we
> should at least make sure we leave no dangling links behind (by
> die()ing early if we find a .git dir we can't handle yet)
> --
> Duy
Stefan Beller· Nov 30, 2016, 20:48 UTC · re: Stefan Beller · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

On Wed, Nov 30, 2016 at 10:04 AM, Stefan Beller <sbeller@google.com> wrote:
>> Submodules and worktrees seem to have many things in common.
>
> Yes. :)

I moved the code to dir.{c,h} and renamed it to a more generic "move_gitdir", but then I am thinking further about how to align worktrees and submodules here, specifically the recursive part.

Whenever submodules and recursion is involved so far we spawned off a child process to take care of the issue in the submodule itself. Here we followed the same pattern and called the submodule--helper to embed the git dirs in the submodule recursively.

As this function is not supposed to be submodule specific anymore, I'd rather not want to have the recursive childrens code live inside the submodule helper, so we also need a neutral helper for moving git directories?

So there are a couple ways forward:
* We declare the "recursive" flag to be submodule specific and keep
the recursion
  in the submodule helper
* the recursive flag is generic enough and we invent a plumbing helper.
  Analogous to the submodule--helper, that was originally invented as a C aid
  for the git-submodule shell script, we could have a "git--helper" or just
  "git plumbing <subcmd>", though that sounds like reinventing the wheel as
  traditionally we'd just have a top level command to do a specific thing.
  (side note: Some parts of git-rev-parse could go into such a plumbing
  command)

For now I'd think to declare recursion in this function to be submodule specific.

Stefan
Junio C Hamano· Nov 30, 2016, 20:51 UTC · re: Duy Nguyen · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

Duy Nguyen <pclouds@gmail.com> writes:
Show 14 quoted lines
> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
>> +/*
>> + * Migrate the given submodule (and all its submodules recursively) from
>> + * having its git directory within the working tree to the git dir nested
>> + * in its superprojects git dir under modules/.
>> + */
>> +void migrate_submodule_gitdir(const char *prefix, const char *path,
>> +                             int recursive)
>
> Submodules and worktrees seem to have many things in common. The first
> one is this. "git worktree move" on a worktree that contains
> submodules .git also benefits from something like this [1]. I suggest
> you move this function to some neutral place and maybe rename it to
> relocate_gitdir() or something.

Yeah, good suggestion (including name; first round used "intern" I had trouble with, then "embed" which was OK-ish, but probably "relocate" is better choice. If anything, what Stefan's series adds is a command to un-embed embedded one).

Show 5 quoted lines
> It probably should take a bit flag instead of "recursive" here. One
> thing I would need is the ability to tell this function "I have moved
> all these .git dirs already (because I move whole worktree in one
> operation), here are the old and new locations of them, fix them up!".
> In other words, no rename() could be optionally skipped.
Thanks two of you for working well together ;-)
Stefan Beller· Nov 30, 2016, 21:00 UTC · re: Junio C Hamano · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

On Wed, Nov 30, 2016 at 12:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 21 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Wed, Nov 23, 2016 at 2:22 AM, Stefan Beller <sbeller@google.com> wrote:
>>> +/*
>>> + * Migrate the given submodule (and all its submodules recursively) from
>>> + * having its git directory within the working tree to the git dir nested
>>> + * in its superprojects git dir under modules/.
>>> + */
>>> +void migrate_submodule_gitdir(const char *prefix, const char *path,
>>> +                             int recursive)
>>
>> Submodules and worktrees seem to have many things in common. The first
>> one is this. "git worktree move" on a worktree that contains
>> submodules .git also benefits from something like this [1]. I suggest
>> you move this function to some neutral place and maybe rename it to
>> relocate_gitdir() or something.
>
> Yeah, good suggestion (including name; first round used "intern" I
> had trouble with, then "embed" which was OK-ish, but probably
> "relocate" is better choice.  If anything, what Stefan's series adds
> is a command to un-embed embedded one).

Heh, good counter perspective. I thought about embedding it "into the superprojects git directory" whereas your un-embed sounds like you'd assume embedding is targetd towards the working dir.

relocate as a fancy name for move sounds like it expects 2 arguments (source and destination), but maybe we could go with:

    git relocate-git-dir (--into-workingtree|--into-gitdir) \
      [--recurse-submodules] \
      [--only-fix-gitfile-links-and-core-setting-as-I-moved-it-myself-already] \
      [--dryrun] [--verbose] [--unsafe-move]

No need to have a pathspec here IMHO. Later it could have another flag to specify the "main" worktree or such as well.

Junio C Hamano· Nov 30, 2016, 21:39 UTC · re: Stefan Beller · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

Stefan Beller <sbeller@google.com> writes:
>     git relocate-git-dir (--into-workingtree|--into-gitdir) \

I am not sure if you meant this as a submodule-specific subcommand or more general helper. "into-workingtree" suggests to me that it is submodule specific, so I'll base my response on that assumption.

Would there ever be a situation where you already have submodule repositories in the right place (according to the more modern practice, to keep them in .git/modules/ of superproject) and want to move them to embed them in worktrees of submodules? I do not think of any.

If there is no such situation, I do not think we want a verb that is direction-neutral (e.g. "move" or "relocate") with two options. Rather we would want "git submodule unembed-git-dir" or something like that.

Stefan Beller· Nov 30, 2016, 21:56 UTC · re: Junio C Hamano · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

On Wed, Nov 30, 2016 at 1:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>>     git relocate-git-dir (--into-workingtree|--into-gitdir) \
>
> I am not sure if you meant this as a submodule-specific subcommand
> or more general helper.  "into-workingtree" suggests to me that it
> is submodule specific, so I'll base my response on that assumption.
>
> Would there ever be a situation where you already have submodule
> repositories in the right place (according to the more modern
> practice, to keep them in .git/modules/ of superproject) and want to
> move them to embed them in worktrees of submodules?  I do not think
> of any.
 "Hi, I made a mistake by using submodules. I don't want to use
  them any more, I rather want to:
  A) make it a separate git repo again and I'll keep them in sync myself
  B) ... "
 "I abuse submodules for what git-LFS was designed for, and the
  submodule is on a different mount point, please keep the git directory
  also at that mount point".

Not sure I agree these problems and the proposed solutions are beautiful, but that is what people may think of as a fast hack?

Show 5 quoted lines
>
> If there is no such situation, I do not think we want a verb that is
> direction-neutral (e.g. "move" or "relocate") with two options.
> Rather we would want "git submodule unembed-git-dir" or something
> like that.

So when we want to have a generic function in C ("relocate_gitdir") for both worktree and submodules, the recursive flag is not supposed to invoke a submodule specific helper, but a generic helper.

Alternatively we make the function not as generic and claim the recursive part is submodule specific and we can happily call "git submodule [un]embed-git-dir" recursively.

Junio C Hamano· Nov 30, 2016, 22:18 UTC · re: Stefan Beller · lore

Re: [PATCHv2 4/4] submodule: add embed-git-dir function

Stefan Beller <sbeller@google.com> writes:
Show 19 quoted lines
> On Wed, Nov 30, 2016 at 1:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>>     git relocate-git-dir (--into-workingtree|--into-gitdir) \
>>
>> I am not sure if you meant this as a submodule-specific subcommand
>> or more general helper.  "into-workingtree" suggests to me that it
>> is submodule specific, so I'll base my response on that assumption.
>>
>> Would there ever be a situation where you already have submodule
>> repositories in the right place (according to the more modern
>> practice, to keep them in .git/modules/ of superproject) and want to
>> move them to embed them in worktrees of submodules?  I do not think
>> of any.
>
>  "Hi, I made a mistake by using submodules. I don't want to use
>   them any more, I rather want to:
>   A) make it a separate git repo again and I'll keep them in sync myself
>   B) ... "
OK, I can buy that.  Thanks.

← back to recent threads