search

scikit-build / scikit-build-core

A next generation Python CMake adaptor and Python API for plugins
https://scikit-build-core.readthedocs.io
Apache License 2.0
247 stars 52 forks source link

Problems finding Python on manylinux #472

Open burgholzer opened 1 year ago

burgholzer commented 1 year ago

Scikit-build-core seems to have problems finding Python on manylinux containers since recently. A simple example demonstrating the problem:

git clone https://github.com/pybind/scikit_build_example
cd scikit_build_example
pipx run cibuildwheel --platform linux

results in many warnings of the sort

 *** scikit-build-core 0.4.8 using CMake 3.27.1 (wheel)
 *** Configuring CMake...
 2023-08-21 06:43:26,999 - scikit_build_core - WARNING - libdir/ldlibrary: /opt/_internal/cpython-3.8.17/lib/libpython3.8.a is not a real file!
 2023-08-21 06:43:27,000 - scikit_build_core - WARNING - Can't find a Python library, got libdir=/opt/_internal/cpython-3.8.17/lib, ldlibrary=libpython3.8.a, multiarch=aarch64-linux-gnu, masd=None

These errors seem to have appeared when CMake 3.27 was released, although I am not 100% certain that's the issue here. Most likely something related to the newer FindPython becoming the default.

I am currently in the process of migrating some of the tools we develop from setuptools to scikit-build-core (see, e.g., https://github.com/cda-tum/mqt-qcec/pull/301) and cibuildwheel on linux keeps failing on me. From the logs (https://github.com/cda-tum/mqt-qcec/actions/runs/5922551488/job/16056693817?pr=301#step:4:257), I can extract the following

-- pybind11 v2.12.0 dev1
  CMake Error at /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
    Could NOT find Python (missing: Python_LIBRARIES Development
    Development.Embed) (found suitable version "3.8.17", minimum required is
    "3.6")
  Call Stack (most recent call first):
    /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
    /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPython/Support.cmake:3824 (find_package_handle_standard_args)
    /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPython.cmake:574 (include)
    extern/mqt-core/extern/pybind11/tools/pybind11NewTools.cmake:35 (find_package)
    extern/mqt-core/extern/pybind11/tools/pybind11Common.cmake:183 (include)
    extern/mqt-core/extern/pybind11/CMakeLists.txt:210 (include)

which kind of points to pybind11 not being able to correctly detect Python. So maybe that's a separate issue.

Any help is appreciated!

LecrisUT commented 1 year ago

Try find_package(Python3) (instead of find_package(Python)) just in case.

Edit: Oh I looked at your PR and you have it. Why do you include the 3.8 though? I see that the venv is for 3.10 and it is not piccked up

burgholzer commented 1 year ago

Try find_package(Python3) (instead of find_package(Python)) just in case.

Edit: Oh I looked at your PR and you have it. Why do you include the 3.8 though? I see that the venv is for 3.10 and it is not piccked up

The call trace from above is not from a find(Python ...) call I have control over. It comes straight from pybind11 that is being used as a submodule here: https://github.com/cda-tum/mqt-core/blob/cb3420728d60f4c791fb042814341073d35b73f1/src/CMakeLists.txt#L96-L97 The respective CMake code is being run before anything in the top-level project (https://github.com/cda-tum/mqt-qcec/blob/scikit-build-core/src/python/CMakeLists.txt) is executed. I just managed to get around the build errors by removing the set(PYBIND11_FINDPYTHON ON) I had placed before the pybind11 import. I guess that part is rather an issue with pybind11 than with scikit-build-core though.

As for the 3.8: that was (blindly) adapted from https://github.com/wjakob/nanobind_example/blob/b372b366057d344e94dd8beb894fe308b276cee9/CMakeLists.txt#L28-L31. Might be off-topic for this issue, but should that even be considered an error?

Even with all the above, the scikit-build-core warnings from the issue description are still present.

LecrisUT commented 1 year ago

As for the 3.8: that was (blindly) adapted from https://github.com/wjakob/nanobind_example/blob/b372b366057d344e94dd8beb894fe308b276cee9/CMakeLists.txt#L28-L31. Might be off-topic for this issue, but should that even be considered an error?

I was concerned how FindPython marks COMPATIBLE_VERSION. It seems to work as major version so it is all ok.

The call trace from above is not from a find(Python ...) call I have control over. It comes straight from pybind11 that is being used as a submodule here: https://github.com/cda-tum/mqt-core/blob/cb3420728d60f4c791fb042814341073d35b73f1/src/CMakeLists.txt#L96-L97 The respective CMake code is being run before anything in the top-level project (https://github.com/cda-tum/mqt-qcec/blob/scikit-build-core/src/python/CMakeLists.txt) is executed. I just managed to get around the build errors by removing the set(PYBIND11_FINDPYTHON ON) I had placed before the pybind11 import. I guess that part is rather an issue with pybind11 than with scikit-build-core though.

Indeed you should configure the external packages. One note, it's supposed to be set(PYBIND11_FINDPYTHON ON CACHE BOOL) in order for it to be overwritten e.g. in the top level project. Pybind's cmake file is rather old and hard to follow. You can also follow PYBIND11_NOPYTHON to avoid loading python altogether there. But why do you need to build the pybind11 manually? You could get it from PyPI, distro package etc., e.g. getting_started example. You might have to configure the pybind11_ROOT or equivalents to detect the bundled cmake files.

As for the warning itself:

CMake Warning (dev) at extern/mqt-core/extern/pybind11/tools/FindPythonLibsNew.cmake:98 (find_package):

This would probably be resolved in https://github.com/pybind/pybind11/pull/4786

LecrisUT commented 1 year ago

Hmm, I took a closer look and it is weird because you are supposed to be hitting this branch: https://github.com/pybind/pybind11/blob/b9359ceadbf4d4b22509b684eea107d055b77901/tools/pybind11Common.cmake#L176-L184 Which does not call FindPythonLibsNew. Can you run it with trace to see why that one is running? Python3_FOUND should be set to True or at least PYBIND11_FINDPYTHON should be set to ON manually to overwrite it.

burgholzer commented 1 year ago

I was concerned how FindPython marks COMPATIBLE_VERSION. It seems to work as major version so it is all ok.

Thanks for the clarification.

But why do you need to build the pybind11 manually? You could get it from PyPI, distro package etc., e.g. getting_started example. You might have to configure the pybind11_ROOT or equivalents to detect the bundled cmake files.

Mostly historic reasons and how our projects evolved. We have one core package that provides pybind11 as a submodule. This core package is then added as a submodule to many top-level projects that all build their own python bindings based on pybind (using that submodule). I'll probably have to rework that system at some point, but it is what we are working with right now.

As for the warning itself:

CMake Warning (dev) at extern/mqt-core/extern/pybind11/tools/FindPythonLibsNew.cmake:98 (find_package):

This would probably be resolved in pybind/pybind11#4786

Yeah. But just to be clear; I was concerned about these warnings:

2023-08-21 06:43:26,999 - scikit_build_core - WARNING - libdir/ldlibrary: /opt/_internal/cpython-3.8.17/lib/libpython3.8.a is not a real file! 2023-08-21 06:43:27,000 - scikit_build_core - WARNING - Can't find a Python library, got libdir=/opt/_internal/cpython-3.8.17/lib, ldlibrary=libpython3.8.a, multiarch=aarch64-linux-gnu, masd=None

Hmm, I took a closer look and it is weird because you are supposed to be hitting this branch: https://github.com/pybind/pybind11/blob/b9359ceadbf4d4b22509b684eea107d055b77901/tools/pybind11Common.cmake#L176-L184 Which does not call FindPythonLibsNew. Can you run it with trace to see why that one is running? Python3_FOUND should be set to True or at least PYBIND11_FINDPYTHON should be set to ON manually to overwrite it.

I just created a scratch PR in our core repository to debug this further; see https://github.com/cda-tum/mqt-core/pull/404. The call stack (https://github.com/cda-tum/mqt-core/actions/runs/5924226258/job/16061359774?pr=404#step:3:261) in the newly added test workflow actually shows that that branch is hit if PYBIND11_FINDPYTHON. It still fails though.

  Call Stack (most recent call first):
    /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
    /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPython/Support.cmake:3824 (find_package_handle_standard_args)
    /opt/_internal/pipx/venvs/cmake/lib/python3.10/site-packages/cmake/data/share/cmake-3.27/Modules/FindPython.cmake:574 (include)
    extern/pybind11/tools/pybind11NewTools.cmake:35 (find_package)
    extern/pybind11/tools/pybind11Common.cmake:183 (include)
    extern/pybind11/CMakeLists.txt:210 (include)

It's this find_package(Python ...) call that triggers the error. I'll probably file this as an issue over at the pybind11 repo as it does not seem to be related to scikit-build-core.

LecrisUT commented 1 year ago

Please run find_package(Python3) before importing pybind. Note that find_package is directory scoped so it should be in a top level CMakeLists.txt. That section is if-guared by Python3_FOUND so if find_package(Python3) is run before-hand, it should not be triggered. But good point, upstream should change it Python3 as well, it's less error-prone.

burgholzer commented 1 year ago

Please run find_package(Python3) before importing pybind. Note that find_package is directory scoped so it should be in a top level CMakeLists.txt. That section is if-guared by Python3_FOUND so if find_package(Python3) is run before-hand, it should not be triggered. But good point, upstream should change it Python3 as well, it's less error-prone.

While that does fix the issue (see https://github.com/cda-tum/mqt-core/actions/runs/5924838270/job/16063131999?pr=404#step:3:255), this still seems like an issue in pybind to me. It should not be required to bypass the "modern" Python finder in pybind. Note that adding find_package(Python REQUIRED COMPONENTS Interpreter Development.Module) before the pybind import (even with just Python and not Python3) does the trick.

Edit: Just for reference, I opened https://github.com/pybind/pybind11/issues/4802

LecrisUT commented 1 year ago

Yes, it is partially a pybind issue, but also using find_package(Python3) before pybind is perfectly fine and many cases recommended. That ensures that you are finding the python version you need in the parent project. Same goes for the FetchContent I mentioned in your upstream. That allows a parent project to link to a different version of pybind, etc.

burgholzer commented 1 year ago

Yes, it is partially a pybind issue, but also using find_package(Python3) before pybind is perfectly fine and many cases recommended. That ensures that you are finding the python version you need in the parent project. Same goes for the FetchContent I mentioned in your upstream. That allows a parent project to link to a different version of pybind, etc.

Totally agreed and thanks for the recommendations! That only leaves the core warnings surfaced in this PR unresolved

*** scikit-build-core 0.4.8 using CMake 3.27.1 (wheel)
*** Configuring CMake...
2023-08-21 06:43:26,999 - scikit_build_core - WARNING - libdir/ldlibrary: /opt/_internal/cpython-3.8.17/lib/libpython3.8.a is not a real file!
2023-08-21 06:43:27,000 - scikit_build_core - WARNING - Can't find a Python library, got libdir=/opt/_internal/cpython-3.8.17/lib, ldlibrary=libpython3.8.a, multiarch=aarch64-linux-gnu, masd=None

Those are independent of the above pybind11 issues.

LecrisUT commented 1 year ago

Those I'm afraid I'm not familiar with. It's on the container side of cibuild_wheel, @henryiii should be able to help with this when he's on.

henryiii commented 1 year ago

The warning about the libraries can be ignored. Manylinux doesn't contain the Python libraries, since you aren't supposed to link to them and it saves space to delete them. The problem is you are making them wrong call for FindPython. If you request the "Development" component, you get both Development.Module and Development.Embed. Manylinux doesn't support Development.Embed, since the Python libraries are deleted from the image during creation. You must either follow Pybind11's docs (AHH! docs are wrong. Will need to fix!) and write:

find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
find_package(pybind11 CONFIG REQUIRED)

Or you can use

set(PYBIND11_FINDPYTHON ON)
find_package(pybind11 CONFIG REQUIRED)

I'm planning on improving the library not found warning a bit.

henryiii commented 1 year ago

Actually the docs do include the correct way to refer to this in a callout with a warning about manylinux, and the reason the "wrong" version is shown is due to trying to support 3.15+ instead of 3.18+. IMO we should probably swap that.

burgholzer commented 1 year ago

Or you can use

set(PYBIND11_FINDPYTHON ON)
find_package(pybind11 CONFIG REQUIRED)

Thanks for the reply. I figured that the warning about the libraries isn't to important. The snipped above does not work though, as it triggers https://github.com/pybind/pybind11/blob/b9359ceadbf4d4b22509b684eea107d055b77901/tools/pybind11NewTools.cmake#L35 which does request the Development component.

henryiii commented 1 year ago

Fixing that in https://github.com/pybind/pybind11/pull/4805, thanks! I think most users either just use the default FindPythonInterp (bad), or call FindPython first (I hope).