-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
CMake Conan 2.0 compatibility #13739
Changes from 19 commits
8014395
3279e24
312df47
3498d3f
bb9383d
31ab1a3
05f13da
5769d2c
b00e0bd
520b6e1
4be0900
8a86742
cb56f47
fe4cb86
8154df4
887635c
a4b5d3a
cede0fe
b6accd3
8c33609
f0fea9e
2e008ee
ed5ef30
0485ca3
4713991
4354d5e
e582a53
be5ffcf
568f04c
5836bfc
19f2c05
dd2bdee
58ff30a
c9345af
bb59eb4
6b5e6e2
b100df8
7fa560a
3bf716f
8fa91ae
182c7d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,24 @@ | ||
import os | ||
from conan import ConanFile | ||
from conan.tools.files import chdir, copy, rmdir, get | ||
from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout | ||
from conan.tools.gnu import Autotools, AutotoolsToolchain | ||
from conan.tools.layout import basic_layout | ||
from conan.tools.build import build_jobs, cross_building, check_min_cppstd | ||
from conan.tools.scm import Version | ||
from conan.tools.files import rmdir, get | ||
from conans import tools, AutoToolsBuildEnvironment, CMake | ||
from conan.errors import ConanInvalidConfiguration, ConanException | ||
from conan.errors import ConanInvalidConfiguration | ||
from conan.errors import ConanException | ||
import os | ||
|
||
required_conan_version = ">=1.49.0" | ||
required_conan_version = ">=1.53.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does it need 1.53.0? 1.50.0 seems to be enough There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trying to stay current with what CCI is using although, regrettably, it is several releases out-of-date despite guidance to always test with the latest release There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's orthogonal. required_conan_version should be the minimum conan version being able to execute this conanfile, not conan version running in c3i. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The c3i version of Conan is the only one that (as far as I know) the recipes get tested with, so unless one does off-line regression testing, there is no guarantee that an earlier version is suitable--especially across the large number of configurations that are tested here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. c3i doesn't test 1.54.0 or 1.55.0 either, so it's not how we reason. It's like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said it's orthogonal, please change required_conan_version to >=1.50.0 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @prince-chrismc, HI @memsharded, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as we are concerned from our side, we appreciate that this is a community-lead effort, and we do not expect contributors to spend significant effort in working out the exact minimum Conan version with which a recipe is compatible, although we do please ask that it is not any lower than what is actually needed, so that users will get a clear error that they can resolve for themselves if they are using a version of Conan that is too old for the recipe. Indeed, our CI does not check this so this is a best-effort situation. If this recipe were to be merged with a version higher than strictly necessary I would not expect significant disruption at this point - given that theres a great number of recipes already requiring 1.53, and we do expect users of Conan Center to be on recent versions - chances are most users already are, making this a moot point by now. On the other hand, if a reviewer kindly spots that this as an improvement, there's no harm in addressing it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn’t agree more, it's best effort to accurately get the true "minimum" and a good default is the one being tested in CCI. Most often we are updating recipes to that latest version so generally those coincide. I like to drop links with "hey this looks like the most recent feature being used" such that I can politely get them lower it when I know it's too high. If a reviewer shows an accurate version, that would improve the quality of the recipe making it more widely available which is not a bad things. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be transparent, the behind the scenes we've had lots of conversations about the role of the linters. Just my quick summary
I personally would love to see the pylint improved with a table of "these imports were added in this version" and double check that and report an error. However unless it's in the linter and enforced (and my example is not enough to guarantee that) its best effort to get the exactly right one. we kindly ask contributors do their best and reviewers to helpfully share details they know so together we get the best recipe. |
||
|
||
class CMakeConan(ConanFile): | ||
name = "cmake" | ||
package_type = "application" | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
description = "Conan installer for CMake" | ||
topics = ("cmake", "build", "installer") | ||
url = "https://github.com/conan-io/conan-center-index" | ||
homepage = "https://github.com/Kitware/CMake" | ||
license = "BSD-3-Clause" | ||
generators = "cmake" | ||
settings = "os", "arch", "compiler", "build_type" | ||
|
||
options = { | ||
|
@@ -26,9 +30,6 @@ class CMakeConan(ConanFile): | |
"bootstrap": False, | ||
} | ||
|
||
_source_subfolder = "source_subfolder" | ||
_cmake = None | ||
|
||
def config_options(self): | ||
if self.settings.os == "Windows": | ||
self.options.with_openssl = False | ||
|
@@ -37,16 +38,16 @@ def requirements(self): | |
if self.options.with_openssl: | ||
self.requires("openssl/1.1.1q") | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def validate(self): | ||
def validate_build(self): | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if self.settings.os == "Macos" and self.settings.arch == "x86": | ||
raise ConanInvalidConfiguration("CMake does not support x86 for macOS") | ||
|
||
if self.settings.os == "Windows" and self.options.bootstrap: | ||
raise ConanInvalidConfiguration("CMake does not support bootstrapping on Windows") | ||
|
||
minimal_cpp_standard = "11" | ||
if self.settings.compiler.cppstd: | ||
tools.check_min_cppstd(self, minimal_cpp_standard) | ||
if self.settings.get_safe("compiler.cppstd"): | ||
check_min_cppstd(self, minimal_cpp_standard) | ||
|
||
minimal_version = { | ||
"gcc": "4.8", | ||
|
@@ -57,9 +58,9 @@ def validate(self): | |
|
||
compiler = str(self.settings.compiler) | ||
if compiler not in minimal_version: | ||
self.output.warn( | ||
self.output.warning( | ||
"{} recipe lacks information about the {} compiler standard version support".format(self.name, compiler)) | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.output.warn( | ||
self.output.warning( | ||
"{} requires a compiler that supports at least C++{}".format(self.name, minimal_cpp_standard)) | ||
return | ||
|
||
|
@@ -68,71 +69,69 @@ def validate(self): | |
raise ConanInvalidConfiguration( | ||
"{} requires a compiler that supports at least C++{}".format(self.name, minimal_cpp_standard)) | ||
|
||
def layout(self): | ||
if self.options.bootstrap: | ||
basic_layout(self, src_folder="src") | ||
else: | ||
cmake_layout(self, src_folder="src") | ||
|
||
def source(self): | ||
get(self, **self.conan_data["sources"][self.version], strip_root=True, destination=self._source_subfolder) | ||
get(self, **self.conan_data["sources"][self.version], | ||
destination=self.source_folder, strip_root=True) | ||
|
||
def _configure_cmake(self): | ||
if not self._cmake: | ||
self._cmake = CMake(self) | ||
def generate(self): | ||
if self.options.bootstrap: | ||
tc = AutotoolsToolchain(self) | ||
tc.generate() | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
else: | ||
tc = CMakeToolchain(self) | ||
if not self.settings.compiler.cppstd: | ||
self._cmake.definitions["CMAKE_CXX_STANDARD"] = 11 | ||
self._cmake.definitions["CMAKE_BOOTSTRAP"] = False | ||
tc.variables["CMAKE_CXX_STANDARD"] = 11 | ||
tc.variables["CMAKE_BOOTSTRAP"] = False | ||
if self.settings.os == "Linux": | ||
self._cmake.definitions["CMAKE_USE_OPENSSL"] = self.options.with_openssl | ||
tc.variables["CMAKE_USE_OPENSSL"] = self.options.with_openssl | ||
if self.options.with_openssl: | ||
self._cmake.definitions["OPENSSL_USE_STATIC_LIBS"] = not self.options["openssl"].shared | ||
if tools.cross_building(self): | ||
self._cmake.definitions["HAVE_POLL_FINE_EXITCODE"] = '' | ||
self._cmake.definitions["HAVE_POLL_FINE_EXITCODE__TRYRUN_OUTPUT"] = '' | ||
self._cmake.configure(source_folder=self._source_subfolder) | ||
|
||
return self._cmake | ||
openssl = self.dependencies["openssl"] | ||
tc.variables["OPENSSL_USE_STATIC_LIBS"] = not openssl.options.shared | ||
if cross_building(self): | ||
tc.variables["HAVE_POLL_FINE_EXITCODE"] = '' | ||
tc.variables["HAVE_POLL_FINE_EXITCODE__TRYRUN_OUTPUT"] = '' | ||
tc.generate() | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def build(self): | ||
if self.options.bootstrap: | ||
with tools.chdir(self._source_subfolder): | ||
self.run(['./bootstrap', '--prefix={}'.format(self.package_folder), '--parallel={}'.format(tools.cpu_count())]) | ||
autotools = AutoToolsBuildEnvironment(self) | ||
with chdir(self, self.source_folder): | ||
self.run('./bootstrap --prefix="" --parallel={}'.format(build_jobs(self))) | ||
autotools = Autotools(self) | ||
autotools.make() | ||
else: | ||
tools.replace_in_file(os.path.join(self._source_subfolder, "CMakeLists.txt"), | ||
"project(CMake)", | ||
"project(CMake)\ninclude(\"{}/conanbuildinfo.cmake\")\nconan_basic_setup(NO_OUTPUT_DIRS)".format( | ||
self.install_folder.replace("\\", "/"))) | ||
if self.settings.os == "Linux": | ||
tools.replace_in_file(os.path.join(self._source_subfolder, "Utilities", "cmcurl", "CMakeLists.txt"), | ||
"list(APPEND CURL_LIBS ${OPENSSL_LIBRARIES})", | ||
"list(APPEND CURL_LIBS ${OPENSSL_LIBRARIES} ${CMAKE_DL_LIBS} pthread)") | ||
|
||
cmake = self._configure_cmake() | ||
cmake = CMake(self) | ||
cmake.configure() | ||
cmake.build() | ||
|
||
def package(self): | ||
self.copy("Copyright.txt", dst="licenses", src=self._source_subfolder) | ||
copy(self, "Copyright.txt", self.source_folder, os.path.join(self.package_folder, "licenses"), keep_path=False) | ||
if self.options.bootstrap: | ||
with tools.chdir(self._source_subfolder): | ||
autotools = AutoToolsBuildEnvironment(self) | ||
with chdir(self, self.source_folder): | ||
autotools = Autotools(self) | ||
autotools.install() | ||
else: | ||
cmake = self._configure_cmake() | ||
cmake = CMake(self) | ||
cmake.configure() | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
cmake.install() | ||
rmdir(self, os.path.join(self.package_folder, "doc")) | ||
|
||
def package_id(self): | ||
del self.info.settings.compiler | ||
del self.info.options.bootstrap | ||
|
||
def package_info(self): | ||
module_version = "{}.{}".format(Version(self.version).major, Version(self.version).minor) | ||
|
||
bindir = os.path.join(self.package_folder, "bin") | ||
self.output.info("Appending PATH environment variable: {}".format(bindir)) | ||
self.env_info.PATH.append(bindir) | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
self.buildenv_info.prepend_path("CMAKE_ROOT", self.package_folder) | ||
self.env_info.CMAKE_ROOT = self.package_folder | ||
self.runenv_info.define_path("CMAKE_ROOT", self.package_folder) | ||
module_version = "{}.{}".format(Version(self.version).major, Version(self.version).minor) | ||
System-Arch marked this conversation as resolved.
Show resolved
Hide resolved
|
||
mod_path = os.path.join(self.package_folder, "share", f"cmake-{module_version}", "Modules") | ||
self.buildenv_info.prepend_path("CMAKE_MODULE_PATH", mod_path) | ||
self.env_info.CMAKE_MODULE_PATH = mod_path | ||
self.runenv_info.define_path("CMAKE_MODULE_PATH", mod_path) | ||
if not os.path.exists(mod_path): | ||
raise ConanException("Module path not found: %s" % mod_path) | ||
|
||
|
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.
No AutotoolsDeps? Is it robust?
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.
Could you please elaborate, not an autotools users so I am not sure what you might be referring too?
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.
OpenSSL is an optional dependency. AutotoolsDeps injects dependencies informations in compile and link flags of autotools, so that it can work out of the box. Usually if upstram configure.ac relies on pkgconf to find & inject these flags, AutotoolsDeps is not needed (but PkgConfigDeps is), otherwise it may be fragile and AutotoolsDeps is here to improve robustness.
Legacy AutotoolsBuildEnvironment was like an union of AutotoolsToolchain, AutotoolsDeps & Autotools conan v2 build helpers.