[PATCH] submodule update --init: displays correct path from submodule

Subsystems: the rest

STALE3494d

3 messages, 2 authors, 2017-01-07 · open the first message on its own page

[PATCH] submodule update --init: displays correct path from submodule

From: Stefan Beller <hidden>
Date: 2017-01-07 00:20:27

In the submodule helper we did not correctly handled the display path
for initializing submodules when both the submodule is inside a
subdirectory as well as the command being invoked from a subdirectory
(as viewed from the superproject).

This was broken in 3604242f080, which was written at a time where
there was no super-prefix available, so we abused the --prefix option
for the same purpose and could get only one case right (the call from
within a subdirectory, not the submodule being in a subdirectory).

Test-provided-by: David Turner [off-list ref]
Signed-off-by: Stefan Beller <redacted>
---

  applies on sb/submodule-embed-gitdir as that contains 89c862655
  (submodule helper: support super prefix)

 builtin/submodule--helper.c | 13 +++++++------
 git-submodule.sh            |  2 +-
 t/t7406-submodule-update.sh | 17 +++++++++++++++++
 3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 242d9911a6..7b3f9fc293 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -305,32 +305,36 @@ static int module_list(int argc, const char **argv, const char *prefix)
 
 		utf8_fprintf(stdout, "%s\n", ce->name);
 	}
 	return 0;
 }
 
 static void init_submodule(const char *path, const char *prefix, int quiet)
 {
 	const struct submodule *sub;
 	struct strbuf sb = STRBUF_INIT;
 	char *upd = NULL, *url = NULL, *displaypath;
 
 	/* Only loads from .gitmodules, no overlay with .git/config */
 	gitmodules_config();
 
-	if (prefix) {
-		strbuf_addf(&sb, "%s%s", prefix, path);
+	if (prefix && get_super_prefix())
+		die("BUG: cannot have prefix and superprefix");
+	else if (prefix)
+		displaypath = xstrdup(relative_path(path, prefix, &sb));
+	else if (get_super_prefix()) {
+		strbuf_addf(&sb, "%s%s", get_super_prefix(), path);
 		displaypath = strbuf_detach(&sb, NULL);
 	} else
 		displaypath = xstrdup(path);
 
 	sub = submodule_from_path(null_sha1, path);
 
 	if (!sub)
 		die(_("No url found for submodule path '%s' in .gitmodules"),
 			displaypath);
 
 	/*
 	 * Copy url setting when it is not set yet.
 	 * To look up the url in .git/config, we must not fall back to
 	 * .gitmodules, so look it up directly.
 	 */
@@ -391,33 +395,30 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 	}
 	strbuf_release(&sb);
 	free(displaypath);
 	free(url);
 	free(upd);
 }
 
 static int module_init(int argc, const char **argv, const char *prefix)
 {
 	struct pathspec pathspec;
 	struct module_list list = MODULE_LIST_INIT;
 	int quiet = 0;
 	int i;
 
 	struct option module_init_options[] = {
-		OPT_STRING(0, "prefix", &prefix,
-			   N_("path"),
-			   N_("alternative anchor for relative paths")),
 		OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")),
 		OPT_END()
 	};
 
 	const char *const git_submodule_helper_usage[] = {
 		N_("git submodule--helper init [<path>]"),
 		NULL
 	};
 
 	argc = parse_options(argc, argv, prefix, module_init_options,
 			     git_submodule_helper_usage, 0);
 
 	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
 		return 1;
 
@@ -1117,31 +1118,31 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
 
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
 	unsigned option;
 };
 
 static struct cmd_struct commands[] = {
 	{"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},
+	{"init", module_init, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	if (argc < 2)
 		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)) {
 			if (get_super_prefix() &&
 			    !(commands[i].option & SUPPORT_SUPER_PREFIX))
diff --git a/git-submodule.sh b/git-submodule.sh
index 9285b5c43d..4e47ff8ad8 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -362,31 +362,31 @@ cmd_init()
 			;;
 		--)
 			shift
 			break
 			;;
 		-*)
 			usage
 			;;
 		*)
 			break
 			;;
 		esac
 		shift
 	done
 
-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} "$@"
+	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet}  "$@"
 }
 
 #
 # Unregister submodules from .git/config and remove their work tree
 #
 cmd_deinit()
 {
 	# parse $args after "submodule ... deinit".
 	deinit_all=
 	while test $# -ne 0
 	do
 		case "$1" in
 		-f|--force)
 			force=$1
 			;;
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 64f322c4cc..725bbed1f8 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -128,30 +128,47 @@ done.
 Cloning into '$pwd/recursivesuper/super/submodule'...
 done.
 EOF
 
 test_expect_success 'submodule update --init --recursive from subdirectory' '
 	git -C recursivesuper/super reset --hard HEAD^ &&
 	(cd recursivesuper &&
 	 mkdir tmp &&
 	 cd tmp &&
 	 git submodule update --init --recursive ../super >../../actual 2>../../actual2
 	) &&
 	test_i18ncmp expect actual &&
 	test_i18ncmp expect2 actual2
 '
 
+cat <<EOF >expect2
+Submodule 'foo/sub' ($pwd/withsubs/../rebasing) registered for path 'sub'
+EOF
+
+test_expect_success 'submodule update --init from and of subdirectory' '
+	git init withsubs &&
+	(cd withsubs &&
+	 mkdir foo &&
+	 git submodule add "$(pwd)/../rebasing" foo/sub &&
+	 (cd foo &&
+	  git submodule deinit -f sub &&
+	  git submodule update --init sub 2>../../actual2
+	 )
+	) &&
+	test_i18ncmp expect2 actual2
+'
+
 apos="'";
 test_expect_success 'submodule update does not fetch already present commits' '
 	(cd submodule &&
 	  echo line3 >> file &&
 	  git add file &&
 	  test_tick &&
 	  git commit -m "upstream line3"
 	) &&
 	(cd super/submodule &&
 	  head=$(git rev-parse --verify HEAD) &&
 	  echo "Submodule path ${apos}submodule$apos: checked out $apos$head$apos" > ../../expected &&
 	  git reset --hard HEAD~1
 	) &&
 	(cd super &&
 	  git submodule update > ../actual 2> ../actual.err
-- 
2.11.0.rc2.30.g7c4be45.dirty

Re: [PATCH] submodule update --init: displays correct path from submodule

From: David Turner <hidden>
Date: 2017-01-07 00:55:09

(for the test)
Signed-off-by: David Turner <redacted>

TIL: super-prefix!


On Fri, 2017-01-06 at 16:19 -0800, Stefan Beller wrote:
quoted hunk
In the submodule helper we did not correctly handled the display path
for initializing submodules when both the submodule is inside a
subdirectory as well as the command being invoked from a subdirectory
(as viewed from the superproject).

This was broken in 3604242f080, which was written at a time where
there was no super-prefix available, so we abused the --prefix option
for the same purpose and could get only one case right (the call from
within a subdirectory, not the submodule being in a subdirectory).

Test-provided-by: David Turner [off-list ref]
Signed-off-by: Stefan Beller <redacted>
---

  applies on sb/submodule-embed-gitdir as that contains 89c862655
  (submodule helper: support super prefix)

 builtin/submodule--helper.c | 13 +++++++------
 git-submodule.sh            |  2 +-
 t/t7406-submodule-update.sh | 17 +++++++++++++++++
 3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 242d9911a6..7b3f9fc293 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -305,32 +305,36 @@ static int module_list(int argc, const char **argv, const char *prefix)
 
 		utf8_fprintf(stdout, "%s\n", ce->name);
 	}
 	return 0;
 }
 
 static void init_submodule(const char *path, const char *prefix, int quiet)
 {
 	const struct submodule *sub;
 	struct strbuf sb = STRBUF_INIT;
 	char *upd = NULL, *url = NULL, *displaypath;
 
 	/* Only loads from .gitmodules, no overlay with .git/config */
 	gitmodules_config();
 
-	if (prefix) {
-		strbuf_addf(&sb, "%s%s", prefix, path);
+	if (prefix && get_super_prefix())
+		die("BUG: cannot have prefix and superprefix");
+	else if (prefix)
+		displaypath = xstrdup(relative_path(path, prefix, &sb));
+	else if (get_super_prefix()) {
+		strbuf_addf(&sb, "%s%s", get_super_prefix(), path);
 		displaypath = strbuf_detach(&sb, NULL);
 	} else
 		displaypath = xstrdup(path);
 
 	sub = submodule_from_path(null_sha1, path);
 
 	if (!sub)
 		die(_("No url found for submodule path '%s' in .gitmodules"),
 			displaypath);
 
 	/*
 	 * Copy url setting when it is not set yet.
 	 * To look up the url in .git/config, we must not fall back to
 	 * .gitmodules, so look it up directly.
 	 */
@@ -391,33 +395,30 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
 	}
 	strbuf_release(&sb);
 	free(displaypath);
 	free(url);
 	free(upd);
 }
 
 static int module_init(int argc, const char **argv, const char *prefix)
 {
 	struct pathspec pathspec;
 	struct module_list list = MODULE_LIST_INIT;
 	int quiet = 0;
 	int i;
 
 	struct option module_init_options[] = {
-		OPT_STRING(0, "prefix", &prefix,
-			   N_("path"),
-			   N_("alternative anchor for relative paths")),
 		OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")),
 		OPT_END()
 	};
 
 	const char *const git_submodule_helper_usage[] = {
 		N_("git submodule--helper init [<path>]"),
 		NULL
 	};
 
 	argc = parse_options(argc, argv, prefix, module_init_options,
 			     git_submodule_helper_usage, 0);
 
 	if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
 		return 1;
 
@@ -1117,31 +1118,31 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
 
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
 	unsigned option;
 };
 
 static struct cmd_struct commands[] = {
 	{"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},
+	{"init", module_init, SUPPORT_SUPER_PREFIX},
 	{"remote-branch", resolve_remote_submodule_branch, 0},
 	{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	if (argc < 2)
 		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)) {
 			if (get_super_prefix() &&
 			    !(commands[i].option & SUPPORT_SUPER_PREFIX))
diff --git a/git-submodule.sh b/git-submodule.sh
index 9285b5c43d..4e47ff8ad8 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -362,31 +362,31 @@ cmd_init()
 			;;
 		--)
 			shift
 			break
 			;;
 		-*)
 			usage
 			;;
 		*)
 			break
 			;;
 		esac
 		shift
 	done
 
-	git ${wt_prefix:+-C "$wt_prefix"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} "$@"
+	git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet}  "$@"
 }
 
 #
 # Unregister submodules from .git/config and remove their work tree
 #
 cmd_deinit()
 {
 	# parse $args after "submodule ... deinit".
 	deinit_all=
 	while test $# -ne 0
 	do
 		case "$1" in
 		-f|--force)
 			force=$1
 			;;
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 64f322c4cc..725bbed1f8 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -128,30 +128,47 @@ done.
 Cloning into '$pwd/recursivesuper/super/submodule'...
 done.
 EOF
 
 test_expect_success 'submodule update --init --recursive from subdirectory' '
 	git -C recursivesuper/super reset --hard HEAD^ &&
 	(cd recursivesuper &&
 	 mkdir tmp &&
 	 cd tmp &&
 	 git submodule update --init --recursive ../super >../../actual 2>../../actual2
 	) &&
 	test_i18ncmp expect actual &&
 	test_i18ncmp expect2 actual2
 '
 
+cat <<EOF >expect2
+Submodule 'foo/sub' ($pwd/withsubs/../rebasing) registered for path 'sub'
+EOF
+
+test_expect_success 'submodule update --init from and of subdirectory' '
+	git init withsubs &&
+	(cd withsubs &&
+	 mkdir foo &&
+	 git submodule add "$(pwd)/../rebasing" foo/sub &&
+	 (cd foo &&
+	  git submodule deinit -f sub &&
+	  git submodule update --init sub 2>../../actual2
+	 )
+	) &&
+	test_i18ncmp expect2 actual2
+'
+
 apos="'";
 test_expect_success 'submodule update does not fetch already present commits' '
 	(cd submodule &&
 	  echo line3 >> file &&
 	  git add file &&
 	  test_tick &&
 	  git commit -m "upstream line3"
 	) &&
 	(cd super/submodule &&
 	  head=$(git rev-parse --verify HEAD) &&
 	  echo "Submodule path ${apos}submodule$apos: checked out $apos$head$apos" > ../../expected &&
 	  git reset --hard HEAD~1
 	) &&
 	(cd super &&
 	  git submodule update > ../actual 2> ../actual.err

Re: [PATCH] submodule update --init: displays correct path from submodule

From: Stefan Beller <hidden>
Date: 2017-01-07 01:01:21

On Fri, Jan 6, 2017 at 4:55 PM, David Turner [off-list ref] wrote:
(for the test)
Signed-off-by: David Turner <redacted>

TIL: super-prefix!
yeah that was introduced recently for prefixes from "outside the repository"
e.g. a superproject, its first user is grep --recurse-submodules.

Also I realize I have set diff.context to 15 as I was experimenting
with it earlier,
and forgot to turn it off. So a lot of context in the diff for the patch.

Thanks for the bug report,
Stefan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help