-
Notifications
You must be signed in to change notification settings - Fork 294
Add an integration test and debug issues with the meson backend #2718
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
base: main
Are you sure you want to change the base?
Conversation
|
|
Well, the example config test did fail on windows. I'm a little disheartened that the error message doesn't match the one we saw in #2663. |
|
Aha! But it is the same error as we see in #2701 ! So I think perhaps this is the same issue appearing in different ways. |
|
Hm... well the integration test (which is unaffected by the action.yml changes) is failing with the same error- Perhaps I'm missing something in my meson setup? Test Meson sources here. |
|
Ah, this error is in the win32 build. We have this warning earlier in the file- So we've given meson a Python.exe that's x86, but it thinks it wants the x86_64 for some reason. Edit: It's chosen x86_64 because the compiler it found on PATH is defaulting to that. |
This reverts commit 50378a1.
|
Here's what I've discovered. By default, meson searches PATH for the first available cc compiler. On Github, even when running in pwsh, that compiler is MinGW. Most projects out there have found ways to work around this. I've seen two ways-
The other wrinkle is win32 builds. As far as I can tell, this isn't too easy with meson-python. Many of the scientific stack projects have already dropped win32. Most projects that support it seem to use separate builds and use ilammy/msvc-dev-cmd to choose a 32-bit toolchain before starting cibuildwheel. Curious if @rgommers has a take here? Also I wonder if build-details.json PEP 739 could help here. Anyway, for now, I've disabled win32 builds in this test. I'd love to improve this interplay between cibuildwheel and meson-python. One idea I've had would be for cibuildwheel to call 'vsdevcmd.bat', like how we already do it for GraalPy. I suspect we might have to make that configurable, though. Some users might already have explicitly setup compilers on PATH. |
This is so it can be tested with bin/run_example_ci_configs.py
|
|
Thanks for the ping @joerick. Yes I can see a few possible solutions. I don't think
That's one of the way. The others ways are:
Note that if I'll also note that CMake is quite similar to Meson with these options, and this
Agreed, it's quite hard to change defaults without breaking some build setup or use case for other users. You can't really go check all the ways existing compilers may already be configured. So opt-in is a safer. Or, if the backwards compat impact seems acceptable, then there should be an easy opt-out at the very least. Meson is a generic build system, it's not going to change its compiler selection logic here (it's been considered and rejected, for good reasons). That means that if we want a more polished user experience by special-casing 32-bit Python on 64-bit Windows, we can either do it in There is one relevant Meson issue which would make it easier for the user to opt in to using 32-bit MSVC: mesonbuild/meson#11435 (this comment in particular). (Second post with solution options coming) |
|
Actually, on second thought, the list of solution options is shorter than I thought. I think the main one is to do both of these:
That makes it very easy for package authors to opt in to the automatic selection by using Cc @eli-schwartz and @dnicolodi for thoughts on this.
I think that this is also reasonable to do, especially if it's easy and it's already being done for GraalPy. I'm not 100% sure that it won't be overridden anyway by |
|
Please note that the The MSVC activation logic is unconditionally run for all builds on all platforms. It:
Thus, passing |
Yup -- note probably the biggest practical (rather than philosophical) issue here is that compiler detection has to happen during project() which occurs before
This seems reasonable to me.
This sounds like it might require parsing the meson command line and also
As per my previous comment, this will be fine. |
Yeah, that's what I was thinking, and is as far as I would go. |
Xref #2663.
Trying to make a test that recreates the issue in #2663.
The integration test for meson probably won't recreate it, but the bin/run_example_ci_configs.py should be able to.