Skip to content

CMakeDeps managing CMAKE_MAP_IMPORTED_CONFIG_XXX - #12049

Merged
jcar87 merged 9 commits into
conan-io:developfrom
lasote:feature/test_cmakedeps_config_mapping
Nov 22, 2022
Merged

jcar87 merged 9 commits into
conan-io:developfrom
lasote:feature/test_cmakedeps_config_mapping

Conversation

@lasote

@lasote lasote commented Sep 7, 2022

Copy link
Copy Markdown
Contributor

Changelog: Bugfix: The CMakeDeps generator was not working properly when the consumer maps configurations of the dependencies using CMAKE_MAP_IMPORTED_CONFIG_XXX.
Docs: conan-io/docs#2826

Close #12041

@lasote lasote added this to the 1.53 milestone Sep 7, 2022
@lasote
lasote requested review from jcar87 and memsharded September 7, 2022 08:04

@memsharded memsharded left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good, a bit risky, but IMPORTED is what should be used, and config suffix too

@czoido czoido modified the milestones: 1.53, 1.54 Oct 4, 2022
@memsharded memsharded modified the milestones: 1.54, 1.55 Nov 2, 2022

@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.

This looks good to me with two caveats:

  • Without this fix, a situation where a consumer project as "Debug" as the build type, and the dependencies where installed as "Release" - CMake gives you a warning (or an error dependency on policy). The warning is indicative of the mismatch and the potential problems. With this fix in the same situation, there's no longer a CMake warning, but compilation will fail. - this may make the user experience confusing.
    If the consumer project does not explicitly do mapping, the observed behaviour is the same with and without this PR.

  • The CMAKE_MAP_IMPORTED_CONFIG_ in the consumers side accept, as value, a list, not just a single config. If we have a situation where a consumer runs conan install for multiple build types, and the CMake side has a configuration that can be satisfied by "any" of those - they could express something like set(CMAKE_MAP_IMPORTED_CONFIG_RELEASE Release RelWithDebInfo MinSizeRel Debug ""). I'd say this is very unusual, but as explained in this CMake issue, that would cause multiple generator expressions to be matched, likely resulting in libraries from different build times to be simultaneously linked...

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.

[bug] MAP_IMPORTED_CONFIG_* does not work in CMakeDeps

4 participants