Greetings, This is the v3 of the patch series titled 'submodule: port subcommand 'summary' from shell to C': https://lore.kernel.org/git/20200806164102.6707-1-shouryashukla.oo@xxxxxxxxx/ After suggestions from Junio, Kaartic and Christian I made some changes: -> Drop the commit c063390a94 (submodule: expose the '--for-status' option of summary, 2020-08-02) since it felt a bit unnecessary to expose the option given its lack of major use cases. Hence, after feedback from Junio, Kaartic and Christian I dropped this patch for now. -> Adapt the patch to jk/strvec. It is a big change for Git hence it was necessary to make the port as per this patch. Feedback and reviews are appreciated. I am tagging along a range-diff between the v3 and v2 for ease of review. Regards, Shourya Shukla ----- range-diff: 1: c063390a94 < -: ---------- submodule: expose the '--for-status' option of summary 2: 561d03351b = 1: 2a66d8c2bb submodule: remove extra line feeds between callback struct and macro 3: e0e0c3ba4b = 2: 0ae3b92e31 submodule: rename helper functions to avoid ambiguity 4: ec6e6c9e64 ! 3: e2912a042f t7421: introduce a test script for verifying 'summary' output @@ Commit message do not work as expected. So, introduce a test script for verifying the 'summary' output for - submodules added using 'git submodule add'. + submodules added using 'git submodule add' and notify regarding the + above mentioned behaviour in t7401 itself. Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@xxxxxxxxx> Signed-off-by: Shourya Shukla <shouryashukla.oo@xxxxxxxxx> + ## t/t7401-submodule-summary.sh ## +@@ t/t7401-submodule-summary.sh: test_description='Summary support for submodules + + This test tries to verify the sanity of summary subcommand of git submodule. + ' ++# NOTE: This test script uses 'git add' instead of 'git submodule add' to add ++# submodules to the superproject. Some submodule subcommands such as init and ++# deinit might not work as expected in this script. t7421 does not have this ++# caveat. + + . ./test-lib.sh + + ## t/t7421-submodule-summary-add.sh (new) ## @@ +#!/bin/sh 5: 9222b4168b ! 4: 879cb902b2 submodule: port submodule subcommand 'summary' from shell to C @@ builtin/submodule--helper.c: static int module_name(int argc, const char **argv, + cp_rev_parse.git_cmd = 1; + cp_rev_parse.dir = sm_path; + prepare_submodule_repo_env(&cp_rev_parse.env_array); -+ argv_array_pushl(&cp_rev_parse.args, "rev-parse", "-q", -+ "--short", NULL); -+ argv_array_pushf(&cp_rev_parse.args, "%s^0", committish); -+ argv_array_push(&cp_rev_parse.args, "--"); ++ strvec_pushl(&cp_rev_parse.args, "rev-parse", "-q", "--short", NULL); ++ strvec_pushf(&cp_rev_parse.args, "%s^0", committish); ++ strvec_push(&cp_rev_parse.args, "--"); + + if (capture_command(&cp_rev_parse, &result, 0)) + return NULL; @@ builtin/submodule--helper.c: static int module_name(int argc, const char **argv, + cp_log.git_cmd = 1; + cp_log.dir = p->sm_path; + prepare_submodule_repo_env(&cp_log.env_array); -+ argv_array_pushl(&cp_log.args, "log", NULL); ++ strvec_pushl(&cp_log.args, "log", NULL); + + if (S_ISGITLINK(p->mod_src) && S_ISGITLINK(p->mod_dst)) { + if (info->summary_limit > 0) -+ argv_array_pushf(&cp_log.args, "-%d", -+ info->summary_limit); -+ -+ argv_array_pushl(&cp_log.args, "--pretty= %m %s", -+ "--first-parent", NULL); -+ argv_array_pushf(&cp_log.args, "%s...%s", -+ src_abbrev, -+ dst_abbrev); ++ strvec_pushf(&cp_log.args, "-%d", ++ info->summary_limit); ++ ++ strvec_pushl(&cp_log.args, "--pretty= %m %s", ++ "--first-parent", NULL); ++ strvec_pushf(&cp_log.args, "%s...%s", ++ src_abbrev, dst_abbrev); + } else if (S_ISGITLINK(p->mod_dst)) { -+ argv_array_pushl(&cp_log.args, "--pretty= > %s", -+ "-1", dst_abbrev, NULL); ++ strvec_pushl(&cp_log.args, "--pretty= > %s", ++ "-1", dst_abbrev, NULL); + } else { -+ argv_array_pushl(&cp_log.args, "--pretty= < %s", -+ "-1", src_abbrev, NULL); ++ strvec_pushl(&cp_log.args, "--pretty= < %s", ++ "-1", src_abbrev, NULL); + } + run_command(&cp_log); + } @@ builtin/submodule--helper.c: static int module_name(int argc, const char **argv, + struct child_process cp_rev_list = CHILD_PROCESS_INIT; + struct strbuf sb_rev_list = STRBUF_INIT; + -+ argv_array_pushl(&cp_rev_list.args, "rev-list", -+ "--first-parent", "--count", NULL); ++ strvec_pushl(&cp_rev_list.args, "rev-list", ++ "--first-parent", "--count", NULL); + if (S_ISGITLINK(p->mod_src) && S_ISGITLINK(p->mod_dst)) -+ argv_array_pushf(&cp_rev_list.args, "%s...%s", -+ src_abbrev, -+ dst_abbrev); ++ strvec_pushf(&cp_rev_list.args, "%s...%s", ++ src_abbrev, dst_abbrev); + else -+ argv_array_push(&cp_rev_list.args, -+ S_ISGITLINK(p->mod_src) ? -+ src_abbrev : -+ dst_abbrev); -+ argv_array_push(&cp_rev_list.args, "--"); ++ strvec_push(&cp_rev_list.args, S_ISGITLINK(p->mod_src) ? ++ src_abbrev : dst_abbrev); ++ strvec_push(&cp_rev_list.args, "--"); + + cp_rev_list.git_cmd = 1; + cp_rev_list.dir = p->sm_path; @@ builtin/submodule--helper.c: static int module_name(int argc, const char **argv, + struct summary_cb *info, + enum diff_cmd diff_cmd) +{ -+ struct argv_array diff_args = ARGV_ARRAY_INIT; ++ struct strvec diff_args = STRVEC_INIT; + struct rev_info rev; + struct module_cb_list list = MODULE_CB_LIST_INIT; + -+ argv_array_push(&diff_args, get_diff_cmd(diff_cmd)); ++ strvec_push(&diff_args, get_diff_cmd(diff_cmd)); + if (info->cached) -+ argv_array_push(&diff_args, "--cached"); -+ argv_array_pushl(&diff_args, "--ignore-submodules=dirty", "--raw", -+ NULL); ++ strvec_push(&diff_args, "--cached"); ++ strvec_pushl(&diff_args, "--ignore-submodules=dirty", "--raw", NULL); + if (head_oid) -+ argv_array_push(&diff_args, oid_to_hex(head_oid)); -+ argv_array_push(&diff_args, "--"); ++ strvec_push(&diff_args, oid_to_hex(head_oid)); ++ strvec_push(&diff_args, "--"); + if (info->argc) -+ argv_array_pushv(&diff_args, info->argv); ++ strvec_pushv(&diff_args, info->argv); + + git_config(git_diff_basic_config, NULL); + init_revisions(&rev, info->prefix); + rev.abbrev = 0; -+ precompose_argv(diff_args.argc, diff_args.argv); -+ -+ diff_args.argc = setup_revisions(diff_args.argc, diff_args.argv, -+ &rev, NULL); ++ precompose_argv(diff_args.nr, diff_args.v); ++ setup_revisions(diff_args.nr, diff_args.v, &rev, NULL); + rev.diffopt.output_format = DIFF_FORMAT_NO_OUTPUT | DIFF_FORMAT_CALLBACK; + rev.diffopt.format_callback = submodule_summary_callback; + rev.diffopt.format_callback_data = &list; @@ builtin/submodule--helper.c: static int module_name(int argc, const char **argv, + else + run_diff_files(&rev, 0); + prepare_submodule_summary(info, &list); ++ strvec_clear(&diff_args); + return 0; +} + ----- Prathamesh Chavan (1): submodule: port submodule subcommand 'summary' from shell to C Shourya Shukla (3): submodule: remove extra line feeds between callback struct and macro submodule: rename helper functions to avoid ambiguity t7421: introduce a test script for verifying 'summary' output builtin/submodule--helper.c | 432 ++++++++++++++++++++++++++++++- diff.c | 2 +- git-submodule.sh | 186 +------------ submodule.c | 10 +- submodule.h | 2 +- t/t7401-submodule-summary.sh | 4 + t/t7421-submodule-summary-add.sh | 69 +++++ 7 files changed, 510 insertions(+), 195 deletions(-) create mode 100755 t/t7421-submodule-summary-add.sh -- 2.28.0
