{"thread":{"id":"48403","subject":"[PATCH 0/5] Rebooting pc/submodule-helper-foreach","startedAt":"2018-05-03T00:54:17Z","lastAt":"2018-05-10T21:25:12Z","messageCount":22,"participants":["Stefan Beller","Ramsay Jones","Jonathan Tan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"346488","messageId":"20180503005358.89082-1-sbeller@google.com","threadId":"48403","inReplyTo":null,"subject":"[PATCH 0/5] Rebooting pc/submodule-helper-foreach","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T00:53:53Z","receivedAt":"2018-05-03T00:54:17Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The \"What's cooking\" email carried this series for some time now:\n> * pc/submodule-helper-foreach (2018-02-02) 5 commits\n> - submodule: port submodule subcommand 'foreach' from shell to C\n> - submodule foreach: document variable '$displaypath'\n> - submodule foreach: clarify the '$toplevel' variable documentation\n> - submodule foreach: document '$sm_path' instead of '$path'\n> - submodule foreach: correct '$path' in nested submodules from a subdirectory\n> \n> Expecting a response to review comments\n> e.g. cf. <20180206150044.1bffbb573c088d38c8e44bf5@google.com>\n\nI reworded the commit message of the first patch and nearly confused\nmyself again, as \"toplevel\" doesn't refer to the \"topmost\" superproject,\njust the direct superproject of the submodule.\n\nHowever I think the code of the first patch is correct as originally presented.\nJust the wording of the commit message was changed to explain the reasoning\nmore extensively.\n\nWith this series, we get\n* keep the invariant of $toplevel + $path to be an absolute path that is\n correctly pointing at the submodule. \"git -C $toplevel config\" + $name\n allowing to ask configuration of the submodule. \n* $displaypath will have the relative path form $pwd to the submodule root.\n* better documentation.\n\nIn later patches we could add $topmost, that points at the superproject\nin which the command was started from, and $path_from_topmost, that would\nbe the relative path from $topmost to the submodule, potentially skipping\nintermediate superprojects.\n\nThanks,\nStefan\n\nPrathamesh Chavan (5):\n submodule foreach: correct '$path' in nested submodules from a\n subdirectory\n submodule foreach: document '$sm_path' instead of '$path'\n submodule foreach: clarify the '$toplevel' variable documentation\n submodule foreach: document variable '$displaypath'\n submodule: port submodule subcommand 'foreach' from shell to C\n\n Documentation/git-submodule.txt | 15 ++--\n builtin/submodule--helper.c | 148 ++++++++++++++++++++++++++++++++\n git-submodule.sh | 40 +--------\n t/t7407-submodule-foreach.sh | 38 +++++++-\n 4 files changed, 194 insertions(+), 47 deletions(-)\n\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346489","messageId":"20180503005358.89082-2-sbeller@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-1-sbeller@google.com","subject":"[PATCH 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T00:53:54Z","receivedAt":"2018-05-03T00:54:20Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nWhen running 'git submodule foreach --recursive' from a subdirectory of\nyour repository, nested submodules get a bogus value for $path:\nFor a submodule 'sub' that contains a nested submodule 'nested',\nrunning 'git -C dir submodule foreach echo $path' from the root of the\nsuperproject would report path='../nested' for the nested submodule.\nThe first part '../' is derived from the logic computing the relative\npath from $pwd to the root of the superproject. The second part is the\nsubmodule path inside the submodule. This value is of little use and is\nhard to document.\n\nThere are three different possible solutions that have more value:\n(a) The path value is documented as the path from the toplevel of the\n superproject to the mount point of the submodule. If 'the' refers to\n the superproject holding this submodule ('sub' holding 'nested'),\n the path would be expected to be path='nested'.\n(b) In case 'the' superproject is referring to the toplevel, which\n is the superproject in which the original command was invoked,\n then path is expected to be path='sub/nested'.\n(c) The documentation explains $path as [...] \"relative to the\n superproject\", following 091a6eb0fe (submodule: drop the\n top-level requirement, 2013-06-16), such that the nested submodule\n would be expected as path='../sub/nested', when \"the\" superproject\n is the superproject, where the command was run from\n(d) or the value of path='nested' is expected if we take the\n intermediate superproject into account. [This is the same as\n (a); it highlights that the documentation is not clear, but\n technically correct if we were to revert 091a6eb0fe.]\n\nThe behavior for (c) was introduced in 091a6eb0fe (submodule: drop the\ntop-level requirement, 2013-06-16) the intent for $path seemed to be\nrelative to $cwd to the submodule worktree, but that did not work for\nnested submodules, as the intermittent submodules were not included in\nthe path.\n\nIf we were to fix the meaning of the $path using (a), (d) such that \"path\"\nis \"the path from the intermediate superproject to the mount point of the\nsubmodule\", we would break any existing submodule user that runs foreach\nfrom non-root of the superproject as the non-nested submodule\n'../sub' would change its path to 'sub'.\n\nIf we were to fix the meaning of $path using (b) such that \"path\"\nis \"the path from the topmost superproject to the mount point of the\nsubmodule\", then we would break any user that uses nested submodules\n(even from the root directory) as the 'nested' would become 'sub/nested'.\n\nIf we were to fix the meaning of $path using (c) such that \"path\"\nis \"the display path from where the original command was invoked to the\nsubmodule\", then we would break users that rely on the assumption that\n\"$toplevel / $path\" is the absolute path of the submodule.\n\nAll groups can be found in the wild. The author has no data if one group\noutweighs the other by large margin, and offending each one seems equally\nbad at first. However in the authors imagination it is better to go with\n(a) as running from a sub directory sounds like it is carried out by a\nhuman rather than by some automation task. With a human on the keyboard\nthe feedback loop is short and the changed behavior can be adapted to\nquickly unlike some automation that can break silently.\n\nAnother argument in favor of (a) is the consistency of the variables\nprovided, \"$toplevel / $path\" gives the absolute path of the submodule,\nwith 'toplevel' (both the variable as well as the documentation) referring\nto the immediate superproject of the submodule.\n\nDocumentation of the variable is adjusted in a follow-up patch.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \nSigned-off-by: Stefan Beller \n---\n git-submodule.sh | 1 -\n t/t7407-submodule-foreach.sh | 36 ++++++++++++++++++++++++++++++++++--\n 2 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 24914963ca2..331d71c908b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -345,7 +345,6 @@ cmd_foreach()\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n \t\t\t\tsanitize_submodule_env\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\tsm_path=$(git submodule--helper relative-path \"$sm_path\" \"$wt_prefix\") &&\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path &&\n \t\t\t\tif test $# -eq 1\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 6ba5daf42ee..5144cc6926b 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -82,9 +82,9 @@ test_expect_success 'test basic \"submodule foreach\" usage' '\n \n cat >expect <expect <../../actual\n+\t) &&\n+\ttest_i18ncmp expect actual\n+'\n \n cat > expect <\n\nIt does not contain the topmost superproject as the author assumed,\nbut the direct superproject, such that $toplevel/$sm_path is the\nactual absolute path of the submodule.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Junio C Hamano \nSigned-off-by: Stefan Beller \n---\n Documentation/git-submodule.txt | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 755ed695f08..408d5a0387f 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -188,7 +188,8 @@ foreach [--recursive] ::\n \t$name is the name of the relevant submodule section in `.gitmodules`,\n \t$sm_path is the path of the submodule as recorded in the superproject,\n \t$sha1 is the commit as recorded in the superproject, and\n-\t$toplevel is the absolute path to the top-level of the superproject.\n+\t$toplevel is the absolute path to its direct superproject, such that\n+\t$toplevel/$sm_path is the absolute path of the submodule.\n \tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n \tvariable is now a deprecated synonym of '$sm_path' variable.\n \tAny submodules defined in the superproject but not checked out are\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346491","messageId":"20180503005358.89082-3-sbeller@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-1-sbeller@google.com","subject":"[PATCH 2/5] submodule foreach: document '$sm_path' instead of '$path'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T00:53:55Z","receivedAt":"2018-05-03T00:54:25Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nAs using a variable '$path' may be harmful to users due to\ncapitalization issues, see 64394e3ae9 (git-submodule.sh: Don't\nuse $path variable in eval_gettext string, 2012-04-17). Adjust\nthe documentation to advocate for using $sm_path, which contains\nthe same value. We still make the 'path' variable available and\ndocument it as a deprecated synonym of 'sm_path'.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-submodule.txt | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 71c5618e82a..755ed695f08 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -183,12 +183,14 @@ information too.\n \n foreach [--recursive] ::\n \tEvaluates an arbitrary shell command in each checked out submodule.\n-\tThe command has access to the variables $name, $path, $sha1 and\n+\tThe command has access to the variables $name, $sm_path, $sha1 and\n \t$toplevel:\n \t$name is the name of the relevant submodule section in `.gitmodules`,\n-\t$path is the name of the submodule directory relative to the\n-\tsuperproject, $sha1 is the commit as recorded in the superproject,\n-\tand $toplevel is the absolute path to the top-level of the superproject.\n+\t$sm_path is the path of the submodule as recorded in the superproject,\n+\t$sha1 is the commit as recorded in the superproject, and\n+\t$toplevel is the absolute path to the top-level of the superproject.\n+\tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n+\tvariable is now a deprecated synonym of '$sm_path' variable.\n \tAny submodules defined in the superproject but not checked out are\n \tignored by this command. Unless given `--quiet`, foreach prints the name\n \tof each submodule before evaluating the command.\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346492","messageId":"20180503005358.89082-5-sbeller@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-1-sbeller@google.com","subject":"[PATCH 4/5] submodule foreach: document variable '$displaypath'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T00:53:57Z","receivedAt":"2018-05-03T00:54:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nIt was observed that the variable '$displaypath' was accessible but\nundocumented. Hence, document it.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-submodule.txt | 6 ++++--\n t/t7407-submodule-foreach.sh | 22 +++++++++++-----------\n 2 files changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 408d5a0387f..4372d00c42e 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -183,10 +183,12 @@ information too.\n \n foreach [--recursive] ::\n \tEvaluates an arbitrary shell command in each checked out submodule.\n-\tThe command has access to the variables $name, $sm_path, $sha1 and\n-\t$toplevel:\n+\tThe command has access to the variables $name, $sm_path, $displaypath,\n+\t$sha1 and $toplevel:\n \t$name is the name of the relevant submodule section in `.gitmodules`,\n \t$sm_path is the path of the submodule as recorded in the superproject,\n+\t$displaypath contains the relative path from the current working\n+\tdirectory to the submodules root directory,\n \t$sha1 is the commit as recorded in the superproject, and\n \t$toplevel is the absolute path to its direct superproject, such that\n \t$toplevel/$sm_path is the absolute path of the submodule.\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 5144cc6926b..77729ac4aa1 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -82,16 +82,16 @@ test_expect_success 'test basic \"submodule foreach\" usage' '\n \n cat >expect <../../actual\n+\t\tgit submodule foreach \"echo \\$toplevel-\\$name-\\$sm_path-\\$displaypath-\\$sha1\" >../../actual\n \t) &&\n \ttest_i18ncmp expect actual\n '\n@@ -206,25 +206,25 @@ submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEA\n \n cat >expect <../../actual\n+\t\tgit submodule foreach --recursive \"echo toplevel: \\$toplevel name: \\$name path: \\$sm_path displaypath: \\$displaypath hash: \\$sha1\" >../../actual\n \t) &&\n \ttest_i18ncmp expect actual\n '\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346493","messageId":"20180503005358.89082-6-sbeller@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-1-sbeller@google.com","subject":"[PATCH 5/5] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T00:53:58Z","receivedAt":"2018-05-03T00:54:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nThis aims to make git-submodule foreach a builtin. 'foreach' is ported to\nthe submodule--helper, and submodule--helper is called from\ngit-submodule.sh.\n\nHelped-by: Brandon Williams \nMentored-by: Christian Couder \nMentored-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 148 ++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 39 +---------\n 2 files changed, 149 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a404df3ea49..bbbea5868de 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -439,6 +439,153 @@ static void for_each_listed_submodule(const struct module_list *list,\n \t\tfn(list->entries[i], cb_data);\n }\n \n+struct cb_foreach {\n+\tint argc;\n+\tconst char **argv;\n+\tconst char *prefix;\n+\tchar *toplevel;\n+\tint quiet;\n+\tint recursive;\n+};\n+#define CB_FOREACH_INIT { 0 }\n+\n+static void runcommand_in_submodule_cb(const struct cache_entry *list_item,\n+\t\t\t\t void *cb_data)\n+{\n+\tstruct cb_foreach *info = cb_data;\n+\tconst char *path = list_item->name;\n+\tconst struct object_id *ce_oid = &list_item->oid;\n+\n+\tconst struct submodule *sub;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *displaypath;\n+\n+\tdisplaypath = get_submodule_displaypath(path, info->prefix);\n+\n+\tsub = submodule_from_path(&null_oid, path);\n+\n+\tif (!sub)\n+\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n+\t\t\tdisplaypath);\n+\n+\tif (!is_submodule_populated_gently(path, NULL))\n+\t\tgoto cleanup;\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\n+\t/*\n+\t* For the purpose of executing in the submodule,\n+\t* separate shell is used for the purpose of running the\n+\t* child process.\n+\t*/\n+\tcp.use_shell = 1;\n+\tcp.dir = path;\n+\n+\t/*\n+\t* NEEDSWORK: the command currently has access to the variables $name,\n+\t* $sm_path, $displaypath, $sha1 and $toplevel only when the command\n+\t* contains a single argument. This is done for maintaining a faithful\n+\t* translation from shell script.\n+\t*/\n+\tif (info->argc == 1) {\n+\t\tchar *toplevel = xgetcwd();\n+\n+\t\targv_array_pushf(&cp.env_array, \"name=%s\", sub->name);\n+\t\targv_array_pushf(&cp.env_array, \"sm_path=%s\", path);\n+\t\targv_array_pushf(&cp.env_array, \"displaypath=%s\", displaypath);\n+\t\targv_array_pushf(&cp.env_array, \"sha1=%s\",\n+\t\t\t\toid_to_hex(ce_oid));\n+\t\targv_array_pushf(&cp.env_array, \"toplevel=%s\", toplevel);\n+\n+\t\t/*\n+\t\t* Since the path variable was accessible from the script\n+\t\t* before porting, it is also made available after porting.\n+\t\t* The environment variable \"PATH\" has a very special purpose\n+\t\t* on windows. And since environment variables are\n+\t\t* case-insensitive in windows, it interferes with the\n+\t\t* existing PATH variable. Hence, to avoid that, we expose\n+\t\t* path via the args argv_array and not via env_array.\n+\t\t*/\n+\t\targv_array_pushf(&cp.args, \"path=%s; %s\",\n+\t\t\t\tpath, info->argv[0]);\n+\n+\t\tfree(toplevel);\n+\t} else {\n+\t\targv_array_pushv(&cp.args, info->argv);\n+\t}\n+\n+\tif (!info->quiet)\n+\t\tprintf(_(\"Entering '%s'\\n\"), displaypath);\n+\n+\tif (info->argv[0] && run_command(&cp))\n+\t\tdie(_(\"run_command returned non-zero status for %s\\n.\"),\n+\t\t\tdisplaypath);\n+\n+\tif (info->recursive) {\n+\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n+\n+\t\tcpr.git_cmd = 1;\n+\t\tcpr.dir = path;\n+\t\tprepare_submodule_repo_env(&cpr.env_array);\n+\n+\t\targv_array_pushl(&cpr.args, \"--super-prefix\", NULL);\n+\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"foreach\", \"--recursive\",\n+\t\t\t\tNULL);\n+\n+\t\tif (info->quiet)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\tif (info->toplevel)\n+\t\t\targv_array_pushf(&cpr.args, \"--toplevel=%s\", info->toplevel);\n+\n+\t\targv_array_pushv(&cpr.args, info->argv);\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"run_command returned non-zero status while\"\n+\t\t\t\t\"recursing in the nested submodules of %s\\n.\"),\n+\t\t\t\tdisplaypath);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+}\n+\n+static int module_foreach(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct cb_foreach info = CB_FOREACH_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\n+\tstruct option module_foreach_options[] = {\n+\t\tOPT__QUIET(&info.quiet, N_(\"Suppress output of entering each submodule command\")),\n+\t\tOPT_BOOL(0, \"recursive\", &info.recursive,\n+\t\t\t N_(\"Recurse into nested submodules\")),\n+\t\tOPT_STRING(0, \"toplevel\", &info.toplevel, N_(\"path\"),\n+\t\t\t N_(\"path from the top level of the invocation\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper foreach [--quiet] [--recursive] \"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_foreach_options,\n+\t\t\t git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN);\n+\n+\tif (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tinfo.argc = argc;\n+\tinfo.argv = argv;\n+\tinfo.prefix = prefix;\n+\n+\tfor_each_listed_submodule(&list, runcommand_in_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n struct init_cb {\n \tconst char *prefix;\n \tunsigned int flags;\n@@ -1838,6 +1985,7 @@ static struct cmd_struct commands[] = {\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{\"foreach\", module_foreach, SUPPORT_SUPER_PREFIX},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n \t{\"print-default-remote\", print_default_remote, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 331d71c908b..cba585f0754 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -323,44 +323,7 @@ cmd_foreach()\n \t\tshift\n \tdone\n \n-\ttoplevel=$(pwd)\n-\n-\t# dup stdin so that it can be restored when running the external\n-\t# command in the subshell (and a recursive call to this function)\n-\texec 3<&0\n-\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n-\t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\t\t\tsay \"$(eval_gettext \"Entering '\\$displaypath'\")\"\n-\t\t\tname=$(git submodule--helper name \"$sm_path\")\n-\t\t\t(\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\tsanitize_submodule_env\n-\t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\t# we make $path available to scripts ...\n-\t\t\t\tpath=$sm_path &&\n-\t\t\t\tif test $# -eq 1\n-\t\t\t\tthen\n-\t\t\t\t\teval \"$1\"\n-\t\t\t\telse\n-\t\t\t\t\t\"$@\"\n-\t\t\t\tfi &&\n-\t\t\t\tif test -n \"$recursive\"\n-\t\t\t\tthen\n-\t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n-\t\t\t\tfi\n-\t\t\t) <&3 3<&- ||\n-\t\t\tdie \"$(eval_gettext \"Stopping at '\\$displaypath'; script returned non-zero status.\")\"\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} \"$@\"\n }\n \n #\n-- \n2.17.0.441.gb46fe60e1d-goog\n\n"},{"id":"346494","messageId":"CAGZ79kZnOhq4iSrM7wRmXsTF4ugLUxQcf0cN-SJFwoJHge1rWw@mail.gmail.com","threadId":"48403","inReplyTo":"20180503005358.89082-6-sbeller@google.com","subject":"Re: [PATCH 5/5] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T01:06:43Z","receivedAt":"2018-05-03T01:06:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, May 2, 2018 at 5:53 PM, Stefan Beller wrote:\n\n> +struct cb_foreach {\n> + char *toplevel;\n...\n> + OPT_STRING(0, \"toplevel\", &info.toplevel, N_(\"path\"),\n> + N_(\"path from the top level of the invocation\")),\n\nThis is a leftover from my experimentation that I hinted at in the cover letter\nthat would be used to implement $topmost. I'll remove this in a reroll.\n"},{"id":"346514","messageId":"f8c0fb71-4bb1-3793-76ef-1052e739849b@ramsayjones.plus.com","threadId":"48403","inReplyTo":"20180503005358.89082-2-sbeller@google.com","subject":"Re: [PATCH 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-05-03T13:29:46Z","receivedAt":"2018-05-03T13:29:52Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 03/05/18 01:53, Stefan Beller wrote:\n> From: Prathamesh Chavan \n> \n> When running 'git submodule foreach --recursive' from a subdirectory of\n> your repository, nested submodules get a bogus value for $path:\n> For a submodule 'sub' that contains a nested submodule 'nested',\n> running 'git -C dir submodule foreach echo $path' from the root of the\n> superproject would report path='../nested' for the nested submodule.\n> The first part '../' is derived from the logic computing the relative\n> path from $pwd to the root of the superproject. The second part is the\n> submodule path inside the submodule. This value is of little use and is\n> hard to document.\n> \n> There are three different possible solutions that have more value:\n> (a) The path value is documented as the path from the toplevel of the\n> superproject to the mount point of the submodule. If 'the' refers to\n> the superproject holding this submodule ('sub' holding 'nested'),\n> the path would be expected to be path='nested'.\n> (b) In case 'the' superproject is referring to the toplevel, which\n> is the superproject in which the original command was invoked,\n> then path is expected to be path='sub/nested'.\n> (c) The documentation explains $path as [...] \"relative to the\n> superproject\", following 091a6eb0fe (submodule: drop the\n> top-level requirement, 2013-06-16), such that the nested submodule\n> would be expected as path='../sub/nested', when \"the\" superproject\n> is the superproject, where the command was run from\n> (d) or the value of path='nested' is expected if we take the\n> intermediate superproject into account. [This is the same as\n> (a); it highlights that the documentation is not clear, but\n> technically correct if we were to revert 091a6eb0fe.]\n> \n> The behavior for (c) was introduced in 091a6eb0fe (submodule: drop the\n> top-level requirement, 2013-06-16) the intent for $path seemed to be\n> relative to $cwd to the submodule worktree, but that did not work for\n> nested submodules, as the intermittent submodules were not included in\n----------------------------^^^^^^^^^^^^\nintermediate\n\nATB,\nRamsay Jones\n"},{"id":"346554","messageId":"20180503104754.6aefd8ba38eb5a8871281b01@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-2-sbeller@google.com","subject":"Re: [PATCH 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T17:47:54Z","receivedAt":"2018-05-03T17:48:00Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 2 May 2018 17:53:54 -0700\nStefan Beller wrote:\n\n> From: Prathamesh Chavan \n> \n> When running 'git submodule foreach --recursive' from a subdirectory of\n> your repository, nested submodules get a bogus value for $path:\n\nI know I said in a previous e-mail [1] that we should use $path instead\nof $sm_path, but now I got confused because the test shows a difference\nin $sm_path, not $path. Maybe add \"(and $sm_path, which is an alias of\n$path)\".\n\n[1] https://public-inbox.org/git/20180206145406.b759164cead02cd3bb3fdce0@google.com/\n\n> There are three different possible solutions that have more value:\n> (a) The path value is documented as the path from the toplevel of the\n> superproject to the mount point of the submodule. If 'the' refers to\n> the superproject holding this submodule ('sub' holding 'nested'),\n> the path would be expected to be path='nested'.\n\nWhat is \"the\", and why is it quoted?\n\nAlso, in (c), you use the same indicative present tense as \"The path\nvalue is documented\" to describe the current situation, whereas this\nseems like a situation you're proposing. It would be clearer to use the\nimperative here (\"Document $path to be the path from the toplevel...\").\nDo the same for the others.\n\nAlso, whenever you mention \"superproject\", make it clear which\nsuperproject you're referring to (\"outermost superproject\" and\n\"immediate superproject\" seem like good terms to me).\n\n> (b) In case 'the' superproject is referring to the toplevel, which\n> is the superproject in which the original command was invoked,\n> then path is expected to be path='sub/nested'.\n\nSame comment about \"the\", and I think s/toplevel, which is the\nsuperproject in which the original command was invoked/outermost\nsuperproject/.\n\n> (c) The documentation explains $path as [...] \"relative to the\n> superproject\", following 091a6eb0fe (submodule: drop the\n> top-level requirement, 2013-06-16), such that the nested submodule\n> would be expected as path='../sub/nested', when \"the\" superproject\n> is the superproject, where the command was run from\n\nHow does \"relative to the superproject\" result in \"../\" appearing in the\npath? I would expect \"../\" to appear only if a path is relative to $pwd.\n\n> (d) or the value of path='nested' is expected if we take the\n> intermediate superproject into account. [This is the same as\n> (a); it highlights that the documentation is not clear, but\n> technically correct if we were to revert 091a6eb0fe.]\n\nHow exactly are we taking the intermediate superproject into account?\n\n> The behavior for (c) was introduced in 091a6eb0fe (submodule: drop the\n> top-level requirement, 2013-06-16) the intent for $path seemed to be\n> relative to $cwd to the submodule worktree, but that did not work for\n> nested submodules, as the intermittent submodules were not included in\n> the path.\n\nI think \"pwd\" is more used in the Git project than \"cwd\", so maybe use\n$pwd here.\n\n> If we were to fix the meaning of the $path using (a), (d) such that \"path\"\n> is \"the path from the intermediate superproject to the mount point of the\n> submodule\", we would break any existing submodule user that runs foreach\n> from non-root of the superproject as the non-nested submodule\n> '../sub' would change its path to 'sub'.\n> \n> If we were to fix the meaning of $path using (b) such that \"path\"\n> is \"the path from the topmost superproject to the mount point of the\n> submodule\", then we would break any user that uses nested submodules\n> (even from the root directory) as the 'nested' would become 'sub/nested'.\n> \n> If we were to fix the meaning of $path using (c) such that \"path\"\n> is \"the display path from where the original command was invoked to the\n> submodule\", then we would break users that rely on the assumption that\n> \"$toplevel / $path\" is the absolute path of the submodule.\n> \n> All groups can be found in the wild. The author has no data if one group\n> outweighs the other by large margin, and offending each one seems equally\n> bad at first. However in the authors imagination it is better to go with\n> (a) as running from a sub directory sounds like it is carried out by a\n> human rather than by some automation task. With a human on the keyboard\n> the feedback loop is short and the changed behavior can be adapted to\n> quickly unlike some automation that can break silently.\n\nAs I said in my previous e-mail, this is a good analysis.\n\n> Another argument in favor of (a) is the consistency of the variables\n> provided, \"$toplevel / $path\" gives the absolute path of the submodule,\n> with 'toplevel' (both the variable as well as the documentation) referring\n> to the immediate superproject of the submodule.\n\nIt's confusing to me that $toplevel is not the path to the outermost\nsuperproject, but the path to the immediate superproject, so I'm not\nsure that the goodness of \"$toplevel/$path\" exists. I would omit this\nparagraph.\n\n> Documentation of the variable is adjusted in a follow-up patch.\n\nBy \"the variable\", I assume you mean $toplevel? If yes, this doesn't\nseem relevant to this patch, since this patch does not change the\nmeaning of $toplevel at all.\n"},{"id":"346555","messageId":"20180503105002.d59dcfc0600878f2ad1a7daa@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-3-sbeller@google.com","subject":"Re: [PATCH 2/5] submodule foreach: document '$sm_path' instead of '$path'","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T17:50:02Z","receivedAt":"2018-05-03T17:50:10Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 2 May 2018 17:53:55 -0700\nStefan Beller wrote:\n\n> foreach [--recursive] ::\n> \tEvaluates an arbitrary shell command in each checked out submodule.\n> -\tThe command has access to the variables $name, $path, $sha1 and\n> +\tThe command has access to the variables $name, $sm_path, $sha1 and\n> \t$toplevel:\n> \t$name is the name of the relevant submodule section in `.gitmodules`,\n> -\t$path is the name of the submodule directory relative to the\n> -\tsuperproject, $sha1 is the commit as recorded in the superproject,\n> -\tand $toplevel is the absolute path to the top-level of the superproject.\n> +\t$sm_path is the path of the submodule as recorded in the superproject,\n> +\t$sha1 is the commit as recorded in the superproject, and\n> +\t$toplevel is the absolute path to the top-level of the superproject.\n> +\tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n> +\tvariable is now a deprecated synonym of '$sm_path' variable.\n> \tAny submodules defined in the superproject but not checked out are\n> \tignored by this command. Unless given `--quiet`, foreach prints the name\n> \tof each submodule before evaluating the command.\n\nThis patch is fine as-is. I would go further and replace all mentions of\n\"the superproject\" to \"its immediate superproject\", but that is\noptional.\n"},{"id":"346556","messageId":"20180503105135.5fbb2b85e2bdbc117a08b627@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-4-sbeller@google.com","subject":"Re: [PATCH 3/5] submodule foreach: clarify the '$toplevel' variable documentation","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T17:51:35Z","receivedAt":"2018-05-03T17:51:40Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 2 May 2018 17:53:56 -0700\nStefan Beller wrote:\n\n> \t$name is the name of the relevant submodule section in `.gitmodules`,\n> \t$sm_path is the path of the submodule as recorded in the superproject,\n> \t$sha1 is the commit as recorded in the superproject, and\n> -\t$toplevel is the absolute path to the top-level of the superproject.\n> +\t$toplevel is the absolute path to its direct superproject, such that\n> +\t$toplevel/$sm_path is the absolute path of the submodule.\n> \tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n> \tvariable is now a deprecated synonym of '$sm_path' variable.\n> \tAny submodules defined in the superproject but not checked out are\n\nAh, I commented on patch 2 before reading patch 3 properly. I think that\nif you follow my suggestions on patch 2, it will be obvious that\n$toplevel/$sm_path is the absolute path of the submodule, so that this\npatch is no longer needed.\n"},{"id":"346557","messageId":"20180503110500.cbabf44370b28302d74d9701@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-6-sbeller@google.com","subject":"Re: [PATCH 5/5] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-03T18:05:00Z","receivedAt":"2018-05-03T18:05:05Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 2 May 2018 17:53:58 -0700\nStefan Beller wrote:\n\n> +\t\targv_array_pushf(&cp.args, \"path=%s; %s\",\n> +\t\t\t\tpath, info->argv[0]);\n\nDo we need to quote the path here? (For example, what if path has a\nquotation mark?)\n\nAlso, would it be useful to have a test testing a submodule with a\n\"weird\" character (e.g. greater-than or single quote) in the name and\nthe path?\n"},{"id":"346559","messageId":"CAGZ79kYq0Qs7tvG0NEYwKHeYxF_tN1-eZA2skdjCiDZ4JkyuRQ@mail.gmail.com","threadId":"48403","inReplyTo":"20180503104754.6aefd8ba38eb5a8871281b01@google.com","subject":"Re: [PATCH 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-03T18:12:27Z","receivedAt":"2018-05-03T18:12:32Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, May 3, 2018 at 10:47 AM, Jonathan Tan wrote:\n> On Wed, 2 May 2018 17:53:54 -0700\n> Stefan Beller wrote:\n>\n>> From: Prathamesh Chavan \n>>\n>> When running 'git submodule foreach --recursive' from a subdirectory of\n>> your repository, nested submodules get a bogus value for $path:\n>\n> I know I said in a previous e-mail [1] that we should use $path instead\n> of $sm_path, but now I got confused because the test shows a difference\n> in $sm_path, not $path. Maybe add \"(and $sm_path, which is an alias of\n> $path)\".\n>\n> [1] https://public-inbox.org/git/20180206145406.b759164cead02cd3bb3fdce0@google.com/\n>\n>> There are three different possible solutions that have more value:\n>> (a) The path value is documented as the path from the toplevel of the\n>> superproject to the mount point of the submodule. If 'the' refers to\n>> the superproject holding this submodule ('sub' holding 'nested'),\n>> the path would be expected to be path='nested'.\n>\n> What is \"the\", and why is it quoted?\n\nbecause it is unclear what is emphasizes.\nIt could be the intermediate (direct) superproject, or it\ncould be the topmost superproject (where the command was run from).\n\nJust having \"the\", makes it unclear which of both it refers to.\n\n> Also, in (c), you use the same indicative present tense as \"The path\n> value is documented\" to describe the current situation, whereas this\n> seems like a situation you're proposing. It would be clearer to use the\n> imperative here (\"Document $path to be the path from the toplevel...\").\n> Do the same for the others.\n>\n> Also, whenever you mention \"superproject\", make it clear which\n> superproject you're referring to (\"outermost superproject\" and\n> \"immediate superproject\" seem like good terms to me).\n\nok, I'll rewrite the commit message with clearer superproject\nannotations.\n\n>> (b) In case 'the' superproject is referring to the toplevel, which\n>> is the superproject in which the original command was invoked,\n>> then path is expected to be path='sub/nested'.\n>\n> Same comment about \"the\", and I think s/toplevel, which is the\n> superproject in which the original command was invoked/outermost\n> superproject/.\n\nThe outermost superproject may not be the one you invoke the\ncommand in.\n\nWe have\n* the direct superproject. You can access it currently via $toplevel,\n which is misleading\n* the superproject, where the command was invoked from,\n Currently only the undocumented $displaypath gives hints\n at this\n* the outermost superproject, which may be even further\n out than the previous superproject in this list; Consider\n a layout with 4 git repositories nested as follows:\n\n outmost/invoked/direct/submodule\n\nSubmodule is part of the superproject \"direct\", but the command\ncould have been invoked from outmost/invoked/dir which has \"direct\"\nas a submodule at '../direct' and itself is a submodule of outmost.\n\nIMHO 'outmost' is of no relevance to the discussion. If you care about\nit, discover it yourself via repeated calls to\n'git rev-parse --show-superproject-working-tree'\n\n'invoked' is only interesting for $displaypath, but apart from that\nthere is no benefit in knowing that at the time of processing\n'submodule'. (It doesn't contain information for submodule, as\nall those configs are in 'direct')\n\n'direct' is a better name than 'toplevel', which is confusing as it\ncould be understood as any of 'direct', 'invoked' or 'outmost'.\n\n>> (c) The documentation explains $path as [...] \"relative to the\n>> superproject\", following 091a6eb0fe (submodule: drop the\n>> top-level requirement, 2013-06-16), such that the nested submodule\n>> would be expected as path='../sub/nested', when \"the\" superproject\n>> is the superproject, where the command was run from\n>\n> How does \"relative to the superproject\" result in \"../\" appearing in the\n> path? I would expect \"../\" to appear only if a path is relative to $pwd.\n\nGah. I messed that up. I wanted to emphasize *relative* and not the\nsuperproject that is merely mentioned to form a sentence there.\n\n>\n>> (d) or the value of path='nested' is expected if we take the\n>> intermediate superproject into account. [This is the same as\n>> (a); it highlights that the documentation is not clear, but\n>> technically correct if we were to revert 091a6eb0fe.]\n>\n> How exactly are we taking the intermediate superproject into account?\n\n'nested' is the full in-tree path from the intermediate (direct) superproject\nto that submodule.\n\n>> The behavior for (c) was introduced in 091a6eb0fe (submodule: drop the\n>> top-level requirement, 2013-06-16) the intent for $path seemed to be\n>> relative to $cwd to the submodule worktree, but that did not work for\n>> nested submodules, as the intermittent submodules were not included in\n>> the path.\n>\n> I think \"pwd\" is more used in the Git project than \"cwd\", so maybe use\n> $pwd here.\n\nok.\n\n>\n>> If we were to fix the meaning of the $path using (a), (d) such that \"path\"\n>> is \"the path from the intermediate superproject to the mount point of the\n>> submodule\", we would break any existing submodule user that runs foreach\n>> from non-root of the superproject as the non-nested submodule\n>> '../sub' would change its path to 'sub'.\n>>\n>> If we were to fix the meaning of $path using (b) such that \"path\"\n>> is \"the path from the topmost superproject to the mount point of the\n>> submodule\", then we would break any user that uses nested submodules\n>> (even from the root directory) as the 'nested' would become 'sub/nested'.\n>>\n>> If we were to fix the meaning of $path using (c) such that \"path\"\n>> is \"the display path from where the original command was invoked to the\n>> submodule\", then we would break users that rely on the assumption that\n>> \"$toplevel / $path\" is the absolute path of the submodule.\n>>\n>> All groups can be found in the wild. The author has no data if one group\n>> outweighs the other by large margin, and offending each one seems equally\n>> bad at first. However in the authors imagination it is better to go with\n>> (a) as running from a sub directory sounds like it is carried out by a\n>> human rather than by some automation task. With a human on the keyboard\n>> the feedback loop is short and the changed behavior can be adapted to\n>> quickly unlike some automation that can break silently.\n>\n> As I said in my previous e-mail, this is a good analysis.\n>\n>> Another argument in favor of (a) is the consistency of the variables\n>> provided, \"$toplevel / $path\" gives the absolute path of the submodule,\n>> with 'toplevel' (both the variable as well as the documentation) referring\n>> to the immediate superproject of the submodule.\n>\n> It's confusing to me that $toplevel is not the path to the outermost\n> superproject,\n\nyes. That confused me for a while, too. Maybe deprecate that (just like we\ndeprecate $path) and introduce $superproject to mean the direct\nsuperproject that holds the submodule currently being processed?\n\n\n> but the path to the immediate superproject, so I'm not\n> sure that the goodness of \"$toplevel/$path\" exists. I would omit this\n> paragraph.\n\nThis exists for all nested submodules currently as these are run\nfrom its root tree. For non-nested submodules, you still have the\npart that is relative from pwd to the submodule.\n\n\n>> Documentation of the variable is adjusted in a follow-up patch.\n>\n> By \"the variable\", I assume you mean $toplevel? If yes, this doesn't\n> seem relevant to this patch, since this patch does not change the\n> meaning of $toplevel at all.\n\nok.\n"},{"id":"346693","messageId":"20180504140346.f088452c4a73539ef140a1da@google.com","threadId":"48403","inReplyTo":"CAGZ79kYq0Qs7tvG0NEYwKHeYxF_tN1-eZA2skdjCiDZ4JkyuRQ@mail.gmail.com","subject":"Re: [PATCH 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-04T21:03:46Z","receivedAt":"2018-05-04T21:03:53Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Thu, 3 May 2018 11:12:27 -0700\nStefan Beller wrote:\n\n> >> There are three different possible solutions that have more value:\n> >> (a) The path value is documented as the path from the toplevel of the\n> >> superproject to the mount point of the submodule. If 'the' refers to\n> >> the superproject holding this submodule ('sub' holding 'nested'),\n> >> the path would be expected to be path='nested'.\n> >\n> > What is \"the\", and why is it quoted?\n> \n> because it is unclear what is emphasizes.\n> It could be the intermediate (direct) superproject, or it\n> could be the topmost superproject (where the command was run from).\n> \n> Just having \"the\", makes it unclear which of both it refers to.\n\nAh, I see - so s/'the'/'the superproject'/\n\n> >> (b) In case 'the' superproject is referring to the toplevel, which\n> >> is the superproject in which the original command was invoked,\n> >> then path is expected to be path='sub/nested'.\n\nAnd here, s/'the' superproject/'the superproject'/\n\n> > Same comment about \"the\", and I think s/toplevel, which is the\n> > superproject in which the original command was invoked/outermost\n> > superproject/.\n> \n> The outermost superproject may not be the one you invoke the\n> command in.\n\nGood point. Maybe \"the superproject the original command was run from\",\nbut I'm open to a better name. So I would write the beginning as\nfollows:\n\n \n\n Also, in git-submodule.txt, $path is documented to be the \"name of the\n submodule directory relative to the superproject\", but \"the\n superproject\" is ambiguous.\n\n To resolve both these issues, we could:\n (a) Change \"the superproject\" to \"its immediate superproject\", so\n $path would be \"nested\" instead of \"../nested\".\n (b) Change \"the superproject\" to \"the superproject the original\n command was run from\", so $path would be \"sub/nested\" instead of\n \"../nested\".\n (c) Change \"the superproject\" to \"the directory the original command\n was run from\", so $path would be \"../sub/nested\" instead of\n \"../nested\".\n\nGoing back to the original patch:\n\n> The behavior for (c) was introduced in 091a6eb0fe (submodule: drop the\n> top-level requirement, 2013-06-16) the intent for $path seemed to be\n> relative to $cwd to the submodule worktree, but that did not work for\n> nested submodules, as the intermittent submodules were not included in\n> the path.\n\nThe (c) behavior was never really introduced, right? 091a6eb0fe\nattempted to introduce (c), but it didn't work when --recursive was\nspecified.\n"},{"id":"347036","messageId":"20180509002952.172347-1-sbeller@google.com","threadId":"48403","inReplyTo":"20180503005358.89082-1-sbeller@google.com","subject":"[PATCHv2 0/4] Rebooting pc/submodule-helper-foreach","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-09T00:29:48Z","receivedAt":"2018-05-09T00:30:06Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\n* rebased onto origin/master\n* dropped leftover \"toplevel\" variable from experimentation\n* reworded the commit message for the first patch extensively\n* dropped the third patch\n* see \"branch-diff\" below.\n\nv1:\nThe \"What's cooking\" email carried this series for some time now:\n> * pc/submodule-helper-foreach (2018-02-02) 5 commits\n> - submodule: port submodule subcommand 'foreach' from shell to C\n> - submodule foreach: document variable '$displaypath'\n> - submodule foreach: clarify the '$toplevel' variable documentation\n> - submodule foreach: document '$sm_path' instead of '$path'\n> - submodule foreach: correct '$path' in nested submodules from a subdirectory\n> \n> Expecting a response to review comments\n> e.g. cf. <20180206150044.1bffbb573c088d38c8e44bf5@google.com>\n\nI reworded the commit message of the first patch and nearly confused\nmyself again, as \"toplevel\" doesn't refer to the \"topmost\" superproject,\njust the direct superproject of the submodule.\n\nHowever I think the code of the first patch is correct as originally presented.\nJust the wording of the commit message was changed to explain the reasoning\nmore extensively.\n\nWith this series, we get\n* keep the invariant of $toplevel + $path to be an absolute path that is\n correctly pointing at the submodule. \"git -C $toplevel config\" + $name\n allowing to ask configuration of the submodule. \n* $displaypath will have the relative path form $pwd to the submodule root.\n* better documentation.\n\nIn later patches we could add $topmost, that points at the superproject\nin which the command was started from, and $path_from_topmost, that would\nbe the relative path from $topmost to the submodule, potentially skipping\nintermediate superprojects.\n\nThanks,\nStefan\n\nPrathamesh Chavan (4):\n submodule foreach: correct '$path' in nested submodules from a\n subdirectory\n submodule foreach: document '$sm_path' instead of '$path'\n submodule foreach: document variable '$displaypath'\n submodule: port submodule subcommand 'foreach' from shell to C\n\n Documentation/git-submodule.txt | 15 ++--\n builtin/submodule--helper.c | 144 ++++++++++++++++++++++++++++++++\n git-submodule.sh | 40 +--------\n t/t7407-submodule-foreach.sh | 38 ++++++++-\n 4 files changed, 190 insertions(+), 47 deletions(-)\n\n-- \n2.17.0.255.g8bfb7c0704\n\n1: 0c5f405db24 ! 1: 85f91b48379 submodule foreach: correct '$path' in nested submodules from a subdirectory\n @@ -12,45 +12,38 @@\n submodule path inside the submodule. This value is of little use and is\n hard to document.\n \n - There are three different possible solutions that have more value:\n - (a) The path value is documented as the path from the toplevel of the\n - superproject to the mount point of the submodule. If 'the' refers to\n - the superproject holding this submodule ('sub' holding 'nested'),\n - the path would be expected to be path='nested'.\n - (b) In case 'the' superproject is referring to the toplevel, which\n - is the superproject in which the original command was invoked,\n - then path is expected to be path='sub/nested'.\n - (c) The documentation explains $path as [...] \"relative to the\n - superproject\", following 091a6eb0fe (submodule: drop the\n - top-level requirement, 2013-06-16), such that the nested submodule\n - would be expected as path='../sub/nested', when \"the\" superproject\n - is the superproject, where the command was run from\n - (d) or the value of path='nested' is expected if we take the\n - intermediate superproject into account. [This is the same as\n - (a); it highlights that the documentation is not clear, but\n - technically correct if we were to revert 091a6eb0fe.]\n + Also, in git-submodule.txt, $path is documented to be the \"name of the\n + submodule directory relative to the superproject\", but \"the\n + superproject\" is ambiguous.\n \n - The behavior for (c) was introduced in 091a6eb0fe (submodule: drop the\n - top-level requirement, 2013-06-16) the intent for $path seemed to be\n - relative to $cwd to the submodule worktree, but that did not work for\n - nested submodules, as the intermittent submodules were not included in\n - the path.\n + To resolve both these issues, we could:\n + (a) Change \"the superproject\" to \"its immediate superproject\", so\n + $path would be \"nested\" instead of \"../nested\".\n + (b) Change \"the superproject\" to \"the superproject the original\n + command was run from\", so $path would be \"sub/nested\" instead of\n + \"../nested\".\n + (c) Change \"the superproject\" to \"the directory the original command\n + was run from\", so $path would be \"../sub/nested\" instead of\n + \"../nested\".\n \n - If we were to fix the meaning of the $path using (a), (d) such that \"path\"\n - is \"the path from the intermediate superproject to the mount point of the\n - submodule\", we would break any existing submodule user that runs foreach\n - from non-root of the superproject as the non-nested submodule\n - '../sub' would change its path to 'sub'.\n + The behavior for (c) was attempted to be introduced in 091a6eb0fe\n + (submodule: drop the top-level requirement, 2013-06-16) with the intent\n + for $path to be relative from $pwd to the submodule worktree, but that\n + did not work for nested submodules, as the intermittent submodules\n + were not included in the path.\n \n - If we were to fix the meaning of $path using (b) such that \"path\"\n - is \"the path from the topmost superproject to the mount point of the\n - submodule\", then we would break any user that uses nested submodules\n - (even from the root directory) as the 'nested' would become 'sub/nested'.\n + If we were to fix the meaning of the $path using (a), we would break\n + any existing submodule user that runs foreach from non-root of the\n + superproject as the non-nested submodule '../sub' would change its\n + path to 'sub'.\n \n - If we were to fix the meaning of $path using (c) such that \"path\"\n - is \"the display path from where the original command was invoked to the\n - submodule\", then we would break users that rely on the assumption that\n - \"$toplevel / $path\" is the absolute path of the submodule.\n + If we were to fix the meaning of $path using (b), then we would break\n + any user that uses nested submodules (even from the root directory)\n + as the 'nested' would become 'sub/nested'.\n +\n + If we were to fix the meaning of $path using (c), then we would break\n + the same users as in (b) as 'nested' would become 'sub/nested' from\n + the root directory of the superproject.\n \n All groups can be found in the wild. The author has no data if one group\n outweighs the other by large margin, and offending each one seems equally\n @@ -60,13 +53,6 @@\n the feedback loop is short and the changed behavior can be adapted to\n quickly unlike some automation that can break silently.\n \n - Another argument in favor of (a) is the consistency of the variables\n - provided, \"$toplevel / $path\" gives the absolute path of the submodule,\n - with 'toplevel' (both the variable as well as the documentation) referring\n - to the immediate superproject of the submodule.\n -\n - Documentation of the variable is adjusted in a follow-up patch.\n -\n Discussed-with: Ramsay Jones \n Signed-off-by: Prathamesh Chavan \n Signed-off-by: Stefan Beller \n2: ae5e280a8d3 ! 2: 3a2cb4e0b01 submodule foreach: document '$sm_path' instead of '$path'\n @@ -28,9 +28,10 @@\n -\t$path is the name of the submodule directory relative to the\n -\tsuperproject, $sha1 is the commit as recorded in the superproject,\n -\tand $toplevel is the absolute path to the top-level of the superproject.\n -+\t$sm_path is the path of the submodule as recorded in the superproject,\n -+\t$sha1 is the commit as recorded in the superproject, and\n -+\t$toplevel is the absolute path to the top-level of the superproject.\n ++\t$sm_path is the path of the submodule as recorded in the immediate\n ++\tsuperproject, $sha1 is the commit as recorded in the immediate\n ++\tsuperproject, and $toplevel is the absolute path to the top-level\n ++\tof the immediate superproject.\n +\tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n +\tvariable is now a deprecated synonym of '$sm_path' variable.\n \tAny submodules defined in the superproject but not checked out are\n3: 23a5f04f846 < -: ----------- submodule foreach: clarify the '$toplevel' variable documentation\n4: 3efed1cdb80 ! 3: 3f4686e8395 submodule foreach: document variable '$displaypath'\n @@ -22,12 +22,14 @@\n +\tThe command has access to the variables $name, $sm_path, $displaypath,\n +\t$sha1 and $toplevel:\n \t$name is the name of the relevant submodule section in `.gitmodules`,\n - \t$sm_path is the path of the submodule as recorded in the superproject,\n -+\t$displaypath contains the relative path from the current working\n -+\tdirectory to the submodules root directory,\n - \t$sha1 is the commit as recorded in the superproject, and\n - \t$toplevel is the absolute path to its direct superproject, such that\n - \t$toplevel/$sm_path is the absolute path of the submodule.\n + \t$sm_path is the path of the submodule as recorded in the immediate\n +-\tsuperproject, $sha1 is the commit as recorded in the immediate\n ++\tsuperproject, $displaypath contains the relative path from the\n ++\tcurrent working directory to the submodules root directory,\n ++\t$sha1 is the commit as recorded in the immediate\n + \tsuperproject, and $toplevel is the absolute path to the top-level\n + \tof the immediate superproject.\n + \tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n \n diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\n --- a/t/t7407-submodule-foreach.sh\n5: 2b17d6891b3 ! 4: 0c677680928 submodule: port submodule subcommand 'foreach' from shell to C\n @@ -23,7 +23,6 @@\n +\tint argc;\n +\tconst char **argv;\n +\tconst char *prefix;\n -+\tchar *toplevel;\n +\tint quiet;\n +\tint recursive;\n +};\n @@ -42,7 +41,7 @@\n +\n +\tdisplaypath = get_submodule_displaypath(path, info->prefix);\n +\n -+\tsub = submodule_from_path(&null_oid, path);\n ++\tsub = submodule_from_path(the_repository, &null_oid, path);\n +\n +\tif (!sub)\n +\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n @@ -69,6 +68,7 @@\n +\t*/\n +\tif (info->argc == 1) {\n +\t\tchar *toplevel = xgetcwd();\n ++\t\tstruct strbuf sb = STRBUF_INIT;\n +\n +\t\targv_array_pushf(&cp.env_array, \"name=%s\", sub->name);\n +\t\targv_array_pushf(&cp.env_array, \"sm_path=%s\", path);\n @@ -86,9 +86,10 @@\n +\t\t* existing PATH variable. Hence, to avoid that, we expose\n +\t\t* path via the args argv_array and not via env_array.\n +\t\t*/\n ++\t\tsq_quote_buf(&sb, path);\n +\t\targv_array_pushf(&cp.args, \"path=%s; %s\",\n -+\t\t\t\tpath, info->argv[0]);\n -+\n ++\t\t\t\t sb.buf, info->argv[0]);\n ++\t\tstrbuf_release(&sb);\n +\t\tfree(toplevel);\n +\t} else {\n +\t\targv_array_pushv(&cp.args, info->argv);\n @@ -116,9 +117,6 @@\n +\t\tif (info->quiet)\n +\t\t\targv_array_push(&cpr.args, \"--quiet\");\n +\n -+\t\tif (info->toplevel)\n -+\t\t\targv_array_pushf(&cpr.args, \"--toplevel=%s\", info->toplevel);\n -+\n +\t\targv_array_pushv(&cpr.args, info->argv);\n +\n +\t\tif (run_command(&cpr))\n @@ -141,8 +139,6 @@\n +\t\tOPT__QUIET(&info.quiet, N_(\"Suppress output of entering each submodule command\")),\n +\t\tOPT_BOOL(0, \"recursive\", &info.recursive,\n +\t\t\t N_(\"Recurse into nested submodules\")),\n -+\t\tOPT_STRING(0, \"toplevel\", &info.toplevel, N_(\"path\"),\n -+\t\t\t N_(\"path from the top level of the invocation\")),\n +\t\tOPT_END()\n +\t};\n +\n"},{"id":"347037","messageId":"20180509002952.172347-2-sbeller@google.com","threadId":"48403","inReplyTo":"20180509002952.172347-1-sbeller@google.com","subject":"[PATCH 1/4] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-09T00:29:49Z","receivedAt":"2018-05-09T00:30:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nWhen running 'git submodule foreach --recursive' from a subdirectory of\nyour repository, nested submodules get a bogus value for $path:\nFor a submodule 'sub' that contains a nested submodule 'nested',\nrunning 'git -C dir submodule foreach echo $path' from the root of the\nsuperproject would report path='../nested' for the nested submodule.\nThe first part '../' is derived from the logic computing the relative\npath from $pwd to the root of the superproject. The second part is the\nsubmodule path inside the submodule. This value is of little use and is\nhard to document.\n\nAlso, in git-submodule.txt, $path is documented to be the \"name of the\nsubmodule directory relative to the superproject\", but \"the\nsuperproject\" is ambiguous.\n\nTo resolve both these issues, we could:\n(a) Change \"the superproject\" to \"its immediate superproject\", so\n $path would be \"nested\" instead of \"../nested\".\n(b) Change \"the superproject\" to \"the superproject the original\n command was run from\", so $path would be \"sub/nested\" instead of\n \"../nested\".\n(c) Change \"the superproject\" to \"the directory the original command\n was run from\", so $path would be \"../sub/nested\" instead of\n \"../nested\".\n\nThe behavior for (c) was attempted to be introduced in 091a6eb0fe\n(submodule: drop the top-level requirement, 2013-06-16) with the intent\nfor $path to be relative from $pwd to the submodule worktree, but that\ndid not work for nested submodules, as the intermittent submodules\nwere not included in the path.\n\nIf we were to fix the meaning of the $path using (a), we would break\nany existing submodule user that runs foreach from non-root of the\nsuperproject as the non-nested submodule '../sub' would change its\npath to 'sub'.\n\nIf we were to fix the meaning of $path using (b), then we would break\nany user that uses nested submodules (even from the root directory)\nas the 'nested' would become 'sub/nested'.\n\nIf we were to fix the meaning of $path using (c), then we would break\nthe same users as in (b) as 'nested' would become 'sub/nested' from\nthe root directory of the superproject.\n\nAll groups can be found in the wild. The author has no data if one group\noutweighs the other by large margin, and offending each one seems equally\nbad at first. However in the authors imagination it is better to go with\n(a) as running from a sub directory sounds like it is carried out by a\nhuman rather than by some automation task. With a human on the keyboard\nthe feedback loop is short and the changed behavior can be adapted to\nquickly unlike some automation that can break silently.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \nSigned-off-by: Stefan Beller \n---\n git-submodule.sh | 1 -\n t/t7407-submodule-foreach.sh | 36 ++++++++++++++++++++++++++++++++++--\n 2 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 24914963ca2..331d71c908b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -345,7 +345,6 @@ cmd_foreach()\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n \t\t\t\tsanitize_submodule_env\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\tsm_path=$(git submodule--helper relative-path \"$sm_path\" \"$wt_prefix\") &&\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path &&\n \t\t\t\tif test $# -eq 1\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 6ba5daf42ee..5144cc6926b 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -82,9 +82,9 @@ test_expect_success 'test basic \"submodule foreach\" usage' '\n \n cat >expect <expect <../../actual\n+\t) &&\n+\ttest_i18ncmp expect actual\n+'\n \n cat > expect <\n\nAs using a variable '$path' may be harmful to users due to\ncapitalization issues, see 64394e3ae9 (git-submodule.sh: Don't\nuse $path variable in eval_gettext string, 2012-04-17). Adjust\nthe documentation to advocate for using $sm_path, which contains\nthe same value. We still make the 'path' variable available and\ndocument it as a deprecated synonym of 'sm_path'.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-submodule.txt | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 630999f41a9..066c7b6c18e 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -183,12 +183,15 @@ information too.\n \n foreach [--recursive] ::\n \tEvaluates an arbitrary shell command in each checked out submodule.\n-\tThe command has access to the variables $name, $path, $sha1 and\n+\tThe command has access to the variables $name, $sm_path, $sha1 and\n \t$toplevel:\n \t$name is the name of the relevant submodule section in `.gitmodules`,\n-\t$path is the name of the submodule directory relative to the\n-\tsuperproject, $sha1 is the commit as recorded in the superproject,\n-\tand $toplevel is the absolute path to the top-level of the superproject.\n+\t$sm_path is the path of the submodule as recorded in the immediate\n+\tsuperproject, $sha1 is the commit as recorded in the immediate\n+\tsuperproject, and $toplevel is the absolute path to the top-level\n+\tof the immediate superproject.\n+\tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\n+\tvariable is now a deprecated synonym of '$sm_path' variable.\n \tAny submodules defined in the superproject but not checked out are\n \tignored by this command. Unless given `--quiet`, foreach prints the name\n \tof each submodule before evaluating the command.\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"347039","messageId":"20180509002952.172347-4-sbeller@google.com","threadId":"48403","inReplyTo":"20180509002952.172347-1-sbeller@google.com","subject":"[PATCH 3/4] submodule foreach: document variable '$displaypath'","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-09T00:29:51Z","receivedAt":"2018-05-09T00:30:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nIt was observed that the variable '$displaypath' was accessible but\nundocumented. Hence, document it.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Junio C Hamano \n---\n Documentation/git-submodule.txt | 8 +++++---\n t/t7407-submodule-foreach.sh | 22 +++++++++++-----------\n 2 files changed, 16 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 066c7b6c18e..500dfdafd16 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -183,11 +183,13 @@ information too.\n \n foreach [--recursive] ::\n \tEvaluates an arbitrary shell command in each checked out submodule.\n-\tThe command has access to the variables $name, $sm_path, $sha1 and\n-\t$toplevel:\n+\tThe command has access to the variables $name, $sm_path, $displaypath,\n+\t$sha1 and $toplevel:\n \t$name is the name of the relevant submodule section in `.gitmodules`,\n \t$sm_path is the path of the submodule as recorded in the immediate\n-\tsuperproject, $sha1 is the commit as recorded in the immediate\n+\tsuperproject, $displaypath contains the relative path from the\n+\tcurrent working directory to the submodules root directory,\n+\t$sha1 is the commit as recorded in the immediate\n \tsuperproject, and $toplevel is the absolute path to the top-level\n \tof the immediate superproject.\n \tNote that to avoid conflicts with '$PATH' on Windows, the '$path'\ndiff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh\nindex 5144cc6926b..77729ac4aa1 100755\n--- a/t/t7407-submodule-foreach.sh\n+++ b/t/t7407-submodule-foreach.sh\n@@ -82,16 +82,16 @@ test_expect_success 'test basic \"submodule foreach\" usage' '\n \n cat >expect <../../actual\n+\t\tgit submodule foreach \"echo \\$toplevel-\\$name-\\$sm_path-\\$displaypath-\\$sha1\" >../../actual\n \t) &&\n \ttest_i18ncmp expect actual\n '\n@@ -206,25 +206,25 @@ submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEA\n \n cat >expect <../../actual\n+\t\tgit submodule foreach --recursive \"echo toplevel: \\$toplevel name: \\$name path: \\$sm_path displaypath: \\$displaypath hash: \\$sha1\" >../../actual\n \t) &&\n \ttest_i18ncmp expect actual\n '\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"347040","messageId":"20180509002952.172347-5-sbeller@google.com","threadId":"48403","inReplyTo":"20180509002952.172347-1-sbeller@google.com","subject":"[PATCH 4/4] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-09T00:29:52Z","receivedAt":"2018-05-09T00:30:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nThis aims to make git-submodule foreach a builtin. 'foreach' is ported to\nthe submodule--helper, and submodule--helper is called from\ngit-submodule.sh.\n\nHelped-by: Brandon Williams \nMentored-by: Christian Couder \nMentored-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 144 ++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 39 +---------\n 2 files changed, 145 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c2403a915ff..4b0b773c06b 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -439,6 +439,149 @@ static void for_each_listed_submodule(const struct module_list *list,\n \t\tfn(list->entries[i], cb_data);\n }\n \n+struct cb_foreach {\n+\tint argc;\n+\tconst char **argv;\n+\tconst char *prefix;\n+\tint quiet;\n+\tint recursive;\n+};\n+#define CB_FOREACH_INIT { 0 }\n+\n+static void runcommand_in_submodule_cb(const struct cache_entry *list_item,\n+\t\t\t\t void *cb_data)\n+{\n+\tstruct cb_foreach *info = cb_data;\n+\tconst char *path = list_item->name;\n+\tconst struct object_id *ce_oid = &list_item->oid;\n+\n+\tconst struct submodule *sub;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *displaypath;\n+\n+\tdisplaypath = get_submodule_displaypath(path, info->prefix);\n+\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n+\n+\tif (!sub)\n+\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n+\t\t\tdisplaypath);\n+\n+\tif (!is_submodule_populated_gently(path, NULL))\n+\t\tgoto cleanup;\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\n+\t/*\n+\t* For the purpose of executing in the submodule,\n+\t* separate shell is used for the purpose of running the\n+\t* child process.\n+\t*/\n+\tcp.use_shell = 1;\n+\tcp.dir = path;\n+\n+\t/*\n+\t* NEEDSWORK: the command currently has access to the variables $name,\n+\t* $sm_path, $displaypath, $sha1 and $toplevel only when the command\n+\t* contains a single argument. This is done for maintaining a faithful\n+\t* translation from shell script.\n+\t*/\n+\tif (info->argc == 1) {\n+\t\tchar *toplevel = xgetcwd();\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\targv_array_pushf(&cp.env_array, \"name=%s\", sub->name);\n+\t\targv_array_pushf(&cp.env_array, \"sm_path=%s\", path);\n+\t\targv_array_pushf(&cp.env_array, \"displaypath=%s\", displaypath);\n+\t\targv_array_pushf(&cp.env_array, \"sha1=%s\",\n+\t\t\t\toid_to_hex(ce_oid));\n+\t\targv_array_pushf(&cp.env_array, \"toplevel=%s\", toplevel);\n+\n+\t\t/*\n+\t\t* Since the path variable was accessible from the script\n+\t\t* before porting, it is also made available after porting.\n+\t\t* The environment variable \"PATH\" has a very special purpose\n+\t\t* on windows. And since environment variables are\n+\t\t* case-insensitive in windows, it interferes with the\n+\t\t* existing PATH variable. Hence, to avoid that, we expose\n+\t\t* path via the args argv_array and not via env_array.\n+\t\t*/\n+\t\tsq_quote_buf(&sb, path);\n+\t\targv_array_pushf(&cp.args, \"path=%s; %s\",\n+\t\t\t\t sb.buf, info->argv[0]);\n+\t\tstrbuf_release(&sb);\n+\t\tfree(toplevel);\n+\t} else {\n+\t\targv_array_pushv(&cp.args, info->argv);\n+\t}\n+\n+\tif (!info->quiet)\n+\t\tprintf(_(\"Entering '%s'\\n\"), displaypath);\n+\n+\tif (info->argv[0] && run_command(&cp))\n+\t\tdie(_(\"run_command returned non-zero status for %s\\n.\"),\n+\t\t\tdisplaypath);\n+\n+\tif (info->recursive) {\n+\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n+\n+\t\tcpr.git_cmd = 1;\n+\t\tcpr.dir = path;\n+\t\tprepare_submodule_repo_env(&cpr.env_array);\n+\n+\t\targv_array_pushl(&cpr.args, \"--super-prefix\", NULL);\n+\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"foreach\", \"--recursive\",\n+\t\t\t\tNULL);\n+\n+\t\tif (info->quiet)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\targv_array_pushv(&cpr.args, info->argv);\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"run_command returned non-zero status while\"\n+\t\t\t\t\"recursing in the nested submodules of %s\\n.\"),\n+\t\t\t\tdisplaypath);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+}\n+\n+static int module_foreach(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct cb_foreach info = CB_FOREACH_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\n+\tstruct option module_foreach_options[] = {\n+\t\tOPT__QUIET(&info.quiet, N_(\"Suppress output of entering each submodule command\")),\n+\t\tOPT_BOOL(0, \"recursive\", &info.recursive,\n+\t\t\t N_(\"Recurse into nested submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper foreach [--quiet] [--recursive] \"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_foreach_options,\n+\t\t\t git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN);\n+\n+\tif (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tinfo.argc = argc;\n+\tinfo.argv = argv;\n+\tinfo.prefix = prefix;\n+\n+\tfor_each_listed_submodule(&list, runcommand_in_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n struct init_cb {\n \tconst char *prefix;\n \tunsigned int flags;\n@@ -1841,6 +1984,7 @@ static struct cmd_struct commands[] = {\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{\"foreach\", module_foreach, SUPPORT_SUPER_PREFIX},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n \t{\"print-default-remote\", print_default_remote, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 331d71c908b..cba585f0754 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -323,44 +323,7 @@ cmd_foreach()\n \t\tshift\n \tdone\n \n-\ttoplevel=$(pwd)\n-\n-\t# dup stdin so that it can be restored when running the external\n-\t# command in the subshell (and a recursive call to this function)\n-\texec 3<&0\n-\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n-\t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\t\t\tsay \"$(eval_gettext \"Entering '\\$displaypath'\")\"\n-\t\t\tname=$(git submodule--helper name \"$sm_path\")\n-\t\t\t(\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\tsanitize_submodule_env\n-\t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\t# we make $path available to scripts ...\n-\t\t\t\tpath=$sm_path &&\n-\t\t\t\tif test $# -eq 1\n-\t\t\t\tthen\n-\t\t\t\t\teval \"$1\"\n-\t\t\t\telse\n-\t\t\t\t\t\"$@\"\n-\t\t\t\tfi &&\n-\t\t\t\tif test -n \"$recursive\"\n-\t\t\t\tthen\n-\t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n-\t\t\t\tfi\n-\t\t\t) <&3 3<&- ||\n-\t\t\tdie \"$(eval_gettext \"Stopping at '\\$displaypath'; script returned non-zero status.\")\"\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} \"$@\"\n }\n \n #\n-- \n2.17.0.255.g8bfb7c0704\n\n"},{"id":"347105","messageId":"20180509101330.f41a1d9c03f6af759398137b@google.com","threadId":"48403","inReplyTo":"20180509002952.172347-1-sbeller@google.com","subject":"Re: [PATCHv2 0/4] Rebooting pc/submodule-helper-foreach","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-05-09T17:13:30Z","receivedAt":"2018-05-09T17:13:37Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 8 May 2018 17:29:48 -0700\nStefan Beller wrote:\n\n> v2:\n> * rebased onto origin/master\n> * dropped leftover \"toplevel\" variable from experimentation\n> * reworded the commit message for the first patch extensively\n> * dropped the third patch\n> * see \"branch-diff\" below.\n\nPatches 1-3 look good to me.\n\nI also can't see anything wrong with patch 4, but I am not an expert at\nshell and how we call it from C, so a review from another reviewer would\nbe appreciated.\n"},{"id":"347180","messageId":"xmqqo9hoj9s7.fsf@gitster-ct.c.googlers.com","threadId":"48403","inReplyTo":"20180509002952.172347-5-sbeller@google.com","subject":"Re: [PATCH 4/4] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-05-10T06:37:44Z","receivedAt":"2018-05-10T06:37:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +static void runcommand_in_submodule_cb(const struct cache_entry *list_item,\n> +\t\t\t\t void *cb_data)\n> +{\n> +\tstruct cb_foreach *info = cb_data;\n> +\tconst char *path = list_item->name;\n> +\tconst struct object_id *ce_oid = &list_item->oid;\n> +\n> +\tconst struct submodule *sub;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tchar *displaypath;\n> +\n> +\tdisplaypath = get_submodule_displaypath(path, info->prefix);\n> +\n> +\tsub = submodule_from_path(the_repository, &null_oid, path);\n> +\n> +\tif (!sub)\n> +\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n> +\t\t\tdisplaypath);\n> +\n> +\tif (!is_submodule_populated_gently(path, NULL))\n> +\t\tgoto cleanup;\n> +\n> +\tprepare_submodule_repo_env(&cp.env_array);\n> +\n> +\t/*\n> +\t* For the purpose of executing in the submodule,\n> +\t* separate shell is used for the purpose of running the\n> +\t* child process.\n> +\t*/\n\nMicronit: this multi-line comment is indented in a funny way.\n\n> +\tcp.use_shell = 1;\n> +\tcp.dir = path;\n> +\n> +\t/*\n> +\t* NEEDSWORK: the command currently has access to the variables $name,\n> +\t* $sm_path, $displaypath, $sha1 and $toplevel only when the command\n> +\t* contains a single argument. This is done for maintaining a faithful\n> +\t* translation from shell script.\n> +\t*/\n\nSame micronit.\n\nThe scripted version does 'eval \"$1\"', so $1 could be something like \n\n\tfor-each 'echo \"$name:$sm_path:$displaypath:$sha1:$toplevel\"'\n\nand it can see any variable, not just these 5 (i.e. we could have\nfed e.g. $wt_prefix and $mode to the above 'echo' and with the\nscripted version the script would have learned their values; with\nthis version it no longer does, but only these 5 are part of the\ndocumented API, so we choose not to consider it a regression).\n\n> +\tif (info->argc == 1) {\n> +\t\tchar *toplevel = xgetcwd();\n> +\t\tstruct strbuf sb = STRBUF_INIT;\n> +\n> +\t\targv_array_pushf(&cp.env_array, \"name=%s\", sub->name);\n> +\t\targv_array_pushf(&cp.env_array, \"sm_path=%s\", path);\n> +\t\targv_array_pushf(&cp.env_array, \"displaypath=%s\", displaypath);\n> +\t\targv_array_pushf(&cp.env_array, \"sha1=%s\",\n> +\t\t\t\toid_to_hex(ce_oid));\n> +\t\targv_array_pushf(&cp.env_array, \"toplevel=%s\", toplevel);\n> +\n> +\t\t/*\n> +\t\t* Since the path variable was accessible from the script\n> +\t\t* before porting, it is also made available after porting.\n> +\t\t* The environment variable \"PATH\" has a very special purpose\n> +\t\t* on windows. And since environment variables are\n> +\t\t* case-insensitive in windows, it interferes with the\n> +\t\t* existing PATH variable. Hence, to avoid that, we expose\n> +\t\t* path via the args argv_array and not via env_array.\n> +\t\t*/\n> +\t\tsq_quote_buf(&sb, path);\n> +\t\targv_array_pushf(&cp.args, \"path=%s; %s\",\n> +\t\t\t\t sb.buf, info->argv[0]);\n\nOK, so we do the equivalent of\n\n\tname=... sm_path=... displaypath=... sha1=... toplevel=... \\\n\tsh -c 'path=...; echo \"$name:$sm_path:...\"'\n\nwhen doing\n\n\tfor-each 'echo \"$name:$sm_path:...\"'\n\nwith parts denoted with ... correctly quoted as necessary. I guess\nit would be the best we could do.\n\nI myself do not know if it is true that bash ported to Windows won't\nget confused with the above \"we use path (all lowercase) only as a\npure shell variable without exporting it ourselves\"; I'd trust those\nwho are more familiar with the platform to raise objections and\nsuggest a better alternative if it is not the case. \n\nThanks for the (malformatted;-) leading comment to highlight why the\n'path' variable alone is treated differently from the others.\n\n> +\t\tstrbuf_release(&sb);\n> +\t\tfree(toplevel);\n> +\t} else {\n> +\t\targv_array_pushv(&cp.args, info->argv);\n> +\t}\n> +\n> +\tif (!info->quiet)\n> +\t\tprintf(_(\"Entering '%s'\\n\"), displaypath);\n> +\n> +\tif (info->argv[0] && run_command(&cp))\n> +\t\tdie(_(\"run_command returned non-zero status for %s\\n.\"),\n> +\t\t\tdisplaypath);\n> +\n> +\tif (info->recursive) {\n> +\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n> +\n> +\t\tcpr.git_cmd = 1;\n> +\t\tcpr.dir = path;\n> +\t\tprepare_submodule_repo_env(&cpr.env_array);\n> +\n> +\t\targv_array_pushl(&cpr.args, \"--super-prefix\", NULL);\n> +\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n> +\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"foreach\", \"--recursive\",\n> +\t\t\t\tNULL);\n> +\n> +\t\tif (info->quiet)\n> +\t\t\targv_array_push(&cpr.args, \"--quiet\");\n> +\n> +\t\targv_array_pushv(&cpr.args, info->argv);\n> +\n> +\t\tif (run_command(&cpr))\n> +\t\t\tdie(_(\"run_command returned non-zero status while\"\n> +\t\t\t\t\"recursing in the nested submodules of %s\\n.\"),\n> +\t\t\t\tdisplaypath);\n> +\t}\n> +\n> +cleanup:\n> +\tfree(displaypath);\n> +}\n\n"},{"id":"347315","messageId":"20180510212501.140660-1-sbeller@google.com","threadId":"48403","inReplyTo":"xmqqo9hoj9s7.fsf@gitster-ct.c.googlers.com","subject":"[PATCH] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-05-10T21:25:01Z","receivedAt":"2018-05-10T21:25:12Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Prathamesh Chavan \n\nThis aims to make git-submodule foreach a builtin. 'foreach' is ported to\nthe submodule--helper, and submodule--helper is called from\ngit-submodule.sh.\n\nHelped-by: Brandon Williams \nMentored-by: Christian Couder \nMentored-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \nSigned-off-by: Stefan Beller \n---\n\nThis is a resend of the last commit in origin/pc/submodule-helper-foreach\nIt addresses the micro nits of funny comment indentation.\n\nThanks,\nStefan\n\n builtin/submodule--helper.c | 144 ++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 39 +---------\n 2 files changed, 145 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c2403a915ff..4002026d1ac 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -439,6 +439,149 @@ static void for_each_listed_submodule(const struct module_list *list,\n \t\tfn(list->entries[i], cb_data);\n }\n \n+struct cb_foreach {\n+\tint argc;\n+\tconst char **argv;\n+\tconst char *prefix;\n+\tint quiet;\n+\tint recursive;\n+};\n+#define CB_FOREACH_INIT { 0 }\n+\n+static void runcommand_in_submodule_cb(const struct cache_entry *list_item,\n+\t\t\t\t void *cb_data)\n+{\n+\tstruct cb_foreach *info = cb_data;\n+\tconst char *path = list_item->name;\n+\tconst struct object_id *ce_oid = &list_item->oid;\n+\n+\tconst struct submodule *sub;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *displaypath;\n+\n+\tdisplaypath = get_submodule_displaypath(path, info->prefix);\n+\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n+\n+\tif (!sub)\n+\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n+\t\t\tdisplaypath);\n+\n+\tif (!is_submodule_populated_gently(path, NULL))\n+\t\tgoto cleanup;\n+\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\n+\t/*\n+\t * For the purpose of executing in the submodule,\n+\t * separate shell is used for the purpose of running the\n+\t * child process.\n+\t */\n+\tcp.use_shell = 1;\n+\tcp.dir = path;\n+\n+\t/*\n+\t * NEEDSWORK: the command currently has access to the variables $name,\n+\t * $sm_path, $displaypath, $sha1 and $toplevel only when the command\n+\t * contains a single argument. This is done for maintaining a faithful\n+\t * translation from shell script.\n+\t */\n+\tif (info->argc == 1) {\n+\t\tchar *toplevel = xgetcwd();\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\targv_array_pushf(&cp.env_array, \"name=%s\", sub->name);\n+\t\targv_array_pushf(&cp.env_array, \"sm_path=%s\", path);\n+\t\targv_array_pushf(&cp.env_array, \"displaypath=%s\", displaypath);\n+\t\targv_array_pushf(&cp.env_array, \"sha1=%s\",\n+\t\t\t\toid_to_hex(ce_oid));\n+\t\targv_array_pushf(&cp.env_array, \"toplevel=%s\", toplevel);\n+\n+\t\t/*\n+\t\t * Since the path variable was accessible from the script\n+\t\t * before porting, it is also made available after porting.\n+\t\t * The environment variable \"PATH\" has a very special purpose\n+\t\t * on windows. And since environment variables are\n+\t\t * case-insensitive in windows, it interferes with the\n+\t\t * existing PATH variable. Hence, to avoid that, we expose\n+\t\t * path via the args argv_array and not via env_array.\n+\t\t */\n+\t\tsq_quote_buf(&sb, path);\n+\t\targv_array_pushf(&cp.args, \"path=%s; %s\",\n+\t\t\t\t sb.buf, info->argv[0]);\n+\t\tstrbuf_release(&sb);\n+\t\tfree(toplevel);\n+\t} else {\n+\t\targv_array_pushv(&cp.args, info->argv);\n+\t}\n+\n+\tif (!info->quiet)\n+\t\tprintf(_(\"Entering '%s'\\n\"), displaypath);\n+\n+\tif (info->argv[0] && run_command(&cp))\n+\t\tdie(_(\"run_command returned non-zero status for %s\\n.\"),\n+\t\t\tdisplaypath);\n+\n+\tif (info->recursive) {\n+\t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n+\n+\t\tcpr.git_cmd = 1;\n+\t\tcpr.dir = path;\n+\t\tprepare_submodule_repo_env(&cpr.env_array);\n+\n+\t\targv_array_pushl(&cpr.args, \"--super-prefix\", NULL);\n+\t\targv_array_pushf(&cpr.args, \"%s/\", displaypath);\n+\t\targv_array_pushl(&cpr.args, \"submodule--helper\", \"foreach\", \"--recursive\",\n+\t\t\t\tNULL);\n+\n+\t\tif (info->quiet)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\targv_array_pushv(&cpr.args, info->argv);\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"run_command returned non-zero status while\"\n+\t\t\t\t\"recursing in the nested submodules of %s\\n.\"),\n+\t\t\t\tdisplaypath);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+}\n+\n+static int module_foreach(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct cb_foreach info = CB_FOREACH_INIT;\n+\tstruct pathspec pathspec;\n+\tstruct module_list list = MODULE_LIST_INIT;\n+\n+\tstruct option module_foreach_options[] = {\n+\t\tOPT__QUIET(&info.quiet, N_(\"Suppress output of entering each submodule command\")),\n+\t\tOPT_BOOL(0, \"recursive\", &info.recursive,\n+\t\t\t N_(\"Recurse into nested submodules\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper foreach [--quiet] [--recursive] \"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_foreach_options,\n+\t\t\t git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN);\n+\n+\tif (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)\n+\t\treturn 1;\n+\n+\tinfo.argc = argc;\n+\tinfo.argv = argv;\n+\tinfo.prefix = prefix;\n+\n+\tfor_each_listed_submodule(&list, runcommand_in_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n struct init_cb {\n \tconst char *prefix;\n \tunsigned int flags;\n@@ -1841,6 +1984,7 @@ static struct cmd_struct commands[] = {\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{\"foreach\", module_foreach, SUPPORT_SUPER_PREFIX},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\n \t{\"status\", module_status, SUPPORT_SUPER_PREFIX},\n \t{\"print-default-remote\", print_default_remote, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 331d71c908b..cba585f0754 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -323,44 +323,7 @@ cmd_foreach()\n \t\tshift\n \tdone\n \n-\ttoplevel=$(pwd)\n-\n-\t# dup stdin so that it can be restored when running the external\n-\t# command in the subshell (and a recursive call to this function)\n-\texec 3<&0\n-\n-\t{\n-\t\tgit submodule--helper list --prefix \"$wt_prefix\" ||\n-\t\techo \"#unmatched\" $?\n-\t} |\n-\twhile read -r mode sha1 stage sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n-\t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\t\t\tsay \"$(eval_gettext \"Entering '\\$displaypath'\")\"\n-\t\t\tname=$(git submodule--helper name \"$sm_path\")\n-\t\t\t(\n-\t\t\t\tprefix=\"$prefix$sm_path/\"\n-\t\t\t\tsanitize_submodule_env\n-\t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\t# we make $path available to scripts ...\n-\t\t\t\tpath=$sm_path &&\n-\t\t\t\tif test $# -eq 1\n-\t\t\t\tthen\n-\t\t\t\t\teval \"$1\"\n-\t\t\t\telse\n-\t\t\t\t\t\"$@\"\n-\t\t\t\tfi &&\n-\t\t\t\tif test -n \"$recursive\"\n-\t\t\t\tthen\n-\t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n-\t\t\t\tfi\n-\t\t\t) <&3 3<&- ||\n-\t\t\tdie \"$(eval_gettext \"Stopping at '\\$displaypath'; script returned non-zero status.\")\"\n-\t\tfi\n-\tdone\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} ${prefix:+--super-prefix \"$prefix\"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} \"$@\"\n }\n \n #\n-- \n2.17.0.255.g8bfb7c0704\n\n"}]}