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

Fix filename in FindPackageHandleStandardArgs for cmake_find_package generator #7610

Merged
merged 4 commits into from Aug 30, 2020

Conversation

danimtb
Copy link
Member

@danimtb danimtb commented Aug 28, 2020

Changelog: Bugfix: Fix cpp_info filename in FindPackageHandleStandardArgs for cmake_find_package generator.
Docs: omit

#tags: slow

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@danimtb danimtb added this to the 1.28.2 milestone Aug 28, 2020
@danimtb danimtb requested a review from jgsogo August 28, 2020 12:14
@uilianries
Copy link
Member

This PR should solve Protobuf names: https://cmake.org/cmake/help/v3.9/module/FindProtobuf.html 💯

@jgsogo
Copy link
Contributor

jgsogo commented Aug 28, 2020

I see _VERSION, at least for Protobuf, follows the filename and not the namespace. With this change it would still be protobuf_VERSION instead of Protobuf_VERSION, right?

@danimtb
Copy link
Member Author

danimtb commented Aug 28, 2020

@jgsogo I did not think about this issue but I think it will be useful to have that change as well

@jgsogo
Copy link
Contributor

jgsogo commented Aug 28, 2020

I think that the orthodox way is to use the filename for all these variables involved in the FindPackageHandleStandardArgs. Again, I'm sure there are exceptions out there 😢 , but I really think those exceptions should be handle by some build-module, we cannot add more items to cpp_info just because CMake 🥊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants