{"thread":{"id":"44831","subject":"[PATCH] submodule update --init: displays correct path from submodule","startedAt":"2017-01-07T00:20:27Z","lastAt":"2017-01-07T01:01:21Z","messageCount":3,"participants":["Stefan Beller","David Turner"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"308903","messageId":"20170107001953.3196-1-sbeller@google.com","threadId":"44831","inReplyTo":null,"subject":"[PATCH] submodule update --init: displays correct path from submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-07T00:19:53Z","receivedAt":"2017-01-07T00:20:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In the submodule helper we did not correctly handled the display path\nfor initializing submodules when both the submodule is inside a\nsubdirectory as well as the command being invoked from a subdirectory\n(as viewed from the superproject).\n\nThis was broken in 3604242f080, which was written at a time where\nthere was no super-prefix available, so we abused the --prefix option\nfor the same purpose and could get only one case right (the call from\nwithin a subdirectory, not the submodule being in a subdirectory).\n\nTest-provided-by: David Turner <novalis@novalis.org>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n  applies on sb/submodule-embed-gitdir as that contains 89c862655\n  (submodule helper: support super prefix)\n\n builtin/submodule--helper.c | 13 +++++++------\n git-submodule.sh            |  2 +-\n t/t7406-submodule-update.sh | 17 +++++++++++++++++\n 3 files changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 242d9911a6..7b3f9fc293 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -305,32 +305,36 @@ static int module_list(int argc, const char **argv, const char *prefix)\n \n \t\tutf8_fprintf(stdout, \"%s\\n\", ce->name);\n \t}\n \treturn 0;\n }\n \n static void init_submodule(const char *path, const char *prefix, int quiet)\n {\n \tconst struct submodule *sub;\n \tstruct strbuf sb = STRBUF_INIT;\n \tchar *upd = NULL, *url = NULL, *displaypath;\n \n \t/* Only loads from .gitmodules, no overlay with .git/config */\n \tgitmodules_config();\n \n-\tif (prefix) {\n-\t\tstrbuf_addf(&sb, \"%s%s\", prefix, path);\n+\tif (prefix && get_super_prefix())\n+\t\tdie(\"BUG: cannot have prefix and superprefix\");\n+\telse if (prefix)\n+\t\tdisplaypath = xstrdup(relative_path(path, prefix, &sb));\n+\telse if (get_super_prefix()) {\n+\t\tstrbuf_addf(&sb, \"%s%s\", get_super_prefix(), path);\n \t\tdisplaypath = strbuf_detach(&sb, NULL);\n \t} else\n \t\tdisplaypath = xstrdup(path);\n \n \tsub = submodule_from_path(null_sha1, path);\n \n \tif (!sub)\n \t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n \t\t\tdisplaypath);\n \n \t/*\n \t * Copy url setting when it is not set yet.\n \t * To look up the url in .git/config, we must not fall back to\n \t * .gitmodules, so look it up directly.\n \t */\n@@ -391,33 +395,30 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t}\n \tstrbuf_release(&sb);\n \tfree(displaypath);\n \tfree(url);\n \tfree(upd);\n }\n \n static int module_init(int argc, const char **argv, const char *prefix)\n {\n \tstruct pathspec pathspec;\n \tstruct module_list list = MODULE_LIST_INIT;\n \tint quiet = 0;\n \tint i;\n \n \tstruct option module_init_options[] = {\n-\t\tOPT_STRING(0, \"prefix\", &prefix,\n-\t\t\t   N_(\"path\"),\n-\t\t\t   N_(\"alternative anchor for relative paths\")),\n \t\tOPT__QUIET(&quiet, N_(\"Suppress output for initializing a submodule\")),\n \t\tOPT_END()\n \t};\n \n \tconst char *const git_submodule_helper_usage[] = {\n \t\tN_(\"git submodule--helper init [<path>]\"),\n \t\tNULL\n \t};\n \n \targc = parse_options(argc, argv, prefix, module_init_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n \tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n \t\treturn 1;\n \n@@ -1117,31 +1118,31 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n \n struct cmd_struct {\n \tconst char *cmd;\n \tint (*fn)(int, const char **, const char *);\n \tunsigned option;\n };\n \n static struct cmd_struct commands[] = {\n \t{\"list\", module_list, 0},\n \t{\"name\", module_name, 0},\n \t{\"clone\", module_clone, 0},\n \t{\"update-clone\", update_clone, 0},\n \t{\"relative-path\", resolve_relative_path, 0},\n \t{\"resolve-relative-url\", resolve_relative_url, 0},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n-\t{\"init\", module_init, 0},\n+\t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n \tif (argc < 2)\n \t\tdie(_(\"submodule--helper subcommand must be \"\n \t\t      \"called with a subcommand\"));\n \n \tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n \t\tif (!strcmp(argv[1], commands[i].cmd)) {\n \t\t\tif (get_super_prefix() &&\n \t\t\t    !(commands[i].option & SUPPORT_SUPER_PREFIX))\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9285b5c43d..4e47ff8ad8 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -362,31 +362,31 @@ cmd_init()\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n \t\t\t;;\n \t\t-*)\n \t\t\tusage\n \t\t\t;;\n \t\t*)\n \t\t\tbreak\n \t\t\t;;\n \t\tesac\n \t\tshift\n \tdone\n \n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix \"$prefix\"} \"$@\"\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper init ${GIT_QUIET:+--quiet}  \"$@\"\n }\n \n #\n # Unregister submodules from .git/config and remove their work tree\n #\n cmd_deinit()\n {\n \t# parse $args after \"submodule ... deinit\".\n \tdeinit_all=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n \t\t-f|--force)\n \t\t\tforce=$1\n \t\t\t;;\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 64f322c4cc..725bbed1f8 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -128,30 +128,47 @@ done.\n Cloning into '$pwd/recursivesuper/super/submodule'...\n done.\n EOF\n \n test_expect_success 'submodule update --init --recursive from subdirectory' '\n \tgit -C recursivesuper/super reset --hard HEAD^ &&\n \t(cd recursivesuper &&\n \t mkdir tmp &&\n \t cd tmp &&\n \t git submodule update --init --recursive ../super >../../actual 2>../../actual2\n \t) &&\n \ttest_i18ncmp expect actual &&\n \ttest_i18ncmp expect2 actual2\n '\n \n+cat <<EOF >expect2\n+Submodule 'foo/sub' ($pwd/withsubs/../rebasing) registered for path 'sub'\n+EOF\n+\n+test_expect_success 'submodule update --init from and of subdirectory' '\n+\tgit init withsubs &&\n+\t(cd withsubs &&\n+\t mkdir foo &&\n+\t git submodule add \"$(pwd)/../rebasing\" foo/sub &&\n+\t (cd foo &&\n+\t  git submodule deinit -f sub &&\n+\t  git submodule update --init sub 2>../../actual2\n+\t )\n+\t) &&\n+\ttest_i18ncmp expect2 actual2\n+'\n+\n apos=\"'\";\n test_expect_success 'submodule update does not fetch already present commits' '\n \t(cd submodule &&\n \t  echo line3 >> file &&\n \t  git add file &&\n \t  test_tick &&\n \t  git commit -m \"upstream line3\"\n \t) &&\n \t(cd super/submodule &&\n \t  head=$(git rev-parse --verify HEAD) &&\n \t  echo \"Submodule path ${apos}submodule$apos: checked out $apos$head$apos\" > ../../expected &&\n \t  git reset --hard HEAD~1\n \t) &&\n \t(cd super &&\n \t  git submodule update > ../actual 2> ../actual.err\n-- \n2.11.0.rc2.30.g7c4be45.dirty\n\n"},{"id":"308904","messageId":"1483750501.31165.9.camel@frank","threadId":"44831","inReplyTo":"20170107001953.3196-1-sbeller@google.com","subject":"Re: [PATCH] submodule update --init: displays correct path from submodule","fromName":"David Turner","fromEmail":"novalis@novalis.org","sentAt":"2017-01-07T00:55:01Z","receivedAt":"2017-01-07T00:55:09Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"(for the test)\nSigned-off-by: David Turner <dturner@twosigma.com>\n\nTIL: super-prefix!\n\n\nOn Fri, 2017-01-06 at 16:19 -0800, Stefan Beller wrote:\n> In the submodule helper we did not correctly handled the display path\n> for initializing submodules when both the submodule is inside a\n> subdirectory as well as the command being invoked from a subdirectory\n> (as viewed from the superproject).\n> \n> This was broken in 3604242f080, which was written at a time where\n> there was no super-prefix available, so we abused the --prefix option\n> for the same purpose and could get only one case right (the call from\n> within a subdirectory, not the submodule being in a subdirectory).\n> \n> Test-provided-by: David Turner <novalis@novalis.org>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n> \n>   applies on sb/submodule-embed-gitdir as that contains 89c862655\n>   (submodule helper: support super prefix)\n> \n>  builtin/submodule--helper.c | 13 +++++++------\n>  git-submodule.sh            |  2 +-\n>  t/t7406-submodule-update.sh | 17 +++++++++++++++++\n>  3 files changed, 25 insertions(+), 7 deletions(-)\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 242d9911a6..7b3f9fc293 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -305,32 +305,36 @@ static int module_list(int argc, const char **argv, const char *prefix)\n>  \n>  \t\tutf8_fprintf(stdout, \"%s\\n\", ce->name);\n>  \t}\n>  \treturn 0;\n>  }\n>  \n>  static void init_submodule(const char *path, const char *prefix, int quiet)\n>  {\n>  \tconst struct submodule *sub;\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \tchar *upd = NULL, *url = NULL, *displaypath;\n>  \n>  \t/* Only loads from .gitmodules, no overlay with .git/config */\n>  \tgitmodules_config();\n>  \n> -\tif (prefix) {\n> -\t\tstrbuf_addf(&sb, \"%s%s\", prefix, path);\n> +\tif (prefix && get_super_prefix())\n> +\t\tdie(\"BUG: cannot have prefix and superprefix\");\n> +\telse if (prefix)\n> +\t\tdisplaypath = xstrdup(relative_path(path, prefix, &sb));\n> +\telse if (get_super_prefix()) {\n> +\t\tstrbuf_addf(&sb, \"%s%s\", get_super_prefix(), path);\n>  \t\tdisplaypath = strbuf_detach(&sb, NULL);\n>  \t} else\n>  \t\tdisplaypath = xstrdup(path);\n>  \n>  \tsub = submodule_from_path(null_sha1, path);\n>  \n>  \tif (!sub)\n>  \t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n>  \t\t\tdisplaypath);\n>  \n>  \t/*\n>  \t * Copy url setting when it is not set yet.\n>  \t * To look up the url in .git/config, we must not fall back to\n>  \t * .gitmodules, so look it up directly.\n>  \t */\n> @@ -391,33 +395,30 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n>  \t}\n>  \tstrbuf_release(&sb);\n>  \tfree(displaypath);\n>  \tfree(url);\n>  \tfree(upd);\n>  }\n>  \n>  static int module_init(int argc, const char **argv, const char *prefix)\n>  {\n>  \tstruct pathspec pathspec;\n>  \tstruct module_list list = MODULE_LIST_INIT;\n>  \tint quiet = 0;\n>  \tint i;\n>  \n>  \tstruct option module_init_options[] = {\n> -\t\tOPT_STRING(0, \"prefix\", &prefix,\n> -\t\t\t   N_(\"path\"),\n> -\t\t\t   N_(\"alternative anchor for relative paths\")),\n>  \t\tOPT__QUIET(&quiet, N_(\"Suppress output for initializing a submodule\")),\n>  \t\tOPT_END()\n>  \t};\n>  \n>  \tconst char *const git_submodule_helper_usage[] = {\n>  \t\tN_(\"git submodule--helper init [<path>]\"),\n>  \t\tNULL\n>  \t};\n>  \n>  \targc = parse_options(argc, argv, prefix, module_init_options,\n>  \t\t\t     git_submodule_helper_usage, 0);\n>  \n>  \tif (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)\n>  \t\treturn 1;\n>  \n> @@ -1117,31 +1118,31 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n>  \n>  struct cmd_struct {\n>  \tconst char *cmd;\n>  \tint (*fn)(int, const char **, const char *);\n>  \tunsigned option;\n>  };\n>  \n>  static struct cmd_struct commands[] = {\n>  \t{\"list\", module_list, 0},\n>  \t{\"name\", module_name, 0},\n>  \t{\"clone\", module_clone, 0},\n>  \t{\"update-clone\", update_clone, 0},\n>  \t{\"relative-path\", resolve_relative_path, 0},\n>  \t{\"resolve-relative-url\", resolve_relative_url, 0},\n>  \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n> -\t{\"init\", module_init, 0},\n> +\t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n>  \t{\"remote-branch\", resolve_remote_submodule_branch, 0},\n>  \t{\"absorb-git-dirs\", absorb_git_dirs, SUPPORT_SUPER_PREFIX},\n>  };\n>  \n>  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint i;\n>  \tif (argc < 2)\n>  \t\tdie(_(\"submodule--helper subcommand must be \"\n>  \t\t      \"called with a subcommand\"));\n>  \n>  \tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n>  \t\tif (!strcmp(argv[1], commands[i].cmd)) {\n>  \t\t\tif (get_super_prefix() &&\n>  \t\t\t    !(commands[i].option & SUPPORT_SUPER_PREFIX))\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 9285b5c43d..4e47ff8ad8 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -362,31 +362,31 @@ cmd_init()\n>  \t\t\t;;\n>  \t\t--)\n>  \t\t\tshift\n>  \t\t\tbreak\n>  \t\t\t;;\n>  \t\t-*)\n>  \t\t\tusage\n>  \t\t\t;;\n>  \t\t*)\n>  \t\t\tbreak\n>  \t\t\t;;\n>  \t\tesac\n>  \t\tshift\n>  \tdone\n>  \n> -\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper init ${GIT_QUIET:+--quiet} ${prefix:+--prefix \"$prefix\"} \"$@\"\n> +\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper init ${GIT_QUIET:+--quiet}  \"$@\"\n>  }\n>  \n>  #\n>  # Unregister submodules from .git/config and remove their work tree\n>  #\n>  cmd_deinit()\n>  {\n>  \t# parse $args after \"submodule ... deinit\".\n>  \tdeinit_all=\n>  \twhile test $# -ne 0\n>  \tdo\n>  \t\tcase \"$1\" in\n>  \t\t-f|--force)\n>  \t\t\tforce=$1\n>  \t\t\t;;\n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> index 64f322c4cc..725bbed1f8 100755\n> --- a/t/t7406-submodule-update.sh\n> +++ b/t/t7406-submodule-update.sh\n> @@ -128,30 +128,47 @@ done.\n>  Cloning into '$pwd/recursivesuper/super/submodule'...\n>  done.\n>  EOF\n>  \n>  test_expect_success 'submodule update --init --recursive from subdirectory' '\n>  \tgit -C recursivesuper/super reset --hard HEAD^ &&\n>  \t(cd recursivesuper &&\n>  \t mkdir tmp &&\n>  \t cd tmp &&\n>  \t git submodule update --init --recursive ../super >../../actual 2>../../actual2\n>  \t) &&\n>  \ttest_i18ncmp expect actual &&\n>  \ttest_i18ncmp expect2 actual2\n>  '\n>  \n> +cat <<EOF >expect2\n> +Submodule 'foo/sub' ($pwd/withsubs/../rebasing) registered for path 'sub'\n> +EOF\n> +\n> +test_expect_success 'submodule update --init from and of subdirectory' '\n> +\tgit init withsubs &&\n> +\t(cd withsubs &&\n> +\t mkdir foo &&\n> +\t git submodule add \"$(pwd)/../rebasing\" foo/sub &&\n> +\t (cd foo &&\n> +\t  git submodule deinit -f sub &&\n> +\t  git submodule update --init sub 2>../../actual2\n> +\t )\n> +\t) &&\n> +\ttest_i18ncmp expect2 actual2\n> +'\n> +\n>  apos=\"'\";\n>  test_expect_success 'submodule update does not fetch already present commits' '\n>  \t(cd submodule &&\n>  \t  echo line3 >> file &&\n>  \t  git add file &&\n>  \t  test_tick &&\n>  \t  git commit -m \"upstream line3\"\n>  \t) &&\n>  \t(cd super/submodule &&\n>  \t  head=$(git rev-parse --verify HEAD) &&\n>  \t  echo \"Submodule path ${apos}submodule$apos: checked out $apos$head$apos\" > ../../expected &&\n>  \t  git reset --hard HEAD~1\n>  \t) &&\n>  \t(cd super &&\n>  \t  git submodule update > ../actual 2> ../actual.err\n\n\n"},{"id":"308905","messageId":"CAGZ79kZDebuqV7q-bcqG-g7E8hBq3RMa0jSDgD1Ho5BN0sByog@mail.gmail.com","threadId":"44831","inReplyTo":"1483750501.31165.9.camel@frank","subject":"Re: [PATCH] submodule update --init: displays correct path from submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-01-07T01:01:15Z","receivedAt":"2017-01-07T01:01:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Jan 6, 2017 at 4:55 PM, David Turner <novalis@novalis.org> wrote:\n> (for the test)\n> Signed-off-by: David Turner <dturner@twosigma.com>\n>\n> TIL: super-prefix!\n\nyeah that was introduced recently for prefixes from \"outside the repository\"\ne.g. a superproject, its first user is grep --recurse-submodules.\n\nAlso I realize I have set diff.context to 15 as I was experimenting\nwith it earlier,\nand forgot to turn it off. So a lot of context in the diff for the patch.\n\nThanks for the bug report,\nStefan\n"}]}