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

Support cross-compiling with MinGW #72

Merged
merged 1 commit into from Mar 3, 2024
Merged

Conversation

nthirtyone
Copy link
Contributor

Fixes: #71

Toolchains and everything around them is primarily there to support CI. If you don't want that, changes in src/system/displays/displays_windows.cpp and CMakeLists.txt:148,171ff are enough to support MinGW.

@@ -0,0 +1,14 @@
set(TOOLCHAIN_PREFIX i686-w64-mingw32)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can reduce duplication by making one of the toolchains a symlink to another and then guess TOOLCHAIN_PREFIX based on CMAKE_CURRENT_LIST_FILE. That was my first solution, but I recalled that without Developer Mode enabled Windows users could encounter problems with the symlinks.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the other "toolchain" file looks, to me, like system- and build-specific config, with no business being shipped; I think move these to .github (.github/workflows?), and maybe merge them (looks like they're identical except for TOOLCHAIN_PREFIX and CMAKE_SYSTEM_PROCESSOR, which maybe can go into -D?) or inline them as -D wholesale.

CMakeLists.txt Outdated Show resolved Hide resolved
tools/wine.in Outdated Show resolved Hide resolved
@nabijaczleweli
Copy link
Collaborator

Wait, is pci_generator being compiled for the target and then run via wine? Does CMake really not support compiling build-step generators for the host instead?

@nthirtyone
Copy link
Contributor Author

nthirtyone commented Feb 24, 2024

Yep, it does not. I can try switching to use ExternalProject hack (for the lack of better term) instead (e.g., vtkiOS.cmake:62-77, tl;dr self import as external project, run entire cmake cycle, and get the binary back to main project), but that would not help get rid off wine entirely, unless we do not run tests for built library.

@nabijaczleweli
Copy link
Collaborator

Okay well that kinda fucking sucks, what's the point of CMake.
Yes, if you could build the generator for the host through the "hack", that'd be much more palatable; I don't think explicitly supporting running foreign tests is necessary (I'd call it a nice-to-have, and it should probably work with wine as a binfmt?).

@nthirtyone
Copy link
Contributor Author

nthirtyone commented Feb 26, 2024

It won't work with just binfmt sadly, because Wine does not use MinGW bin directories (or Ubuntu custom paths for runtime) by default and CMake does not propagete environmental variables (e.g., ENV{WINEPATH}) to custom target processes. Alternatives to the current solution with wrapping script are: prepending COMMAND ${CMAKE_COMMAND} -E env ... to custom target (and tests) or offloading all the work to system provided wrappers for toolchains (e.g., i686-w64-mingw-{cmake,wine} commands; Ubuntu does not have such hence, again, own toolchain and wrapper).

@nthirtyone
Copy link
Contributor Author

nthirtyone commented Feb 26, 2024

Rebase might be done to hastily, I don't know what's your preferred workflow, sorry. I removed wine dependency altogether and switched to use ExternalProject as discussed. The conditions are made to not be MinGW dependent, so naively it should work in other cross-compilation environments, too. The good news is that the jobs are down to sub-30-second times. If we remove MinGW CI jobs, I could remove the toolchains, too. I walked through the changes only once this time, so there might be more clean-up chances I missed.

CMakeLists.txt Outdated Show resolved Hide resolved
@nabijaczleweli nabijaczleweli merged commit 8162673 into ThePhD:main Mar 3, 2024
12 of 14 checks passed
@nabijaczleweli
Copy link
Collaborator

Looks fine, CI passes, works for me on native builds; thanks!

@nthirtyone nthirtyone deleted the mingw branch March 3, 2024 00:18
@nthirtyone
Copy link
Contributor Author

Cheers!

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

Successfully merging this pull request may close these issues.

Cannot build with MinGW on Linux for Windows
2 participants