Re: [PATCH 4/4] submodule: port submodule subcommand 'foreach' from shell to C
Stefan Bellerwrites: > +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 = _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, _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(_array); > + > + /* > + * For the purpose of executing in the submodule, > + * separate shell is used for the purpose of running the > + * child process. > + */ Micronit: this multi-line comment is indented in a funny way. > + 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. > + */ Same micronit. The scripted version does 'eval "$1"', so $1 could be something like for-each 'echo "$name:$sm_path:$displaypath:$sha1:$toplevel"' and it can see any variable, not just these 5 (i.e. we could have fed e.g. $wt_prefix and $mode to the above 'echo' and with the scripted version the script would have learned their values; with this version it no longer does, but only these 5 are part of the documented API, so we choose not to consider it a regression). > + if (info->argc == 1) { > + char *toplevel = xgetcwd(); > + struct strbuf sb = STRBUF_INIT; > + > + argv_array_pushf(_array, "name=%s", sub->name); > + argv_array_pushf(_array, "sm_path=%s", path); > + argv_array_pushf(_array, "displaypath=%s", displaypath); > + argv_array_pushf(_array, "sha1=%s", > + oid_to_hex(ce_oid)); > + argv_array_pushf(_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(, path); > + argv_array_pushf(, "path=%s; %s", > + sb.buf, info->argv[0]); OK, so we do the equivalent of name=... sm_path=... displaypath=... sha1=... toplevel=... \ sh -c 'path=...; echo "$name:$sm_path:..."' when doing for-each 'echo "$name:$sm_path:..."' with parts denoted with ... correctly quoted as necessary. I guess it would be the best we could do. I myself do not know if it is true that bash ported to Windows won't get confused with the above "we use path (all lowercase) only as a pure shell variable without exporting it ourselves"; I'd trust those who are more familiar with the platform to raise objections and suggest a better alternative if it is not the case. Thanks for the (malformatted;-) leading comment to highlight why the 'path' variable alone is treated differently from the others. > + strbuf_release(); > + free(toplevel); > + } else { > + argv_array_pushv(, info->argv); > + } > + > + if (!info->quiet) > + printf(_("Entering '%s'\n"), displaypath); > + > + if (info->argv[0] && run_command()) > + 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(_array); > + > + argv_array_pushl(, "--super-prefix", NULL); > + argv_array_pushf(, "%s/", displaypath); > + argv_array_pushl(, "submodule--helper", "foreach", > "--recursive", > + NULL); > + > + if (info->quiet) > + argv_array_push(, "--quiet"); > + > + argv_array_pushv(, info->argv); > + > + if (run_command()) > + die(_("run_command returned non-zero status while"
[PATCH 4/4] submodule: port submodule subcommand 'foreach' from shell to C
From: Prathamesh ChavanThis 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 --- 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 c2403a915ff..4b0b773c06b 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 = _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, _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(_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(_array, "name=%s", sub->name); + argv_array_pushf(_array, "sm_path=%s", path); + argv_array_pushf(_array, "displaypath=%s", displaypath); + argv_array_pushf(_array, "sha1=%s", + oid_to_hex(ce_oid)); + argv_array_pushf(_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(, path); + argv_array_pushf(, "path=%s; %s", +sb.buf, info->argv[0]); + strbuf_release(); + free(toplevel); + } else { + argv_array_pushv(, info->argv); + } + + if (!info->quiet) + printf(_("Entering '%s'\n"), displaypath); + + if (info->argv[0] && run_command()) + 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(_array); + + argv_array_pushl(, "--super-prefix", NULL); + argv_array_pushf(, "%s/", displaypath); + argv_array_pushl(, "submodule--helper", "foreach", "--recursive", + NULL); + + if (info->quiet) + argv_array_push(, "--quiet"); + + argv_array_pushv(, info->argv); + + if (run_command()) + 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