Added components to fix pybind11 #4445

danimtb · 2021-02-01T10:52:58Z

Specify library name and version: pybind11/2.6.0, pybind11/2.6.1

First try to model the pybind11 components. I got the test_package example working but I am sure there are cases that are still not covered. However, this is a start and the recipe works without doing tricks to package the CMake config file.

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

ghost · 2021-02-07T13:47:35Z

I detected other pull requests that are modifying pybind11/all recipe:

enable usage of new FindPython modules #4641

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

baptisteesteban · 2021-02-17T15:59:55Z

Hi,
I am really interested by this PR and I would like to know what is its status.
Thanks

danimtb · 2021-02-17T16:04:27Z

Waiting for the update of Conan version to add this support in the recipe

SpaceIm · 2021-02-17T16:32:59Z

recipes/pybind11/all/conanfile.py

+            self.cpp_info.components["main"].names["cmake_find_package"] = "pybind11"
+            self.cpp_info.components["main"].builddirs = [cmake_base_path]
+            for generator in ["cmake_find_package", "cmake_find_package_multi"]:
+                self.cpp_info.components["main"].build_modules[generator].append(os.path.join(cmake_base_path, "FindPythonLibsNew.cmake"))
+                self.cpp_info.components["main"].build_modules[generator].append(os.path.join(cmake_base_path, "pybind11Tools.cmake"))
+            self.cpp_info.components["headers"].includedirs = [os.path.join("include", "pybind11")]
+            self.cpp_info.components["headers"].requires = ["main"]
+            self.cpp_info.components["embed"].requires = ["main"]
+            self.cpp_info.components["module"].requires = ["main"]
+            self.cpp_info.components["python_link_helper"].requires = ["main"]
+            self.cpp_info.components["windows_extras"].requires = ["main"]
+            self.cpp_info.components["lto"].requires = ["main"]
+            self.cpp_info.components["thin_lto"].requires = ["main"]
+            self.cpp_info.components["opt_size"].requires = ["main"]
+            self.cpp_info.components["python2_no_register"].requires = ["main"]


those components names are too generic for pkg-config generator

those are the names of the CMake targets in pybind11. Do you have any suggestions? I will resume work on this PR once Conan is updated and let's see if those components really make sense

yes, add a prefix for conan components names, and set cmake_find_package and cmake_find_package_multi names explicitly.

pkg_config names will inherit from conan components names (if it's too generic they might conflict with others pkg_config files in the dependency graph even if consumer doesn't care of pybind11 pkgconfig files)

Consider my comment #4445 (comment), I'd prefer to move this forward and then improve the components' related stuff.

Components are part of the interface (downstream recipes might rely on those conan components names in their package_info). It's better to not introduce generic component names, because we would have to remove them later, and it would be a breaking change in the recipe.

planetmarshall · 2021-02-22T20:39:36Z

recipes/pybind11/all/conanfile.py

+            self.cpp_info.components["main"].names["cmake_find_package"] = "pybind11"
+            self.cpp_info.components["main"].builddirs = [cmake_base_path]
+            for generator in ["cmake_find_package", "cmake_find_package_multi"]:
+                self.cpp_info.components["main"].build_modules[generator].append(os.path.join(cmake_base_path, "FindPythonLibsNew.cmake"))


Including FindPythonLibsNew will enforce use of the deprecated FindPythonInterp etc. pybind supports the new FindPython modules (CMake 3.12+) but if you remove pybind11NewTools etc, it won't be able to use it.

Maybe make it an option? See also #4641

Recent work on this PR may address the issue described in #4641. Please review it if possible and consider closing the #4641 PR if that is the case. Thank you!

nabbelbabbel · 2021-03-19T01:32:03Z

Given there are now recipes on the index that require conan v1.33 and up (e.g. openssl), the workaround of using an older conan version with the old pybind11 recipe to have a working combination has been disabled.

For this reason, I am interested in the status of this PR.
(Or in the status/requirements for any fix of this recipe for that matter.)

There was some reference to wait for a conan update made above, I guess that v1.34.1 was that update?

Thanks.

Jean1995 · 2021-03-30T15:05:49Z

Are there any updates on this PR / issue?

conan-center-bot · 2021-04-16T16:40:53Z

All green in build 11 (fb33697556051393333e9e802063094e670549f7):

pybind11/2.5.0@:
All packages built successfully! (All logs)
pybind11/2.6.1@:
All packages built successfully! (All logs)
pybind11/2.6.0@:
All packages built successfully! (All logs)
pybind11/2.4.3@:
All packages built successfully! (All logs)
pybind11/2.6.2@:
All packages built successfully! (All logs)

danimtb · 2021-04-16T16:46:38Z

I know it is far from complete regarding components and it will require some more time to understand each of the cmake targets and relations among them, but at least this works as expected without requiring packaging config files. I'd prefer to have this merge in order to fix #4407 and then continue the work in the recipe to improve it

prince-chrismc · 2021-04-19T21:25:19Z

recipes/pybind11/all/conanfile.py

        cmake_base_path = os.path.join("lib", "cmake", "pybind11")
-        self.cpp_info.builddirs = [cmake_base_path]
+        if tools.Version(self.version) >= "2.6.0":
+            self.cpp_info.components["main"].names["cmake_find_package"] = "pybind11"


I comment I see often (but I am no expert) we need to be caution about generic conan component names

yes, already mentioned here: #4445 (comment)

prince-chrismc · 2021-04-19T21:30:36Z

We got a few related issues to this pr

fixes #4322
closes #4641

There a "not a bug" we can close 🙏
#3743

m-drozd · 2021-04-20T12:44:21Z

Hi, these changes have broken my builds and I can't seem to find a way to use the new revision of 2.6.1 version.
Up to this point I've been using this package more or less like this:

find_package (Python COMPONENTS Interpreter Development)
find_package (Python3 COMPONENTS Interpreter Development)
include(${CMAKE_BINARY_DIR}/build/conanbuildinfo.cmake)
conan_basic_setup(NO_OUTPUT_DIRS TARGETS)

and then linking with pybind11::embed target worked flawlessly but now i get bunch of

Target "xyz" links to target "pybind11::embed"
  but the target was not found.

If dropping the pybind:: targets was intended what's the replacement here?

prince-chrismc · 2021-04-20T13:14:13Z

You should be able to use the cmake_find_package generator from Conan and use find_package(...) in CMake to gain access to the targets

jornb · 2021-04-20T19:53:34Z

These changes seem to have broken our builds.

Our setup is super simple. In our CMakeLists.txt we have

pybind11_add_module(mytarget main.cpp)
target_link_libraries(mytarget PRIVATE CONAN_PKG::pybind11)

Invoking with

cmake -DPython3_EXECUTABLE=/my/python /my/code

gives this output

CMake Error at CMakeLists.txt:2 (pybind11_add_module):
  Unknown CMake command "pybind11_add_module".

How is pybind11_add_module supposed to be invoked with these changes?

find_package(pybind11 REQUIRED)

Results in

CMake Error at CMakeLists.txt:2 (find_package):
  By not providing "Findpybind11.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "pybind11",
  but CMake did not find one.

  Could not find a package configuration file provided by "pybind11" with any
  of the following names:

    pybind11Config.cmake
    pybind11-config.cmake

  Add the installation prefix of "pybind11" to CMAKE_PREFIX_PATH or set
  "pybind11_DIR" to a directory containing one of the above files.  If
  "pybind11" provides a separate development package or SDK, be sure it has
  been installed.

prince-chrismc · 2021-04-21T01:00:23Z

use the cmake_find_package generator

To get the missing file

danimtb · 2021-04-21T08:55:51Z

@m-drozd @jornb please check the test_package of the recipe https://github.com/conan-io/conan-center-index/tree/master/recipes/pybind11/all/test_package to see how this should be consumed correctly. Generators in consumers should be cmake_find_package or cmake_find_package_multi and CMakeLists.txt with

find_package(pybind11 REQUIRED)
pybind11_add_module(test_package MODULE test_package.cpp)
set_property(TARGET test_package PROPERTY CXX_STANDARD 11)

m-drozd · 2021-04-21T11:02:48Z

Yeah I managed to get it working after messing with cmake_find_package and cmake_path generators. It's still not ideal because, as far as I can tell, the replacements done in conanfile make pybind unusable without conan after using the deploy generator (it worked fine up untill now), but I can work around that.

* Added components to fix pybind11 * required conan version * remove config files * remove cmake files not needed * fix unlink * add rest of macros * order * patch pybind11common file * set standard * remove other modules from build_modules

ericriff · 2021-07-30T22:15:28Z

Posting here to get some attention from the community.
This PR added support for the components, but the build module links most of them to pybind11::pybind11 which happens to match the global target.
As a consequence, no matter which target you link to, you end up linking to all of them. This is annoying since those targets are used to control build flags, so your build ends up not using the flags you expect.
See #6605 for details.

macdew · 2021-08-04T15:59:41Z

I'm also having an issue with the pybind11 package which I think is related to these changes. I'm using cmake_find_package_multi together with CMake Generator for Visual Studio (multi-configuration build), and find_package(pybind11 REQUIRED) is throwing the following error:

CMake Error at C:/<conanpath>/pybind11/2.7.1/ies/testing/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/lib/cmake/pybind11/pybind11NewTools.cmake:138 (add_library):
  add_library cannot create imported target "pybind11::python_headers"
  because another target with the same name already exists.

I wonder if it is trying to execute this script for all configurations (Debug/Release/RelWithDebInfo) and there is no guard against multiple-includes?

macdew · 2021-08-04T16:14:12Z

I'm getting another error at this location:

CMake Error at C:/<conanpath>/pybind11/2.7.1/ies/testing/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/lib/cmake/pybind11/pybind11Tools.cmake:97 (add_library):
  add_library cannot create imported target "pybind11::python_headers"
  because another target with the same name already exists.

It looks like both pybind11NewTools.cmake AND pybind11Tools.cmake are being run, but they create the same targets, and presumably only one should be used?

Holt59 · 2021-08-26T08:47:05Z

Same as above, I'm completely stuck... Any news on this?

macdew · 2021-08-26T09:12:21Z

I modified the recipe (we fork them locally, so was fortunate to be able to do this) to patch the sources as follows:

            tools.replace_in_file(os.path.join(self.package_folder, "lib", "cmake", "pybind11", "pybind11Common.cmake"),
                                  "if(TARGET pybind11::lto)",
                                  "include_guard(GLOBAL)\nif(FALSE)")
            tools.replace_in_file(os.path.join(self.package_folder, "lib", "cmake", "pybind11", "pybind11Common.cmake"),
                                  "add_library(",
                                  "# add_library(")

I appreciate that this is a total hack, but the scripts are completely broken as they are so had no other solution... (note: CMake's include_guard requires cmake 3.10 or higher)

The include guard was required because the file gets loaded multiple times, resulting in the same target being created multiple times. The add_library call was to fix the actual issue here of both scripts getting run (I'm going from memory here...).

My usage in CMake is as follows... using conan's cmake_find_package_multi generator:

set(Python3_FIND_REGISTRY LAST)
set(Python3_FIND_UNVERSIONED_NAMES FIRST)

message("Python bindings: searching for Python3")
find_package(Python3 COMPONENTS Interpreter Development)

if (Python3_FOUND)
  set(CMAKE_PREFIX_PATH "${CMAKE_BINARY_DIR};${CMAKE_PREFIX_PATH}")
  find_package(pybind11 CONFIG REQUIRED)

  pybind11_add_module(my_python_module MODULE ...)
endif()

CPickens42 · 2022-04-01T00:30:11Z

Hello, I don't understand why the the pybind11 recipe does removes the "pybind11Targets.cmake", "pybind11Config.cmake", "pybind11ConfigVersion.cmake". I cannot use cmake_find_package because of some issues with other packages (conflict stuff)

I ended up using the following (as mentioned in #9343):

It seems that the official pybind11 package from conan doesn't expose all cmake files from pybind11. For instance share/cmake/pybind11/pybind11Config.cmake was not present at all.

I ended up using a custom conan package that works nicely with CMakeDeps and CMakeToolchain generators (that are the recommended way for conan 2.0):

import os
from conans import ConanFile, tools, CMake


class PyBind11Conan(ConanFile):
    name = "pybind11"
    version = "2.9.2"
    settings = "os", "compiler", "arch", "build_type"
    description = "Seamless operability between C++11 and Python"
    homepage = "https://github.com/pybind/pybind11"
    license = "BSD Style: https://github.com/pybind/pybind11/blob/master/LICENSE"
    url = "https://github.com/conan-community/conan-pybind11"
    no_copy_sources = True

    def source(self):
        tools.get("%s/archive/v%s.tar.gz" % (self.homepage, self.version))

    def build(self):
        cmake = CMake(self)
        cmake.definitions["PYBIND11_TEST"] = False
        cmake.configure(source_folder="pybind11-%s" % self.version)
        cmake.build()
        cmake.install()

    def package(self):
        self.copy("*LICENSE", keep_path=False)

    def package_id(self):
        # Make all options and dependencies (direct and transitive) contribute
        # to the package id
        self.info.requires.full_package_mode()

    def package_info(self):
        self.cpp_info.builddirs.append(os.path.join("share", "cmake", "pybind11"))

It is an adaptation of this https://github.com/conan-community/conan-pybind11.
The code in package_info permits to have the CMakeToolchain generators to overload CMakeDeps when necessary.

My CMakeLists.txt looks like that:

include(${CMAKE_SOURCE_DIR}/.conanvirtualenv/conanbuildinfo.cmake)

# load pybind11
set(Python_FIND_VIRTUALENV FIRST)
find_package(Python REQUIRED COMPONENTS Interpreter Development)
find_package(pybind11 CONFIG REQUIRED)

@danimtb Do you think I should do a PR? I'm sure I'm missing a lot of stuff. I'm really new to conan. It would be great to have the support of pybind11 with the new recommended generators.

danimtb added 3 commits February 1, 2021 11:44

Added components to fix pybind11

f1e9632

required conan version

4e1ab60

remove config files

fcff035