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

Force backslashes to slashes for embedded paths #35

Merged
merged 1 commit into from Dec 30, 2022

Conversation

skeeto
Copy link
Contributor

@skeeto skeeto commented Dec 29, 2022

On Windows the paths are joined using backslashes, but these backslashes are not properly escaped for a C string when printed, resulting in the wrong string in the include. I ran into this when building csprite natively on Windows using Mingw-w64 (via).

More generally, to successfully build and run (in place) on Windows — which I was happy to see working! — I also had to:

  1. Adjust subprocess.run(['clang++', ...] to 'g++'
  2. Add SDL2 -I/-L paths to CCFLAGS/LFLAGS (alternative: install SDL2 in the GCC sysroot like on other platforms)
  3. Adjust python3 to python since that's what it's named on Windows

For (1), this could be 'c++' to invoke the system's primary C++ compiler, whether that's clang++ or g++, just like cc for the C compiler. (Note: You can do the same with CXX in the Makefile.)

For (2) the Makefile could use sdl2-config, which is part of SDL2 and exists specifically to solve this problem. (Note the double $$.)

CCFLAGS:=... $$(sdl2-config --cflags)
LFLAGS+=... $$(sdl2-config --static-libs) ...   # static link
LFLAGS+=... $$(sdl2-config --libs) ...          # dynamic link

The sources also need to be updated to include SDL.h rather than SDL2/SDL.h which is the incorrect include anyway. (Maybe also get rid of -DSDL_MAIN_HANDLED=1 since it doesn't seem to serve a purpose in this case? I suspect it's only there to simplify linking.)

For (3) add a PYTHON variable a la CXX. This would also work better in virtual environments, e.g. make PYTHON=venv/bin/python.

On Windows the paths are joined using backslashes, but these backslashes
are not properly escaped for a C string when printed, resulting in the
wrong string in the include.
@pegvin pegvin added this to the release 1.0 milestone Dec 30, 2022
@pegvin pegvin added bug Something isn't working enhancement New feature or request labels Dec 30, 2022
@pegvin pegvin merged commit 16b7bf7 into csprite:master Dec 30, 2022
@pegvin
Copy link
Collaborator

pegvin commented Dec 30, 2022

Hey, thanks for the PR! i didn't really thought of this as my whole workflow is generally on linux.

sorry for the inconvenience with the build system, i will address all those issues by raising a new issue.

Happy New Year!

@pegvin
Copy link
Collaborator

pegvin commented Dec 30, 2022

also the makefile was designed according to msys2 environment on windows so it might or might not fail when building on any other environment, i will be adding msvc support & mingw soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants