Re: [PATCH 3/3] t7421: eliminate 'grep' check in t7421.4 for mingw compatibility

[Shourya Shukla <shouryashukla.oo@gmail.com>]

On 25/08 08:03, Kaartic Sivaraam wrote:
> On 25-08-2020 17:00, Shourya Shukla wrote:
> > 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,dst}_abbrev' by guarding the
> 
> The change only affects `src_abbrev`. So, it's misleading to mention
> `dst_abbrev` here.

I forgot to change that. Thank you for pointing this out.

> > '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.
> > 
> > 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 <Johannes.Schindelin@gmx.de>
> > Helped-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> > Mentored-by: Christian Couder <chriscool@tuxfamily.org>
> > Mentored-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> > Signed-off-by: Shourya Shukla <shouryashukla.oo@gmail.com>
> > ---
> >  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 93d0700891..f1951680f7 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 = NULL;
> 
> Unlike `src_abbrev`, I don't think we need to initilialize `dst_abbrev`
> to NULL here as it would be assigned in all code paths.

Alright. Changed!