skeeto / w64devkit

Portable C and C++ Development Kit for x64 (and x86) Windows
The Unlicense
3k stars 211 forks source link

Add ability to choose shell for running build rules #17

Closed Peter0x44 closed 2 years ago

Peter0x44 commented 2 years ago

Certain Makefiles, like the one raylib has use the cmd.exe builtin command del to delete files for targets like make clean. That means these targets fail while running under w64devkit's make and mingw32-make

make clean
del *.o /s
sh: del: not found
make: *** [Makefile:734: clean] Error 127

https://github.com/raysan5/raylib/blob/1e436be51d4f8853c3494a0753eabe7628ac6d90/src/Makefile#L733 Is there any way that you would suggest to deal with this from the perspective of the Makefile? Because I can't really think of a solution to the problem as-is. Perhaps mingw32-make should run build rules under cmd.exe, as is the behavior for most other distributions of mingw-w64, while make should run build rules under the busybox shell, for a user expecting to use unix commands in his Makefile.

skeeto commented 2 years ago

Short answer: Set the SHELL macro to "cmd" either in the Makefile itself or in the command.

make SHELL=cmd

This is the standard way to choose a different shell when the default shell is inappropriate. I understand that few do it (most Makefiles are poorly written), but any Makefile that uses cmd.exe features like del should be doing this in the first place. Otherwise they're relying on implicit defaults that may not match expectations. Fortunately that command line override is an easy way to deal with it (though it's explicitly ignored by the "-e" flag).

GNU Make prefers a unix shell (see GNU Make's README.W32), so unless a specific shell is selected via SHELL, on Windows it searches the PATH for a "sh.exe" and uses it if found (see variable.c). As far as I know this is how "mingw32-make" has also always worked, and Makefiles expecting cmd.exe from it is simply because, when called by that name, it's typically used outside of the presence of a sh.exe.

I spent time investigating this when adding w64devkit command aliases, where I had observed make's behavior change once sh.exe existed on the path. The one hidden inside BusyBox was invisible to GNU Make — and also unusable, due being a command with an argument ("busybox sh"). It also ignores sh.bat, which was another reason I couldn't rely on batch scripts for command aliases. For a time, I even considered patching Make to use sh.bat.

Related: GNU Make has a crude, built-in pseudo-shell, and uses it to improve efficiency (avoids a shell spawn) when the SHELL is /bin/sh and the command is simple enough. I don't think "sh.exe" counts, but it's something to be aware of. See commit f9194ec and this busybox-w32 patch: https://lists.sr.ht/~skeeto/public-inbox/patches/20756

Peter0x44 commented 2 years ago

Thanks for the clarification! I really understand things clearly now, and the broken assumptions the Makefile was using.

Peter0x44 commented 2 years ago

To follow up to this, I ran in to this problem in premake generated makefiles

I have a test Makefile here:

.PHONY: default
default:
        $(info SHELL is $(SHELL))
        del

running this under make with NO sh.exe present in the PATH reports this:

SHELL is sh.exe
del
The syntax of the command is incorrect.
make: *** [Makefile:4: default] Error 1

With sh.exe present in PATH, all that changed was that the full path to sh.exe (from w64devkit) was present, and del failed with the CreateProcess error as expected.

Now, because it doesn't fail with the CreateProcess error, this must be running under cmd.exe, so checking the value of SHELL is not a method you can use to determine which shell it is running under

Do you have any other ideas how I could do this reliably within a Makefile?

The current way premake does this is with this code:

SHELLTYPE := posix
ifeq (.exe,$(findstring .exe,$(ComSpec)))
        SHELLTYPE := msdos
endi

This does not work properly at all, I could not find any shell that changed the value of ComSpec Powershell did not, nor did msys2/cygwin, and nor did busybox-w32 I determined running make SHELLTYPE=posix also worked fine, with the Makefile running under sh.exe, so all that needs to be changed is making this detection work properly.

Peter0x44 commented 2 years ago

Actually, I am now observing something even weirder that I can't explain

SHELLTYPE := posix
ifeq (.exe,$(findstring .exe,$(ComSpec)))
        SHELLTYPE := msdos
endif

default:
        $(info SHELLTYPE is $(SHELLTYPE))

If I invoke make from cmd.exe: SHELLTYPE is msdos If I invoke make from sh.exe: SHELLTYPE is posix

Both are reading the same Makefile, and indeed both are executing using sh.exe Inserting del fails on both with the CreateProcess error.

Do you have any idea how the shell that invoked make has an effect on this?