make: normalize backslashes in env-imported variable values#376
make: normalize backslashes in env-imported variable values#376Peter0x44 wants to merge 1 commit into
Conversation
On Windows, the VULKAN_SDK environment variable, as set by the Vulkan SDK qt ifw installer contain backslash-separated paths. When GNU Make expands these into recipe lines passed to a unixy shell, the backslashes make it over to the shell command, where they are treated as escape characters, mangling the paths. As a result, using `make -f llama.mak -j32` would fail to build, if invoked from powershell. Normalize backslashes to forward slashes in all variable values imported from the Windows environment at startup, when a "unixy" shell is being used. This is the same thing that BusyBox-w32 ash does in setwinxp() on shell startup.
|
I've found powershell to be quite useful for running agents, so I noticed this issue when I went to build llama.mak. I figured it would be sensible to do the same change busybox-w32 does to the environment variables expanded into recipes when SHELL is "unixy". But I'm a little unsure if it could have other consequences, so opened as a PR instead of pushing it. |
I appreciate it. I too am a little worried about edge cases a dumb conversion might break. The conversion in busybox-w32 breaks some things (which is why we need |
|
This shouldn't affect |
|
@skeeto worth merging now? To summarize, this will make no difference if you invoke make from within w64devkit.exe, as busybox-w32 already did the transformation in the environment variables. It will make no difference if you invoke |
|
A real use case, here in a contrived example: SHELL = cmd
DIR = .
example:
dir $(DIR)Where this currently works: But would no longer work after these changes, as I'd love a solution like the one in u-config where path variables are escaped for the receiving shell when interpolated. However, Make doesn't have the luxury of knowing which variables are paths and which are shell fragments that should be passed through unmodified, e.g. Perhaps we still merge this to try it out, but it might be reverted before the next release. |
|
Yeah, it ultimately is a hack, but it is one I find useful occasionally. For cmd, this is not a major issue, as \ isn't used to escape things (that's ^ or ` I forgot) and nobody really designs cli arguments around \ (since it's also a path separator, for which / works equally well in the vast majority of contexts) That's why I think it's a good idea. And since BusyBox is already doing this translation, doing it in make too would only be more consistent, not less. I could put it behind an env var or add a way to disable it if that would make it any better. I could also just fix my environment variables anyway. |
|
Alright, let's try it out for awhile. |
On Windows, the VULKAN_SDK environment variable, as set by the Vulkan SDK qt ifw installer contain backslash-separated paths. When GNU Make expands these into recipe lines passed to a unixy shell, the backslashes make it over to the shell command, where they are treated as escape characters, mangling the paths.
As a result, using
make -f llama.makwould fail to build, if invoked from powershell, on a system with the vulkan SDK installed.Normalize backslashes to forward slashes in all variable values imported from the Windows environment at startup, when a "unixy" shell is being used. This is the same thing that BusyBox-w32 ash does in setwinxp() on shell startup.