pybind / cmake_example

Example pybind11 module built with a CMake-based build system
Other
626 stars 220 forks source link

Command line arguments OR calling python from CMake rather than the inverse #44

Closed tdegeus closed 3 years ago

tdegeus commented 3 years ago

Thanks for the nice template. However...

I'm sometimes running into build problems, most recently I have some unresolved problem here (https://github.com/conda-forge/prrng-feedstock). The problem is that the CMake call is quite buried, and that makes the whole thing hard to debug, or adapt when needed. Did you even consider having either:

henryiii commented 3 years ago

Note that the future is with scikit_build_example, as scikit_build is the official adaptor for CMake + setuptools.

Depending on how you wrote your CMakeLists, you often can call CMake directly for debugging, though.

I don't see any unresolved problems in that feedstock?

tdegeus commented 3 years ago

Thanks for your very clear explanation @henryiii . For creating a distribution from CMake, through the lines I read: wait a bit and hope for improvements in scikit-build.

Indeed, I too call CMake often directly, and it took me a second to realise that I can just as well do it on conda-build. The issue there (https://github.com/conda-forge/prrng-feedstock/pull/7) is the following. In my CMakeLists.txt I have

find_package(Python REQUIRED COMPONENTS NumPy)

But CMake fails to find NumPy:

 CMake Error at /home/conda/feedstock_root/build_artifacts/prrng-split_1621505432677/_build_env/share/cmake-3.20/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
      Could NOT find Python (missing: Python_NumPy_INCLUDE_DIRS NumPy) (found
      version "2.6.6")

(or in fact the correct Python executable all-together, it find version 2.6.6 which I'm guessing is the OS's default). Even though it is clearly specified: https://github.com/pybind/cmake_example/blob/8fa57a713ec594bc3aef49aee2caac4a174dba48/setup.py#L46 (I also tried to directly call CMake with that option, same result).

What is strange is that I had no problem building and installing without the line

find_package(Python REQUIRED COMPONENTS NumPy)

i.e. it did find the correct executable...

henryiii commented 3 years ago

FindPython uses Python_EXECUTABLE not PYTHON_EXECUTABLE. (Capital P, lower case ython). You are passing the values for FindPythonLibs/FindPythonInterp.

henryiii commented 3 years ago

Also, you should not mix and match. Either only use FindPython or only use FindPythonLibs/FindPythonInterp. Pybind11 will automatically use FindPython if you call that first; without any configuration, it will use the classic FindPythonLibs/FindPythonInterp.

henryiii commented 3 years ago

Finally, if you do use FindPython, you must find the Development component too, otherwise you can't build against it.

tdegeus commented 3 years ago

Thanks @henryiii I did not spot that... If I need NumPy's headers I'm forced to use FindPython I guess, correct? Regarding pybind11's behaviour, could you suggest the correct syntax for using FindPython. If I use your suggestion this is what I would have:

find_package(pybind11 CONFIG REQUIRED)
find_package(Python REQUIRED COMPONENTS Development NumPy)
pybind11_add_module(mypython python/main.cpp)
set_target_properties(mypython PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
target_link_libraries(mypython PUBLIC pybind11::module Python::NumPy)
tdegeus commented 3 years ago

(BTW I tried locally without Development and it worked fine)

henryiii commented 3 years ago

You must do the FindPython before Findpybind11, since pybind11 has to be able to detect you've found Python already with the new method.

If I need NumPy's headers I'm forced to use FindPython I guess, correct?

Well, if you want to use the built-in support for it; otherwise you could manually do the appropriate calls and get the NumPy header locations, which isn't too hard. Though FYI, you don't need the NumPy headers to use NumPy in pybind11 (and if you do, you need to build wheels with the minimum versions of NumPy supported, etc, which pybind11 normally frees you from).

tdegeus commented 3 years ago

I started using https://github.com/xtensor-stack/xtensor-python that seems to need the NumPy header. I could consider the manual call, is there an advantage?

tdegeus commented 3 years ago

Actually I'm getting the following errors:

CMake Warning (dev) at /Users/tdegeus/opt/miniconda3/envs/stable/share/cmake/pybind11/pybind11NewTools.cmake:233 (if):
  Policy CMP0057 is not set: Support new IN_LIST if() operator.  Run "cmake
  --help-policy CMP0057" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  IN_LIST will be interpreted as an operator when the policy is set to NEW.
  Since the policy is not set the OLD behavior will be used.
Call Stack (most recent call first):
  CMakeLists.txt:99 (pybind11_add_module)
This warning is for project developers.  Use -Wno-dev to suppress it.

CMake Error at /Users/tdegeus/opt/miniconda3/envs/stable/share/cmake/pybind11/pybind11NewTools.cmake:233 (if):
  if given arguments:

    "NOT" "ARG_WITHOUT_SOABI" "OR" "NOT" "WITH_SOABI" "IN_LIST" "ARG_UNPARSED_ARGUMENTS"

  Unknown arguments specified
Call Stack (most recent call first):
  CMakeLists.txt:99 (pybind11_add_module)

-- Configuring incomplete, errors occurred!

with

find_package(Python REQUIRED COMPONENTS Interpreter Development NumPy)
find_package(pybind11 REQUIRED CONFIG)
pybind11_add_module(mypython python/main.cpp)

which I took from the docs

henryiii commented 3 years ago

You need to set CMake minimum required to something more recent than 3.1. Probably due to the bug here:

- cmake_minimum_required(VERSION 3.1..3.19)
+ cmake_minimum_required(VERSION 3.1...3.19)

But why have a minimum of 3.1? I think FindPython's NumPy addition was 3.18 or something like that.

tdegeus commented 3 years ago

Thanks that did it!

tdegeus commented 3 years ago

Thanks a lot for your help, I was really stuck

henryiii commented 3 years ago

NumPy was added 3.14: https://cmake.org/cmake/help/latest/module/FindPython.html - though I wouldn't use FindPython before 3.15 and 3.18 is much better (PyPy support, etc).

hameer-spire commented 2 years ago

Hello, I'm running into a very similar error. Here's my CMakeLists.txt:

cmake_minimum_required(VERSION 3.18..3.20)

project(project_name VERSION 0.0.1)

find_package(Python3 REQUIRED COMPONENTS Development NumPy)
find_package(pybind11 REQUIRED CONFIG)
find_package(xtensor REQUIRED)
find_package(xtensor-python REQUIRED)

pybind11_add_module(project_name src/main.cpp)
set_target_properties(project_name PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
target_link_libraries(project_name PUBLIC pybind11::module xtensor-python Python3::NumPy)

target_compile_definitions(project_name PRIVATE VERSION_INFO=0.0.1)

And the (relevant part of the) error is:

Could NOT find Python3 (missing: Python3_NumPy_INCLUDE_DIRS NumPy) (found
      version "3.10.4")

If I change to Python (without 3), it doesn't find Python at all. I have already followed the steps here already, including bumping the CMake required version as well as defining the correct variable instead of the one in the template. With a local installation of CMake it finds everything fine, it's only when invoking via setuptools that I see an issue.

tdegeus commented 2 years ago

@hameer-spire is this helpful : https://conda-forge.org/docs/maintainer/knowledge_base.html#using-cmake ?

hameer-spire commented 2 years ago

@tdegeus Unfortunately not, using CMake directly builds the extension fine, it's building via setup.py that I have issues. I'm using the template in this repository exactly for setup.py and pyproject.toml and have changed the CMake flag to the correct capitalization. Changing it to add 3 also has no effect, neither does changing the CMakeLists.txt to be without the 3, in which case even Python isn't found.

tdegeus commented 2 years ago

Too bad. A side comment: I nowadays build exclusively with scikit-build + CMake. See for example https://github.com/tdegeus/prrng/blob/main/CMakeLists.txt and https://github.com/tdegeus/prrng/blob/main/setup.py . Notably, in build with Python I no longer use FindPython as I only use NumPy :

https://github.com/tdegeus/prrng/blob/1705895ad3ac216982965ceb194e841ed29dcc12/CMakeLists.txt#L130

henryiii commented 2 years ago

I really want to make FindPython work properly with scikit-build. Anyway, the problem I believe is you have to pass through the right variables to FindPython. You don't want to "find" python, you are literally running from Python, and you want to use that Python. The example is probably set up with FindPythonInterp/FindPythonLibs, which use a different set of discovery variables. You'll need to change those for the FindPython variables.

henryiii commented 2 years ago

This is the wrong setting for FindPython: https://github.com/pybind/cmake_example/blob/cda58ce64954daf75f3dd83bff1274d278d8cab6/setup.py#L47

See https://cmake.org/cmake/help/latest/module/FindPython.html#hints (I think the root dir one is the one you'd go for, though it's annoying, since the root dir is not as easy to compute, especially cross platform, while you always have sys.executable).

henryiii commented 2 years ago

Ahh, this is probably exactly what you want: https://cmake.org/cmake/help/latest/module/FindPython.html#artifacts-specification

hameer-spire commented 2 years ago

Update: FindPython3 works with -DPython3_EXECUTABLE and I had to add numpy in the build_requires. Build system isolation was kicking in.