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

CMakeToolchain: proper support of find_*() & include() commands #10186

Merged
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
f2e977d
CMakeToolchain: proper support of find_*() & include() commands
SpaceIm Dec 16, 2021
f04fb1b
CMakeToolchain: remove find_builddirs
SpaceIm Dec 16, 2021
ff262ba
don't test include() not in toolchain file
SpaceIm Dec 16, 2021
f21ed65
test builddirs other that root package folder
SpaceIm Dec 16, 2021
16c941a
unconditionally force CMAKE_FIND_ROOT_PATH_MODE_*
SpaceIm Dec 16, 2021
dba1989
CMakeToolchain: android workaround not needed anymore
SpaceIm Dec 16, 2021
be4b91d
CMakeToolchain: fix comments
SpaceIm Dec 16, 2021
5204b73
cmaketoolchain: allow to find host executables if not cross-building
SpaceIm Dec 17, 2021
9aa71cf
CMakeToolchain: intermediate module for host & build context
SpaceIm Dec 17, 2021
029dfc0
factorize
SpaceIm Dec 17, 2021
ab11029
rename variables to distinguish host & build
SpaceIm Dec 17, 2021
75812f4
allow to add root package folder in cmake_module_path
SpaceIm Dec 17, 2021
858fab8
Merge branch 'develop' into feature/cmake-toolchain-robust-find-commands
SpaceIm Dec 18, 2021
e34655a
test include() of cmake modules from requires & build_requires
SpaceIm Dec 19, 2021
f9978e8
tests of find_file()/find_path()/find_library()/find_program()
SpaceIm Dec 19, 2021
e020731
fix test_cmaketoolchain_path_find_program test on windows
SpaceIm Dec 19, 2021
89e1e40
allow again to add root package folder to CMAKE_PREFIX_PATH
SpaceIm Dec 19, 2021
46cc18a
Revert "allow again to add root package folder to CMAKE_PREFIX_PATH"
SpaceIm Dec 19, 2021
4b0c6d7
improve test_cmaketoolchain_path_find_program
SpaceIm Dec 19, 2021
aca09f0
simplify tests
SpaceIm Dec 19, 2021
5a1adfc
simplify again
SpaceIm Dec 19, 2021
8aa3443
simplify again
SpaceIm Dec 19, 2021
54cefa9
add cross-build tests
SpaceIm Dec 20, 2021
af6afcf
add cross-build in real config test
SpaceIm Dec 20, 2021
05b83f4
try to honor CMAKE_FIND_ROOT_PATH_MODE_* from user
SpaceIm Dec 20, 2021
e60bfe7
use 2 profiles in test_apple_own_framework_cross_build
SpaceIm Dec 21, 2021
edff4ac
improvement
SpaceIm Dec 21, 2021
cbca474
simplify tests
SpaceIm Dec 21, 2021
b679c0e
factorize ios settings
SpaceIm Dec 21, 2021
905ff07
fix some issues in tests
SpaceIm Dec 21, 2021
6778102
simplify
SpaceIm Dec 21, 2021
68d4616
test typical CMAKE_FIND_ROOT_PATH_MODE_* values
SpaceIm Dec 22, 2021
1e89fe9
simplify
SpaceIm Dec 29, 2021
8a5ee2b
Merge branch 'develop' into feature/cmake-toolchain-robust-find-commands
SpaceIm Jan 1, 2022
c244e30
cleanup
SpaceIm Jan 1, 2022
fd72d4a
Merge branch 'develop' into feature/cmake-toolchain-robust-find-commands
SpaceIm Jan 14, 2022
121b61a
Merge branch 'develop' into feature/cmake-toolchain-robust-find-commands
memsharded Jan 19, 2022
6bb0be8
using CMake list(APPEND instead of set with unitialized variables
memsharded Jan 19, 2022
5d19b4f
fixed absolute paths issues
memsharded Jan 19, 2022
b1d69a7
fix iOS tests missing os.sdk
memsharded Jan 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 79 additions & 40 deletions conan/tools/cmake/toolchain.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
import os
import re
import textwrap
Expand All @@ -11,6 +10,7 @@
from conan.tools._compilers import architecture_flag, use_win_mingw
from conan.tools.build import build_jobs
from conan.tools.cmake.utils import is_multi_configuration
from conan.tools.cross_building import cross_building
from conan.tools.files import save_toolchain_args
from conan.tools.intel import IntelCC
from conan.tools.microsoft import VCVars
Expand Down Expand Up @@ -395,28 +395,56 @@ def context(self):
return ctxt_toolchain


class FindConfigFiles(Block):
class FindFiles(Block):
template = textwrap.dedent("""
{% if find_package_prefer_config %}
set(CMAKE_FIND_PACKAGE_PREFER_CONFIG {{ find_package_prefer_config }})
{% endif %}
# To support the generators based on find_package()
{% if cmake_module_path %}
set(CMAKE_MODULE_PATH {{ cmake_module_path }} ${CMAKE_MODULE_PATH})
{% if generators_folder or host_build_paths_root or host_build_paths_noroot or build_build_paths %}
set(CMAKE_MODULE_PATH {{ generators_folder }} {{ host_build_paths_root }} {{ host_build_paths_noroot }} {{ build_build_paths }} ${CMAKE_MODULE_PATH})
lasote marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% if cmake_prefix_path %}
set(CMAKE_PREFIX_PATH {{ cmake_prefix_path }} ${CMAKE_PREFIX_PATH})
{% if generators_folder or host_build_paths_noroot %}
lasote marked this conversation as resolved.
Show resolved Hide resolved
set(CMAKE_PREFIX_PATH {{ generators_folder }} {{ host_build_paths_noroot }} ${CMAKE_PREFIX_PATH})
{% endif %}
{% if android_prefix_path %}
set(CMAKE_FIND_ROOT_PATH {{ android_prefix_path }} ${CMAKE_FIND_ROOT_PATH})
{% if cmake_program_path %}
set(CMAKE_PROGRAM_PATH {{ cmake_program_path }} ${CMAKE_PROGRAM_PATH})
{% endif %}
{% if cmake_library_path %}
set(CMAKE_LIBRARY_PATH {{ cmake_library_path }} ${CMAKE_LIBRARY_PATH})
{% endif %}
{% if is_apple and cmake_framework_path %}
set(CMAKE_FRAMEWORK_PATH {{ cmake_framework_path }} ${CMAKE_FRAMEWORK_PATH})
lasote marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}
{% if cmake_include_path %}
set(CMAKE_INCLUDE_PATH {{ cmake_include_path }} ${CMAKE_INCLUDE_PATH})
{% endif %}

# To support cross building to iOS, watchOS and tvOS where CMake looks for config files
# only in the system frameworks unless you declare the XXX_DIR variables
{% if cross_ios %}
set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE BOTH)
{% if cross_building %}
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_PACKAGE OR CMAKE_FIND_ROOT_PATH_MODE_PACKAGE STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE "BOTH")
endif()
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_PROGRAM OR CMAKE_FIND_ROOT_PATH_MODE_PROGRAM STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM "BOTH")
endif()
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_LIBRARY OR CMAKE_FIND_ROOT_PATH_MODE_LIBRARY STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY "BOTH")
endif()
{% if is_apple %}
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_FRAMEWORK OR CMAKE_FIND_ROOT_PATH_MODE_FRAMEWORK STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_FRAMEWORK "BOTH")
endif()
{% endif %}
""")
if(NOT DEFINED CMAKE_FIND_ROOT_PATH_MODE_INCLUDE OR CMAKE_FIND_ROOT_PATH_MODE_INCLUDE STREQUAL "ONLY")
set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE "BOTH")
endif()
{% endif %}
""")

@staticmethod
lasote marked this conversation as resolved.
Show resolved Hide resolved
def _join_paths(paths):
return " ".join(['"{}"'.format(p.replace('\\', '/')
.replace('$', '\\$')
.replace('"', '\\"')) for p in paths])

def context(self):
# To find the generated cmake_find_package finders
Expand All @@ -427,32 +455,46 @@ def context(self):
find_package_prefer_config = "OFF"

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

Choose a reason for hiding this comment

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

This was necessary at some point, to have the Android build finding the xxx-config.cmake by Conan. Maybe it is no longer needed, but it is difficult to know, because we are not running yet Android build tests in our CI

Copy link
Contributor Author

@SpaceIm SpaceIm Dec 17, 2021

Choose a reason for hiding this comment

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

Yes, it's the same issue than cross-build to iOS/tvOS/watchOS, but 2 different solutions have been implemented. Here it's fixed by forcing CMAKE_FIND_ROOT_PATH_MODE_PACKAGE to "BOTH" (same fix than iOS/tvOS/watchOS).

Of course, the best solution would be to not have to manipulate CMAKE_FIND_ROOT_PATH_MODE_* but other solutions raise other issues (conflicts between host & build context), and I think this one is harmless compare to other solutions.

is_apple_ = os_ and os_ in ["Macos", "iOS", "watchOS", "tvOS"]

# Read information from host context
host_req = self._conanfile.dependencies.host.values()
cross_ios = os_ in ('iOS', "watchOS", "tvOS")

# Read the buildirs
build_paths = []
host_build_paths_root = []
host_build_paths_noroot = []
host_lib_paths = []
host_framework_paths = []
host_include_paths = []
for req in host_req:
cppinfo = req.cpp_info.aggregated_components()
build_paths.extend([os.path.join(req.package_folder,
p.replace('\\', '/').replace('$', '\\$').replace('"', '\\"'))
for p in cppinfo.builddirs])

if self._toolchain.find_builddirs:
build_paths = " ".join(['"{}"'.format(b.replace('\\', '/')
.replace('$', '\\$')
.replace('"', '\\"')) for b in build_paths])
else:
build_paths = ""

return {"find_package_prefer_config": find_package_prefer_config,
"cmake_prefix_path": "${CMAKE_CURRENT_LIST_DIR} " + build_paths,
"cmake_module_path": "${CMAKE_CURRENT_LIST_DIR} " + build_paths,
"android_prefix_path": android_prefix,
"cross_ios": cross_ios,
"generators_folder": "${CMAKE_CURRENT_LIST_DIR}"}
host_build_paths_root.extend([os.path.join(req.package_folder, p) for p in cppinfo.builddirs if p == ""])
lasote marked this conversation as resolved.
Show resolved Hide resolved
host_build_paths_noroot.extend([os.path.join(req.package_folder, p) for p in cppinfo.builddirs if p != ""])
host_lib_paths.extend([os.path.join(req.package_folder, p) for p in cppinfo.libdirs])
if is_apple_:
host_framework_paths.extend([os.path.join(req.package_folder, p) for p in cppinfo.frameworkdirs])
host_include_paths.extend([os.path.join(req.package_folder, p) for p in cppinfo.includedirs])

# Read information from build context
build_req = self._conanfile.dependencies.build.values()
build_build_paths = []
build_bin_paths = []
for req in build_req:
lasote marked this conversation as resolved.
Show resolved Hide resolved
cppinfo = req.cpp_info.aggregated_components()
build_build_paths.extend([os.path.join(req.package_folder, p) for p in cppinfo.builddirs])
build_bin_paths.extend([os.path.join(req.package_folder, p) for p in cppinfo.bindirs])

return {
"find_package_prefer_config": find_package_prefer_config,
"generators_folder": "${CMAKE_CURRENT_LIST_DIR}",
"host_build_paths_root": self._join_paths(host_build_paths_root),
"host_build_paths_noroot": self._join_paths(host_build_paths_noroot),
"build_build_paths": self._join_paths(build_build_paths),
"cmake_program_path": self._join_paths(build_bin_paths),
"cmake_library_path": self._join_paths(host_lib_paths),
"cmake_framework_path": self._join_paths(host_framework_paths),
"cmake_include_path": self._join_paths(host_include_paths),
"is_apple": is_apple_,
"cross_building": cross_building(self._conanfile),
}


class UserToolchain(Block):
Expand Down Expand Up @@ -740,13 +782,10 @@ def __init__(self, conanfile, generator=None, namespace=None):
("parallel", ParallelBlock),
("cmake_flags_init", CMakeFlagsInitBlock),
("try_compile", TryCompileBlock),
("find_paths", FindConfigFiles),
("find_paths", FindFiles),
("rpath", SkipRPath),
("shared", SharedLibBock)])

# Set the CMAKE_MODULE_PATH and CMAKE_PREFIX_PATH to the deps .builddirs
self.find_builddirs = True

check_using_build_profile(self._conanfile)

def _context(self):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ class HELLO_EXPORT Hello
@pytest.mark.skipif(platform.system() != "Darwin", reason="Only OSX")
@pytest.mark.parametrize("settings",
[('',),
('-s os=iOS -s os.version=10.0 -s arch=armv8',),
("-s os=tvOS -s os.version=11.0 -s arch=armv8",)])
('-pr:b default -s os=iOS -s os.version=10.0 -s arch=armv8',),
("-pr:b default -s os=tvOS -s os.version=11.0 -s arch=armv8",)])
def test_apple_own_framework_cross_build(settings):
client = TestClient()

Expand Down
Loading