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

CMakeDeps imported target multiconfig #11691

Merged

Conversation

lasote
Copy link
Contributor

@lasote lasote commented Jul 22, 2022

Changelog: omit
Docs: omit

@@ -41,7 +41,7 @@ def template(self):
endif()
endmacro()

function(conan_package_library_targets libraries package_libdir deps out_libraries out_libraries_target config_suffix package_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.

out_libraries arg was not used.

@lasote lasote marked this pull request as ready for review July 26, 2022 13:10
@lasote lasote changed the title WIP CMakeDeps imported target multiconfig CMakeDeps imported target multiconfig Jul 26, 2022
@lasote lasote requested review from memsharded and jcar87 July 26, 2022 13:11
@lasote lasote added this to the 1.51 milestone Jul 27, 2022

# FIXME: What is the result of this for multi-config? All configs adding themselves to path?
set(CMAKE_MODULE_PATH {{ '${' }}{{ pkg_name }}_BUILD_DIRS{{ config_suffix }}} {{ '${' }}CMAKE_MODULE_PATH})
set(CMAKE_PREFIX_PATH {{ '${' }}{{ pkg_name }}_BUILD_DIRS{{ config_suffix }}} {{ '${' }}CMAKE_PREFIX_PATH})

{%- for comp_variable_name, comp_target_name in components_names %}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved below, with the other components code to be easier.

@jcar87 jcar87 merged commit dd4e9fa into conan-io:develop Jul 28, 2022
list(APPEND {{ pkg_name }}_LIBRARIES{{ config_suffix }} ${_SYSTEM_LIB})
endforeach()

# We need to add our requirements too
set({{ pkg_name }}_LIBRARIES_TARGETS{{ config_suffix }} {{ '"${' }}{{ pkg_name }}_LIBRARIES_TARGETS{{ config_suffix }}{{ '};' }}{{ deps_targets_names }}")
set({{ pkg_name }}_LIBRARIES{{ config_suffix }} {{ '"${' }}{{ pkg_name }}_LIBRARIES{{ config_suffix }}{{ '};' }}{{ deps_targets_names }}")
set({{ pkg_name }}_LIBRARIES{{ config_suffix }} "")
Copy link

@alex-700 alex-700 Aug 1, 2022

Choose a reason for hiding this comment

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

Hey, @lasote !

I think that you have broken everything here :) The variable was not cleared here before. It seems strange even when looking to several "list(APPEND ...)" above, which actively push something to this variable.

I didn't understand whole picture though. But the specific problem is: conan=1.51 broke a build of zstd dependant, where I'm using ${zstd_LIBRARIES}, which become empty after update. And I'm pretty sure that bug in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking.

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

4 participants