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

[fmt] conan v2 compatibility #10456

Merged
merged 26 commits into from
Jul 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 0 additions & 7 deletions recipes/fmt/all/CMakeLists.txt

This file was deleted.

1 change: 0 additions & 1 deletion recipes/fmt/all/conandata.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ sources:
patches:
"5.3.0":
- patch_file: "patches/fix-install-5.3.0.patch"
base_path: "source_subfolder"
# "6.0.0":
# - patch_file: "patches/fix-install-6.0.0.patch"
# base_path: "source_subfolder"
Expand Down
121 changes: 61 additions & 60 deletions recipes/fmt/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import os
import shutil

from conan.tools.microsoft import msvc_runtime_flag
from conans import ConanFile, CMake, tools
from conans.errors import ConanInvalidConfiguration
from conan import ConanFile
from conan.tools.scm import Version
from conan.tools.cmake import CMake, CMakeToolchain, cmake_layout
from conan.tools.files import get, apply_conandata_patches, copy, rmdir

required_conan_version = ">=1.43.0"

required_conan_version = ">=1.47.0"


class FmtConan(ConanFile):
Expand All @@ -14,6 +17,7 @@ class FmtConan(ConanFile):
topics = ("fmt", "format", "iostream", "printf")
url = "https://github.com/conan-io/conan-center-index"
license = "MIT"
exports_sources = "patches/*"

settings = "os", "arch", "compiler", "build_type"
options = {
Expand All @@ -31,29 +35,17 @@ class FmtConan(ConanFile):
"with_os_api": True,
}

generators = "cmake"
_cmake = None

@property
def _source_subfolder(self):
return "source_subfolder"

@property
def _build_subfolder(self):
return "build_subfolder"

@property
def _is_msvc(self):
return str(self.settings.compiler) in ["Visual Studio", "msvc"]

@property
def _has_with_os_api_option(self):
return tools.Version(self.version) >= "7.0.0"
return Version(str(self.version)) >= "7.0.0"

def export_sources(self):
self.copy("CMakeLists.txt")
for patch in self.conan_data.get("patches", {}).get(self.version, []):
self.copy(patch["patch_file"])
def generate(self):
if not self.options.header_only:
tc = CMakeToolchain(self)
tc.generate()

def layout(self):
cmake_layout(self)

def config_options(self):
if self.settings.os == "Windows":
Expand All @@ -71,65 +63,74 @@ def configure(self):
elif self.options.shared:
del self.options.fPIC

def validate(self):
if self.options.get_safe("shared") and self._is_msvc and "MT" in msvc_runtime_flag(self):
raise ConanInvalidConfiguration(
"Visual Studio build for shared library with MT runtime is not supported"
)

def package_id(self):
if self.options.header_only:
if self.info.options.header_only: # might be changed to self.info.clear() in 1.50
self.info.header_only()
else:
del self.info.options.with_fmt_alias

def source(self):
tools.get(**self.conan_data["sources"][self.version],
destination=self._source_subfolder, strip_root=True)

def _configure_cmake(self):
if self._cmake:
return self._cmake
self._cmake = CMake(self)
self._cmake.definitions["FMT_DOC"] = False
self._cmake.definitions["FMT_TEST"] = False
self._cmake.definitions["FMT_INSTALL"] = True
self._cmake.definitions["FMT_LIB_DIR"] = "lib"
if self._has_with_os_api_option:
self._cmake.definitions["FMT_OS"] = self.options.with_os_api
self._cmake.configure(build_folder=self._build_subfolder)
return self._cmake
get(self, **self.conan_data["sources"][str(self.version)], strip_root=True)

def build(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
tools.patch(**patch)
apply_conandata_patches(self)
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
if not self.options.header_only:
cmake = self._configure_cmake()
cmake = CMake(self)
# FIXME : https://github.com/conan-io/conan/issues/11476
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
# can be replaced by https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html#cache-variables in 1.50
cache_entries = {
"FMT_DOC": "False",
"FMT_TEST": "False",
"FMT_INSTALL": "True",
"FMT_LIB_DIR": "lib"
}
if self._has_with_os_api_option:
cache_entries["FMT_OS"] = self.options.with_os_api
Copy link
Member

Choose a reason for hiding this comment

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

This should be at the CMakeToolchain level, that allows local user builds to be consistent with options

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can't be changed until c3i uses Conan 1.50 and then must be translated to https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html#cache-variables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CCI uses 1.49 ATM

Copy link
Contributor

Choose a reason for hiding this comment

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

I am curious about the other variable mechanics https://docs.conan.io/en/latest/reference/conanfile/tools/cmake/cmaketoolchain.html#variables

If the toolchain sets the value before the options are declared then they do nothing according to https://cmake.org/cmake/help/latest/command/option.html

If this is set to use 1.49 then I think we could get away for using just tc.variables however I agree the cache ones are better UX this seems to be working for my project

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there was a bug using tc.variables had a zero effect. it causes CI to build more things than needed (e.g. it build tests that were intended to be disabled via FMT_TEST).
it turns out to be that diff:
https://github.com/fmtlib/fmt/blob/f94b7364b9409f05207c3af3fa4666730e11a854/CMakeLists.txt#L41-L49
https://github.com/fmtlib/fmt/blob/b6f4ceaed0a0a24ccf575fab6c56dd50ccf6f1a9/CMakeLists.txt#L66-L81
in general it harmless in this particular case (just causes longer builds), but in other cases it can produce a wrong package if some option is ignored.
more info at conan-io/conan#11476

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhh, there will need to be a hook so people dont use the other one in CCI 🙈

The options before project is an obvious limitation + it's easy to imagine the separate steps for the c3i could be problematic

Thanks for filling in the details

cmake.configure(variables=cache_entries)
cmake.build()

def package(self):
self.copy("LICENSE.rst", dst="licenses", src=self._source_subfolder)
copy(self, pattern="*LICENSE.rst", src=self.source_folder, dst=os.path.join(self.package_folder, "licenses"))
if self.options.header_only:
self.copy("*.h", dst="include", src=os.path.join(self._source_subfolder, "include"))
copy(self, pattern="*.h", src=os.path.join(self.source_folder, "include"), dst=os.path.join(self.package_folder, "include"))
else:
cmake = self._configure_cmake()
cmake = CMake(self)
cmake.install()
tools.rmdir(os.path.join(self.package_folder, "lib", "cmake"))
tools.rmdir(os.path.join(self.package_folder, "lib", "pkgconfig"))
tools.rmdir(os.path.join(self.package_folder, "share"))
rmdir(self, os.path.join(self.package_folder, "lib", "cmake"))
rmdir(self, os.path.join(self.package_folder, "lib", "pkgconfig"))
rmdir(self, os.path.join(self.package_folder, "res"))
rmdir(self, os.path.join(self.package_folder, "share"))

def package_info(self):
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
self.cpp_info.names["cmake_find_package"] = "fmt"
self.cpp_info.names["cmake_find_package_multi"] = "fmt"
self.cpp_info.names["pkg_config"] = "fmt"
SSE4 marked this conversation as resolved.
Show resolved Hide resolved

target = "fmt-header-only" if self.options.header_only else "fmt"
self.cpp_info.set_property("cmake_target_name", "fmt::{}".format(target))

# TODO: back to global scope in conan v2 once cmake_find_package* generators removed
self.cpp_info.components["_fmt"].includedirs.extend(["include"])
if self.options.header_only:
self.cpp_info.components["fmt-header-only"].defines.append("FMT_HEADER_ONLY=1")
self.cpp_info.components["_fmt"].defines.append("FMT_HEADER_ONLY=1")
if self.options.with_fmt_alias:
self.cpp_info.components["fmt-header-only"].defines.append("FMT_STRING_ALIAS=1")
self.cpp_info.components["_fmt"].defines.append("FMT_STRING_ALIAS=1")
else:
postfix = "d" if self.settings.build_type == "Debug" else ""
self.cpp_info.libs = ["fmt" + postfix]
libname = "fmt" + postfix
self.cpp_info.components["_fmt"].libs = [libname]
if self.settings.os == "Linux":
self.cpp_info.components["_fmt"].system_libs.extend(["m"])
# FIXME: remove when Conan 1.50 is used in c3i and update the Conan required version
# from that version components don't have empty libdirs by default
self.cpp_info.components["_fmt"].libdirs.extend(["lib"])
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
self.cpp_info.components["_fmt"].bindirs.extend(["bin"])
if self.options.with_fmt_alias:
self.cpp_info.defines.append("FMT_STRING_ALIAS=1")
self.cpp_info.components["_fmt"].defines.append("FMT_STRING_ALIAS=1")
if self.options.shared:
self.cpp_info.defines.append("FMT_SHARED")
self.cpp_info.components["_fmt"].defines.append("FMT_SHARED")

# TODO: to remove in conan v2 once cmake_find_package* generators removed
self.cpp_info.components["_fmt"].names["cmake_find_package"] = target
self.cpp_info.components["_fmt"].names["cmake_find_package_multi"] = target
self.cpp_info.components["_fmt"].set_property("cmake_target_name", "fmt::{}".format(target))
30 changes: 0 additions & 30 deletions recipes/fmt/all/test_cmakedeps/conanfile.py

This file was deleted.

3 changes: 0 additions & 3 deletions recipes/fmt/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
cmake_minimum_required(VERSION 3.15)
project(test_package CXX)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

find_package(fmt REQUIRED CONFIG)

# TEST_PACKAGE #################################################################
Expand Down
29 changes: 21 additions & 8 deletions recipes/fmt/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,32 @@
import os
from conan import ConanFile
from conan.tools.cmake import CMake, CMakeToolchain
from conan.tools.build import can_run
from conan.tools.cmake import cmake_layout

from conans import ConanFile, CMake, tools

required_conan_version = ">=1.43.0"

class TestPackageConan(ConanFile):
settings = "os", "arch", "compiler", "build_type"
generators = "cmake", "cmake_find_package_multi"
settings = "os", "compiler", "build_type", "arch"
generators = "CMakeDeps", "VirtualRunEnv"

def requirements(self):
self.requires(self.tested_reference_str)

def generate(self):
tc = CMakeToolchain(self)
tc.variables["FMT_HEADER_ONLY"] = self.dependencies["fmt"].options.header_only
tc.generate()

def layout(self):
cmake_layout(self)

def build(self):
cmake = CMake(self)
cmake.definitions["FMT_HEADER_ONLY"] = self.options["fmt"].header_only
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
self.run(os.path.join("bin", "test_package"), run_environment=True)
self.run(os.path.join("bin", "test_ranges"), run_environment=True)
if can_run(self):
self.run(os.path.join(self.cpp.build.bindirs[0], "test_package"), env="conanrun")
self.run(os.path.join(self.cpp.build.bindirs[0], "test_ranges"), env="conanrun")
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
cmake_minimum_required(VERSION 3.15)
project(test_package CXX)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

find_package(fmt REQUIRED CONFIG)

# TEST_PACKAGE #################################################################
Expand Down
19 changes: 19 additions & 0 deletions recipes/fmt/all/test_v1_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# pylint: skip-file
from conans import ConanFile, CMake, tools
SSE4 marked this conversation as resolved.
Show resolved Hide resolved
import os


class TestPackageConan(ConanFile):
settings = "os", "arch", "compiler", "build_type"
generators = "cmake", "cmake_find_package_multi"

def build(self):
cmake = CMake(self)
cmake.definitions["FMT_HEADER_ONLY"] = self.options["fmt"].header_only
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
self.run(os.path.join("bin", "test_package"), run_environment=True)
self.run(os.path.join("bin", "test_ranges"), run_environment=True)
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <vector>
#include <limits>


#include <fmt/format.h>
#include <fmt/printf.h>
#include <fmt/ostream.h>
Expand Down