* [PATCH] submodule update --init: displays correct path from submodule
@ 2017-01-07 0:19 Stefan Beller
2017-01-07 0:55 ` David Turner
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Beller @ 2017-01-07 0:19 UTC (permalink / raw)
To: gitster; +Cc: novalis, git, Stefan Beller
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 <novalis@novalis.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] submodule update --init: displays correct path from submodule
2017-01-07 0:19 [PATCH] submodule update --init: displays correct path from submodule Stefan Beller
@ 2017-01-07 0:55 ` David Turner
2017-01-07 1:01 ` Stefan Beller
0 siblings, 1 reply; 3+ messages in thread
From: David Turner @ 2017-01-07 0:55 UTC (permalink / raw)
To: Stefan Beller; +Cc: gitster, git
(for the test)
Signed-off-by: David Turner <dturner@twosigma.com>
TIL: super-prefix!
On Fri, 2017-01-06 at 16:19 -0800, Stefan Beller wrote:
> 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 <novalis@novalis.org>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] submodule update --init: displays correct path from submodule
2017-01-07 0:55 ` David Turner
@ 2017-01-07 1:01 ` Stefan Beller
0 siblings, 0 replies; 3+ messages in thread
From: Stefan Beller @ 2017-01-07 1:01 UTC (permalink / raw)
To: David Turner; +Cc: Junio C Hamano, git@vger.kernel.org
On Fri, Jan 6, 2017 at 4:55 PM, David Turner <novalis@novalis.org> wrote:
> (for the test)
> Signed-off-by: David Turner <dturner@twosigma.com>
>
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-07 1:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-07 0:19 [PATCH] submodule update --init: displays correct path from submodule Stefan Beller
2017-01-07 0:55 ` David Turner
2017-01-07 1:01 ` Stefan Beller
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 NNTP newsgroup(s).