pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.57k stars 2.09k forks source link

[BUG] 2.11.1: `pybind11` cmake module broken with `-D USE_PYTHON_INCLUDE_DIR=OFF` #4865

Open kloczek opened 1 year ago

kloczek commented 1 year ago

Required prerequisites

What version (or hash if on master) of pybind11 are you using?

2.11.1

Problem description

Looks like when pybind11is build with cmake -D USE_PYTHON_INCLUDE_DIR=OFF generated pybind11 cmake modules still expects to find header files in python siterlib tree. cmake output:

----------------------------- Captured stdout call -----------------------------
*** scikit-build-core 0.5.1 using CMake 3.27.6 (wheel)
*** Configuring CMake...
loading initial cache file /tmp/tmpcr9l27qj/build/CMakeInit.txt
-- The CXX compiler identification is GNU 13.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Python: /usr/bin/python3 (found version "3.8.18") found components: Interpreter Development.Module
-- Performing Test HAS_FLTO
-- Performing Test HAS_FLTO - Success
-- Found pybind11: /usr/lib/python3.8/site-packages/pybind11/include (found version "2.11.1")
-- Configuring done (1.2s)
-- Generating done (0.0s)
----------------------------- Captured stderr call -----------------------------
CMake Error in CMakeLists.txt:
  Imported target "pybind11::headers" includes non-existent path

    "/usr/lib/python3.8/site-packages/pybind11/include"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

CMake Generate step failed.  Build files cannot be regenerated correctly.

Reproducible example code

I found that issue on testing scikit-build-core https://github.com/scikit-build/scikit-build-core/issues/368#issuecomment-1742178319

Is this a regression? Put the last known working version here if it is.

Not a regression

LecrisUT commented 1 year ago

We have discussed a bit on the scikit-build-core issue and we suspect it is because there are two cmake calls being done, a native %cmake_build and one through %pyproject_wheel which could overwrite the cmake files.

But as I inspect the installed files, it is harder to pin down the issue:

# /usr/share/cmake/pybind11/pybind11Config.cmake
get_filename_component(PACKAGE_PREFIX_DIR "${CMAKE_CURRENT_LIST_DIR}/../../../" ABSOLUTE)
...
set(pybind11_INCLUDE_DIR "${PACKAGE_PREFIX_DIR}/include")

@kloczek Does your file look the same? Also worth doing a sanity check with cmake --debug-find-pkg=pybind11.

kloczek commented 1 year ago

We have discussed a bit on the scikit-build-core issue and we suspect it is because there are two cmake calls being done, a native %cmake_build and one through %pyproject_wheel which could overwrite the cmake files.

Not possible. cmake uses -B x86_64-redhat-linux-gnu and pep517 based build uses as standard build/. In bot cases are used off-source-tree builds.

@kloczek Does your file look the same? Also worth doing a sanity check with cmake --debug-find-pkg=pybind11.

I've described the cause of that behaviour and it is caused by how I'm packaging pybind11. All details are https://github.com/scikit-build/scikit-build-core/issues/368#issuecomment-1742199738 and after that comment.

In this case it is not exactly bug in pybind11 but what I've done exposed some build procedure issue which is use symlink to the inside python sitelib tree when is used USE_PYTHON_INCLUDE_DIR=OFF.

As nature of this issue is at the moment known and with present symlik everything is OK I'm leaving you decision about close this ticket or proceed to add some modification in cmake based build/install procedure to drop that hack. I'm fully aware of that that prepare clean build procedure without that symlink may be a bit tricky/hard so it may be (for now) acceptable) to live everything as it is now.

Sorry to raise this as hard bug .. 😋

FYI: I'm going to restore that symlink in my packaging procedure with annotation pointing to this ticket that this link is kind of JFDI solution which replacing may be not worth time necessary to spend to get rid of it.

LecrisUT commented 1 year ago

We have discussed a bit on the scikit-build-core issue and we suspect it is because there are two cmake calls being done, a native %cmake_build and one through %pyproject_wheel which could overwrite the cmake files.

Not possible. cmake uses -B x86_64-redhat-linux-gnu and pep517 based build uses as standard build/. In bot cases are used off-source-tree builds.

They both call %cmake_install which is where the conflict can occur.

@kloczek Does your file look the same? Also worth doing a sanity check with cmake --debug-find-pkg=pybind11.

In this case it is not exactly bug in pybind11 but what I've done exposed some build procedure issue which is use symlink to the inside python sitelib tree when is used USE_PYTHON_INCLUDE_DIR=OFF.

Yes, this is why I want to confirm the contents of /usr/share/cmake/pybind11/pybind11Config.cmake. That would at least rule out if it is a conflict issue that I described above or there is something more fundamental: https://github.com/pybind/pybind11/blob/0a756c0bb2f7e7ac6cf4e2c4de26211409fadbe4/CMakeLists.txt#L227-L231 https://github.com/pybind/pybind11/blob/0a756c0bb2f7e7ac6cf4e2c4de26211409fadbe4/tools/pybind11Config.cmake.in#L205

If those are not the case, it could be that it is an issue of find_package() where for some reason it uses the python ones (if there are any) instead of system.

Also, for the record here, Fedora packaging is not so clear about this kind of issue either. I would probably make a PR with configuration to Fedora CI so that this can be patched up.

kloczek commented 1 year ago

Some thought .. as long as pybind11 provides only python module use cmake and than pep517 builds is probably a bit incorrect. Initially my spec file has been copied from fedora spec file in which are those two stages. Probably better would be just perform pep517 build. Only dilemma is: how to pass cmake options to pep517 based build procedure? 🤔 Any comments? 🤔

LecrisUT commented 1 year ago

Well that's the issue with the current custom build system, it is not that flexible and it always defines some defaults. You can pass more variables by setting the environment variable CMAKE_ARGS.

But even so, I would not recommend that approach. The %cmake macro defines some OS specific configurations which can change as the system evolves, while the cmake script in setup.py is built specifically for PyPI packaging.