diff mbox series

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

Message ID 05b4b69fee2b8c32769dd72dea182cfb72a14876.1601155970.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series CMake and Visual Studio | expand

Commit Message

Nicolas Guichard via GitGitGadget Sept. 26, 2020, 9:32 p.m. UTC 
From: Johannes Schindelin <johannes.schindelin@gmx.de>

By default, Git for Windows does not install its `sh.exe` into the
`PATH`. However, our current `CMakeLists.txt` expects to find a shell
interpreter in the `PATH`.

So let's fall back to looking in the default location where Git for
Windows _does_ install a relatively convenient `sh.exe`:
`C:\Program Files\Git\bin\sh.exe`

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 contrib/buildsystems/CMakeLists.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Øystein Walle Sept. 28, 2020, 11:17 a.m. UTC | #1 
>  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.

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.

For the curious I have attached the patch that ended up working, but I doubt
it's worth including (except the simplified if(NOT...) logic).

Øsse.

[1]: https://cmake.org/cmake/help/latest/command/find_program.html

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 5007f173f1..baa46e4e97 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -41,10 +41,17 @@ cmake_minimum_required(VERSION 3.14)
 #set the source directory to root of git
 set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 
-find_program(SH_EXE sh)
+find_program(SH_EXE sh
+    PATHS  "C:/Program Files/Git/bin"
+           "$ENV{VSINSTALLDIR}/Common7/IDE/CommonExtensions/Microsoft/TeamFoundation/Team Explorer/Git/usr/bin"
+ )
 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/")
+else()
+    # Make sure various utilities are available in PATH
+    get_filename_component(dir "${SH_EXE}" DIRECTORY)
+    set(ENV{PATH} "$ENV{PATH};${dir}")
 endif()
 
 #Create GIT-VERSION-FILE using GIT-VERSION-GEN
Johannes Schindelin Sept. 28, 2020, 7:39 p.m. UTC | #2 
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
diff mbox series

Patch

diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt
index 5007f173f1..d14fa4f3dc 100644
--- a/contrib/buildsystems/CMakeLists.txt
+++ b/contrib/buildsystems/CMakeLists.txt
@@ -43,8 +43,11 @@  set(CMAKE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/../..)
 
 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()
 
 #Create GIT-VERSION-FILE using GIT-VERSION-GEN