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

Windows build failing due to the use of ls #73

Open
mniak opened this issue May 29, 2022 · 5 comments
Open

Windows build failing due to the use of ls #73

mniak opened this issue May 29, 2022 · 5 comments

Comments

@mniak
Copy link

mniak commented May 29, 2022

++++ info: using Makefile.pdlibbuilder version 0.6.0
process_begin: CreateProcess(NULL, ls C:/Users/andre/Projects/Pd/pure-data/src/m_pd.h, ...) failed.
pd-lib-builder/Makefile.pdlibbuilder:788: pipe: No such file or directory
process_begin: CreateProcess(NULL, ls "D:\ProgramFiles\Purr Data\bin/pd.dll", ...) failed.
pd-lib-builder/Makefile.pdlibbuilder:793: pipe: No such file or directory
pd-lib-builder/Makefile.pdlibbuilder:798: Where is Pd API m_pd.h? Do 'make help' for info.

Windows doesn't have the ls command by default.

When I tried to use powershell instead, it ran into other issues.
The easiest way is to use dir, since it works in both powershell and cmd.

I have a PR for that: #72

@umlaeute
Copy link
Contributor

i figure, pd-lib-builder targets MinGW/MSYS environments (on Windows), where MSYS should provide ls.

however, i wonder why we can't use a Make-builtin instead of using external programs, e.g. a simple $(wildcard "$(PDINCLUDEDIR)/m_pd.h") (on all platforms).

probably @katjav can share some thoughts on why it uses $(shell ls ...) instead

@katjav
Copy link
Contributor

katjav commented Dec 15, 2022

probably @katjav can share some thoughts on why it uses $(shell ls ...) instead

Thoughts are in the file itself but probably not at the appropriate place (starting at line 542):

# Standard paths on Windows contain spaces, and GNU make functions treat such
# paths as lists, with unintended effects. Therefore we must use shell function
# ls instead of make's wildcard when probing for a path, and use double quotes
# when specifying a path in a command argument.

Definitive expansion of paths as they will be used in commands is deferred to the section starting at line 787 and applicable for all platforms. The shell ls function is used there for platform-independent expansion. So maybe the explanation should go there.

@umlaeute
Copy link
Contributor

thanks for the clarification.

it's just amazing how things can go awry when you combine the GNU and redmont powers.

@katjav
Copy link
Contributor

katjav commented Dec 15, 2022

In addition to my previous comment: Makefile.pdilbbuilder explicitly depends on GNU make in order to be portable. GNU make documentation specifies which (POSIX compliant) tools can be used in a Makefile:

https://www.gnu.org/software/make/manual/make.html#Makefile-Conventions

But the same Makefile conventions also say:

Every Makefile should contain this line:

SHELL = /bin/sh

to avoid trouble on systems where the SHELL variable might be inherited from the environment.

Makefile.pdlibbuilder doesn't define SHELL and I forgot if that is for a good reason. Should it be introduced?

@rbeesley
Copy link

I've got a personal variant of Makefile.pdlibbuilder I'm working on which can build under WSL, which is an improvement in some ways to using MinGW. The issue with spaces in the path isn't such a big deal if you escape the spaces. For example, /mnt/c/Program\ Files. This is a subsection of make vars output:

variable PDBINDIR = /mnt/c/Program\ Files/Pd/bin
variable PDDIR = /mnt/c/Program\ Files/Pd
variable PDINCLUDEDIR = /mnt/c/Program\ Files/Pd/src
variable PDLIBBUILDER_DIR = pd-lib-builder/
variable PDLIBDIR = /mnt/c/AddData/Roaming/Pd

Defined in Makefile as:

PLATFORM=x86_64-w64-mingw32
PDDIR=/mnt/c/Program\ Files/Pd
PDINCLUDEDIR=/mnt/c/Program\ Files/Pd/src
PDBINDIR=/mnt/c/Program\ Files/Pd/bin
PDLIBDIR=/mnt/c/AddData/Roaming/Pd

I also added a cflags variable to point to the Windows SDK:

# enable to build under WSL
cflags += -I ~/dev/xwin/sdk/include

I'm using xwin to make the Windows SDK available in WSL, which just seemed easier than dealing with the installed SDK on the Windows side of things. Then to make this work with other commands, you need to remove the quotes as the escapement means that the argument is recognized as a single argument anyway.

I haven't shared a PR back yet as this seems to cause problems with MinGW and untested for Linux or MacOS.

I've also started looking at if I could install GNU Make and try to use a Visual Studio environment to build directly in Windows using pd-lib-builder, but that's something for another day.

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

4 participants