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

Bugfixes 11743 and 11755 #11756

Merged
merged 2 commits into from Aug 4, 2022
Merged

Bugfixes 11743 and 11755 #11756

merged 2 commits into from Aug 4, 2022

Conversation

lasote
Copy link
Contributor

@lasote lasote commented Aug 2, 2022

Changelog: Bugfix: The CMakeDeps generator failed for consumers with CMake projects doing an add_subdirectory with two different find_package calls to the same package, one in the main CMakeLists.txt and the other in the subdirectory.
Changelog: Bugfix: The CMakeDeps generator stopped to fill the XXX_LIBRARIES CMake variable, especially when using components.
Docs: omit

Close #11755
Close #11743

@lasote lasote added this to the 1.51.1 milestone Aug 2, 2022
@lasote lasote requested review from jcar87 and czoido August 2, 2022 12:04
@@ -121,6 +109,9 @@ def template(self):
$<$<CONFIG:{{configuration}}>:${{'{'}}{{pkg_name}}_LIB_DIRS{{config_suffix}}}> APPEND)
{%- endif %}

# For the modules (FindXXX)
list(APPEND {{ pkg_name }}_LIBRARIES{{ config_suffix }} {{root_target_name}})
Copy link
Contributor Author

@lasote lasote Aug 2, 2022

Choose a reason for hiding this comment

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

Note, here I'm attaching the target instead of the path to the libraries, like in previous versions. That is because the macro conan_package_library_targets was simplified and only returns the targets.

When someone uses the Boost_LIBRARIES is typically to link a target with them so it should be ok and it is wide easier in code, and also, the transitive dependencies of the package were always added as targets to the list.

@lasote lasote changed the title Bugfixes 11743 Bugfixes 11743 and 11755 Aug 2, 2022
@@ -174,9 +168,7 @@ def template(self):

########## AGGREGATED GLOBAL TARGET WITH THE COMPONENTS #####################
{%- for comp_variable_name, comp_target_name in components_names %}

target_link_libraries({{root_target_name}} INTERFACE {{ comp_target_name }})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is what it was causing the #11755 error. When using an add_subdirectory the target exists but fails to use target_link_libraries. This is apparently related to the scope of the target. Creating the targets as INTERFACE GLOBAL worked too but looked more dangerous.

t.run("install .")
# only doing the configure failed before #11743 fix
t.run("build .")
assert "AGGREGATED LIBS: boost::B;boost::A" in t.out
Copy link
Contributor Author

@lasote lasote Aug 2, 2022

Choose a reason for hiding this comment

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

These are the targets, in previous versions of Conan in that variable appeared the path to the libraries files, except for the dependencies that were also targets and are kept as targets

@lasote lasote marked this pull request as ready for review August 3, 2022 06:30
Copy link
Contributor

@jcar87 jcar87 left a comment

Choose a reason for hiding this comment

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

LGTM!! This was a tricky one!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants