From e0f7ae564eecd5e43704c9640ea68ff93a8f316e Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Tue, 25 Aug 2020 17:00:18 +0530 Subject: [PATCH 1/3] submodule: eliminate unused parameters from print_submodule_summary() Eliminate the parameters 'missing_{src,dst}' from the 'print_submodule_summary()' function call since they are not used anywhere in the function. Reported-by: Jeff King Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 63ea39025d037a..b83f840251503d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -982,7 +982,6 @@ static char* verify_submodule_committish(const char *sm_path, static void print_submodule_summary(struct summary_cb *info, char* errmsg, int total_commits, const char *displaypath, const char *src_abbrev, const char *dst_abbrev, - int missing_src, int missing_dst, struct module_cb *p) { if (p->status == 'T') { @@ -1154,8 +1153,7 @@ static void generate_submodule_summary(struct summary_cb *info, print_submodule_summary(info, errmsg, total_commits, displaypath, src_abbrev, - dst_abbrev, missing_src, - missing_dst, p); + dst_abbrev, p); free(displaypath); free(src_abbrev); From f0c6b6467d60d9c8442ebf4a4b4bd53ebc9bcb4e Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Tue, 25 Aug 2020 17:00:19 +0530 Subject: [PATCH 2/3] submodule: fix style in function definition The definitions of 'verify_submodule_committish()' and 'print_submodule_summary()' had wrong styling in terms of the asterisk placement. Amend them. Also, the warning printed in case of an unexpected file mode printed the mode in decimal. Print it in octal for enhanced readability. Reported-by: Kaartic Sivaraam Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Helped-by: Junio C Hamano Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b83f840251503d..93d0700891993e 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -959,7 +959,7 @@ enum diff_cmd { DIFF_FILES }; -static char* verify_submodule_committish(const char *sm_path, +static char *verify_submodule_committish(const char *sm_path, const char *committish) { struct child_process cp_rev_parse = CHILD_PROCESS_INIT; @@ -979,7 +979,7 @@ static char* verify_submodule_committish(const char *sm_path, return strbuf_detach(&result, NULL); } -static void print_submodule_summary(struct summary_cb *info, char* errmsg, +static void print_submodule_summary(struct summary_cb *info, char *errmsg, int total_commits, const char *displaypath, const char *src_abbrev, const char *dst_abbrev, struct module_cb *p) @@ -1056,7 +1056,7 @@ static void generate_submodule_summary(struct summary_cb *info, } else { /* for a submodule removal (mode:0000000), don't warn */ if (p->mod_dst) - warning(_("unexpected mode %d\n"), p->mod_dst); + warning(_("unexpected mode %o\n"), p->mod_dst); } } From d79b14556922e5b7139268e92f2dface8d174278 Mon Sep 17 00:00:00 2001 From: Shourya Shukla Date: Thu, 27 Aug 2020 23:15:01 +0530 Subject: [PATCH 3/3] t7421: eliminate 'grep' check in t7421.4 for mingw compatibility The 'grep' check in test 4 of t7421 resulted in the failure of t7421 on Windows due to a different error message error: cannot spawn git: No such file or directory instead of fatal: exec 'rev-parse': cd to 'my-subm' failed: No such file or directory Tighten up the check to compute 'src_abbrev' by guarding the 'verify_submodule_committish()' call using `p->status !='D'`, so that the former isn't called in case of non-existent submodule directory, consequently, there is no such error message on any execution environment. The same need not be implemented for 'dst_abbrev' and is rather redundant since the conditional 'if (S_ISGITLINK(p->mod_dst))' already guards the 'verify_submodule_committish()' when we have a status of 'D'. Therefore, eliminate the 'grep' check in t7421. Instead, verify the absence of an error message by doing a 'test_must_be_empty' on the file containing the error. Reported-by: Johannes Schindelin Helped-by: Kaartic Sivaraam Mentored-by: Christian Couder Mentored-by: Kaartic Sivaraam Signed-off-by: Shourya Shukla Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 7 ++++--- t/t7421-submodule-summary-add.sh | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 93d0700891993e..1db1176e48dd9d 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1035,7 +1035,7 @@ static void print_submodule_summary(struct summary_cb *info, char *errmsg, static void generate_submodule_summary(struct summary_cb *info, struct module_cb *p) { - char *displaypath, *src_abbrev, *dst_abbrev; + char *displaypath, *src_abbrev = NULL, *dst_abbrev; int missing_src = 0, missing_dst = 0; char *errmsg = NULL; int total_commits = -1; @@ -1061,8 +1061,9 @@ static void generate_submodule_summary(struct summary_cb *info, } if (S_ISGITLINK(p->mod_src)) { - src_abbrev = verify_submodule_committish(p->sm_path, - oid_to_hex(&p->oid_src)); + if (p->status != 'D') + src_abbrev = verify_submodule_committish(p->sm_path, + oid_to_hex(&p->oid_src)); if (!src_abbrev) { missing_src = 1; /* diff --git a/t/t7421-submodule-summary-add.sh b/t/t7421-submodule-summary-add.sh index 59a9b00467dcb0..b070f13714a7ee 100755 --- a/t/t7421-submodule-summary-add.sh +++ b/t/t7421-submodule-summary-add.sh @@ -58,7 +58,7 @@ test_expect_success 'submodule summary output for submodules with changed paths' git commit -m "change submodule path" && rev=$(git -C sm rev-parse --short HEAD^) && git submodule summary HEAD^^ -- my-subm >actual 2>err && - grep "fatal:.*my-subm" err && + test_must_be_empty err && cat >expected <<-EOF && * my-subm ${rev}...0000000: