-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add cmake_extra_variables
property for CMakeConfigDeps
and CMakeDeps
#18822
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
base: develop2
Are you sure you want to change the base?
Add cmake_extra_variables
property for CMakeConfigDeps
and CMakeDeps
#18822
Conversation
|
||
def is_multi_configuration(generator): | ||
if not generator: | ||
return False | ||
return "Visual" in generator or "Xcode" in generator or "Multi-Config" in generator | ||
|
||
|
||
def parse_extra_variable(source, key, value): |
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.
Not a fan of moving this to a utils file, but didn't find a better suited place to share between CMakeToolchain and CMakeConfigDeps, happy to hear alternatives
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.
Looks good to me.
test/functional/toolchains/cmake/cmakedeps2/test_cmakeconfigdeps_new.py
Outdated
Show resolved
Hide resolved
test/functional/toolchains/cmake/cmakedeps2/test_cmakeconfigdeps_new.py
Outdated
Show resolved
Hide resolved
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.
Maybe we need to provide this for CMakeDeps
as well? As CMakeConfigDeps
will still take a bit to be production ready, do we want to have this available for ConanCenter sooner?
@@ -28,6 +28,19 @@ def additional_variables_prefixes(self): | |||
self.cmakedeps.get_property("cmake_additional_variables_prefixes", self.conanfile, check_type=list) or []) | |||
return list(set([self.file_name] + prefix_list)) | |||
|
|||
@property | |||
def parsed_extra_variables(self): |
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.
Note quite sure if the config is the best place for this, I write them here for symmetry with CMakeConfigDeps where they are written next to the cmake_additional_variables_prefixes
too, maybe we want to move them elsewhere?
cmake_extra_variables
property for CMakeConfigDeps
cmake_extra_variables
property for CMakeConfigDeps
cmake_extra_variables
property for CMakeConfigDeps
cmake_extra_variables
property for CMakeConfigDeps
and CMakeDeps
I would probably suggest:
And my only comment is to make sure that we generate these variables (the |
It was named
Yes ,this is already this way in |
the recommendation for what variables should be covered by so it's probably okay for a toolchain related recipe (e.g. if I have a recipe for a compiler, or a build tool, and there's something for convenience that I'd like users to generate in a toolchain file), but less okay to use in a recipe for a library (our goal for this is to match upstream config files generated by the project, and those cannot define variables for a toolchain (obviously, as by the time you reach find_package, project() has already run). There are no uses of The only concern would be, users relying on |
Yes, it is very rarely used there, it is not the intention. But it is widely used by users defining it in profile files. My concern is a user defining If this is the case, at this moment, the dependency value will have priority over the profile value, which is unexpected and can be frustrating for users. |
Changelog: Feature: Add
cmake_extra_variables
property forCMakeConfigDeps
Changelog: Feature: Add
cmake_extra_variables
property forCMakeDeps
Docs: TODO
Closes #18683, this comes from a discussion with the team where it could help around some shortcomings in a few CCI recipes