-
Notifications
You must be signed in to change notification settings - Fork 981
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
preparing migration of names to CMakeDeps #8568
preparing migration of names to CMakeDeps #8568
Conversation
does easy migration worth making this implicit magic (then setting one generator suddenly impacts on another one)? |
I can try to implement this magic in the |
@solvingj WDYT? my personal opinion: I'd like to avoid hidden magic in 2.0 as much as possible, and prefer explicit approach for everything. |
The problem is that we need users to try it now. There are users that are already using it (@mpusz, for example), and we are breaking them (in the past version I am fine with not mapping CMakeDeps => cmake_find_package_multi, but I strongly believe that at least a mapping cmake_find_package_multi => CMakeDeps is necessary in this release, and until a majority of ConanCenter recipes have started to use |
probably, it's fair, the anyway, up to you, my point is just try to move away from implicit magic things in 2.0, instead of adding more of them. |
Totally, this is a temporary thing, to be removed once ConanCenter has catched up. |
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.
Please, add components to the test.
As this magic is going to be removed in Conan 2.0, I think we should condition this to the conan_v2_mode
(by default the fallback is activated, in CONAN_V2_MODE it isn't). It would be useful to show a warning (or error) if it is consuming the information from cmake_find_package_multi
instead of CMakeDeps
. At some point in time in conan-center-index
we could force the change by activating something that would make PRs fail.
I was thinking about a different approach for this. The issue arises when hardcoding generator-related stuff in recipes and then a generator wanting to use that same information despite the recipe not declaring so. This case is the same when using a custom generator (with a name that it is not built-in in Conan). As a solution, we could provide the information of available generators at def package_info(self):
for generator in self.generators.get_family("cmake"):
self.cpp_info.components["mycomp"].names[generator] = "MySuperPkg1" # Set nfo for all CMake related generators
self.cpp_info.components["mycomp"].names["cmake_find_package"] = "MySuperPkg1" # Override info specific for that generator Just an idea. But probably something to consider regarding this issue with Anyway, I am not blocking this PR and I feel this is something needed to experiment with the CMakeDeps generator probably. As it is experimental I guess this is something we can play with. |
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 that Conan v2 should use a different strategy, maybe it is possible to use just cmake
(or the build system name) as the entry in the dictionary and the generators choose the information to consume. Now it's harder because of the cmake
generator.
I'm not sure if making it too fine-grained (cmake-targets
, cmake-extra
, cmake-???
) will be needed or not.
Ok, lets merge it at the moment, and open an issue for discussion. |
Changelog: Fix: Allow
cmake_find_package_multi
andCMakeDeps
to be aliases forcpp_info.names
andcpp_info.filenames
to allow easy migration.Docs: Omit
cc/ @mpusz