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

Toolchain declare DIR find package variables #9032

Merged

Conversation

lasote
Copy link
Contributor

@lasote lasote commented May 31, 2021

Changelog: Feature: The conan_toolchain.cmake now includes xxx_DIR variables for the dependencies to ease the find_package mechanism to locate them. The declaration of these directories is a must when cross-building in OSX where CMake ignores CMAKE_PREFIX_PATH and CMAKE_MODULE_PATH to look only at the system framework directories.
Docs: conan-io/docs#2108

@lasote lasote requested a review from memsharded May 31, 2021 14:10
@lasote lasote added this to the 1.38 milestone May 31, 2021
conan/tools/cmake/toolchain.py Outdated Show resolved Hide resolved
conan/tools/cmake/toolchain.py Outdated Show resolved Hide resolved

def get_file_name(conanfile):
"""Get the name of the file for the find_package(XXX)"""
ret = conanfile.new_cpp_info.get_property("cmake_file_name", "CMakeDeps")
Copy link
Member

Choose a reason for hiding this comment

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

This is not very great now, that it depends explicitly on CMakeDeps, while it has been made common and is being used not in CMakeDeps, but in CMakeToolchain. I understand why, but reads a bit confusing if you don't know it.

Copy link
Contributor Author

@lasote lasote Jun 1, 2021

Choose a reason for hiding this comment

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

It is used in both of them. Maybe that property shouldn't be attached to the CMakeDeps generator by the recipe creator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a comment. Suggestion welcome.

Copy link
Member

Choose a reason for hiding this comment

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

I am not fully convinced on the proposition of both having the property name and the specific generator as a specialization. We might want to revisit that, but yes, I guess at the moment this is fine here.

conan/tools/cmake/toolchain.py Outdated Show resolved Hide resolved

def get_file_name(conanfile):
"""Get the name of the file for the find_package(XXX)"""
ret = conanfile.new_cpp_info.get_property("cmake_file_name", "CMakeDeps")
Copy link
Member

Choose a reason for hiding this comment

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

I am not fully convinced on the proposition of both having the property name and the specific generator as a specialization. We might want to revisit that, but yes, I guess at the moment this is fine here.

@@ -409,10 +401,17 @@ def context(self):

os_ = self._conanfile.settings.get_safe("os")
android_prefix = "${CMAKE_CURRENT_LIST_DIR}" if os_ == "Android" else None

host_req = self._conanfile.dependencies.transitive_host_requires
find_names_needed = os_ in ('iOS', "watchOS", "tvOS")
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't happen in OSX, right, only for these systems?

@memsharded memsharded merged commit f809644 into conan-io:develop Jun 7, 2021
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

2 participants