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

Build failures using meson-python 0.11.0 with cibuildwheel #222

Closed
dnicolodi opened this issue Nov 21, 2022 · 58 comments
Closed

Build failures using meson-python 0.11.0 with cibuildwheel #222

dnicolodi opened this issue Nov 21, 2022 · 58 comments
Labels
bug Something isn't working dependency-bug A bug experienced by users of meson-python caused by a dependency, rather than in code in this repo

Comments

@dnicolodi
Copy link
Member

dnicolodi commented Nov 21, 2022

On Windows, building for win32 fails with

Program python found: YES (C:\Users\runneradmin\AppData\Local\Temp\cibw-run-dbnhe2vj\cp37-win32\build\venv\Scripts\python.exe)
Need python for 64-bit, but found 32-bit
        
..\..\meson.build:24:3: ERROR: Python dependency not found

it may be a cibuildwheel issue or a Meson issue. Full log here https://github.com/dnicolodi/python-siphash24/actions/runs/3517527064/jobs/5895402010

On macOS, building succeeds but test fail. The test pass when the package is not built with cibuildwheel. No idea what is going on as it seems that the test process segfaults. Maybe a linking issue. Full log here https://github.com/dnicolodi/python-siphash24/actions/runs/3517527064/jobs/5895402097

On Linux, linking issue:

FAILED: siphash24.cpython-37m-x86_64-linux-gnu.so
        cc  -o siphash24.cpython-37m-x86_64-linux-gnu.so siphash24.cpython-37m-x86_64-linux-gnu.so.p/meson-generated_siphash24.pyx.c.o -Wl,--as-needed -Wl,--allow-shlib-undefined -shared -fPIC -Wl,--start-group subprojects/libcsiphash-1/src/libcsiphash-1.a -L/opt/_internal/cpython-3.7.13/lib -lpython3.7m -Wl,--end-group
        /opt/rh/devtoolset-10/root/usr/libexec/gcc/x86_64-redhat-linux/10/ld: cannot find -lpython3.7m

it may be Python 3.7 specific. Again not seen when building without cibuildwheel, maybe related to the setup used in the manylinux container. Full log here https://github.com/dnicolodi/python-siphash24/actions/runs/3517527064/jobs/5895401917

EDIT: a summary of the actual issue is in #222 (comment), and a Meson issue is linked from #222 (comment). There is also useful discussion on cross-compiling in the comments below.

@dnicolodi
Copy link
Member Author

@rgommers SciPy uses cibuildwheel. Did you bump into any of these issues? Don't you build for these platforms?

@henryiii
Copy link
Contributor

henryiii commented Nov 21, 2022

Side comment: When Meson fails, we should raise SystemExit (like setuptools usually does), not throw an exception. Pip/build report a mess when there's an uncaught exception from the build hooks, while SystemExit is handled gracefully without nearly as many extra reports about missing build hooks and changed exceptions. The actual failure is printed by Meson.

@henryiii
Copy link
Contributor

henryiii commented Nov 21, 2022

Is this an extension or an app? You should not be linking to Python on macOS/linux if it's an extension. The libs are intentionally deleted from the manylinux images to stop users from trying to link to them.

That Windows failure is supposed to be building a 32-bit wheel, so guessing it's either meson or meson-python not respecting the attempt to build for 32-bit? It does find the 32-bit Python then complains when it's 32-bit (which is what it is supposed to be).

@dnicolodi
Copy link
Member Author

Is this an extension or an app? You should not be linking to Python on macOS/linux if it's an extension. The libs are intentionally deleted from the manylinux images to stop users from trying to link to them.

It is an extension. Meson takes care of the linking, when needed. I don't do it explicitly. Interestingly, it is a problem only for Python 3.7 in cibuildwheel. I've no idea what's going on.

That Windows failure is supposed to be building a 32-bit wheel, so guessing it's either meson or meson-python not respecting the attempt to build for 32-bit? It does find the 32-bit Python then complains when it's 32-bit (which is what it is supposed to be).

The Windows issue I think is due to cibuildwheel not setting up things for Meson to find the MSVC compiler. I'm trying to setup the correct environment variables from outside to see if this fixes cibuildwheel.

@dnicolodi
Copy link
Member Author

dnicolodi commented Nov 21, 2022

The Windows issue I think is due to cibuildwheel not setting up things for Meson to find the MSVC compiler. I'm trying to setup the correct environment variables from outside to see if this fixes cibuildwheel.

The one solution I've tried https://github.com/bus1/cabuild/tree/main/action/msdevshell does not work:

        The Meson build system
        Version: 0.64.0
        Source dir: D:\a\python-siphash24\python-siphash24
        Build dir: D:\a\python-siphash24\python-siphash24\.mesonpy-lnm4qgxr\build
        Build type: native build
        Project name: python-siphash24
        Project version: undefined
        
        ..\..\meson.build:1:0: ERROR: Found GNU link.exe instead of MSVC link.exe in C:\Program Files\Git\usr\bin\link.EXE.
        This link.exe is not a linker.
        You may need to reorder entries to your %PATH% variable to resolve this.

@henryiii
Copy link
Contributor

henryiii commented Nov 21, 2022

Are you using the action? There's a report that by using the bash shell in the aciton this breaks Meson's discovery. I think we can fix it by using the native shell and splitting the action up (though, why does this break meson? CMake and setuptools are just fine as it is).

Meson should not be putting in -lpython3.7(m), that's a bug when building something to be distributable. You can't link to Python's libs. That will cause segfaults on macOS and libraries not found on manylinux. It will work when building locally, since you are using the same patch release of Python that you built with, so it won't segfault (unless you upgrade Python to a new patch release without rebuilding, or sending it to someone else, etc.)

@henryiii
Copy link
Contributor

This one: pypa/cibuildwheel#1338

@henryiii
Copy link
Contributor

Also, ilammy/msvc-dev-cmd is what I usually use.

@dnicolodi
Copy link
Member Author

Meson does the same as thing distutils does to decide when to link to libpython. It actually calls into distutils, see https://github.com/mesonbuild/meson/blob/3ae89a7150264ae5a112975af1377ee1c04cb994/mesonbuild/modules/python.py#L366-L370 I'm surprised that distutils is broken in this way.

@dnicolodi
Copy link
Member Author

dnicolodi commented Nov 21, 2022

Also, ilammy/msvc-dev-cmd is what I usually use.

In combination with the cibuildwheel action? How do you set the architecture when you use cibuildwheel to compile both amd64 and x86?

@henryiii
Copy link
Contributor

That rather scares me - is Meson being tested on Python 3.12a2? Distutils has been removed there. I thought the point of moving to Meson was to be ready for distutils' removal?

If that's the built-in distutils, it wouldn't horribly surprise me, as it's usually filtered by setuptools, and would explain why it would be Python version specific. https://github.com/pypa/setuptools/blob/59ee4980a0f49ea610e26a1aca104334ae03d140/setuptools/_distutils/command/build_ext.py#L736-L789

Hmm, see https://github.com/pypa/setuptools/blob/59ee4980a0f49ea610e26a1aca104334ae03d140/setuptools/_distutils/command/py37compat.py#L6. See pypa/distutils#9.

@henryiii
Copy link
Contributor

henryiii commented Nov 21, 2022

I generally don't mix the two because both setuptools and CMake are smart enough to work without ilammy/msvc-dev-cmd. The first thing I'd recommend trying is swapping the action for run: pipx run cibuildwheel==2.9.0 and see if that fixes it. If it does, I'll see if we can split the action up into Unix and Windows steps. (Also, the main reason to use the action is to use dependabot to keep it up to date, which I'm guessing you aren't doing since you are using 2.9.0 instead of 2.11.2).

Otherwise, you can split the job up into 32 bit and 64 bit steps. It's fine to run msvc-dev-cmd twice. (In fact, I vaguely think I have done this before for something).

@dnicolodi
Copy link
Member Author

The fact that distutils is going away is know, but there are reasons why Meson depends on it. @eli-schwartz is the one that has the whole picture and working on a solution. You may want to discuss the details with him.

@dnicolodi
Copy link
Member Author

cibuildwheel uses bash in GitHub Actions and this puts the GNU compilers on the $PATH making a mess for Meson. The solutions for having MSVC compilers on the path do not work for this reason. It seems that cibuildwheel and Meson simply do not mix. One way around is to use the Mingw compilers adding the required configuration to meson.build, but this does not allow to build win32 wheels, I think.

@henryiii
Copy link
Contributor

henryiii commented Nov 21, 2022

Did you try my pipx suggestion? That will avoid bash on Windows. (And that's basically the solution we'd propose in cibuildwheel if it works)

@rgommers
Copy link
Contributor

SciPy uses cibuildwheel. Did you bump into any of these issues? Don't you build for these platforms?

On Windows we use Mingw-w64 for 64-bit build, and currently we don't build for 32-bit Python on Windows due to the lack of a suitable compiler toolchain (one is in progress, also Mingw based). We haven't had any problems on macOS.

Code:

@dnicolodi
Copy link
Member Author

Thanks @rgommers. There are two macOS issues: one related to Python 3.7 and one related to universal2 builds. SciPy does not compile for Python 3.7 and probably neither compiles universal binaries.

@rgommers
Copy link
Contributor

universal2: no point in that, best to let it die. just put thin binaries on PyPI I'd say, that's what numpy/scipy/etc do. If you have a user who needs it, point them to delocate, which provides a delocate-fuse tool that just does the right thing.

@dnicolodi
Copy link
Member Author

Still, I think there may be a real bug in meson-python there: with the cibuildwheel setup the wheel tags are wrong.

@eli-schwartz
Copy link
Member

eli-schwartz commented Nov 21, 2022

If both MSVC and the MingGW compilers are on the path, which one should Meson choose, and why? Which one should other build systems choose, and why?

That rather scares me - is Meson being tested on Python 3.12a2? Distutils has been removed there. I thought the point of moving to Meson was to be ready for distutils' removal?

Meson historically relies on distutils for 2 things:

  • figuring out whether the found python thinks it needs to link to libpython
  • checking whether Debian's patched python with patched sys.path has patched distutils to adapt to the patch, but left sysconfig broken

In both cases, the use of distutils is limited to a probe script that dumps json to stdout describing the capabilities of the found python. And in one of those cases, we were limited by "whatever Debian does", which is beyond frustrating but there you have it.

We do intend to fix this before python 3.12 final, and ideally as soon as possible for alpha testing, but the main point is that whether or not Meson uses it isn't something that individual packages like SciPy need to care about.

But "be ready for distutils' removal" is a complex topic, and the main issue that many meson-python adapters have is less about the removal of distutils from the stdlib, and more about what that means for setuptools' release velocity, API stability, and the infeasibility of maintaining numpy.distutils. The use of either distutils or setuptools isn't healthy for large projects especially, because of various limitations which mainly boil down to "use a real, battle-tested build system that supports introspection, parallel builds, custom build rules that aren't just open-coded in python, etc."
Having an import distutils may or may not be a hindrance to these goals, and as it happens, isn't one.

There's a quick hack to make this work on python 3.12 alpha, simply install setuptools as a build dependency -- it includes a copy of distutils, suitable for example for checking whether there's a debian scheme in it :D and which also contains stdlib-compatible logic for determining whether libpython is expected on that OS to be linked to in extensions. This is of course a hack, not a solution, which is why it will not be needed for the final release. I've already been looking into what is needed here.

@dnicolodi
Copy link
Member Author

Did you try my pipx suggestion?

Just now. It works to build the amd64 binaries. I haven't tried to build both the x86 and amd64. Thank you!

@henryiii
Copy link
Contributor

But does it fix Windows? That's what I'd like to know before making a PR to cibuildwheel to split the action.

@dnicolodi
Copy link
Member Author

Oh sorry, I thought the context was clear, it works for building the Windows amd64 wheels.

@henryiii
Copy link
Contributor

Ahh, of course, thinking about universal2 due to convo above 😜. Okay, will see what can be done in cibuildwheel.

@dnicolodi
Copy link
Member Author

I think I found a real bug: meson-python does not generate the right wheel tags when cibuildwheel tries to compile the arm64 or universal2 wheels. @henryiii can you point me to the code in cibuildwheel that sets up the environment for these platforms?

@dnicolodi
Copy link
Member Author

Never mind, found it https://github.com/pypa/cibuildwheel/blob/2e6cbe2435ef410156319c98625002ed76e08785/cibuildwheel/macos.py#L219-L232 . And meson-python does not do what cibuildwheel expects:

>>> str(mesonpy._tags.Tag())
'cp310-cp310-macosx_10_15_x86_64'
>>> os.environ['_PYTHON_HOST_PLATFORM'] = "macosx-11.0-arm64"
>>> str(mesonpy._tags.Tag())
'cp310-cp310-macosx_10_15_x86_64'

@henryiii
Copy link
Contributor

henryiii commented Nov 21, 2022

This should be controlled by ARCHFLAGS, ideally (cibuildwheel does set both).

@rgommers
Copy link
Contributor

@henryiii
Copy link
Contributor

IIRC, that was mostly necessary to try to make 3.8 work, since 3.8's a little weird due to ARM support being experimental with 3.8.

@dnicolodi
Copy link
Member Author

dnicolodi commented Nov 21, 2022

ARCHFLAGS does not change what Python reports. _PYTHON_HOST_PLATFORM changes what sysconfig.get_platforms() reports. The problem is that I based my implementation of the wheel tag code on packaging which uses platform.mac_ver() to derive the platform tag on macOS.platform.mac_ver() is not affected by the environment variables. This is fine for packaging (maybe?) because it is used mostly for generating the tags for installing wheels not for building them thus overwriting the platform makes little sense. Indeed, wheel uses sysconfig.get_platoform() to compute the platform tag.

@henryiii
Copy link
Contributor

henryiii commented Nov 22, 2022

I'm totally okay if you want to come up with a viable alternative and document it (and please include Windows/Linux cross compilation too, even if it's not supported out of the box quite yet!). It cannot be based on --config-settings for cibuildwheel to use it, though - cibuildwheel communicates entirely though environment variables, and that's the only way to communicate to a build backend without knowing what backend you have. It's also necessary for conda-build, since conda doesn't control the pip invocation, but does control the environment.

While both exist, the standard one (some mixture of ARCHFLAGS & _PYTHON_HOST_PLATFORM, either contain enough data independently) should be respected if the new one isn't set. You'll need to get all the major backends, especially Setuptools, on board. Then once everyone supports it, and nobody is putting silly things like setuptools<60 that stop them from getting the new version that supports this ;), then we can drop the old methods and just use the new way. But all major tools, especially setuptools, needs to support it first, and have supported it for enough time for people to upgrade, before the old way is dropped. We can add cibuildwheel support sooner, however, once there's a proposal with some buy in, and probably get Conda-build too. I'd say an appropriate amount of buy-in would be setuptools, or Maturin + meson-python + scikit-build-core.

Cross-complation is supported for macOS (at least if all your dependencies are Universal2). This isn't really supported for Linux and Windows is a mess. I'd really, really like to avoid that sort of thing anywhere else (and remove it for Windows some day). So I do think working on a common way to specify a cross-compilation environment would be useful. I'd especially recommend making sure the setuptools folks are on board if we propose anything.

Is this at all documented anywhere?

It should be here, IMO, but no, not that I'm aware of.

@dnicolodi
Copy link
Member Author

Linking issue identified and reported here mesonbuild/meson#11097 Meson PR on the way.

dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 22, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 22, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 22, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
@rgommers rgommers changed the title Failures across the board building project using meson-python 0.11.0 with cibuildwheel Python 3.7 on Windows and macOS arm64 build failures using meson-python 0.11.0 with cibuildwheel Nov 23, 2022
@rgommers
Copy link
Contributor

Cross-complation is supported for macOS (at least if all your dependencies are Universal2).

That's kinda not really cross-compiling, it's just injecting one flag and tweaking a wheel tag, and then hoping it's a simple package so that will be enough. I suspect there may not be (m)any projects actually needing this. Because the restriction here of all deps must be universal2 is quite severe (e.g., most users are going to depend on numpy, so they're all ruled out).

I'll add the rest of my thoughts on gh-226.

This isn't really supported for Linux and Windows is a mess. I'd really, really like to avoid that sort of thing anywhere else (and remove it for Windows some day).

Ouch, that is messy indeed.

So I do think working on a common way to specify a cross-compilation environment would be useful. I'd especially recommend making sure the setuptools folks are on board if we propose anything.

This is a bigger topic. I'd like to work on it, yes. Specifying the environment would require separate build vs. host dependency specification in pyproject.toml for one. Cross-compiling is not and has never been supported by setuptools, distutils and numpy.distutils. Everything that happens to work for a specific platform (Windows and x86->arm64 macOS only probably) is basically a non-robust hack. To make it better, any build backend has to know it is cross-compiling. I'm not sure what the setuptools folks will say about supporting anything officially, we'll see.

@eli-schwartz
Copy link
Member

Actually Linux has a much more interesting and probably reliable hack which vendors that have fully and officially supported generic full-OS cross compilation have historically used to make python compatible with them.

It's $_PYTHON_SYSCONFIGDATA_NAME as an environment variable, pointing to the cross compiled python, such that the native python's sysconfig.get_config_vars() essentially reports itself as the cross built one. This accounts for a large chunk (but not all) of the information that is used to build and install modules.

@henryiii
Copy link
Contributor

That's kinda not really cross-compiling, it's just injecting one flag

Apple very intentionally made cross compiling very easy, and it in general works really well. Most cibuildwheel projects just do this, and it works. The biggest caveat is dependencies - you can't just grab them from Homebrew, you need dependencies compiled with the same arch flags (ideally universal, which makes things much easier when cross compiling). Which is also true for non-cross compiling, you need 10.9+ dependencies instead of the specific ones homebrew provides, so a few projects have actually been improved by moving to custom builds or downloads of deps instead of (incorrectly) grabbing deps from Homebrew.

I'm only aware of a very small handful of very large projects that do not cross compile.

@dnicolodi
Copy link
Member Author

dnicolodi commented Nov 23, 2022

@rgommers The new title of the issue is wrong. Windows failure is not related to meson-python, Linux and macOS failures on Python 3.7 are due to a bug in Meson, cross compilation is not supported in meson-python. Anyhow, I opened specific tickets for the different issues and everyone is commenting everywhere, except in the tickets that track the specific issues. It does not help.

@rgommers
Copy link
Contributor

Would you mind updating it to something descriptive that looks more correct to you @dnicolodi? "Failures across the board" is too vague. I semi-regularly go through a whole backlog, so having titles that are descriptive is useful to me.

are due to a bug in Meson

We do have a dependency-bug for that - it signals that it's an issue that may affect meson-python users, but has to be solved elsewhere.

@dnicolodi
Copy link
Member Author

The idea was to close this bug once the root cause of the failures was identified. I opened specific bugs for the identified issues, see #222 (comment) and #222 (comment) however this didn't stop people from continuing to comment on this way too vague issue thus I felt it did not make sense to close it in favor of the more specific ones.

@rgommers
Copy link
Contributor

Okay, let's close this then - I'll edit the description to point to those two comments.

@rgommers rgommers added bug Something isn't working dependency-bug A bug experienced by users of meson-python caused by a dependency, rather than in code in this repo labels Nov 23, 2022
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 23, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 23, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 23, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 23, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
@dnicolodi dnicolodi changed the title Python 3.7 on Windows and macOS arm64 build failures using meson-python 0.11.0 with cibuildwheel Build failures using meson-python 0.11.0 with cibuildwheel Nov 24, 2022
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 24, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 24, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Nov 24, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
@dnicolodi
Copy link
Member Author

@henryiii How does cibuildwheel setup the environment for compiling x86 Windows binaries in a x86_64 build machine? Does it do something special to get the right compiler or does it expect to just work? Right now I need to run cibuildwheel twice with different $PATH to have Meson pick the right compiler, but it would be nice if there were a way to avoid this.

dnicolodi added a commit to dnicolodi/meson-python that referenced this issue Dec 11, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See mesonbuild#222 for background.
dnicolodi added a commit that referenced this issue Dec 13, 2022
Use sysconfig.get_platform() instead of platform.mac_ver() to base the
platform tag computation.  The ability to overwrite the value returned
by former via the _PYTHON_HOST_PLATFORM environment variable is used
in macOS extension modules cross-compilation.

See #222 for background.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependency-bug A bug experienced by users of meson-python caused by a dependency, rather than in code in this repo
Projects
None yet
Development

No branches or pull requests

4 participants