Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`cppinfo` improvements: components, exe, system deps... #5090

Open
lasote opened this issue May 3, 2019 · 12 comments

Comments

@lasote
Copy link
Contributor

commented May 3, 2019

Intro

The idea is to be able to specify different components (different libs, different exes) in the cpp_info object so it can help with:

  • Modeling inter-dependencies in the same package, when a package is packaging more than 1 library, ex, OpenSSL (crypto, ssl) or Boost: #2387
  • Modeling also system dependencies: #2382
  • Help with a future find_package generators to be able to give custom names to the target: #4430 (Package::module)
  • Introduce the concept of "executables" in the cppinfo, so a consumer could receive information about the executables that the package exposes in variables, and the generators can also generate targets for the exes (in future refactor), like a target for protoc.

Proposition

  • New fields to cpp_info:

    • self.cpp_info.name: It could be used by the generators to name the targets, config files etc. E.g: ZLIB. By default, the package name will be used.
    • self.cpp_info.exes: List of executable names (located in any self.cpp_info.bindirs). It could be used by the generators to export variables with the paths etc.
    • self.cpp_info.system_deps: List of system dependencies. (["dl", "pthread"])
  • Subcomponents:

Enable self.cpp_info["component_id"], that is a very similar object to cpp_info with some exceptions:

  • The component_id already gives the name of the component, can be used by the generators for example to name the component of the target: OpenSSL::Crypto for self.cpp_info["Crypto"].
  • self.cpp_info["component_id"].name: The name for the subcomponent, by default the component_id will be the name.
  • self.cpp_info["component_id"].lib: The library name
  • self.cpp_info["component_id"].exe: The executable name (incompatible with specifying .lib)
  • self.cpp_info["component_id"].deps: List with componen_ids.
  • self.cpp_info["component_id"].system_deps: List with system library names.
  • self.cpp_info["component_id"].libs: That won't exist
  • self.cpp_info["component_id"].exes: That won't exist
  • self.cpp_info["component_id"].name: The name doesn't exist, the "component_id" gives the name already.
  • self.cpp_info["component_id"].rootpath: No sense

The subcomponents will also have available:

self.cpp_info["component_id"].includedirs 
self.cpp_info["component_id"].libdirs 
self.cpp_info["component_id"].resdirs 
self.cpp_info["component_id"].bindirs
self.cpp_info["component_id"].builddirs 
self.cpp_info["component_id"].defines
self.cpp_info["component_id"].cflags 
self.cpp_info["component_id"].cppflags
self.cpp_info["component_id"].cxxflags
self.cpp_info["component_id"].sharedlinkflags
self.cpp_info["component_id"].exelinkflags

If some of these elements are adjusted in the "general" object, the subcomponents will extend the values automatically, even if they do not use the append syntax:

self.cpp_info.include_dirs = ["include2"]
self.cpp_info["crypto"].include_dirs = ["include3"]

That will propagate ["include2", "include3"] as the include dirs for the crypto component.


Related to #4430

@ohanar

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

For component deps, I think it is very important to be able to have granularity with specifying package dependencies component deps, and whether they are part of the public interface or not.

For instance, lets say I'm creating a package foo that depends upon qt and boost, and my package has a core component and gui component. I want to specify that the core component depends upon QtCore and as an implementation detail (in cmake speak, links privately against) boost::filesystem, and that the gui component additionally depends upon QtWidgets. When using only the core component of my library, you should only get the compilation flags for my library and QtCore (in particular no includedirs for boost), and link flags for my library, QtCore and boost::filesystem.

This sort of logic already exists in modern cmake targets, it would be nice to be able to get that information through the conan generator cmake targets as well. Right now to get this fine-tuned information, you have to create various config.cmake files to export this information, and then import it with find_packages and completely ignore conan's cmake generator.

@lasote

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

Yes, the next needed features would be to improve the generators to allow, for example, to use find_package(XXX COMPONENT YYY). But first, we need the model.

#5091

@lasote lasote modified the milestones: 1.16, 1.17 May 29, 2019

@lasote

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

This is an ongoing effort, moving to 1.17, I didn't expected to have it completed this iteration.

@danimtb

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Some conclusions for ongoing development after discussion:

  • Make global usage (normal usage of cpp_info up-to-date) incompatible with components declaration
  • Remove cpp_info.exes computation in components mode.
  • Take system_deps order in cpp_info.libs computation.
  • Consider usage of a cpp_info.components = True as opt-in.
@lasote

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

Consider usage of a cpp_info.components = True as opt-in.

If you find a clean to detect when doing both adjustments would be better.

@thejohnfreeman

This comment has been minimized.

Copy link

commented Jun 13, 2019

What if a component Y in package B depends on a component X in package A? The component ID cannot be done like this:

self.cpp_info["Y"].deps = ["A.X"]
#             ^^^

Because it means that in X, it names component X "X" instead of "A.X". It has to be done like this:

self.cpp_info["B.Y"].deps = ["A.X"]

Unless Conan adds the "{package.name}." prefix implicitly.

However, this way, the Conan CMake generators should not use the {package.name}::{component.id} format for targets, or else you get B::B.Y, which is not conventional and thus will require changes to consuming CMakeLists.txts.

I think this would probably be most popular:

  • self.cpp_info.__getitem__ accepts both qualified and unqualified component IDs.
  • The .deps can be filled with both qualified and unqualified component IDs.
  • All unqualified component IDs are implicitly prepended with "{package.name}.".
  • The CMake generators name targets in the format {package.name}::{component.unqualified_id}.

To avoid ambiguities and ensure portability, you might want to restrict unqualified component IDs to match [a-zA-Z][a-zA-Z0-9_]*.

@lasote

This comment has been minimized.

Copy link
Contributor Author

commented Jun 14, 2019

What if a component Y in package B depends on a component X in package A?

The requirements between different packages are not modeled in the cpp_info.
If that case A will declare the N components of A in the cpp info and B (the only one knowing which component needs to use) can, for example, by using cmake target, link link only a component of A in the build script. For that, it is necessary that the generators understand the new cppinfo model, so B have available different targets for every component of A. But that will be a second stage of this feature.

@thejohnfreeman

This comment has been minimized.

Copy link

commented Jun 14, 2019

Hmm, I don't quite understand what you wrote after the first sentence...

@jgsogo

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

After having a look at PR #5242 there are some thing I would want to consider related to this new feature, some of them have already been mentioned in this thread:

  • Each component should aggregate the values from its dependencies:

    self.cpp_info["cmp1"].lib = "cmp1"
    self.cpp_info["cmp1"].includedirs = ["cmp1"]
    
    self.cpp_info["cmp2"].lib = "cmp2"
    self.cpp_info["cmp2"].includedirs = ["cmp2"]
    self.cpp_info["cmp2"].deps = ["cmp1"]
    
    assert self.cpp_info["cmp2"].includedirs == ["cmp2", "cmp1"]  # The order here is important too
  • Add a libs property to the components (continuation of above example):

    assert self.cpp_info["cmp2"].libs == ["cmp2", "cmp1"]
  • I would consider what @thejohnfreeman and @ohanar have pointed out. If we are going to create cmake-targets (I assume any other build system has something similar) we need to know which requirements (and components inside the requirements) correspond to each component. Something like:

    class Package(ConanFile):
        requires = "boost/...."
        
        def package_info(self):
            self.cpp_info["cmp1"].requires = ["boost::filesystem", "boost::regex"]
    
            self.cpp_info["cmp2"].deps = ["cmp1"]
            self.cpp_info["cmp2"].requires = ["boost::filesystem"]

    So, if I'm consuming component cmp1 I will link with boost::filesystem and boost:regex too, but if I'm consuming cmp2 I would only link against cmp1 and boost::filesystem.

    • At the root level (without components), you will link with all the boost package by default, but there is not much we can do about it.
    • About the syntax and name of variables, I'm open to suggestions.
@thejohnfreeman

This comment has been minimized.

Copy link

commented Jun 20, 2019

It can be fine if Conan provides a convenient method or property to aggregate attributes over transitive dependencies, but there should remain a way to read only what the user explicitly assigned. Please do not lose that information, if you envision Conan as a platform for other tools. There will exist some downstream component that wants to know.

In @jgsogo's second example, I would expect cmp2 to also link against boost::regex through its dependency on cmp1. Only if boost::regex was in the build_requires of cmp1 (corresponding to a PRIVATE requirement in CMake parlance) would I expect it to be left out of its Transitive Usage Requirements.

@jgsogo

This comment has been minimized.

Copy link
Member

commented Jun 20, 2019

Agree and agree 😸


We must keep the raw information too, sure, it could be useful.

(Implementation detail: ) IMO, I think that the logic about aggregating libraries/paths in the proper order must be inside the Component class (which has the representation of the graph with the internal dependencies) and not delegated to the generators which are the ones that will consume that information (same reason why we are aggregating the libs).


In @jgsogo's second example, I would expect cmp2 to also link against boost::regex through its dependency on cmp1. Only if boost::regex was in the build_requires of cmp1 (corresponding to a PRIVATE requirement in CMake parlance) would I expect it to be left out of its Transitive Usage Requirements.

Yes, I was thinking about a PRIVATE dependency. Here there is nothing as the build_requires concept because Conan is not managing the dependency between cmp1 and cmp2, so it is up to the recipe writer to realize about that private relationship and fill the components fields accordingly.

@lasote

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2019

Add a libs property to the components (continuation of above example):
assert self.cpp_info["cmp2"].libs == ["cmp2", "cmp1"]

I agree that would be a nicer model to have the information automatically aggregated for a component, but not for the self.cpp_info but for the self.deps_cpp_info and should be explicitly specified, something like self.deps_cpp_info["cmp2"].aggregated_libs or similar. But this is only "nice", It doesn't need to be implemented in the first iteration, probably better to do it when we develop some generators and face the real needs of them.

About the second general concern about specifying which components of requirements are required by a component, yes, I think you are right, even if the library depending on Boost knows which components need to link (for example, by using the small targets from the boost), unless it declares that circumstance in the cpp_info, the consumers of the library will lose that information. Even though, until we have smarter generators this would be almost invisible, so let's face the issue on a second stage.

@danimtb danimtb referenced a pull request that will close this issue Jul 1, 2019

Open

cpp_info components #5408

4 of 5 tasks complete

@lasote lasote modified the milestones: 1.17, 1.18 Jul 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.