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

[libiconv] conan v2 update #13263

Merged
merged 7 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 0 additions & 1 deletion recipes/libiconv/all/conandata.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@ sources:
patches:
"1.16":
- patch_file: "patches/0001-libcharset-fix-linkage.patch"
base_path: "source_subfolder"
161 changes: 68 additions & 93 deletions recipes/libiconv/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
from conan.tools.files import rename
from conans import ConanFile, tools, AutoToolsBuildEnvironment
from conan.tools.microsoft import is_msvc
from contextlib import contextmanager
import os
import functools

required_conan_version = ">=1.45.0"
from conan.tools.files import (
apply_conandata_patches,
copy,
export_conandata_patches,
get,
rename,
replace_in_file,
rm,
rmdir
)
from conan import ConanFile
from conan.tools.env import VirtualBuildEnv
from conan.tools.gnu import Autotools, AutotoolsToolchain
from conan.tools.layout import basic_layout
from conan.tools.microsoft import is_msvc, unix_path
from conan.tools.scm import Version

required_conan_version = ">=1.52.0"


class LibiconvConan(ConanFile):
Expand All @@ -26,20 +38,16 @@ class LibiconvConan(ConanFile):
}

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

@property
def _use_winbash(self):
return tools.os_info.is_windows and (self.settings.compiler == "gcc" or tools.cross_building(self))
def _is_clang_cl(self):
return (self.settings.compiler == "clang" and self.settings.os == "Windows") \
or self.settings.get_safe("compiler.toolset") == "ClangCL"

@property
def _is_clang_cl(self):
return self.settings.compiler == "clang" and self.settings.os == "Windows"
def _msvc_tools(self):
return ("clang-cl", "llvm-lib", "lld-link") if self._is_clang_cl else ("cl", "lib", "link")

def export_sources(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
self.copy(patch["patch_file"])
export_conandata_patches(self)

def config_options(self):
if self.settings.os == "Windows":
Expand All @@ -55,99 +63,66 @@ def configure(self):
def _settings_build(self):
return getattr(self, "settings_build", self.settings)

def build_requirements(self):
if self._settings_build.os == "Windows" and not tools.get_env("CONAN_BASH_PATH"):
self.build_requires("msys2/cci.latest")
def layout(self):
basic_layout(self, src_folder="src")

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

Choose a reason for hiding this comment

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

would be nice to move this after source(), otherwise recipe execution flow is a little bit confusing

tc = AutotoolsToolchain(self)
env = tc.environment()

@contextmanager
def _build_context(self):
env_vars = {}
if is_msvc(self) or self._is_clang_cl:
cc = "cl" if is_msvc(self) else os.environ.get("CC", "clang-cl")
cxx = "cl" if is_msvc(self) else os.environ.get("CXX", "clang-cl")
lib = "lib" if is_msvc(self) else os.environ.get("AR", "llvm-lib")
build_aux_path = os.path.join(self.build_folder, self._source_subfolder, "build-aux")
lt_compile = tools.unix_path(os.path.join(build_aux_path, "compile"))
lt_ar = tools.unix_path(os.path.join(build_aux_path, "ar-lib"))
env_vars.update({
"CC": "{} {} -nologo".format(lt_compile, cc),
"CXX": "{} {} -nologo".format(lt_compile, cxx),
"LD": "link",
"STRIP": ":",
"AR": "{} {}".format(lt_ar, lib),
"RANLIB": ":",
"NM": "dumpbin -symbols"
})
env_vars["win32_target"] = "_WIN32_WINNT_VISTA"

if not tools.cross_building(self) or is_msvc(self) or self._is_clang_cl:
rc = None
if self.settings.arch == "x86":
rc = "windres --target=pe-i386"
elif self.settings.arch == "x86_64":
rc = "windres --target=pe-x86-64"
if rc:
env_vars["RC"] = rc
env_vars["WINDRES"] = rc
if self._use_winbash:
env_vars["RANLIB"] = ":"

with tools.vcvars(self.settings) if (is_msvc(self) or self._is_clang_cl) else tools.no_op():
with tools.chdir(self._source_subfolder):
with tools.environment_append(env_vars):
yield

@functools.lru_cache(1)
def _configure_autotools(self):
host = None
build = None
if is_msvc(self) or self._is_clang_cl:
build = False
if self.settings.arch == "x86":
host = "i686-w64-mingw32"
elif self.settings.arch == "x86_64":
host = "x86_64-w64-mingw32"

autotools = AutoToolsBuildEnvironment(self, win_bash=tools.os_info.is_windows)

configure_args = []
if self.options.shared:
configure_args.extend(["--disable-static", "--enable-shared"])
else:
configure_args.extend(["--enable-static", "--disable-shared"])
cc, lib, link = self._msvc_tools
build_aux_path = os.path.join(self.source_folder, "build-aux")
lt_compile = unix_path(self, os.path.join(build_aux_path, "compile"))
lt_ar = unix_path(self, os.path.join(build_aux_path, "ar-lib"))
env.define("CC", f"{lt_compile} {cc} -nologo")
env.define("CXX", f"{lt_compile} {cc} -nologo")
env.define("LD", f"{link}")
env.define("STRIP", ":")
env.define("AR", f"{lt_ar} {lib}")
env.define("RANLIB", ":")
env.define("NM", "dumpbin -symbols")
env.define("win32_target", "_WIN32_WINNT_VISTA")

if is_msvc(self) and Version(self.settings.compiler.version) >= "12":
Copy link
Contributor

@jwillikers jwillikers Oct 3, 2022

Choose a reason for hiding this comment

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

This isn't going to work correctly for the msvc compiler, only Visual Studio. Unfortunately, there won't be a more helpful solution until Conan 1.53.0 for this: conan-io/conan#11158

Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

# tc.extra_cflags is not respected.
# See https://github.com/conan-io/conan/issues/12228
env.define("CFLAGS", tc.cflags + ["-FS"])

tc.generate(env)

env = VirtualBuildEnv(self)
env.generate()
Comment on lines +100 to +101
Copy link
Contributor

@SpaceIm SpaceIm Oct 6, 2022

Choose a reason for hiding this comment

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

should be called before AutotoolsToolchain, otherwise it can defeat msvc env var tricks above.

Copy link
Member

Choose a reason for hiding this comment

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

Good to know! 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

@SpaceIm @uilianries this is still called after AutotoolsToolchain,
can this recipe be fixed up to be done the "proper" way?
I am trying to use this recipe as a model for upgrading libsodium, and its not good if this recipe isn't "gold standard" (or suggests another to follow)

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, how does this even work properly? "env" variable name is reused...
first as env = tc.environment() and then as env=VirtualBuildEnv(self).
how should it be done "before AutotoolsToolchain" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Use the paclage templates as a refernce point, https://github.com/conan-io/conan-center-index/blob/master/docs/package_templates/autotools_package/all/conanfile.py#L91

we are trying to focus on keeping those up to spec


if (self.settings.compiler == "Visual Studio" and tools.Version(self.settings.compiler.version) >= "12") or \
self.settings.compiler == "msvc":
autotools.flags.append("-FS")
def build_requirements(self):
if self._settings_build.os == "Windows":
if not self.conf.get("tools.microsoft.bash:path", default=False, check_type=bool):
self.tool_requires("msys2/cci.latest")
self.win_bash = True

autotools.configure(args=configure_args, host=host, build=build)
return autotools
def source(self):
get(self, **self.conan_data["sources"][self.version], strip_root=True)
planetmarshall marked this conversation as resolved.
Show resolved Hide resolved

def _patch_sources(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
tools.patch(**patch)
apply_conandata_patches(self)
# relocatable shared libs on macOS
for configure in ["configure", os.path.join("libcharset", "configure")]:
tools.replace_in_file(os.path.join(self._source_subfolder, configure),
replace_in_file(self, os.path.join(self.source_folder, configure),
"-install_name \\$rpath/", "-install_name @rpath/")

def build(self):
self._patch_sources()
with self._build_context():
autotools = self._configure_autotools()
autotools.make()
autotools = Autotools(self)
autotools.configure()
autotools.make()

def package(self):
self.copy("COPYING.LIB", src=self._source_subfolder, dst="licenses")
with self._build_context():
autotools = self._configure_autotools()
autotools.install()
copy(self, "COPYING.LIB", self.source_folder, os.path.join(self.package_folder, "licenses"))
autotools = Autotools(self)
autotools.install(args=[f"DESTDIR={unix_path(self, self.package_folder)}"])

tools.remove_files_by_mask(os.path.join(self.package_folder, "lib"), "*.la")
tools.rmdir(os.path.join(self.package_folder, "share"))
rm(self, "*.la", os.path.join(self.package_folder, "lib"))
rmdir(self, os.path.join(self.package_folder, "share"))

if (is_msvc(self) or self._is_clang_cl) and self.options.shared:
for import_lib in ["iconv", "charset"]:
Expand Down
5 changes: 1 addition & 4 deletions recipes/libiconv/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
cmake_minimum_required(VERSION 3.1)
project(test_package C)

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

find_package(Iconv REQUIRED)

add_executable(${PROJECT_NAME} test_package.c)
target_link_libraries(${PROJECT_NAME} Iconv::Iconv)
target_link_libraries(${PROJECT_NAME} PRIVATE Iconv::Iconv)
26 changes: 14 additions & 12 deletions recipes/libiconv/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
from conans import ConanFile, CMake, tools
import os

from conan import ConanFile
from conan.tools.build import can_run
from conan.tools.cmake import CMake, cmake_layout


class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake", "cmake_find_package"
generators = "CMakeToolchain", "CMakeDeps", "VirtualRunEnv"
test_type = "explicit"

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

def build_requirements(self):
if self.settings.os == "Macos" and self.settings.arch == "armv8":
# Workaround for CMake bug with error message:
# Attempting to use @rpath without CMAKE_SHARED_LIBRARY_RUNTIME_C_FLAG being
# set. This could be because you are using a Mac OS X version less than 10.5
# or because CMake's platform configuration is corrupt.
# FIXME: Remove once CMake on macOS/M1 CI runners is upgraded.
self.build_requires("cmake/3.22.0")
def layout(self):
cmake_layout(self)

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
self.run(os.path.join("bin", "test_package"), run_environment=True)
if can_run(self):
bin_path = os.path.join(self.cpp.build.bindirs[0], "test_package")
self.run(bin_path, env="conanrun")
10 changes: 10 additions & 0 deletions recipes/libiconv/all/test_v1_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
cmake_minimum_required(VERSION 3.1)
project(test_package C)

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

find_package(Iconv REQUIRED)

add_executable(${PROJECT_NAME} ../test_package/test_package.c)
target_link_libraries(${PROJECT_NAME} Iconv::Iconv)
planetmarshall marked this conversation as resolved.
Show resolved Hide resolved
16 changes: 16 additions & 0 deletions recipes/libiconv/all/test_v1_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from conans import ConanFile, CMake, tools
import os


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

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
self.run(os.path.join("bin", "test_package"), run_environment=True)