Re: [PATCH v3 4/4] submodule: port submodule subcommand 'summary' from shell to C

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Shourya,

On Thu, 13 Aug 2020, Shourya Shukla wrote:

> [...]
> 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