gmake2: Fix detecting msdos vs posix shell #2039
Conversation
|
Related discussions I had investigating this issue: I have yet to test this on windows directly again, but I am 99% sure this is the correct solution addressing it. |
The current method of searching for ".exe" in the ComSpec environment variable is not reliable, because no shell on Windows I could find updates this. In theory, even if shells did, this still wouldn't work because `sh.exe` is a valid name for shell running build rules. I could not find any posix type shell that actually changes the ComSpec environment variable on Windows. All of these yielded the same result: powershell: $env:ComSpec busybox-w32: echo $COMSPEC msys2: echo $COMSPEC cygwin: echo $COMSPEC returning: C:\Windows\system32\cmd.exe Not even powershell touches this environment variable at all! The only reason this ever appeared to work in the first place, is that most posix type shells on Windows capitalize environment variables, and GNU Make does case-sensitive comparisons of environment variables, even on Windows. What it effectively was doing is detecting the shell type of the shell that was invoking make, not actually what was executing the build rules. The shell executing the build rules and the shell make is invoked from don't have to be the same. In fact, on Windows, GNU Make searches the PATH for an `sh.exe` to run build rules, and prioritizes using it over cmd.exe if it is present (see variable.c for how this is implemented). In an environment like this, invoking `make clean` from cmd.exe will fail, due to trying to execute cmd commands on a posix type shell, which cannot handle them. This can be proven to be the case, because `make SHELL=cmd` works correctly, by forcing GNU Make to invoke cmd.exe instead of its preferred sh.exe As far as I can tell, there is no standard environment variable that indicates whether an msdos or posix type shell is being used to execute the Makefile. The solution I implemented is to use a "polyglot test" executed in the shell, that can differentiate between an msdos and type posix shell in the actual Makefile itself. I used the approach of running `echo "test"` in the shell. A posix type shell will write test without quotes, while an msdos type shell will write "test" with quotes.
Peter0x44
force-pushed
the
fix-detect-shelltype
branch
from
6cc6451 to
564438e
Compare
|
Force pushed to flip the condition of the if because it gives a nicer diff |
|
I can confirm I tested it and it worked how I expected, regardless of the shell make was using, and the shell make was invoked from. |
|
Is there a chance I can get this reviewed soon? I tried my best to explain the change, but if more clarification is needed, I'm happy to provide it. |
|
Hi, I was hoping that someone with more expertise in this area would review your change 😅 EDIT: I'll do it after the weekend. Sorry! |
These are not required anymore, now that premake is updated. Fixed in: premake/premake-core#2039
The current method of searching for ".exe" in the ComSpec environment variable is not reliable, because no shell on Windows I could find updates this. In theory, even if shells did, this still wouldn't work because
sh.exeis a valid name for shell running build rules.I could not find any posix type shell that actually changes the ComSpec environment variable on Windows.
All of these yielded the same result:
powershell: $env:ComSpec
busybox-w32: echo $COMSPEC
msys2: echo $COMSPEC
cygwin: echo $COMSPEC
returning: C:\Windows\system32\cmd.exe
Not even powershell touches this environment variable at all!
The only reason this ever appeared to work in the first place, is that most posix type shells on Windows capitalize environment variables, and GNU Make does case-sensitive comparisons of environment variables, even on Windows. What it effectively was doing is detecting the shell type of the shell that was invoking make, not actually what was executing the build rules.
The shell executing the build rules and the shell make is invoked from don't have to be the same. In fact, on Windows, GNU Make searches the PATH for an
sh.exeto run build rules, and prioritizes using it over cmd.exe if it is present (see variable.c for how this is implemented). In an environment like this, invokingmake cleanfrom cmd.exe will fail, due to trying to execute cmd commands on a posix type shell, which cannot handle them. This can be proven to be the case, becausemake SHELL=cmdworks correctly, by forcing GNU Make to invoke cmd.exe instead of its preferred sh.exeAs far as I can tell, there is no standard environment variable that indicates whether an msdos or posix type shell is being used to execute the Makefile. The solution I implemented is to use a "polyglot test" executed in the shell, that can differentiate between an msdos and type posix shell in the actual Makefile itself.
I used the approach of running
echo "test"in the shell. A posix type shell will write test without quotes, while an msdos type shell will write "test" with quotes.TL:DR:
What does this PR do?
This PR fixes the code which detects whether an "msdos" or "posix" type shell is being used to execute the Makefile
How does this PR change Premake's behavior?
This PR changes the way generated Makefiles try to detect what shell they are running under
Are there any breaking changes? Will any existing behavior change?
This shouldn't be a breaking change to existing users.
Anything else we should know?
To reproduce the issue this patch addresses, follow these steps:
Download w64devkit from:
https://github.com/skeeto/w64devkit/releases/latest
In my last testing I used w64devkit-v1.18.0.zip, though I noticed this issue over a year ago.
Extract it somewhere, and add the
bindirectory contained to your PATH. This is the directory containing gcc, make & friends.Download premake5.exe and put it on your PATH also.
Open cmd.exe, and then generate any project with
premake5 gmake2Build the project using
makeand then try runningmake cleanObserve an error something like the following:
run
make clean SHELL=cmdand observe that it worked correctly.P.S. Sorry for this enormous wall of text, but this issue is rather complex and confused me greatly.
It took quite a while for me to understand the real cause and how everything was interacting, so I want to document it here in as much detail as I can so it confuses no one else.