Re: [PATCH v3 4/4] submodule: port submodule subcommand 'summary' from shell to C
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Aug 21, 2020, 15:17 UTC
- Message-ID
- <nycvar.QRO.7.76.6.2008211708280.56@tvgsbejvaqbjf.bet>
- In-Reply-To
- <20200812194404.17028-5-shouryashukla.oo@gmail.com>
Hi Shourya,
On Thu, 13 Aug 2020, Shourya Shukla wrote:
Show 11 quoted lines
> [...] > diff --git a/t/t7421-submodule-summary-add.sh b/t/t7421-submodule-summary-add.sh > index 829fe26d6d..59a9b00467 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 && > - test_i18ngrep "fatal:.*my-subm" err && > + grep "fatal:.*my-subm" err &&
Sadly, this breaks on Windows: on Linux (and before this patch, also on Windows), the error message reads somewhat like this:
fatal: exec 'rev-parse': cd to 'my-subm' failed: No such file or directory
However, with the built-in `git submodule summary`, on Windows the error message reads like this:
error: cannot spawn git: No such file or directory
Now, this is of course not the best way to present this error message, but please note that even providing a better error message does not fix the erroneous expectation of the `fatal:` prefix (Git typically produces this when `die()`ing, which can be done in the POSIX version that uses `fork()` and `exec()` but not in the Windows version that needs to use `CreateProcessW()` instead).
Therefore, I propose this patch on top:
-- snipsnap -- [PATCH] mingw: mention if `mingw_spawnve()` failed due to a missing directory
When we recently converted the `summary` subcommand of `git submodule` to be mostly built-in, a bug was uncovered where a very unhelpful error message was produced when a process could not be spawned because the directory in which it was supposed to be run does not exist.
Even so, we _still_ have to adjust the `git submodule summary` test, to accommodate for the fact that the `mingw_spawnve()` function will return with an error instead of `die()`ing.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- compat/mingw.c | 4 ++++ t/t7421-submodule-summary-add.sh | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/compat/mingw.c b/compat/mingw.c index 1a64d4efb26b..3c30d0cab589 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1850,6 +1850,10 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen /* Make sure to override previous errors, if any */ errno = 0; + if (dir && !is_directory(dir)) + return error_errno(_("could not exec '%s' in '%s'"), + argv[0], dir); + if (restrict_handle_inheritance < 0) restrict_handle_inheritance = core_restrict_inherited_handles; /* diff --git a/t/t7421-submodule-summary-add.sh b/t/t7421-submodule-summary-add.sh index 59a9b00467dc..f00d69ca29ea 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 && + grep "my-subm" err && cat >expected <<-EOF && * my-subm ${rev}...0000000: -- 2.28.0.windows.1