Re: [PATCH v2 02/10] cmake: do find Git for Windows' shell interpreter

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

[-- Attachment #1: Type: text/plain, Size: 3523 bytes --]

Hi Øystein,

On Mon, 28 Sep 2020, Øystein Walle wrote:

> >  find_program(SH_EXE sh)
> >  if(NOT SH_EXE)
> > -	message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
> > -			"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
> > +	set(SH_EXE "C:/Program Files/Git/bin/sh.exe")
> > +	if(NOT EXISTS ${SH_EXE})
> > +		message(FATAL_ERROR "sh: shell interpreter was not found in your path, please install one."
> > +				"On Windows, you can get it as part of 'Git for Windows' install at https://gitforwindows.org/")
> > +	endif()
> >  endif()
>
> You can write the find_program() command more succinctly as:
>
> 	find_program(SH_EXE sh PATHS "C:/Program Files/Git/bin")
>
> PATHS is is a list of extra directories to search, which are usually hard-coded
> guesses[1]. This way we avoid an extra check and indentation level.

Thank you, I was not aware of this neat feature.

> I found my Visual Studio installation already contains a sh.exe.  I think it
> ships with VS by default; I can't even find a way to remove it. It's located
> at:
>
> C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\Common7\IDE\CommonExtensions\Microsoft\TeamFoundation\Team Explorer\Git\usr\bin\sh.exe
>
> When I started writing this up I figured that could serve as an additional
> fallback. However, if I use that shell I have to add (...)/usr/bin to PATH as
> the various scripts need expr and sed among other things. I get the same result
> if I search "C:/Program Files/Git/usr/bin", but there is no equivalent
> (...)/bin in the Git included with VS for some reason.

Indeed. This is what I get in that case:

-- snip --
1> [CMake] Generating  GIT-VERSION-FILE
1> [CMake] C:/git-sdk-64/usr/src/vs2017-test/contrib/buildsystems/../../GIT-VERSION-GEN: line 24: sed: command not found
1> [CMake] C:/git-sdk-64/usr/src/vs2017-test/contrib/buildsystems/../../GIT-VERSION-GEN: line 29: expr: command not found
1> [CMake] GIT_VERSION =
1> [CMake] CMake Error at C:\git-sdk-64\usr\src\vs2017-test\contrib\buildsystems\CMakeLists.txt:79 (string):
1> [CMake]   string sub-command FIND requires 3 or 4 parameters.
1> [CMake]
1> [CMake]
1> [CMake] CMake Error at C:\git-sdk-64\usr\src\vs2017-test\contrib\buildsystems\CMakeLists.txt:83 (string):
1> [CMake]   string sub-command REGEX, mode MATCH needs at least 5 arguments total to
1> [CMake]   command.
1> [CMake]
1> [CMake]
1> [CMake] CMake Error at C:\git-sdk-64\usr\src\vs2017-test\contrib\buildsystems\CMakeLists.txt:87 (project):
1> [CMake]   VERSION ".0" format invalid.
-- snap --

The explanation is pretty simple: you cannot just call into `sh.exe` via
an absolute path and expect it to add its containing directory to the
`PATH`. It does not, and the symptom is that neither `sed` nor `expr` are
found.

One solution is to add it to the `PATH` manually, which is the original
expectation in our `CMakeLists.txt` version.

Another solution is to point it to `C:\Program Files\Git\bin\sh.exe` which
is not, in fact, a shell, but a small wrapper executable whose job it is
to set up a couple environment variables (`PATH` being one of them) and
then spawning the _actual_ `sh.exe`. The source code for that wrapper:
https://github.com/git-for-windows/MINGW-packages/blob/main/mingw-w64-git/git-wrapper.c

As you figured out, it is _not_ enough to use `...\usr\bin\sh.exe`
directly without adjusting the `PATH`.

Ciao,
Dscho