[package] pybind11 ignores targets and always link against the whole list of components

[ericriff]

pybind11 is a headers only library, but it exposes a bunch of targets that the consumer can link to in order to inherit build options and flags.
Here it is the complete list
https://pybind11.readthedocs.io/en/stable/compiling.html#advanced-interface-library-targets

After the components update to this recipe, our build time increased significantly and we've tracked it down to the lto flag that wasn't there before.

We use pybind11 in 2 different ways, the standard way with pybind11_add_module(module NO_EXTRAS) (the no extras should prevent the lto flag from being added), and we also use the "advanced" mode add_library(module) + target_link_libraries(module pybind11::module). In both situations we now have the lto flag, as if we were linking to pybind11::lto.

I think it is a combination of 2 issues:

1. Conan just declares a bunch of components and it expects the build module to configure them. The main issue is that the build module links all the targets to pybind11::pybind11, which I think should only contain the headers, but in conan that means the whole list of components, right?

2. All the components depend on main, which is also called pybind11::pybind11?

I'm not completely sure if that is the issue, I'm still diving into it. Posting here to hear from the conan experts since I'm not big on components.
Could we make the main target that groups all other targets something different than pybind11::pybind11? Maybe pybind11::pybind11_all? Officially there is no target that groups everything.

@danimtb

This is an extract of the build module

# --------------------- Shared targets ----------------------------

# Build an interface library target:
# add_library(pybind11::pybind11 IMPORTED INTERFACE ${optional_global})
set_property(
  TARGET pybind11::pybind11
  APPEND
  PROPERTY INTERFACE_LINK_LIBRARIES pybind11::headers)

# Build a module target:
# add_library(pybind11::module IMPORTED INTERFACE ${optional_global})
set_property(
  TARGET pybind11::module
  APPEND
  PROPERTY INTERFACE_LINK_LIBRARIES pybind11::pybind11)

# Build an embed library target:
# add_library(pybind11::embed IMPORTED INTERFACE ${optional_global})
set_property(
  TARGET pybind11::embed
  APPEND
  PROPERTY INTERFACE_LINK_LIBRARIES pybind11::pybind11)

[ericriff]

Hey guys sorry for tagging you here but you're the experts here and I'm a bit stuck.
@madebr @uilianries @SpaceIm @ericLemanissier

Can the general target be changed? Or is it tied to the scope name?

[ericLemanissier]

This can be done by setting self.cpp_info.names["cmake_find_package"] = "pybind11_all" and self.cpp_info.components["pybind11-main"].names["cmake_find_package"] = "pybind11"

Pybind11 is already doing the second part, so you only need to set cpp_info.names

[SpaceIm]

Not sufficient, self.cpp_info.names["cmake_find_package"] = "pybind11_all" breaks the namespace.

[ericriff]

Exactly. I want to keep the namespace but change the main target name. Is that even possible?

[ericLemanissier]

Mmmmm, indeed, I didn't think enough before replying to this one, sorry!
It looks like a limitation of conan which should be reported on conan-io/conan

[ericriff]

Should I open a different issue there? I know there is a way of moving issues around, but I don't mind opening a new one there.
In the meantime, any suggestions? Does it make sense to revert to the previous behavior and let the build module handle the targets entirely?

[danimtb]

I think this could be covered by a previous discussion we had here conan-io/conan#8533 (review)

[ericriff]

IIUC that discussion is about creating aliases for the conan generated targets. In this case we need to rename the generic one since it conflicts with a real pybind11 target.
Or are you talking about changing self.cpp_info.names["cmake_find_package"] to some random name and breaking the namespace, and then adding a bunch of aliases to the right names / namespace?

[danimtb]

@ericriff I was thinking about the second approach but it wouldn't be more like a workaround than a real feature.

I have been working again with the components in the recipe and the information you provided in the issue and I agree the main problem here is that in the recipe you cannot override the general target linking with other targets. In the cmake_find_package generator you end up with this:

if(NOT TARGET pybind11::pybind11)
        add_library(pybind11::pybind11 INTERFACE IMPORTED)
endif()
set_property(TARGET pybind11::pybind11 APPEND PROPERTY
             INTERFACE_LINK_LIBRARIES "${pybind11_COMPONENTS}")

and while this allows you to effectively create your own pybind11::pybind11 target with the "main" component, it does not allow you to override the link libraries and you end up with a target graph like this one in the test_package (for windows):

which is kind of a mess.

I have made a POC moving the target linking into the if-clause above like so:

if(NOT TARGET pybind11::pybind11)
        add_library(pybind11::pybind11 INTERFACE IMPORTED)
        set_property(TARGET pybind11::pybind11 APPEND PROPERTY
             INTERFACE_LINK_LIBRARIES "${pybind11_COMPONENTS}")
endif()

and got the following result:

which looks more reasonable at least and makes sense with the information provided here https://pybind11.readthedocs.io/en/stable/compiling.html#advanced-interface-library-targets as well.

Modifications in the recipe were minimal: just removed all the "requires" from the components to let the pybind11Common.cmake do it.

Please let me know what you think about it and I would try to create a branch with my modifications so you can give it a go. Thanks! 😄

[danimtb]

See #6938 for modifications in the recipe

[cqc-alec]

Is there any good workaround for this? Among the unwanted consequences are:

• adding the -flto flag, as already pointed out (pybind11::lto);
• adding the -Os flag, overriding the -O3 flag that I want to use (pybind11::opt_size);
• linking directly against the Python library, causing crashes when run on some platforms (pybind11::embed).

These issues are preventing me from updating from pybind11/2.6.2#66f97eac059bef5b60bd6898ec4869fd.

[slietzau]

We experience the same problem and had to manually build our own target from the variables defined by cmake_find_package.

Is there anything added with the conan versions released since then that helps with this issue?

Does the new CMakeDeps generator have the same problem?

[SpaceIm]

Does the new CMakeDeps generator have the same problem?

Currently yes, but set_property() allows finer grained targets definition, so with proper set_property() in package_info() of pybind11, CMakeDeps could fix this issue.

[feltech]

Just came across this problem. It took serious effort to debug and discover that this is the bug.

In our case as soon as we added Development.Embed to the COMPONENTS option of find_package(Python, our pybind11::module-linked targets started pulling in libpython as a dependency, which is very wrong for a Python extension module, especially if libpython is installed as a static library!

[planetmarshall]

See #13283 for a draft PR in an attempt to resolve this issue.

[Ellis-Anderson]

Did this ever get solved? I see #13283 was merged but I still see -flto and -Os compiler flags being added to my builds when pybind11 is used. Another issue that seems related is that any package that consumes a conan recipe that requires pybind11 gets linked against libpython erroneously. Seemingly the only way to prevent the latter is to add self.requires(pybind11/2.13.6, visible=False) but that brings its own issues along.