* [GSoC][PATCH v1 1/2] submodule: port set_name_rev from shell to C @ 2017-05-21 12:27 Prathamesh Chavan 2017-05-21 12:27 ` [GSoC][PATCH v1 2/2] submodule: port submodule subcommand status Prathamesh Chavan 0 siblings, 1 reply; 9+ messages in thread From: Prathamesh Chavan @ 2017-05-21 12:27 UTC (permalink / raw) To: git; +Cc: sbeller, christian.couder, peff, Prathamesh Chavan Since later on we want to port submodule subcommand status, and since set_name_rev is part of cmd_status, hence this function is ported. It has been ported to function set_name_rev in C, which calls get_name_rev to get the revname, and after formatting it, set_name_prints it. And hence in this way, the command `git submodule--helper set-name-rev "sm_path" "sha1"` sets value of revname in git-submodule.sh The function get_name_rev returns the stdout of the git describe commands. Since there are four different git-describe commands used for generating the name rev, four child_process are introduced, each successive child process running only when previous has no stdout. The order of these four git-describe commands is maintained the same as it was in the function set_name_rev() before porting. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Stefan Beller <sbeller@google.com> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- This series of patch is based on gitster/jk/bug-to-abort for untilizing its BUG() macro. Since submodule subcommand used set_name_rev function, first this function was ported before porting the subcommand to C. Complete build report for this patch is available at: https://travis-ci.org/pratham-pc/git/builds/ Branch: status Build #64 Also, I have updated my Github and pushed this work. It can be seen at: https://github.com/pratham-pc/git/commits/status builtin/submodule--helper.c | 65 +++++++++++++++++++++++++++++++++++++++++++++ git-submodule.sh | 16 ++--------- 2 files changed, 67 insertions(+), 14 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 566a5b6a6..5f0ddd8ad 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -219,6 +219,70 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr return 0; } +enum describe_step { + step_bare = 0, + step_tags, + step_contains, + step_all_always, + step_end +}; + +static char *get_name_rev(int argc, const char **argv, const char *prefix) +{ + struct child_process cp; + struct strbuf sb = STRBUF_INIT; + enum describe_step cur_step; + + for (cur_step = step_bare; cur_step < step_end; cur_step++) { + child_process_init(&cp); + prepare_submodule_repo_env(&cp.env_array); + cp.dir = argv[1]; + cp.no_stderr = 1; + + switch (cur_step) { + case step_bare: + argv_array_pushl(&cp.args, "git", "describe", argv[2], NULL); + break; + case step_tags: + argv_array_pushl(&cp.args, "git", "describe", "--tags", + argv[2], NULL); + break; + case step_contains: + argv_array_pushl(&cp.args, "git", "describe", + "--contains", argv[2], NULL); + break; + case step_all_always: + argv_array_pushl(&cp.args, "git", "describe", "--all", + "--always", argv[2], NULL); + break; + default: + BUG("unknown describe step '%d'", cur_step); + } + + if (!capture_command(&cp, &sb, 0) && sb.len) { + strbuf_strip_suffix(&sb, "\n"); + return strbuf_detach(&sb, NULL); + } + } + + strbuf_release(&sb); + return NULL; +} + + +static int set_name_rev(int argc, const char **argv, const char *prefix) +{ + char *namerev; + if (argc != 3) + die("set-name-rev only accepts two arguments: <path> <sha1>"); + + namerev = get_name_rev(argc, argv, prefix); + if (namerev[0]) + printf(" (%s)\n", namerev); + + return 0; +} + struct module_list { const struct cache_entry **entries; int alloc, nr; @@ -1212,6 +1276,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}, + {"set-name-rev", set_name_rev, 0}, {"init", module_init, SUPPORT_SUPER_PREFIX}, {"remote-branch", resolve_remote_submodule_branch, 0}, {"push-check", push_check, 0}, diff --git a/git-submodule.sh b/git-submodule.sh index c0d0e9a4c..b6eb5bcce 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -758,18 +758,6 @@ cmd_update() } } -set_name_rev () { - revname=$( ( - sanitize_submodule_env - cd "$1" && { - git describe "$2" 2>/dev/null || - git describe --tags "$2" 2>/dev/null || - git describe --contains "$2" 2>/dev/null || - git describe --all --always "$2" - } - ) ) - test -z "$revname" || revname=" ($revname)" -} # # Show commit summary for submodules in index or working tree # @@ -1041,14 +1029,14 @@ cmd_status() fi if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path" then - set_name_rev "$sm_path" "$sha1" + revname=$(git submodule--helper set-name-rev "$sm_path" "$sha1") say " $sha1 $displaypath$revname" else if test -z "$cached" then sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD) fi - set_name_rev "$sm_path" "$sha1" + revname=$(git submodule--helper set-name-rev "$sm_path" "$sha1") say "+$sha1 $displaypath$revname" fi -- 2.11.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [GSoC][PATCH v1 2/2] submodule: port submodule subcommand status 2017-05-21 12:27 [GSoC][PATCH v1 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan @ 2017-05-21 12:27 ` Prathamesh Chavan 2017-05-22 21:28 ` Stefan Beller 0 siblings, 1 reply; 9+ messages in thread From: Prathamesh Chavan @ 2017-05-21 12:27 UTC (permalink / raw) To: git; +Cc: sbeller, christian.couder, peff, Prathamesh Chavan This aims to make git-submodule status a builtin. 'status' is ported to submodule--helper, and submodule--helper is called from git-submodule.sh. For the purpose of porting cmd_status, the code is split up such that one function obtains all the list of submodules, acting as the front-end of git-submodule status. This function later calls the second function for_each_submodule_list,it which basically loops through the list of submodules and calls function fn, which in this case is status_submodule. The third function, status submodule returns the status of submodule and also takes care of the recursive flag. The first function module_status parses the options present in argv, and then with the help of module_list_compute, generates the list of submodules present in the current working tree. The second function for_each_submodule_list traverses through the list, and calls function fn (which in the case of submodule subcommand foreach is status_submodule) is called for each entry. The third function status_foreach checks for the various conditions, and prints the status of the submodule accordingly. Also, this function takes care of the recursive flag by creating a separate child_process and running it inside the submodule. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Stefan Beller <sbeller@google.com> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- A new function, get_submodule_displaypath is also introduced for getting the displaypath of the submodule while taking care of its prefix and superprefix. builtin/submodule--helper.c | 162 ++++++++++++++++++++++++++++++++++++++++++++ git-submodule.sh | 48 +------------ 2 files changed, 163 insertions(+), 47 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 5f0ddd8ad..7c040a375 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -13,6 +13,8 @@ #include "refs.h" #include "connect.h" +typedef void (*submodule_list_func_t)(const struct cache_entry *list_item, void *cb_data); + static char *get_default_remote(void) { char *dest = NULL, *ret; @@ -219,6 +221,23 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr return 0; } +static char *get_submodule_displaypath(const char *path, const char *prefix) +{ + const char *super_prefix = get_super_prefix(); + + if (prefix && super_prefix) { + BUG("cannot have prefix '%s' and superprefix '%s'", + prefix, super_prefix); + } else if (prefix) { + struct strbuf sb = STRBUF_INIT; + return xstrdup(relative_path(path, prefix, &sb)); + } else if (super_prefix) { + return xstrfmt("%s/%s", super_prefix, path); + } else { + return xstrdup(path); + } +} + enum describe_step { step_bare = 0, step_tags, @@ -395,6 +414,13 @@ static int module_list(int argc, const char **argv, const char *prefix) return 0; } +static void for_each_submodule_list(const struct module_list list, submodule_list_func_t fn, void *cb_data) +{ + int i; + for (i = 0; i < list.nr; i++) + fn(list.entries[i], cb_data); +} + static void init_submodule(const char *path, const char *prefix, int quiet) { const struct submodule *sub; @@ -532,6 +558,141 @@ static int module_init(int argc, const char **argv, const char *prefix) return 0; } +struct cb_status { + const char *prefix; + unsigned int quiet: 1; + unsigned int cached: 1; + unsigned int recursive: 1; +}; +#define CB_STATUS_INIT { NULL, 0, 0, 0 } + +static void status_submodule(const struct cache_entry *list_item, void *cb_data) +{ + struct cb_status *info = cb_data; + struct strbuf sub_sha1 = STRBUF_INIT; + char* displaypath = NULL; + struct argv_array diff_files_arg = ARGV_ARRAY_INIT; + + argv_array_pushl(&diff_files_arg, "diff-files", "--ignore-submodules=dirty", + "--quiet", "--", list_item->name, NULL); + + gitmodules_config(); + + displaypath = get_submodule_displaypath(list_item->name, info->prefix); + + if (!submodule_from_path(null_sha1, list_item->name)) + die(_("no submodule mapping found in .gitmodules for path '%s'"), list_item->name); + + strbuf_addstr(&sub_sha1, oid_to_hex(&list_item->oid)); + + if (list_item->ce_flags) { + printf(_("U%s %s\n"), sha1_to_hex(null_sha1), displaypath); + return; + } + + if (!is_submodule_initialized(list_item->name)) { + printf(_("-%s %s\n"), sub_sha1.buf, displaypath); + return; + } + + if (!cmd_diff_files(diff_files_arg.argc, diff_files_arg.argv , info->prefix)) { + struct argv_array name_rev_args = ARGV_ARRAY_INIT; + argv_array_pushl(&name_rev_args, "set-name-rev", list_item->name, + sub_sha1.buf, NULL); + printf(_(" %s %s"), sub_sha1.buf, displaypath); + set_name_rev(name_rev_args.argc, name_rev_args.argv, info->prefix); + } else { + struct argv_array name_rev_args = ARGV_ARRAY_INIT; + + if (info->cached == 0) { + struct child_process cp = CHILD_PROCESS_INIT; + struct strbuf sb = STRBUF_INIT; + + prepare_submodule_repo_env(&cp.env_array); + cp.dir = list_item->name; + + argv_array_pushl(&cp.args, "git", "rev-parse", "--verify", "HEAD", NULL); + + if (capture_command(&cp, &sb, 0)) + die(_("Could not run 'git rev-parse --verify HEAD' in submodule %s"), list_item->name); + + strbuf_strip_suffix(&sb, "\n"); + argv_array_pushl(&name_rev_args, "set-name-rev", list_item->name, + sb.buf, NULL); + + printf(_("+%s %s"), sb.buf, displaypath); + set_name_rev(name_rev_args.argc, name_rev_args.argv, info->prefix); + + strbuf_release(&sb); + } else { + + argv_array_pushl(&name_rev_args, "set-name-rev", list_item->name, + sub_sha1.buf, NULL); + + printf(_("+%s %s"), sub_sha1.buf, displaypath); + set_name_rev(name_rev_args.argc, name_rev_args.argv, info->prefix); + } + } + + if (info->recursive) { + struct child_process cpr = CHILD_PROCESS_INIT; + + cpr.git_cmd = 1; + cpr.dir = list_item->name; + prepare_submodule_repo_env(&cpr.env_array); + + argv_array_pushl(&cpr.args, "--super-prefix", displaypath, + "submodule--helper", "status", "--recursive", NULL); + + if (info->cached) + argv_array_push(&cpr.args, "--cached"); + + if (info->quiet) + argv_array_push(&cpr.args, "--quiet"); + + if (run_command(&cpr)) + die(_("Failed to recurse into submodule path %s"), list_item->name); + } + +} + +static int module_status(int argc, const char **argv, const char *prefix) +{ + struct cb_status info = CB_STATUS_INIT; + struct pathspec pathspec; + struct module_list list = MODULE_LIST_INIT; + int quiet = 0; + int cached = 0; + int recursive = 0; + + struct option module_status_options[] = { + OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")), + OPT_BOOL(0, "cached", &cached, N_("Use commit stored in the index instead of the one stored in the submodule HEAD")), + OPT_BOOL(0, "recursive", &recursive, N_("Recurse into nested submodules")), + OPT_END(), + }; + + const char *const git_submodule_helper_usage[] = { + N_("git submodule status [--quiet] [--cached] [--recursive] [<path>]"), + NULL + }; + + argc = parse_options(argc, argv, prefix, module_status_options, + git_submodule_helper_usage, 0); + + if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) + return 1; + + info.prefix = prefix; + info.quiet = !!quiet; + info.cached = !!cached; + info.recursive = !!recursive; + + for_each_submodule_list(list, status_submodule, &info); + + return 0; +} + static int module_name(int argc, const char **argv, const char *prefix) { const struct submodule *sub; @@ -1278,6 +1439,7 @@ static struct cmd_struct commands[] = { {"resolve-relative-url-test", resolve_relative_url_test, 0}, {"set-name-rev", set_name_rev, 0}, {"init", module_init, SUPPORT_SUPER_PREFIX}, + {"status", module_status, SUPPORT_SUPER_PREFIX}, {"remote-branch", resolve_remote_submodule_branch, 0}, {"push-check", push_check, 0}, {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX}, diff --git a/git-submodule.sh b/git-submodule.sh index b6eb5bcce..a142f9c04 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1004,54 +1004,8 @@ cmd_status() shift done - { - git submodule--helper list --prefix "$wt_prefix" "$@" || - echo "#unmatched" $? - } | - while read -r mode sha1 stage sm_path - do - die_if_unmatched "$mode" "$sha1" - name=$(git submodule--helper name "$sm_path") || exit - displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix") - if test "$stage" = U - then - say "U$sha1 $displaypath" - continue - fi - if ! git submodule--helper is-active "$sm_path" || - { - ! test -d "$sm_path"/.git && - ! test -f "$sm_path"/.git - } - then - say "-$sha1 $displaypath" - continue; - fi - if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path" - then - revname=$(git submodule--helper set-name-rev "$sm_path" "$sha1") - say " $sha1 $displaypath$revname" - else - if test -z "$cached" - then - sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD) - fi - revname=$(git submodule--helper set-name-rev "$sm_path" "$sha1") - say "+$sha1 $displaypath$revname" - fi + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@" - if test -n "$recursive" - then - ( - prefix="$displaypath/" - sanitize_submodule_env - wt_prefix= - cd "$sm_path" && - eval cmd_status - ) || - die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")" - fi - done } # # Sync remote urls for submodules -- 2.11.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [GSoC][PATCH v1 2/2] submodule: port submodule subcommand status 2017-05-21 12:27 ` [GSoC][PATCH v1 2/2] submodule: port submodule subcommand status Prathamesh Chavan @ 2017-05-22 21:28 ` Stefan Beller 2017-06-05 20:25 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan 0 siblings, 1 reply; 9+ messages in thread From: Stefan Beller @ 2017-05-22 21:28 UTC (permalink / raw) To: Prathamesh Chavan; +Cc: git, Christian Couder, Jeff King On Sun, May 21, 2017 at 5:27 AM, Prathamesh Chavan <pc44800@gmail.com> wrote: > This aims to make git-submodule status a builtin. 'status' is ported > to submodule--helper, and submodule--helper is called from > git-submodule.sh. > > For the purpose of porting cmd_status, the code is split up such that > one function obtains all the list of submodules, acting as the > front-end of git-submodule status. This function later calls the > second function for_each_submodule_list,it which basically loops > through the list of submodules and calls function fn, which in this > case is status_submodule. The third function, status submodule returns > the status of submodule and also takes care of the recursive flag. > > The first function module_status parses the options present in argv, > and then with the help of module_list_compute, generates the list of > submodules present in the current working tree. > > The second function for_each_submodule_list traverses through the list, > and calls function fn (which in the case of submodule subcommand > foreach is status_submodule) is called for each entry. > > The third function status_foreach checks for the various conditions, > and prints the status of the submodule accordingly. Also, this > function takes care of the recursive flag by creating a separate > child_process and running it inside the submodule. > > Mentored-by: Christian Couder <christian.couder@gmail.com> > Mentored-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> > --- > A new function, get_submodule_displaypath is also introduced for getting > the displaypath of the submodule while taking care of its prefix and > superprefix. > > builtin/submodule--helper.c | 162 ++++++++++++++++++++++++++++++++++++++++++++ > git-submodule.sh | 48 +------------ > 2 files changed, 163 insertions(+), 47 deletions(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 5f0ddd8ad..7c040a375 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -13,6 +13,8 @@ > #include "refs.h" > #include "connect.h" > > +typedef void (*submodule_list_func_t)(const struct cache_entry *list_item, void *cb_data); > + > static char *get_default_remote(void) > { > char *dest = NULL, *ret; > @@ -219,6 +221,23 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr > return 0; > } > > +static char *get_submodule_displaypath(const char *path, const char *prefix) > +{ > + const char *super_prefix = get_super_prefix(); > + > + if (prefix && super_prefix) { > + BUG("cannot have prefix '%s' and superprefix '%s'", > + prefix, super_prefix); > + } else if (prefix) { > + struct strbuf sb = STRBUF_INIT; > + return xstrdup(relative_path(path, prefix, &sb)); > + } else if (super_prefix) { > + return xstrfmt("%s/%s", super_prefix, path); > + } else { > + return xstrdup(path); > + } > +} > + > enum describe_step { > step_bare = 0, > step_tags, > @@ -395,6 +414,13 @@ static int module_list(int argc, const char **argv, const char *prefix) > return 0; > } > > +static void for_each_submodule_list(const struct module_list list, submodule_list_func_t fn, void *cb_data) > +{ > + int i; > + for (i = 0; i < list.nr; i++) > + fn(list.entries[i], cb_data); > +} Up to here it looks like the patch in https://public-inbox.org/git/20170521125814.26255-2-pc44800@gmail.com/ (without the nit of having an extra void return) Maybe it is worth it to combine the two patch series, such that we'd need to review the common parts only once? > + > static void init_submodule(const char *path, const char *prefix, int quiet) > { > const struct submodule *sub; > @@ -532,6 +558,141 @@ static int module_init(int argc, const char **argv, const char *prefix) > return 0; > } > > +struct cb_status { > + const char *prefix; > + unsigned int quiet: 1; > + unsigned int cached: 1; > + unsigned int recursive: 1; > +}; > +#define CB_STATUS_INIT { NULL, 0, 0, 0 } > + > + if (run_command(&cpr)) > + die(_("Failed to recurse into submodule path %s"), list_item->name); I thought this is a badly worded error message, but it turns out it is just as in the shell code, which is good for a direct translation. Maybe we can adapt the error message in a later follow up to be more aligned to other submodule error messages. (dropping "path" and putting single quotes around %s, also un-capitalize the first letter) > +static int module_status(int argc, const char **argv, const char *prefix) > +{ > + struct cb_status info = CB_STATUS_INIT; > + struct pathspec pathspec; > + struct module_list list = MODULE_LIST_INIT; > + int quiet = 0; > + int cached = 0; > + int recursive = 0; > + > + struct option module_status_options[] = { > + OPT__QUIET(&quiet, N_("Suppress output for initializing a submodule")), > + OPT_BOOL(0, "cached", &cached, N_("Use commit stored in the index instead of the one stored in the submodule HEAD")), > + OPT_BOOL(0, "recursive", &recursive, N_("Recurse into nested submodules")), > + OPT_END(), > + }; > + > + const char *const git_submodule_helper_usage[] = { > + N_("git submodule status [--quiet] [--cached] [--recursive] [<path>]"), > + NULL > + }; > + > + argc = parse_options(argc, argv, prefix, module_status_options, > + git_submodule_helper_usage, 0); > + > + if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) > + return 1; > + > + info.prefix = prefix; > + info.quiet = !!quiet; > + info.cached = !!cached; > + info.recursive = !!recursive; > + > + for_each_submodule_list(list, status_submodule, &info); > + > + return 0; > +} This function looks good. Though my gut reaction was to suggest to add another layer of abstraction. Then I checked wt-status.c, but that makes use of "submodule summary" and not "submodule status". So all is good. > + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@" I'd think we would not need to pass down superprefix here as we do not call "submodule status" in a recursive way. The recursion works on the submodule helper itself, so we could simplify it to just git ${wt_prefix:+-C "$wt_prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@" Another idea that I just had: Maybe we could drop --cached, --recursive as well, as they are just command line options, which could be just part of "$@". For --quiet this is a bit more complicated as it may come in via an environment variable (which we could also check for in C in theory. I know I omitted that when writing some submodule--helper code a couple months ago, but the reason escaped me) Thanks, Stefan ^ permalink raw reply [flat|nested] 9+ messages in thread
* [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C 2017-05-22 21:28 ` Stefan Beller @ 2017-06-05 20:25 ` Prathamesh Chavan 2017-06-05 20:25 ` [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status Prathamesh Chavan ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Prathamesh Chavan @ 2017-06-05 20:25 UTC (permalink / raw) To: sbeller; +Cc: git, christian.couder, Prathamesh Chavan Since later on we want to port submodule subcommand status, and since set_name_rev is part of cmd_status, hence this function is ported. It has been ported to function print_name_rev in C, which calls get_name_rev to get the revname, and after formatting it, print_name_rev prints it. And hence in this way, the command `git submodule--helper print-name-rev "sm_path" "sha1"` sets value of revname in git-submodule.sh The function get_name_rev returns the stdout of the git describe commands. Since there are four different git-describe commands used for generating the name rev, four child_process are introduced, each successive child process running only when previous has no stdout. The order of these four git-describe commands is maintained the same as it was in the function set_name_rev() in shell script. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Stefan Beller <sbeller@google.com> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- builtin/submodule--helper.c | 67 +++++++++++++++++++++++++++++++++++++++++++++ git-submodule.sh | 16 ++--------- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 566a5b6a6..3022118d1 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -219,6 +219,72 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr return 0; } +enum describe_step { + step_bare = 0, + step_tags, + step_contains, + step_all_always, + step_end +}; + +static char *get_name_rev(int argc, const char **argv, const char *prefix) +{ + struct child_process cp; + struct strbuf sb = STRBUF_INIT; + enum describe_step cur_step; + + for (cur_step = step_bare; cur_step < step_end; cur_step++) { + child_process_init(&cp); + prepare_submodule_repo_env(&cp.env_array); + cp.dir = argv[1]; + cp.no_stderr = 1; + + switch (cur_step) { + case step_bare: + argv_array_pushl(&cp.args, "git", "describe", + argv[2], NULL); + break; + case step_tags: + argv_array_pushl(&cp.args, "git", "describe", + "--tags", argv[2], NULL); + break; + case step_contains: + argv_array_pushl(&cp.args, "git", "describe", + "--contains", argv[2], NULL); + break; + case step_all_always: + argv_array_pushl(&cp.args, "git", "describe", + "--all", "--always", argv[2], + NULL); + break; + default: + BUG("unknown describe step '%d'", cur_step); + } + + if (!capture_command(&cp, &sb, 0) && sb.len) { + strbuf_strip_suffix(&sb, "\n"); + return strbuf_detach(&sb, NULL); + } + } + + strbuf_release(&sb); + return NULL; +} + +static int print_name_rev(int argc, const char **argv, const char *prefix) +{ + char *namerev; + if (argc != 3) + die("print-name-rev only accepts two arguments: <path> <sha1>"); + + namerev = get_name_rev(argc, argv, prefix); + if (namerev && namerev[0]) + printf(" (%s)", namerev); + printf("\n"); + + return 0; +} + struct module_list { const struct cache_entry **entries; int alloc, nr; @@ -1212,6 +1278,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}, + {"print-name-rev", print_name_rev, 0}, {"init", module_init, SUPPORT_SUPER_PREFIX}, {"remote-branch", resolve_remote_submodule_branch, 0}, {"push-check", push_check, 0}, diff --git a/git-submodule.sh b/git-submodule.sh index c0d0e9a4c..091051891 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -758,18 +758,6 @@ cmd_update() } } -set_name_rev () { - revname=$( ( - sanitize_submodule_env - cd "$1" && { - git describe "$2" 2>/dev/null || - git describe --tags "$2" 2>/dev/null || - git describe --contains "$2" 2>/dev/null || - git describe --all --always "$2" - } - ) ) - test -z "$revname" || revname=" ($revname)" -} # # Show commit summary for submodules in index or working tree # @@ -1041,14 +1029,14 @@ cmd_status() fi if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path" then - set_name_rev "$sm_path" "$sha1" + revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1") say " $sha1 $displaypath$revname" else if test -z "$cached" then sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD) fi - set_name_rev "$sm_path" "$sha1" + revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1") say "+$sha1 $displaypath$revname" fi -- 2.13.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status 2017-06-05 20:25 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan @ 2017-06-05 20:25 ` Prathamesh Chavan 2017-06-05 23:12 ` Stefan Beller 2017-06-06 4:24 ` Christian Couder 2017-06-05 22:50 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Stefan Beller 2017-06-05 23:20 ` Brandon Williams 2 siblings, 2 replies; 9+ messages in thread From: Prathamesh Chavan @ 2017-06-05 20:25 UTC (permalink / raw) To: sbeller; +Cc: git, christian.couder, Prathamesh Chavan This aims to make git-submodule subcommand status a builtin. Here 'status' is ported to submodule--helper, and submodule--helper is called from git-submodule.sh. For the purpose of porting cmd_status, the code is split up such that one function obtains all the list of submodules, acting as the front-end of git-submodule status. This function later calls the second function for_each_submodule_list,it which basically loops through the list of submodules and calls function fn, which in this case is status_submodule. The third function, status submodule returns the status of submodule and also takes care of the recursive flag. The first function module_status parses the options present in argv, and then with the help of module_list_compute, generates the list of submodules present in the current working tree. The second function for_each_submodule_list traverses through the list, and calls function fn (which in the case of submodule subcommand status is status_submodule) is called for each entry. The third function status_submodule checks for the various conditions, and prints the status of the submodule accordingly. Also, this function takes care of the recursive flag by creating a separate child_process and running it inside the submodule. The function print_status handles the printing of submodule's status. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Stefan Beller <sbeller@google.com> Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> --- In this new version of patch, function print_status is introduced. The functions for_each_submodule_list and get_submodule_displaypath are found to be the same as those in the ported submodule subcommand foreach's patches. The reason for doing so is to keep both the patches independant and on separate branches. Also this patch is build on the branch gitster/jk/bug-to-abort for utilizing its BUG() macro. Complete build report is available at: https://travis-ci.org/pratham-pc/git/builds/ Branch: submodule-status-new Build #91 builtin/submodule--helper.c | 181 ++++++++++++++++++++++++++++++++++++++++++++ git-submodule.sh | 49 +----------- 2 files changed, 182 insertions(+), 48 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 3022118d1..85da05550 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -13,6 +13,9 @@ #include "refs.h" #include "connect.h" +typedef void (*submodule_list_func_t)(const struct cache_entry *list_item, + void *cb_data); + static char *get_default_remote(void) { char *dest = NULL, *ret; @@ -219,6 +222,25 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr return 0; } +static char *get_submodule_displaypath(const char *path, const char *prefix) +{ + const char *super_prefix = get_super_prefix(); + + if (prefix && super_prefix) { + BUG("cannot have prefix '%s' and superprefix '%s'", + prefix, super_prefix); + } else if (prefix) { + struct strbuf sb = STRBUF_INIT; + char *displaypath = xstrdup(relative_path(path, prefix, &sb)); + strbuf_release(&sb); + return displaypath; + } else if (super_prefix) { + return xstrfmt("%s/%s", super_prefix, path); + } else { + return xstrdup(path); + } +} + enum describe_step { step_bare = 0, step_tags, @@ -397,6 +419,13 @@ static int module_list(int argc, const char **argv, const char *prefix) return 0; } +static void for_each_submodule_list(const struct module_list list, submodule_list_func_t fn, void *cb_data) +{ + int i; + for (i = 0; i < list.nr; i++) + fn(list.entries[i], cb_data); +} + static void init_submodule(const char *path, const char *prefix, int quiet) { const struct submodule *sub; @@ -534,6 +563,157 @@ static int module_init(int argc, const char **argv, const char *prefix) return 0; } +struct status_cb { + const char *prefix; + unsigned int quiet: 1; + unsigned int recursive: 1; + unsigned int cached: 1; +}; +#define STATUS_CB_INIT { NULL, 0, 0, 0 } + +static void print_status(struct status_cb *info, char state, const char *path, + char *sub_sha1, char *displaypath) +{ + if (info->quiet) + return; + + printf("%c%s %s", state, sub_sha1, displaypath); + + if (state == ' ' || state == '+') { + struct argv_array name_rev_args = ARGV_ARRAY_INIT; + + argv_array_pushl(&name_rev_args, "print-name-rev", + path, sub_sha1, NULL); + print_name_rev(name_rev_args.argc, name_rev_args.argv, + info->prefix); + } else { + printf("\n"); + } +} + +static void status_submodule(const struct cache_entry *list_item, void *cb_data) +{ + struct status_cb *info = cb_data; + char *sub_sha1 = xstrdup(oid_to_hex(&list_item->oid)); + char *displaypath; + struct argv_array diff_files_args = ARGV_ARRAY_INIT; + + if (!submodule_from_path(null_sha1, list_item->name)) + die(_("no submodule mapping found in .gitmodules for path '%s'"), + list_item->name); + + displaypath = get_submodule_displaypath(list_item->name, info->prefix); + + if (list_item->ce_flags) { + print_status(info, 'U', list_item->name, + sha1_to_hex(null_sha1), displaypath); + goto cleanup; + } + + if (!is_submodule_initialized(list_item->name)) { + print_status(info, '-', list_item->name, sub_sha1, displaypath); + goto cleanup; + } + + argv_array_pushl(&diff_files_args, "diff-files", + "--ignore-submodules=dirty", "--quiet", "--", + list_item->name, NULL); + + if (!cmd_diff_files(diff_files_args.argc, diff_files_args.argv, + info->prefix)) { + print_status(info, ' ', list_item->name, sub_sha1, displaypath); + } else { + if (!info->cached) { + struct child_process cp = CHILD_PROCESS_INIT; + struct strbuf sb = STRBUF_INIT; + + prepare_submodule_repo_env(&cp.env_array); + cp.git_cmd = 1; + cp.dir = list_item->name; + + argv_array_pushl(&cp.args, "rev-parse", + "--verify", "HEAD", NULL); + + if (capture_command(&cp, &sb, 0)) + die(_("could not run 'git rev-parse --verify" + "HEAD' in submodule %s"), + list_item->name); + + strbuf_strip_suffix(&sb, "\n"); + print_status(info, '+', list_item->name, sb.buf, + displaypath); + strbuf_release(&sb); + } else { + print_status(info, '+', list_item->name, sub_sha1, + displaypath); + } + } + + if (info->recursive) { + struct child_process cpr = CHILD_PROCESS_INIT; + + cpr.git_cmd = 1; + cpr.dir = list_item->name; + prepare_submodule_repo_env(&cpr.env_array); + + argv_array_pushl(&cpr.args, "--super-prefix", displaypath, + "submodule--helper", "status", "--recursive", + NULL); + + if (info->cached) + argv_array_push(&cpr.args, "--cached"); + + if (info->quiet) + argv_array_push(&cpr.args, "--quiet"); + + if (run_command(&cpr)) + die(_("failed to recurse into submodule '%s'"), + list_item->name); + } + +cleanup: + free(displaypath); + free(sub_sha1); +} + +static int module_status(int argc, const char **argv, const char *prefix) +{ + struct status_cb info = STATUS_CB_INIT; + struct pathspec pathspec; + struct module_list list = MODULE_LIST_INIT; + int quiet = 0; + int cached = 0; + int recursive = 0; + + struct option module_status_options[] = { + OPT__QUIET(&quiet, N_("Suppress submodule status output")), + OPT_BOOL(0, "cached", &cached, N_("Use commit stored in the index instead of the one stored in the submodule HEAD")), + OPT_BOOL(0, "recursive", &recursive, N_("Recurse into nested submodules")), + OPT_END() + }; + + const char *const git_submodule_helper_usage[] = { + N_("git submodule status [--quiet] [--cached] [--recursive] [<path>]"), + NULL + }; + + argc = parse_options(argc, argv, prefix, module_status_options, + git_submodule_helper_usage, 0); + + if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) + return 1; + + info.prefix = prefix; + info.quiet = !!quiet; + info.recursive = !!recursive; + info.cached = !!cached; + + gitmodules_config(); + for_each_submodule_list(list, status_submodule, &info); + + return 0; +} + static int module_name(int argc, const char **argv, const char *prefix) { const struct submodule *sub; @@ -1280,6 +1460,7 @@ static struct cmd_struct commands[] = { {"resolve-relative-url-test", resolve_relative_url_test, 0}, {"print-name-rev", print_name_rev, 0}, {"init", module_init, SUPPORT_SUPER_PREFIX}, + {"status", module_status, SUPPORT_SUPER_PREFIX}, {"remote-branch", resolve_remote_submodule_branch, 0}, {"push-check", push_check, 0}, {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX}, diff --git a/git-submodule.sh b/git-submodule.sh index 091051891..a24b1b91b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -1004,54 +1004,7 @@ cmd_status() shift done - { - git submodule--helper list --prefix "$wt_prefix" "$@" || - echo "#unmatched" $? - } | - while read -r mode sha1 stage sm_path - do - die_if_unmatched "$mode" "$sha1" - name=$(git submodule--helper name "$sm_path") || exit - displaypath=$(git submodule--helper relative-path "$prefix$sm_path" "$wt_prefix") - if test "$stage" = U - then - say "U$sha1 $displaypath" - continue - fi - if ! git submodule--helper is-active "$sm_path" || - { - ! test -d "$sm_path"/.git && - ! test -f "$sm_path"/.git - } - then - say "-$sha1 $displaypath" - continue; - fi - if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path" - then - revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1") - say " $sha1 $displaypath$revname" - else - if test -z "$cached" - then - sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD) - fi - revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1") - say "+$sha1 $displaypath$revname" - fi - - if test -n "$recursive" - then - ( - prefix="$displaypath/" - sanitize_submodule_env - wt_prefix= - cd "$sm_path" && - eval cmd_status - ) || - die "$(eval_gettext "Failed to recurse into submodule path '\$sm_path'")" - fi - done + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@" } # # Sync remote urls for submodules -- 2.13.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status 2017-06-05 20:25 ` [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status Prathamesh Chavan @ 2017-06-05 23:12 ` Stefan Beller 2017-06-06 4:24 ` Christian Couder 1 sibling, 0 replies; 9+ messages in thread From: Stefan Beller @ 2017-06-05 23:12 UTC (permalink / raw) To: Prathamesh Chavan; +Cc: git, Christian Couder On Mon, Jun 5, 2017 at 1:25 PM, Prathamesh Chavan <pc44800@gmail.com> wrote: > This aims to make git-submodule subcommand status a builtin. Here > 'status' is ported to submodule--helper, and submodule--helper is > called from git-submodule.sh. > > For the purpose of porting cmd_status, the code is split up such that > one function obtains all the list of submodules, acting as the front-end > of git-submodule status. This function later calls the second function > for_each_submodule_list,it which basically loops through the list of > submodules and calls function fn, which in this case is status_submodule. > The third function, status submodule returns the status of submodule and > also takes care of the recursive flag. > > The first function module_status parses the options present in argv, > and then with the help of module_list_compute, generates the list of > submodules present in the current working tree. > > The second function for_each_submodule_list traverses through the list, > and calls function fn (which in the case of submodule subcommand status > is status_submodule) is called for each entry. > > The third function status_submodule checks for the various conditions, > and prints the status of the submodule accordingly. Also, this function > takes care of the recursive flag by creating a separate child_process > and running it inside the submodule. The function print_status handles the > printing of submodule's status. > > Mentored-by: Christian Couder <christian.couder@gmail.com> > Mentored-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> > --- > In this new version of patch, function print_status is introduced. > > The functions for_each_submodule_list and get_submodule_displaypath > are found to be the same as those in the ported submodule subcommand > foreach's patches. The reason for doing so is to keep both the patches > independant and on separate branches. Maybe keep it even in a separate patch, such that the status series becomes: patch 1: introduce for_each_submodule_list and get_submodule_displaypath patch 2: port print_name_rev patch 3: port status whereas the foreach series (and other series later) could re-use patch 1, and build on top of it. For reviewing patches, it is fine to have the get_submodule_displaypath is both series, though for applying patches it for less complication/deduplication from the maintainer I would think. > + > +static void print_status(struct status_cb *info, char state, const char *path, > + char *sub_sha1, char *displaypath) > +{ > + if (info->quiet) > + return; > + > + printf("%c%s %s", state, sub_sha1, displaypath); > + > + if (state == ' ' || state == '+') { > + struct argv_array name_rev_args = ARGV_ARRAY_INIT; > + > + argv_array_pushl(&name_rev_args, "print-name-rev", > + path, sub_sha1, NULL); > + print_name_rev(name_rev_args.argc, name_rev_args.argv, > + info->prefix); ... with the suggestion given in the print_name_rev patch, this would become a one liner. :) The rest looks good to me :) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status 2017-06-05 20:25 ` [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status Prathamesh Chavan 2017-06-05 23:12 ` Stefan Beller @ 2017-06-06 4:24 ` Christian Couder 1 sibling, 0 replies; 9+ messages in thread From: Christian Couder @ 2017-06-06 4:24 UTC (permalink / raw) To: Prathamesh Chavan; +Cc: Stefan Beller, git On Mon, Jun 5, 2017 at 10:25 PM, Prathamesh Chavan <pc44800@gmail.com> wrote: > --- > In this new version of patch, function print_status is introduced. > > The functions for_each_submodule_list and get_submodule_displaypath > are found to be the same as those in the ported submodule subcommand > foreach's patches. The reason for doing so is to keep both the patches > independant and on separate branches. Also this patch is build on > the branch gitster/jk/bug-to-abort for utilizing its BUG() macro. The BUG() macro is now in master. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C 2017-06-05 20:25 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan 2017-06-05 20:25 ` [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status Prathamesh Chavan @ 2017-06-05 22:50 ` Stefan Beller 2017-06-05 23:20 ` Brandon Williams 2 siblings, 0 replies; 9+ messages in thread From: Stefan Beller @ 2017-06-05 22:50 UTC (permalink / raw) To: Prathamesh Chavan; +Cc: git, Christian Couder On Mon, Jun 5, 2017 at 1:25 PM, Prathamesh Chavan <pc44800@gmail.com> wrote: > Since later on we want to port submodule subcommand status, and since > set_name_rev is part of cmd_status, hence this function is ported. It > has been ported to function print_name_rev in C, which calls get_name_rev > to get the revname, and after formatting it, print_name_rev prints it. > And hence in this way, the command `git submodule--helper print-name-rev > "sm_path" "sha1"` sets value of revname in git-submodule.sh > > The function get_name_rev returns the stdout of the git describe > commands. Since there are four different git-describe commands used for > generating the name rev, four child_process are introduced, each successive > child process running only when previous has no stdout. The order of these > four git-describe commands is maintained the same as it was in the function > set_name_rev() in shell script. > > Mentored-by: Christian Couder <christian.couder@gmail.com> > Mentored-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> > --- > builtin/submodule--helper.c | 67 +++++++++++++++++++++++++++++++++++++++++++++ > git-submodule.sh | 16 ++--------- > 2 files changed, 69 insertions(+), 14 deletions(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 566a5b6a6..3022118d1 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -219,6 +219,72 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr > return 0; > } > > +enum describe_step { > + step_bare = 0, Do we rely on step_bare to be equal to 0? (This is the hint I am reading from '=0' here. If we do not, please omit.) > + step_tags, > + step_contains, > + step_all_always, > + step_end > +}; > + > +static char *get_name_rev(int argc, const char **argv, const char *prefix) So we split up the functionality into two functions. get_name_rev, which does the heavy lifting work, and print_name_rev, that is a wrapper around having to deal with going from shell to C and back. One of C strength' compared to shell is type safety, so maybe we can tighten the contract that get_name_rev offers to its callers and make it get_name_rev(const char *sub_path, const char *object_id / sha1) and then have print_name_rev call it via get_name_rev (argv[1], argv[2]) (which coincidentally is right after checking for argc != 3, which reinforces that the contract of the wrapper is "just making sure we have valid input" and this function "just does heavy lifting, assuming input is valid". > +{ > + struct child_process cp; > + struct strbuf sb = STRBUF_INIT; > + enum describe_step cur_step; > + > + for (cur_step = step_bare; cur_step < step_end; cur_step++) { > + child_process_init(&cp); (minor nit, personal opinion, feel free to ignore:) Alternatively, you could declare cp inside the loop assigned to CHILD_PROCESS_INIT. Same for strbuf sb as well, such that you only declare the iterator variable outside the loop. > + prepare_submodule_repo_env(&cp.env_array); > + cp.dir = argv[1]; > + cp.no_stderr = 1; cp.git_cmd = 1; as well? Thanks, Stefan ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C 2017-06-05 20:25 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan 2017-06-05 20:25 ` [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status Prathamesh Chavan 2017-06-05 22:50 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Stefan Beller @ 2017-06-05 23:20 ` Brandon Williams 2 siblings, 0 replies; 9+ messages in thread From: Brandon Williams @ 2017-06-05 23:20 UTC (permalink / raw) To: Prathamesh Chavan; +Cc: sbeller, git, christian.couder On 06/06, Prathamesh Chavan wrote: > Since later on we want to port submodule subcommand status, and since > set_name_rev is part of cmd_status, hence this function is ported. It > has been ported to function print_name_rev in C, which calls get_name_rev > to get the revname, and after formatting it, print_name_rev prints it. > And hence in this way, the command `git submodule--helper print-name-rev > "sm_path" "sha1"` sets value of revname in git-submodule.sh > > The function get_name_rev returns the stdout of the git describe > commands. Since there are four different git-describe commands used for > generating the name rev, four child_process are introduced, each successive > child process running only when previous has no stdout. The order of these > four git-describe commands is maintained the same as it was in the function > set_name_rev() in shell script. > > Mentored-by: Christian Couder <christian.couder@gmail.com> > Mentored-by: Stefan Beller <sbeller@google.com> > Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> > --- > builtin/submodule--helper.c | 67 +++++++++++++++++++++++++++++++++++++++++++++ > git-submodule.sh | 16 ++--------- > 2 files changed, 69 insertions(+), 14 deletions(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 566a5b6a6..3022118d1 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -219,6 +219,72 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr > return 0; > } > > +enum describe_step { > + step_bare = 0, > + step_tags, > + step_contains, > + step_all_always, > + step_end > +}; > + > +static char *get_name_rev(int argc, const char **argv, const char *prefix) > +{ > + struct child_process cp; > + struct strbuf sb = STRBUF_INIT; > + enum describe_step cur_step; > + > + for (cur_step = step_bare; cur_step < step_end; cur_step++) { > + child_process_init(&cp); > + prepare_submodule_repo_env(&cp.env_array); > + cp.dir = argv[1]; > + cp.no_stderr = 1; set cp.git = 1 so that you can avoid pushing "git" onto the arg array. > + > + switch (cur_step) { > + case step_bare: > + argv_array_pushl(&cp.args, "git", "describe", > + argv[2], NULL); > + break; > + case step_tags: > + argv_array_pushl(&cp.args, "git", "describe", > + "--tags", argv[2], NULL); > + break; > + case step_contains: > + argv_array_pushl(&cp.args, "git", "describe", > + "--contains", argv[2], NULL); > + break; > + case step_all_always: > + argv_array_pushl(&cp.args, "git", "describe", > + "--all", "--always", argv[2], > + NULL); > + break; > + default: > + BUG("unknown describe step '%d'", cur_step); > + } > + > + if (!capture_command(&cp, &sb, 0) && sb.len) { > + strbuf_strip_suffix(&sb, "\n"); > + return strbuf_detach(&sb, NULL); > + } > + } > + > + strbuf_release(&sb); > + return NULL; > +} > + > +static int print_name_rev(int argc, const char **argv, const char *prefix) > +{ > + char *namerev; > + if (argc != 3) > + die("print-name-rev only accepts two arguments: <path> <sha1>"); > + > + namerev = get_name_rev(argc, argv, prefix); > + if (namerev && namerev[0]) > + printf(" (%s)", namerev); > + printf("\n"); > + > + return 0; > +} > + > struct module_list { > const struct cache_entry **entries; > int alloc, nr; > @@ -1212,6 +1278,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}, > + {"print-name-rev", print_name_rev, 0}, > {"init", module_init, SUPPORT_SUPER_PREFIX}, > {"remote-branch", resolve_remote_submodule_branch, 0}, > {"push-check", push_check, 0}, > diff --git a/git-submodule.sh b/git-submodule.sh > index c0d0e9a4c..091051891 100755 > --- a/git-submodule.sh > +++ b/git-submodule.sh > @@ -758,18 +758,6 @@ cmd_update() > } > } > > -set_name_rev () { > - revname=$( ( > - sanitize_submodule_env > - cd "$1" && { > - git describe "$2" 2>/dev/null || > - git describe --tags "$2" 2>/dev/null || > - git describe --contains "$2" 2>/dev/null || > - git describe --all --always "$2" > - } > - ) ) > - test -z "$revname" || revname=" ($revname)" > -} > # > # Show commit summary for submodules in index or working tree > # > @@ -1041,14 +1029,14 @@ cmd_status() > fi > if git diff-files --ignore-submodules=dirty --quiet -- "$sm_path" > then > - set_name_rev "$sm_path" "$sha1" > + revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1") > say " $sha1 $displaypath$revname" > else > if test -z "$cached" > then > sha1=$(sanitize_submodule_env; cd "$sm_path" && git rev-parse --verify HEAD) > fi > - set_name_rev "$sm_path" "$sha1" > + revname=$(git submodule--helper print-name-rev "$sm_path" "$sha1") > say "+$sha1 $displaypath$revname" > fi > > -- > 2.13.0 > -- Brandon Williams ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-06-06 4:25 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-21 12:27 [GSoC][PATCH v1 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan 2017-05-21 12:27 ` [GSoC][PATCH v1 2/2] submodule: port submodule subcommand status Prathamesh Chavan 2017-05-22 21:28 ` Stefan Beller 2017-06-05 20:25 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Prathamesh Chavan 2017-06-05 20:25 ` [GSoC][PATCH v2 2/2] submodule: port submodule subcommand status Prathamesh Chavan 2017-06-05 23:12 ` Stefan Beller 2017-06-06 4:24 ` Christian Couder 2017-06-05 22:50 ` [GSoC][PATCH v2 1/2] submodule: port set_name_rev from shell to C Stefan Beller 2017-06-05 23:20 ` Brandon Williams
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.