{"thread":{"id":"47744","subject":"[PATCH v1 0/5] Incremental rewrite of git-submodules","startedAt":"2018-02-02T04:58:00Z","lastAt":"2018-02-06T23:11:36Z","messageCount":9,"participants":["Prathamesh Chavan","Jonathan Tan","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"338071","messageId":"20180202045745.5076-1-pc44800@gmail.com","threadId":"47744","inReplyTo":null,"subject":"[PATCH v1 0/5] Incremental rewrite of git-submodules","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-02-02T04:57:40Z","receivedAt":"2018-02-02T04:58:00Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"Following series of patches focuses on porting submodule subcommand\ngit-foreach from shell to C.\nAn initial attempt for porting was introduced about 9 months back,\nand since then then patches have undergone many changes. Some of the \nnotable discussion thread which I would like to point out is: [1] \nThe previous version of this patch series which was floated is\navailable at: [2].\n\nThe following changes were made to that:\n* As it was observed in other submodule subcommand's ported function\n that the number of params increased a lot, the variables quiet and \n recursive, were replaced in the cb_foreach struct with a single\n unsigned integer variable called flags.\n\n* To accomodate the possiblity of a direct call to the functions\n runcommand_in_submodule(), callback function\n runcommand_in_submodule_cb() was introduced.\n\n[1]: https://public-inbox.org/git/20170419170513.16475-1-pc44800@gmail.com/T/#u\n[2]: https://public-inbox.org/git/20170807211900.15001-14-pc44800@gmail.com/\n\nAs before you can find this series at: \nhttps://github.com/pratham-pc/git/commits/patch-series-3\n\nAnd its build report is available at: \nhttps://travis-ci.org/pratham-pc/git/builds/\nBranch: patch-series-3\nBuild #202\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 | 151 ++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 40 +----------\n t/t7407-submodule-foreach.sh | 38 +++++++++-\n 4 files changed, 197 insertions(+), 47 deletions(-)\n\n-- \n2.15.1\n\n"},{"id":"338072","messageId":"20180202045745.5076-2-pc44800@gmail.com","threadId":"47744","inReplyTo":"20180202045745.5076-1-pc44800@gmail.com","subject":"[PATCH v1 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-02-02T04:57:41Z","receivedAt":"2018-02-02T04:58:04Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"When running 'git submodule foreach' from a subdirectory of your\nrepository, nested submodules get a bogus value for $sm_path:\nFor a submodule 'sub' that contains a nested submodule 'nested',\nrunning 'git -C dir submodule foreach echo $path' would report\npath='../nested' for the nested submodule. The first part '../' is\nderived from the logic computing the relative path from $pwd to the\nroot of the superproject. The second part is the submodule path inside\nthe submodule. This value is of little use and is hard to document.\n\nThere are two 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.\n In this case we would want to have path='sub/nested'.\n\n(b) As Ramsay noticed the documented value is wrong. For the non-nested\n case the path is equal to the relative path from $pwd to the\n submodules working directory. When following this model,\n the expected value would be path='../sub/nested'.\n\nThe behavior for (b) 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) such that \"path\"\nis \"the path from the toplevel of the superproject to the mount point\nof the submodule\", we would break any existing submodule user that runs\nforeach from non-root of the superproject as the non-nested submodule\n'../sub' would change its path to 'sub'.\n\nIf we would fix the meaning of the $path using (b), such that \"path\"\nis \"the relative path from $pwd to the submodule\", then we would break\nany user that uses nested submodules (even from the root directory) as\nthe 'nested' would become 'sub/nested'.\n\nBoth 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\nby a human rather than by some automation task. With a human on\nthe keyboard the feedback loop is short and the changed behavior can be\nadapted to quickly unlike some automation that can break silently.\n\nDiscussed-with: Ramsay Jones \nSigned-off-by: Prathamesh Chavan \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 156255a9e..7305ee25f 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 6ba5daf42..0663622a4 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 <\nSigned-off-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \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 ff612001d..a23baef62 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.15.1\n\n"},{"id":"338074","messageId":"20180202045745.5076-4-pc44800@gmail.com","threadId":"47744","inReplyTo":"20180202045745.5076-1-pc44800@gmail.com","subject":"[PATCH v1 3/5] submodule foreach: clarify the '$toplevel' variable documentation","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-02-02T04:57:43Z","receivedAt":"2018-02-02T04:58:11Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"It 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 \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 a23baef62..8e7930ebc 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 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.15.1\n\n"},{"id":"338075","messageId":"20180202045745.5076-5-pc44800@gmail.com","threadId":"47744","inReplyTo":"20180202045745.5076-1-pc44800@gmail.com","subject":"[PATCH v1 4/5] submodule foreach: document variable '$displaypath'","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-02-02T04:57:44Z","receivedAt":"2018-02-02T04:58:13Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"It 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 \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 8e7930ebc..0cca702cb 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 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 0663622a4..6ad57e061 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-\\$name-\\$sm_path-\\$displaypath-\\$sha1\" >../../actual\n \t) &&\n \ttest_i18ncmp expect actual\n '\n-- \n2.15.1\n\n"},{"id":"338076","messageId":"20180202045745.5076-6-pc44800@gmail.com","threadId":"47744","inReplyTo":"20180202045745.5076-1-pc44800@gmail.com","subject":"[PATCH v1 5/5] submodule: port submodule subcommand 'foreach' from shell to C","fromName":"Prathamesh Chavan","fromEmail":"pc44800@gmail.com","sentAt":"2018-02-02T04:57:45Z","receivedAt":"2018-02-02T04:58:16Z","isPatch":true,"sender":{"key":"pc44800@gmail.com","avatar":"https://avatars.githubusercontent.com/u/17272661?v=4"},"body":"This aims to make git-submodule foreach a builtin. This is the very\nfirst step taken in this direction. Hence, 'foreach' is ported to\nsubmodule--helper, and submodule--helper is called from git-submodule.sh.\nThe code is split up to have one function to obtain all the list of\nsubmodules. This function acts as the front-end of git-submodule foreach\nsubcommand. It calls the function for_each_listed_submodule(), which basically\nloops through the list and calls function fn, which in this case is\nruncommand_in_submodule_cb(). This third function is a callback function that\ncalls runcommand_in_submodule() with the appropriate parameters and then\ntakes care of running the command in that submodule, and recursively\nperforming the same when --recursive is flagged.\n\nThe first function module_foreach first parses the options present in\nargv, and then with the help of module_list_compute(), generates the list of\nsubmodules present in the current working tree.\n\nThe second function for_each_listed_submodule() traverses through the\nlist, and calls function fn (which in case of submodule subcommand\nforeach is runcommand_in_submodule_cb()) is called for each entry.\n\nThe third function runcommand_in_submodule_cb() calls the function\nruncommand_in_submodule() after passing appropraite parameters.\n\nThe fourth function runcommand_in_submodule(), generates a submodule struct sub\nfor $name, value and then later prepends name=sub->name; and other\nvalue assignment to the env argv_array structure of a child_process.\nAlso the of submodule-foreach is push to args argv_array\nstructure and finally, using run_command the commands are executed\nusing a shell.\n\nThe fourth function also takes care of the recursive flag, by creating\na separate child_process structure and prepending \"--super-prefix displaypath\",\nto the args argv_array structure. Other required arguments and the\ninput of submodule-foreach is also appended to this argv_array.\n\nHelped-by: Brandon Williams \nMentored-by: Christian Couder \nMentored-by: Stefan Beller \nSigned-off-by: Prathamesh Chavan \n---\n builtin/submodule--helper.c | 151 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 39 +-----------\n 2 files changed, 152 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a5c4a8a69..46dee6bf5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -718,6 +718,156 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct cb_foreach {\n+\tint argc;\n+\tconst char **argv;\n+\tconst char *prefix;\n+\tunsigned int flags;\n+};\n+#define CB_FOREACH_INIT { 0, NULL, NULL, 0 }\n+\n+static void runcommand_in_submodule(const char *path, const struct object_id *ce_oid,\n+\t\t\t\t int argc, const char **argv, const char *prefix,\n+\t\t\t\t unsigned int flags)\n+{\n+\tconst struct submodule *sub;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tchar *displaypath;\n+\n+\tdisplaypath = get_submodule_displaypath(path, 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 displaypath);\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 maintianing a faithful\n+\t * translation from shell script.\n+\t */\n+\tif (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\t oid_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\t path, argv[0]);\n+\t\tfree(toplevel);\n+\t} else {\n+\t\targv_array_pushv(&cp.args, argv);\n+\t}\n+\n+\tif (!(flags & OPT_QUIET))\n+\t\tprintf(_(\"Entering '%s'\\n\"), displaypath);\n+\n+\tif (argv[0] && run_command(&cp))\n+\t\tdie(_(\"run_command returned non-zero status for %s\\n.\"),\n+\t\t displaypath);\n+\n+\tif (flags & OPT_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\t NULL);\n+\n+\t\tif (flags & OPT_QUIET)\n+\t\t\targv_array_push(&cpr.args, \"--quiet\");\n+\n+\t\targv_array_pushv(&cpr.args, argv);\n+\n+\t\tif (run_command(&cpr))\n+\t\t\tdie(_(\"run_command returned non-zero status while\"\n+\t\t\t \"recursing in the nested submodules of %s\\n.\"),\n+\t\t\t displaypath);\n+\t}\n+\n+cleanup:\n+\tfree(displaypath);\n+}\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+\truncommand_in_submodule(list_item->name, &list_item->oid, info->argc,\n+\t\t\t\tinfo->argv, info->prefix, info->flags);\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+\tint quiet = 0;\n+\tint recursive = 0;\n+\n+\tstruct option module_foreach_options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"Suppress output of entering each submodule command\")),\n+\t\tOPT_BOOL(0, \"recursive\", &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\tBUG(\"module_list_compute should not choke on empty pathspec\");\n+\n+\tinfo.argc = argc;\n+\tinfo.argv = argv;\n+\tinfo.prefix = prefix;\n+\tif (quiet)\n+\t\tinfo.flags |= OPT_QUIET;\n+\tif (recursive)\n+\t\tinfo.flags |= OPT_RECURSIVE;\n+\n+\tfor_each_listed_submodule(&list, runcommand_in_submodule_cb, &info);\n+\n+\treturn 0;\n+}\n+\n static int clone_submodule(const char *path, const char *gitdir, const char *url,\n \t\t\t const char *depth, struct string_list *reference,\n \t\t\t int quiet, int progress)\n@@ -1496,6 +1646,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{\"remote-branch\", resolve_remote_submodule_branch, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 7305ee25f..7627e27c8 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.15.1\n\n"},{"id":"338541","messageId":"20180206145406.b759164cead02cd3bb3fdce0@google.com","threadId":"47744","inReplyTo":"20180202045745.5076-2-pc44800@gmail.com","subject":"Re: [PATCH v1 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-02-06T22:54:06Z","receivedAt":"2018-02-06T22:54:13Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Fri, 2 Feb 2018 10:27:41 +0530\nPrathamesh Chavan wrote:\n\n> When running 'git submodule foreach' from a subdirectory of your\n\nAdd \"--recursive\".\n\n> repository, nested submodules get a bogus value for $sm_path:\n\nMaybe call it $path for now, since $sm_path starts to be recommended\nonly in patches after this one.\n\n> For a submodule 'sub' that contains a nested submodule 'nested',\n> running 'git -C dir submodule foreach echo $path' would report\n\nAdd \"from the root of the superproject\", maybe?\n\n> path='../nested' for the nested submodule. The first part '../' is\n> derived from the logic computing the relative path from $pwd to the\n> root of the superproject. The second part is the submodule path inside\n> the submodule. This value is of little use and is hard to document.\n> \n> There are two 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.\n> In this case we would want to have path='sub/nested'.\n> \n> (b) As Ramsay noticed the documented value is wrong. For the non-nested\n> case the path is equal to the relative path from $pwd to the\n> submodules working directory. When following this model,\n> the expected value would be path='../sub/nested'.\n\nA third solution is to use \"nested\" - that is, the name of the submodule\ndirectory relative to its superproject. (It's currently documented as\n\"the name of the submodule directory relative to the superproject\".)\nHaving said that, (b) is probably better.\n\n> Both 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\n> by a human rather than by some automation task. With a human on\n> the keyboard the feedback loop is short and the changed behavior can be\n> adapted to quickly unlike some automation that can break silently.\n\nThanks - this is a good analysis.\n\n> git-submodule.sh | 1 -\n> t/t7407-submodule-foreach.sh | 36 ++++++++++++++++++++++++++++++++++--\n\nI think the documentation should be changed too - $path is the name of\nthe submodule directory relative to the current directory?\n"},{"id":"338543","messageId":"20180206150044.1bffbb573c088d38c8e44bf5@google.com","threadId":"47744","inReplyTo":"20180206145406.b759164cead02cd3bb3fdce0@google.com","subject":"Re: [PATCH v1 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-02-06T23:00:44Z","receivedAt":"2018-02-06T23:01:05Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 6 Feb 2018 14:54:06 -0800\nJonathan Tan wrote:\n\n> > There are two 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.\n> > In this case we would want to have path='sub/nested'.\n> > \n> > (b) As Ramsay noticed the documented value is wrong. For the non-nested\n> > case the path is equal to the relative path from $pwd to the\n> > submodules working directory. When following this model,\n> > the expected value would be path='../sub/nested'.\n>\n> A third solution is to use \"nested\" - that is, the name of the submodule\n> directory relative to its superproject. (It's currently documented as\n> \"the name of the submodule directory relative to the superproject\".)\n> Having said that, (b) is probably better.\n\n[snip]\n\n> > +cat >expect < > +Entering '../nested1'\n> > +$pwd/clone2-nested1-nested1-$nested1sha1\n> > +Entering '../nested1/nested2'\n> > +$pwd/clone2/nested1-nested2-nested2-$nested2sha1\n> > +Entering '../nested1/nested2/nested3'\n> > +$pwd/clone2/nested1/nested2-nested3-nested3-$nested3sha1\n> > +Entering '../nested1/nested2/nested3/submodule'\n> > +$pwd/clone2/nested1/nested2/nested3-submodule-submodule-$submodulesha1\n> > +Entering '../sub1'\n> > +$pwd/clone2-foo1-sub1-$sub1sha1\n> > +Entering '../sub2'\n> > +$pwd/clone2-foo2-sub2-$sub2sha1\n> > +Entering '../sub3'\n> > +$pwd/clone2-foo3-sub3-$sub3sha1\n> > +EOF\n> > +\n> > +test_expect_success 'test \"submodule foreach --recursive\" from subdirectory' '\n> > +\t(\n> > +\t\tcd clone2/untracked &&\n> > +\t\tgit submodule foreach --recursive \"echo \\$toplevel-\\$name-\\$sm_path-\\$sha1\" >../../actual\n> > +\t) &&\n> > +\ttest_i18ncmp expect actual\n> > +'\n\nWait a minute...this seems like you're using my \"third solution\". If we\nwere using either (a) or (b), $sm_path would contain slashes in the case\nof nested submodules, right?\n"},{"id":"338545","messageId":"CAGZ79kZ-Z7jq7LZQKdyvgk6zUdsGc1dQERTKvGJ2S3=Sb9dFyg@mail.gmail.com","threadId":"47744","inReplyTo":"20180206145406.b759164cead02cd3bb3fdce0@google.com","subject":"Re: [PATCH v1 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-02-06T23:11:27Z","receivedAt":"2018-02-06T23:11:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Feb 6, 2018 at 2:54 PM, Jonathan Tan wrote:\n> On Fri, 2 Feb 2018 10:27:41 +0530\n> Prathamesh Chavan wrote:\n>\n>> When running 'git submodule foreach' from a subdirectory of your\n>\n> Add \"--recursive\".\n>\n>> repository, nested submodules get a bogus value for $sm_path:\n>\n> Maybe call it $path for now, since $sm_path starts to be recommended\n> only in patches after this one.\n>\n>> For a submodule 'sub' that contains a nested submodule 'nested',\n>> running 'git -C dir submodule foreach echo $path' would report\n>\n> Add \"from the root of the superproject\", maybe?\n\nThis command is run from the root, though the\n\"-C dir\" should indicate that the git command runs from the subdirectory.\nNot sure how much slang this is, or if it can be made easier to understand\nby writing\n\n cd dir && git submodule foreach --recursive echo $path\n\nbut adding the \"from root\" part sounds like a clarification nevertheless.\n\n>> path='../nested' for the nested submodule. The first part '../' is\n>> derived from the logic computing the relative path from $pwd to the\n>> root of the superproject. The second part is the submodule path inside\n>> the submodule. This value is of little use and is hard to document.\n>>\n>> There are two 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.\n>> In this case we would want to have path='sub/nested'.\n>>\n>> (b) As Ramsay noticed the documented value is wrong. For the non-nested\n>> case the path is equal to the relative path from $pwd to the\n>> submodules working directory. When following this model,\n>> the expected value would be path='../sub/nested'.\n>\n> A third solution is to use \"nested\" - that is, the name of the submodule\n> directory relative to its superproject. (It's currently documented as\n> \"the name of the submodule directory relative to the superproject\".)\n> Having said that, (b) is probably better.\n\nOh, so the nested would just report \"nested/\" as that is the path\nfrom its superproject to its location. The value does not change depending\non where the command is invoked, or whether it is an actual nested or direct\nsubmodule? The latter part sounds like a slight modification of (a), but the\nformer part sounds like a completely new version (c).\n"}]}