From c033a2f62dda774de8cccc83b7f1918377995d6c Mon Sep 17 00:00:00 2001 From: Prathamesh Chavan Date: Tue, 8 May 2018 17:29:49 -0700 Subject: [PATCH 1/4] submodule foreach: correct '$path' in nested submodules from a subdirectory When running 'git submodule foreach --recursive' from a subdirectory of your repository, nested submodules get a bogus value for $path: For a submodule 'sub' that contains a nested submodule 'nested', running 'git -C dir submodule foreach echo $path' from the root of the superproject would report path='../nested' for the nested submodule. The first part '../' is derived from the logic computing the relative path from $pwd to the root of the superproject. The second part is the submodule path inside the submodule. This value is of little use and is hard to document. Also, in git-submodule.txt, $path is documented to be the "name of the submodule directory relative to the superproject", but "the superproject" is ambiguous. To resolve both these issues, we could: (a) Change "the superproject" to "its immediate superproject", so $path would be "nested" instead of "../nested". (b) Change "the superproject" to "the superproject the original command was run from", so $path would be "sub/nested" instead of "../nested". (c) Change "the superproject" to "the directory the original command was run from", so $path would be "../sub/nested" instead of "../nested". The behavior for (c) was attempted to be introduced in 091a6eb0fe (submodule: drop the top-level requirement, 2013-06-16) with the intent for $path to be relative from $pwd to the submodule worktree, but that did not work for nested submodules, as the intermittent submodules were not included in the path. If we were to fix the meaning of the $path using (a), we would break any existing submodule user that runs foreach from non-root of the superproject as the non-nested submodule '../sub' would change its path to 'sub'. If we were to fix the meaning of $path using (b), then we would break any user that uses nested submodules (even from the root directory) as the 'nested' would become 'sub/nested'. If we were to fix the meaning of $path using (c), then we would break the same users as in (b) as 'nested' would become 'sub/nested' from the root directory of the superproject. All groups can be found in the wild. The author has no data if one group outweighs the other by large margin, and offending each one seems equally bad at first. However in the authors imagination it is better to go with (a) as running from a sub directory sounds like it is carried out by a human rather than by some automation task. With a human on the keyboard the feedback loop is short and the changed behavior can be adapted to quickly unlike some automation that can break silently. Discussed-with: Ramsay Jones Signed-off-by: Prathamesh Chavan Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 1 - t/t7407-submodule-foreach.sh | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 24914963ca..331d71c908 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -345,7 +345,6 @@ cmd_foreach() prefix="$prefix$sm_path/" sanitize_submodule_env cd "$sm_path" && - sm_path=$(git submodule--helper relative-path "$sm_path" "$wt_prefix") && # we make $path available to scripts ... path=$sm_path && if test $# -eq 1 diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 6ba5daf42e..5144cc6926 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -82,9 +82,9 @@ test_expect_success 'test basic "submodule foreach" usage' ' cat >expect <expect <../../actual + ) && + test_i18ncmp expect actual +' cat > expect < Date: Tue, 8 May 2018 17:29:50 -0700 Subject: [PATCH 2/4] submodule foreach: document '$sm_path' instead of '$path' As using a variable '$path' may be harmful to users due to capitalization issues, see 64394e3ae9 (git-submodule.sh: Don't use $path variable in eval_gettext string, 2012-04-17). Adjust the documentation to advocate for using $sm_path, which contains the same value. We still make the 'path' variable available and document it as a deprecated synonym of 'sm_path'. Discussed-with: Ramsay Jones Signed-off-by: Stefan Beller Signed-off-by: Prathamesh Chavan Signed-off-by: Junio C Hamano --- Documentation/git-submodule.txt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 630999f41a..066c7b6c18 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -183,12 +183,15 @@ information too. foreach [--recursive] :: Evaluates an arbitrary shell command in each checked out submodule. - The command has access to the variables $name, $path, $sha1 and + The command has access to the variables $name, $sm_path, $sha1 and $toplevel: $name is the name of the relevant submodule section in `.gitmodules`, - $path is the name of the submodule directory relative to the - superproject, $sha1 is the commit as recorded in the superproject, - and $toplevel is the absolute path to the top-level of the superproject. + $sm_path is the path of the submodule as recorded in the immediate + superproject, $sha1 is the commit as recorded in the immediate + superproject, and $toplevel is the absolute path to the top-level + of the immediate superproject. + Note that to avoid conflicts with '$PATH' on Windows, the '$path' + variable is now a deprecated synonym of '$sm_path' variable. Any submodules defined in the superproject but not checked out are ignored by this command. Unless given `--quiet`, foreach prints the name of each submodule before evaluating the command. From b6f7ac8fd561d6df9c76009898daf96a93765ffc Mon Sep 17 00:00:00 2001 From: Prathamesh Chavan Date: Tue, 8 May 2018 17:29:51 -0700 Subject: [PATCH 3/4] submodule foreach: document variable '$displaypath' It was observed that the variable '$displaypath' was accessible but undocumented. Hence, document it. Discussed-with: Ramsay Jones Signed-off-by: Stefan Beller Signed-off-by: Prathamesh Chavan Signed-off-by: Junio C Hamano --- Documentation/git-submodule.txt | 8 +++++--- t/t7407-submodule-foreach.sh | 22 +++++++++++----------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 066c7b6c18..500dfdafd1 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -183,11 +183,13 @@ information too. foreach [--recursive] :: Evaluates an arbitrary shell command in each checked out submodule. - The command has access to the variables $name, $sm_path, $sha1 and - $toplevel: + The command has access to the variables $name, $sm_path, $displaypath, + $sha1 and $toplevel: $name is the name of the relevant submodule section in `.gitmodules`, $sm_path is the path of the submodule as recorded in the immediate - superproject, $sha1 is the commit as recorded in the immediate + superproject, $displaypath contains the relative path from the + current working directory to the submodules root directory, + $sha1 is the commit as recorded in the immediate superproject, and $toplevel is the absolute path to the top-level of the immediate superproject. Note that to avoid conflicts with '$PATH' on Windows, the '$path' diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 5144cc6926..77729ac4aa 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -82,16 +82,16 @@ test_expect_success 'test basic "submodule foreach" usage' ' cat >expect <../../actual + git submodule foreach "echo \$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual ) && test_i18ncmp expect actual ' @@ -206,25 +206,25 @@ submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEA cat >expect <../../actual + git submodule foreach --recursive "echo toplevel: \$toplevel name: \$name path: \$sm_path displaypath: \$displaypath hash: \$sha1" >../../actual ) && test_i18ncmp expect actual ' From fc1b9243cd5d51ae455272f1f953d4002294edee Mon Sep 17 00:00:00 2001 From: Prathamesh Chavan Date: Thu, 10 May 2018 14:25:01 -0700 Subject: [PATCH 4/4] submodule: port submodule subcommand 'foreach' from shell to C This aims to make git-submodule foreach a builtin. 'foreach' is ported to the submodule--helper, and submodule--helper is called from git-submodule.sh. Helped-by: Brandon Williams Mentored-by: Christian Couder Mentored-by: Stefan Beller Signed-off-by: Prathamesh Chavan Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 144 ++++++++++++++++++++++++++++++++++++ git-submodule.sh | 39 +--------- 2 files changed, 145 insertions(+), 38 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index c2403a915f..4002026d1a 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -439,6 +439,149 @@ static void for_each_listed_submodule(const struct module_list *list, fn(list->entries[i], cb_data); } +struct cb_foreach { + int argc; + const char **argv; + const char *prefix; + int quiet; + int recursive; +}; +#define CB_FOREACH_INIT { 0 } + +static void runcommand_in_submodule_cb(const struct cache_entry *list_item, + void *cb_data) +{ + struct cb_foreach *info = cb_data; + const char *path = list_item->name; + const struct object_id *ce_oid = &list_item->oid; + + const struct submodule *sub; + struct child_process cp = CHILD_PROCESS_INIT; + char *displaypath; + + displaypath = get_submodule_displaypath(path, info->prefix); + + sub = submodule_from_path(the_repository, &null_oid, path); + + if (!sub) + die(_("No url found for submodule path '%s' in .gitmodules"), + displaypath); + + if (!is_submodule_populated_gently(path, NULL)) + goto cleanup; + + prepare_submodule_repo_env(&cp.env_array); + + /* + * For the purpose of executing in the submodule, + * separate shell is used for the purpose of running the + * child process. + */ + cp.use_shell = 1; + cp.dir = path; + + /* + * NEEDSWORK: the command currently has access to the variables $name, + * $sm_path, $displaypath, $sha1 and $toplevel only when the command + * contains a single argument. This is done for maintaining a faithful + * translation from shell script. + */ + if (info->argc == 1) { + char *toplevel = xgetcwd(); + struct strbuf sb = STRBUF_INIT; + + argv_array_pushf(&cp.env_array, "name=%s", sub->name); + argv_array_pushf(&cp.env_array, "sm_path=%s", path); + argv_array_pushf(&cp.env_array, "displaypath=%s", displaypath); + argv_array_pushf(&cp.env_array, "sha1=%s", + oid_to_hex(ce_oid)); + argv_array_pushf(&cp.env_array, "toplevel=%s", toplevel); + + /* + * Since the path variable was accessible from the script + * before porting, it is also made available after porting. + * The environment variable "PATH" has a very special purpose + * on windows. And since environment variables are + * case-insensitive in windows, it interferes with the + * existing PATH variable. Hence, to avoid that, we expose + * path via the args argv_array and not via env_array. + */ + sq_quote_buf(&sb, path); + argv_array_pushf(&cp.args, "path=%s; %s", + sb.buf, info->argv[0]); + strbuf_release(&sb); + free(toplevel); + } else { + argv_array_pushv(&cp.args, info->argv); + } + + if (!info->quiet) + printf(_("Entering '%s'\n"), displaypath); + + if (info->argv[0] && run_command(&cp)) + die(_("run_command returned non-zero status for %s\n."), + displaypath); + + if (info->recursive) { + struct child_process cpr = CHILD_PROCESS_INIT; + + cpr.git_cmd = 1; + cpr.dir = path; + prepare_submodule_repo_env(&cpr.env_array); + + argv_array_pushl(&cpr.args, "--super-prefix", NULL); + argv_array_pushf(&cpr.args, "%s/", displaypath); + argv_array_pushl(&cpr.args, "submodule--helper", "foreach", "--recursive", + NULL); + + if (info->quiet) + argv_array_push(&cpr.args, "--quiet"); + + argv_array_pushv(&cpr.args, info->argv); + + if (run_command(&cpr)) + die(_("run_command returned non-zero status while" + "recursing in the nested submodules of %s\n."), + displaypath); + } + +cleanup: + free(displaypath); +} + +static int module_foreach(int argc, const char **argv, const char *prefix) +{ + struct cb_foreach info = CB_FOREACH_INIT; + struct pathspec pathspec; + struct module_list list = MODULE_LIST_INIT; + + struct option module_foreach_options[] = { + OPT__QUIET(&info.quiet, N_("Suppress output of entering each submodule command")), + OPT_BOOL(0, "recursive", &info.recursive, + N_("Recurse into nested submodules")), + OPT_END() + }; + + const char *const git_submodule_helper_usage[] = { + N_("git submodule--helper foreach [--quiet] [--recursive] "), + NULL + }; + + argc = parse_options(argc, argv, prefix, module_foreach_options, + git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN); + + if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0) + return 1; + + info.argc = argc; + info.argv = argv; + info.prefix = prefix; + + for_each_listed_submodule(&list, runcommand_in_submodule_cb, &info); + + return 0; +} + struct init_cb { const char *prefix; unsigned int flags; @@ -1841,6 +1984,7 @@ static struct cmd_struct commands[] = { {"relative-path", resolve_relative_path, 0}, {"resolve-relative-url", resolve_relative_url, 0}, {"resolve-relative-url-test", resolve_relative_url_test, 0}, + {"foreach", module_foreach, SUPPORT_SUPER_PREFIX}, {"init", module_init, SUPPORT_SUPER_PREFIX}, {"status", module_status, SUPPORT_SUPER_PREFIX}, {"print-default-remote", print_default_remote, 0}, diff --git a/git-submodule.sh b/git-submodule.sh index 331d71c908..cba585f075 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -323,44 +323,7 @@ cmd_foreach() shift done - toplevel=$(pwd) - - # dup stdin so that it can be restored when running the external - # command in the subshell (and a recursive call to this function) - exec 3<&0 - - { - git submodule--helper list --prefix "$wt_prefix" || - echo "#unmatched" $? - } | - while read -r mode sha1 stage sm_path - do - die_if_unmatched "$mode" "$sha1" - if test -e "$sm_path"/.git - then - displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix") - say "$(eval_gettext "Entering '\$displaypath'")" - name=$(git submodule--helper name "$sm_path") - ( - prefix="$prefix$sm_path/" - sanitize_submodule_env - cd "$sm_path" && - # we make $path available to scripts ... - path=$sm_path && - if test $# -eq 1 - then - eval "$1" - else - "$@" - fi && - if test -n "$recursive" - then - cmd_foreach "--recursive" "$@" - fi - ) <&3 3<&- || - die "$(eval_gettext "Stopping at '\$displaypath'; script returned non-zero status.")" - fi - done + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@" } #