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

[feat] Take into account 'requires' information to propagate only those components #7644

Merged
merged 13 commits into from Sep 22, 2020

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Sep 2, 2020

Changelog: Feature: Allow packages that do not declare components to depend on other packages components and manage transitivity correctly, with the new self.cpp_info.requires attribute.
Docs: conan-io/docs#1864

On top of #7641
Closes #7475

This PR introduces a new feature. Generators cmake_find_package[_multi] and pkg_config (those that implement components) will take into account the requirements and components listed in the requires field and will use only those targets when adding dependencies to the consumers.

Example 1

class Recipe:
    names = "middle"
    requires = "top/version"

    def package_info(self):
        self.cpp_info.components["middle1"].requires = ["top::cmp1"]

will generate this equivalent CMake lines:

add_library(middle::middle INTERFACE IMPORTED)
find_dependency(top)
set_property(TARGET middle::middle PROPERTY INTERFACE_LINK_LIBRARIES top::cmp1)

before it was using top::top

Example 2

As we don't want to force every recipe to use components in order to take advantage of this feature, there is a new cpp_info.requires property that offers the same capabilities as the one defined in the components:

class Recipe:
    names = "middle"
    requires = "top/version"

    def package_info(self):
        self.cpp_info.requires = ["top::cmp1"]

will generate this equivalent CMake lines:

add_library(middle::middle INTERFACE IMPORTED)
find_dependency(top)
set_property(TARGET middle::middle PROPERTY INTERFACE_LINK_LIBRARIES top::cmp1)

@jgsogo jgsogo added this to the 1.30 milestone Sep 3, 2020
@jgsogo jgsogo marked this pull request as ready for review September 3, 2020 13:15
@jgsogo jgsogo requested a review from czoido September 3, 2020 13:15
""")

@classmethod
def setUpClass(cls):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the test so slow that a setUpClass would make sense? Why not a setUp and make sure there are no cross-effects between tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was cleaner IMO, the test_xxx contains only the statements that are interesting to the test itself, while the setup is guaranteed to be the same for all of them. When a new generator implements this feature I hope we write a new test_xxxx function for it using the same recipes, this way it is harder to write something custom for that generator.

But I can move it to setUp or even copy it to every single test, we don't have anything written about this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm moving it to a setUp method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Components
  
Awaiting triage
2 participants