Skip to content

Refactored CMakeDep using always targets instead of lists - #11788

Merged
czoido merged 4 commits into
conan-io:release/1.51from
lasote:bugfix/cmake_deps_fixes
Aug 8, 2022
Merged

czoido merged 4 commits into
conan-io:release/1.51from
lasote:bugfix/cmake_deps_fixes

Conversation

@lasote

@lasote lasote commented Aug 5, 2022

Copy link
Copy Markdown
Contributor

Changelog: Bugfix: Refactored CMakeDep using always targets instead of lists.
Docs: omit

The bug appeared when components require system libraries, and those were not linked to the IMPORTED but to the component target. CMake was not able to order the system libraries correctly causing linker errors. So it has been fixed using always targets instead of lists, the dependencies of a package/component (such as the system libraries, apple frameworks, and transitive dependencies) are linked to an INTERFACE target that later is linked to the IMPORTED targets when cpp_info.libs are declared, otherwise to the global or component target.

Close #11773

@lasote
lasote requested review from czoido and jcar87 August 5, 2022 10:31
@lasote lasote added this to the 1.51.1 milestone Aug 5, 2022
set(CMAKE_PREFIX_PATH {{ '${' }}{{ pkg_name }}_BUILD_DIRS{{ config_suffix }}} {{ '${' }}CMAKE_PREFIX_PATH})

{% if not components_names %}
########## GLOBAL TARGET PROPERTIES {{ configuration }} ########################################

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for this diff, it is mostly tabulation of the CMake code, I think now it is much more readable and the generated files don't look so bad, actually improves readability too.

@jcar87 jcar87 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@lasote
lasote marked this pull request as ready for review August 5, 2022 13:17
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.

3 participants