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
Specify build modules by the generator in cpp_info #8232
Specify build modules by the generator in cpp_info #8232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is more explicit behavior, but it will break existing behavior for recipes that are appending files to cpp_info.build_modules.append(...)
, will they lose all the build_modules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As commented by @jgsogo if we would like to keep the previous "global" build_modules so we don't break to users (even if experimental), I would vote for adding the functionality for new generators only, that do not need an aggregated dict of build_modules, because every file will include its own ones.
Added the possibility of reading build_modules as a list and adding the specific |
conans/test/functional/generators/cmake_find_package_multi_test.py
Outdated
Show resolved
Hide resolved
@@ -557,37 +557,49 @@ def setUp(self): | |||
cpp_info = CppInfo(ref.name, "dummy_root_folder1") | |||
cpp_info.filter_empty = False # For testing purposes only | |||
cpp_info.name = ref.name | |||
cpp_info.build_modules = ["my-module.cmake"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will the assignment fail? I see the existing implementation keeps append()
and extend()
, but not sure about the build_modules = ["..."]
one. Would this be breaking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, you have a specific test for that case here https://github.com/conan-io/conan/pull/8232/files#diff-3779db7bbc92daafae06a9e61c096a7970fe9fb3e9345183947061183d7c5612R249
Co-authored-by: James <james@conan.io>
@@ -390,7 +390,7 @@ def package_info(self): | |||
""") | |||
# This is a module that defines some functionality | |||
find_module = textwrap.dedent(""" | |||
function(conan_message MESSAGE_OUTPUT) | |||
function(custom_message MESSAGE_OUTPUT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to avoid using conan_message()
defined by the generator
Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a note to the changelog about "backward compatibility for *.cmake
build-modules added at global scope, but not for file extensions".
added to the changelog! |
Changelog: Feature: Specify build modules by the generator in
cpp_info
. Added backwards compatibility for*.cmake
build modules added at global scope, but not for other file extensions.Docs: conan-io/docs#1986
develop
branch, documenting this one.