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

[feature] Manage shared, fPIC and header_only options automatically #14194

Merged
merged 23 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from 14 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
5 changes: 5 additions & 0 deletions conan/tools/options/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from conan.tools.options.helpers import (
handle_common_config_options,
handle_common_configure_options,
handle_common_package_id_options
)
19 changes: 19 additions & 0 deletions conan/tools/options/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
from conans.model.pkg_type import PackageType


def handle_common_config_options(conanfile):
if conanfile.settings.get_safe("os") == "Windows":
conanfile.options.rm_safe("fPIC")


def handle_common_configure_options(conanfile):
if conanfile.options.get_safe("header_only"):
conanfile.options.rm_safe("fPIC")
conanfile.options.rm_safe("shared")
elif conanfile.options.get_safe("shared"):
conanfile.options.rm_safe("fPIC")


def handle_common_package_id_options(conanfile):
if conanfile.options.get_safe("header_only") or conanfile.package_type == PackageType.HEADER:
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking that if by the point this gets executed the package type has already been properly computed, why rely on the option? I don't actually know if having it is better or worse, so happy to hear your opinion :)

danimtb marked this conversation as resolved.
Show resolved Hide resolved
conanfile.info.clear()
13 changes: 13 additions & 0 deletions conans/client/conanfile/configure.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from conans.errors import conanfile_exception_formatter
from conans.model.pkg_type import PackageType
from conans.model.requires import BuildRequirements, TestRequirements, ToolRequirements
from conan.tools.options import handle_common_config_options, handle_common_configure_options


def run_configure_method(conanfile, down_options, profile_options, ref):
Expand All @@ -9,6 +10,8 @@ def run_configure_method(conanfile, down_options, profile_options, ref):
if hasattr(conanfile, "config_options"):
with conanfile_exception_formatter(conanfile, "config_options"):
conanfile.config_options()
else:
_conanfile_config_options_default(conanfile)

# Assign only the current package options values, but none of the dependencies
is_consumer = conanfile._conan_is_consumer
Expand All @@ -17,6 +20,8 @@ def run_configure_method(conanfile, down_options, profile_options, ref):
if hasattr(conanfile, "configure"):
with conanfile_exception_formatter(conanfile, "configure"):
conanfile.configure()
else:
_conanfile_configure_default(conanfile)

self_options, up_options = conanfile.options.get_upstream_options(down_options, ref,
is_consumer)
Expand All @@ -38,3 +43,11 @@ def run_configure_method(conanfile, down_options, profile_options, ref):
if hasattr(conanfile, "build_requirements"):
with conanfile_exception_formatter(conanfile, "build_requirements"):
conanfile.build_requirements()


def _conanfile_config_options_default(conanfile):
handle_common_config_options(conanfile)


def _conanfile_configure_default(conanfile):
handle_common_configure_options(conanfile)
danimtb marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 7 additions & 1 deletion conans/client/graph/compute_pid.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from conans.errors import conanfile_exception_formatter, ConanInvalidConfiguration, \
conanfile_remove_attr, ConanException
from conans.model.info import ConanInfo, RequirementsInfo, RequirementInfo, PythonRequiresInfo
from conan.tools.options import handle_common_package_id_options


def compute_package_id(node, new_config):
Expand Down Expand Up @@ -81,5 +82,10 @@ def run_validate_package_id(conanfile):
with conanfile_exception_formatter(conanfile, "package_id"):
with conanfile_remove_attr(conanfile, ['cpp_info', 'settings', 'options'], "package_id"):
conanfile.package_id()

else:
_conanfile_package_id_default(conanfile)
conanfile.info.validate()


def _conanfile_package_id_default(conanfile):
danimtb marked this conversation as resolved.
Show resolved Hide resolved
handle_common_package_id_options(conanfile)
2 changes: 2 additions & 0 deletions conans/model/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ def rm_safe(self, field):
delattr(self, field)
except ConanException:
pass
except KeyError:
memsharded marked this conversation as resolved.
Show resolved Hide resolved
pass

def validate(self):
for child in self._data.values():
Expand Down
5 changes: 2 additions & 3 deletions conans/test/functional/toolchains/cmake/test_cmake.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ def test_toolchain_win(self, compiler, build_type, runtime, version, cppstd, arc
options = {"shared": shared}
save(self.client.cache.new_config_path, "tools.build:jobs=1")
install_out = self._run_build(settings, options)
self.assertIn("WARN: Toolchain: Ignoring fPIC option defined for Windows", install_out)

# FIXME: Hardcoded VS version and partial toolset check
toolchain_path = os.path.join(self.client.current_folder, "build",
Expand Down Expand Up @@ -314,7 +313,6 @@ def test_toolchain_mingw_win(self, build_type, libcxx, version, cppstd, arch, sh
}
options = {"shared": shared}
install_out = self._run_build(settings, options)
self.assertIn("WARN: Toolchain: Ignoring fPIC option defined for Windows", install_out)
self.assertIn("The C compiler identification is GNU", self.client.out)
toolchain_path = os.path.join(self.client.current_folder, "build",
"conan_toolchain.cmake").replace("\\", "/")
Expand Down Expand Up @@ -396,7 +394,8 @@ def test_toolchain_linux(self, build_type, cppstd, arch, libcxx, shared):
"CMAKE_SHARED_LINKER_FLAGS": arch_str,
"CMAKE_EXE_LINKER_FLAGS": arch_str,
"COMPILE_DEFINITIONS": defines,
"CMAKE_POSITION_INDEPENDENT_CODE": "ON"
# fPIC is managed automatically depending on the shared option value
"CMAKE_POSITION_INDEPENDENT_CODE": "ON" if not shared else ""
danimtb marked this conversation as resolved.
Show resolved Hide resolved
}

def _verify_out(marker=">>"):
Expand Down
170 changes: 170 additions & 0 deletions conans/test/integration/options/test_configure_options.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import textwrap
import unittest

from parameterized import parameterized

from conans.test.utils.tools import TestClient


class ConfigureOptionsTest(unittest.TestCase):
"""
Test config_options(), configure() and package_id() methods can manage shared, fPIC and
header_only options automatically.
"""

@parameterized.expand([
["Linux", False, False, False, [False, False, False]],
["Windows", False, False, False, [False, None, False]],
["Windows", True, False, False, [True, None, False]],
["Windows", False, False, True, [None, None, True]],
["Linux", False, False, True, [None, None, True]],
["Linux", True, True, False, [True, None, False]],
["Linux", True, False, False, [True, None, False]],
["Linux", True, True, True, [None, None, True]],
["Linux", True, True, True, [None, None, True]],
["Linux", False, True, False, [False, True, False]],
["Linux", False, True, False, [False, True, False]],
])
def test_methods_not_defined(self, settings_os, shared, fpic, header_only, result):
"""
Test that options are managed automatically when methods config_otpions and configure are not
defined.
Check that header only package gets its unique package ID.
"""
client = TestClient()
conanfile = textwrap.dedent(f"""\
from conan import ConanFile

class Pkg(ConanFile):
settings = "os", "compiler", "arch", "build_type"
options = {{"shared": [True, False], "fPIC": [True, False], "header_only": [True, False]}}
default_options = {{"shared": {shared}, "fPIC": {fpic}, "header_only": {header_only}}}

def build(self):
shared = self.options.get_safe("shared")
fpic = self.options.get_safe("fPIC")
header_only = self.options.get_safe("header_only")
self.output.info(f"shared: {{shared}}, fPIC: {{fpic}}, header only: {{header_only}}")
""")
client.save({"conanfile.py": conanfile})
client.run(f"create . --name=pkg --version=0.1 -s os={settings_os}")
result = f"shared: {result[0]}, fPIC: {result[1]}, header only: {result[2]}"
self.assertIn(result, client.out)
if header_only:
self.assertIn("Package 'da39a3ee5e6b4b0d3255bfef95601890afd80709' created", client.out)

@parameterized.expand([
["Linux", False, False, False, [False, False, False]],
["Linux", False, False, True, [False, False, True]],
["Linux", False, True, False, [False, True, False]],
["Linux", False, True, True, [False, True, True]],
["Linux", True, False, False, [True, False, False]],
["Linux", True, False, True, [True, False, True]],
["Linux", True, True, False, [True, True, False]],
["Linux", True, True, True, [True, True, True]],
["Windows", False, False, False, [False, False, False]],
["Windows", False, False, True, [False, False, True]],
["Windows", False, True, False, [False, True, False]],
["Windows", False, True, True, [False, True, True]],
["Windows", True, False, False, [True, False, False]],
["Windows", True, False, True, [True, False, True]],
["Windows", True, True, False, [True, True, False]],
["Windows", True, True, True, [True, True, True]],
])
def test_optout(self, settings_os, shared, fpic, header_only, result):
"""
Test that options are not managed automatically when methods are defined.
Check that header only package gets its unique package ID.
"""
client = TestClient()
conanfile = textwrap.dedent(f"""\
from conan import ConanFile

class Pkg(ConanFile):
settings = "os", "compiler", "arch", "build_type"
options = {{"shared": [True, False], "fPIC": [True, False], "header_only": [True, False]}}
default_options = {{"shared": {shared}, "fPIC": {fpic}, "header_only": {header_only}}}

def config_options(self):
pass

def configure(self):
pass

def build(self):
shared = self.options.get_safe("shared")
fpic = self.options.get_safe("fPIC")
header_only = self.options.get_safe("header_only")
self.output.info(f"shared: {{shared}}, fPIC: {{fpic}}, header only: {{header_only}}")
""")
client.save({"conanfile.py": conanfile})
client.run(f"create . --name=pkg --version=0.1 -s os={settings_os}")
result = f"shared: {result[0]}, fPIC: {result[1]}, header only: {result[2]}"
self.assertIn(result, client.out)
if header_only:
self.assertIn("Package 'da39a3ee5e6b4b0d3255bfef95601890afd80709' created", client.out)

@parameterized.expand([
["Linux", False, False, False, [False, False, False]],
["Windows", False, False, False, [False, None, False]],
["Windows", True, False, False, [True, None, False]],
["Windows", False, False, True, [None, None, True]],
["Linux", False, False, True, [None, None, True]],
["Linux", True, True, False, [True, None, False]],
["Linux", True, False, False, [True, None, False]],
["Linux", True, True, True, [None, None, True]],
["Linux", True, True, True, [None, None, True]],
["Linux", False, True, False, [False, True, False]],
["Linux", False, True, False, [False, True, False]],
])
def test_methods_defined_explicit(self, settings_os, shared, fpic, header_only, result):
"""
Test that options are managed automatically when methods config_otpions and configure are not
danimtb marked this conversation as resolved.
Show resolved Hide resolved
defined.
Check that header only package gets its unique package ID.
"""
client = TestClient()
conanfile = textwrap.dedent(f"""\
from conan import ConanFile
from conan.tools.options import handle_common_config_options, handle_common_configure_options

class Pkg(ConanFile):
settings = "os", "compiler", "arch", "build_type"
options = {{"shared": [True, False], "fPIC": [True, False], "header_only": [True, False]}}
default_options = {{"shared": {shared}, "fPIC": {fpic}, "header_only": {header_only}}}

def config_options(self):
handle_common_config_options(self)

def configure(self):
handle_common_configure_options(self)

def build(self):
shared = self.options.get_safe("shared")
fpic = self.options.get_safe("fPIC")
header_only = self.options.get_safe("header_only")
self.output.info(f"shared: {{shared}}, fPIC: {{fpic}}, header only: {{header_only}}")
""")
client.save({"conanfile.py": conanfile})
client.run(f"create . --name=pkg --version=0.1 -s os={settings_os}")
result = f"shared: {result[0]}, fPIC: {result[1]}, header only: {result[2]}"
self.assertIn(result, client.out)
if header_only:
self.assertIn("Package 'da39a3ee5e6b4b0d3255bfef95601890afd80709' created", client.out)

def test_header_package_type_pid(self):
"""
Test that we get the pid for header only when package type is set to header-library
"""
client = TestClient()
conanfile = textwrap.dedent(f"""\
from conan import ConanFile

class Pkg(ConanFile):
settings = "os", "compiler", "arch", "build_type"
package_type = "header-library"

""")
client.save({"conanfile.py": conanfile})
client.run(f"create . --name=pkg --version=0.1")
self.assertIn("Package 'da39a3ee5e6b4b0d3255bfef95601890afd80709' created", client.out)
Empty file.
68 changes: 68 additions & 0 deletions conans/test/unittests/tools/options/test_options_tools.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
from conan.tools.options import (
danimtb marked this conversation as resolved.
Show resolved Hide resolved
handle_common_config_options,
handle_common_configure_options, handle_common_package_id_options
)
from conans.model.pkg_type import PackageType
from conans.test.utils.mocks import MockSettings, ConanFileMock, MockOptions, MockConanfile


def test_handle_common_config_options():
"""
If windows, then fpic option should be removed
"""
conanfile = ConanFileMock()
conanfile.settings = MockSettings({"os": "Linux"})
conanfile.options = MockOptions({"shared": False, "fPIC": False})
handle_common_config_options(conanfile)
assert conanfile.options.values == {"shared": False, "fPIC": False}
conanfile.settings = MockSettings({"os": "Windows"})
handle_common_config_options(conanfile)
assert conanfile.options.values == {"shared": False}


def test_handle_common_configure_options():
"""
If header only, fpic and shared options should be removed.
If shared, fpic removed and header only is false
"""
conanfile = ConanFileMock()
conanfile.settings = MockSettings({"os": "Linux"})
conanfile.options = MockOptions({"header_only": False, "shared": False, "fPIC": False})
handle_common_configure_options(conanfile)
assert conanfile.options.values == {"header_only": False, "shared": False, "fPIC": False}
conanfile.options = MockOptions({"header_only": True, "shared": False, "fPIC": False})
handle_common_configure_options(conanfile)
assert conanfile.options.values == {"header_only": True}
conanfile.options = MockOptions({"header_only": False, "shared": True, "fPIC": False})
handle_common_configure_options(conanfile)
assert conanfile.options.values == {"header_only": False, "shared": True}
# Wrong options combination, but result should be right as well
conanfile.options = MockOptions({"header_only": True, "shared": True, "fPIC": False})
handle_common_configure_options(conanfile)
assert conanfile.options.values == {"header_only": True}


def test_handle_common_package_id_options():
"""
When header_only option is False, the original settings and options is kept in package id info
When the heacer_only option is True, the package id info is cleared.
When package type is header, then the package id info is cleared
"""
original_settings = {"os": "Linux", "compiler": "gcc"}
options = MockOptions({"header_only": False})
conanfile = MockConanfile(original_settings, options)
handle_common_package_id_options(conanfile)
assert conanfile.info.settings == original_settings
assert conanfile.info.options == options

options = MockOptions({"header_only": True})
conanfile = MockConanfile(original_settings, options)
handle_common_package_id_options(conanfile)
assert conanfile.info.settings == {}
assert conanfile.info.options == {}

conanfile = MockConanfile(original_settings)
conanfile.package_type = PackageType.HEADER
handle_common_package_id_options(conanfile)
assert conanfile.info.settings == {}
assert conanfile.info.options == {}
12 changes: 11 additions & 1 deletion conans/test/utils/mocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ def __getattr__(self, name):
except KeyError:
raise ConanException("'%s' value not defined" % name)

def rm_safe(self, name):
self.values.pop(name)
danimtb marked this conversation as resolved.
Show resolved Hide resolved


class MockCppInfo(object):
def __init__(self):
Expand All @@ -81,6 +84,7 @@ class MockConanfile(ConanFile):
def __init__(self, settings, options=None, runner=None):
self.display_name = ""
self._conan_node = None
self.package_type = "unknown"
self.folders = Folders()
self.settings = settings
self.settings_build = settings
Expand All @@ -90,9 +94,15 @@ def __init__(self, settings, options=None, runner=None):
self.conf = Conf()

class MockConanInfo:
pass
settings = None
options = None

def clear(self):
self.settings = {}
self.options = {}
self.info = MockConanInfo()
self.info.settings = settings # Incomplete, only settings for Cppstd Min/Max tests
self.info.options = options

def run(self, *args, **kwargs):
if self.runner:
Expand Down