Add an integration test and debug issues with the meson backend#2718
Add an integration test and debug issues with the meson backend#2718
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. |
This would help, for sure! Users could set up a config override in cibuildwheel's config to set this depending on architecture, too. Regarding the second solution,
How does a static config file specify both amd64 and x86? These are different per wheel build, not to be statically defined in a config. When cibuildwheel is using these options, it's passing them as On a separate note, it would be really cool if we could find some way, similar to ARCHFLAGS on macOS, for cibuildwheel to just do the right thing. Could we invent a |
I was thinking that if |
|
Ah, I see, yeah that could work. I was just confused because I thought that meson only knows the Python interpreter it's building for later in the meson.build file. Having said that, for cibuildwheel usage, those are the same interpreter. |
Cherry-picked from #2718 so we can test the action.yml file
Cherry-picked from #2718 and modified so we can test the action.yml file
|
meson-python ordinarily instructs Meson to use the python interpreter that meson-python was run with. So it's still the same thing in that case, even outside of cibuildwheel. :) |
* Fix the PATH mangling in action.yml Let's put the `uv` on PATH question aside, and fix #2663. This change ensures that the PATH entries added by Github when you request `shell: bash` don't make it into the pwsh environment. It also keeps the uv binary on PATH if installed via `extras:`. Would love to get this fixed soon, then we can figure out the right approach regarding `uv` discovery. * Modify script to allow testing of the GHA action on a PR Cherry-picked from #2718 and modified so we can test the action.yml file
|
Ready to go in? I want to add a scikit-build-core + uv workspaces test, and I'd like to build on this. |
Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
|
Yes, this is ready to 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.