Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unexpected/Incorrect Path Conversion when Exporting Environment Variables #152

Open
cariad-robert-abel opened this issue Apr 19, 2023 · 4 comments

Comments

@cariad-robert-abel
Copy link

I recently updated my msys and noticed that some Makefile recipes started failing. Basically, whenever more than one path-like string is exported, a weird conversion kicks in that tries to split each single path as if it were a list of paths and then replaces the first forward slash /.

As far as I understand, make exports variables through the environment for subsequent invocations of tools using the export command, so this isn't the usual path mangling when using command-line arguments or at least I assume that make does not invoke a second executable and passes the exported data via command-line arguments to achieve this.

I reduced this down to the following minimal example:

ex
│   Makefile
└───dir
        Makefile

The Makefile contents are simple:

##########################################################################
# Makefile

.PHONY: foo
foo: export SINGLE += $(abspath file1)
foo: export DOUBLE += $(abspath file2) $(abspath file3)
foo:
	@echo "SHELL = $(SHELL)"
	@echo "SINGLE = $(SINGLE)"
	@echo "DOUBLE = $(DOUBLE)"
	+@cd dir && $(MAKE) bar && cd ..

##########################################################################
# dir/Makefile

.PHONY: bar
bar:
	@echo "SHELL = $(SHELL)"
	@echo "SINGLE = $(SINGLE)"
	@echo "DOUBLE = $(DOUBLE)"

The output with msys-2.0.dll version 3004.3.0.0 used to be:

$ mingw32-make foo
SHELL = C:/msys2/usr/bin/sh.exe
SINGLE = C:/tmp/ex/file1
DOUBLE = C:/tmp/ex/file2 C:/tmp/ex/file3
mingw32-make[1]: Entering directory 'C:/tmp/ex/dir'
SHELL = C:/msys2/usr/bin/sh.exe
SINGLE = C:/tmp/ex/file1
DOUBLE = C:/tmp/ex/file2 C:/tmp/ex/file3
mingw32-make[1]: Leaving directory 'C:/tmp/ex/dir'

However with msys-2.0.dll version 3004.6.0.0 the output is:

$ mingw32-make foo
SHELL = C:/msys2/usr/bin/sh.exe
SINGLE = C:/tmp/ex/file1
DOUBLE = C:/tmp/ex/file2 C:/tmp/ex/file3
mingw32-make[1]: Entering directory 'C:/tmp/ex/dir'
SHELL = C:/msys2/usr/bin/sh.exe
SINGLE = C:/tmp/ex/file1
DOUBLE = C;C:\msys2\ex\file2 C;C:\msys2\ex\file3        # <-- error on this line
mingw32-make[1]: Leaving directory 'C:/tmp/ex/dir'

Basically, the each path is treated as if the first colon : was actually a separator within a list of paths, then the first forward slash / is replaced by the msys2 base directory and then the paths are concatenated again using the windows separator for lists of paths, semicolon ;.

I strongly suspect this is related to #150, because as one can see, it does not happen with just a single path.
However, curiously this error does not occur when the exported list itself starts with a space, so using foo: export DOUBLE := $(DOUBLE) $(abspath file2) $(abspath file3) (DOUBLE is initially empty) does not trigger this behavior. This leads me to believe this might be a slightly different case.

I also noticed that the behavior does not occur when using SHELL = cmd, so it's definitely the invocation of sh.exe in line 3 of the foo recipe that's causing the issue. Unfortunately, MSYS2_ARG_CONV_EXCL does not have any effect on this.

I apologize for the somewhat lengthy issue ;)

@dscho
Copy link
Collaborator

dscho commented Apr 19, 2023

I think you're correct, and this is due to the change where spaces are now considered to be perfectly fine in file/directory names.

I'm not quite sure how we can address your scenario without re-breaking the fix where, say, /c/Program Files was considered not to be a path.

Could you elaborate a bit why the path list in the Makefile has to be in mixed format (i.e. DOS drive but no backslashes) and with a space instead of the usual semicolon as separator?

@cariad-robert-abel
Copy link
Author

Could you elaborate a bit why the path list in the Makefile has to be in mixed format (i.e. DOS drive but no backslashes) and with a space instead of the usual semicolon as separator?

The reason I use paths with forward slashes— C:/Project/src instead of C:\Project\src—is that

  1. that's the style returned by $(abspath <relative-path>), and
  2. sh.exe is in the path and make uses that to invoke recipes, so all backslashes would have to be escaped, which is a nightmare especially when appending to already-escaped variables' contents, and
  3. I call native and msys/unixy programs alike and now the translation/escaping of paths would have to be program-specific, which is another headache. Hence the use of mingw32-make instead of regular make.

Why is this list not separated by semi-colons? Because it's actually my INCLUDES variable that is then processed using $(INCLUDES:%=-I%) in the sub-Makefile. It's a regular list of strings separated by whitespace, which is very common in Makefiles. The paths are actually also quoted ("C:/Project/Name/src"), but that does not make a difference to this bug and I left it out for brevity.

I'm not quite sure how we can address your scenario without re-breaking the fix where, say, /c/Program Files was considered not to be a path.

I'm not sure I follow, because wouldn't it have to be /c/Program\ Files anyway in a unixy list of paths, e.g. /c/Program\ Files/Git/bin:/c/Program\ Files/GnuPG/bin? In any case, my use case is really a very common use case when using hierarchical Makefiles, so this will break a lot of use cases, I'm afraid.

I'd be happy if there was a (possibly separate) environment variable that disables this behavior, seeing as even MSYS2_ARG_CONV_EXCL has no effect.

@jeremyd2019
Copy link
Member

I'd be happy if there was a (possibly separate) environment variable that disables this behavior, seeing as even MSYS2_ARG_CONV_EXCL has no effect.

I think MSYS2_ENV_CONV_EXCL is what you are looking for here: https://www.msys2.org/docs/filesystem-paths/#environment-variables

That doesn't negate this issue though, this does seem like a common case to me, at least for a user of mingw32-make.

@cariad-robert-abel
Copy link
Author

I think MSYS2_ENV_CONV_EXCL is what you are looking for

Wow, I wasn't even aware that msys2 did that. Thanks for pointing this out :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants