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

More flexibility in Autotools tools to override arguments and avoid all default arguments #11284

Merged
merged 30 commits into from May 27, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 19 additions & 20 deletions conan/tools/gnu/autotools.py
Expand Up @@ -11,30 +11,30 @@

class Autotools(object):

def __init__(self, conanfile, namespace=None, build_script_folder=None):
def __init__(self, conanfile, namespace=None):
self._conanfile = conanfile

toolchain_file_content = load_toolchain_args(self._conanfile.generators_folder,
namespace=namespace)
self._configure_args = toolchain_file_content.get("configure_args")
self._make_args = toolchain_file_content.get("make_args")
self.default_configure_install_args = True
self.build_script_folder = os.path.join(self._conanfile.source_folder, build_script_folder) \
if build_script_folder else self._conanfile.source_folder

def configure(self):
def configure(self, build_script_folder=None):
"""
http://jingfenghanmax.blogspot.com.es/2010/09/configure-with-host-target-and-build.html
https://gcc.gnu.org/onlinedocs/gccint/Configure-Terms.html
"""
script_folder = os.path.join(self._conanfile.source_folder, build_script_folder) \
if build_script_folder else self._conanfile.source_folder
configure_args = []
if self.default_configure_install_args and self._conanfile.package_folder:
if self.default_configure_install_args:
def _get_argument(argument_name, cppinfo_name):
elements = getattr(self._conanfile.cpp.package, cppinfo_name)
return "--{}=${{prefix}}/{}".format(argument_name, elements[0]) if elements else ""

# If someone want arguments but not the defaults can pass them in args manually
configure_args.extend(["--prefix=%s" % self._conanfile.package_folder.replace("\\", "/"),
configure_args.extend(["--prefix=/",
_get_argument("bindir", "bindirs"),
_get_argument("sbindir", "bindirs"),
_get_argument("libdir", "libdirs"),
Expand All @@ -45,23 +45,24 @@ def _get_argument(argument_name, cppinfo_name):
self._configure_args = "{} {}".format(self._configure_args, args_to_string(configure_args)) \
if configure_args else self._configure_args

configure_cmd = "{}/configure".format(self.build_script_folder)
configure_cmd = "{}/configure".format(script_folder)
subsystem = deduce_subsystem(self._conanfile, scope="build")
configure_cmd = subsystem_path(subsystem, configure_cmd)
cmd = '"{}" {}'.format(configure_cmd, self._configure_args)
self._conanfile.output.info("Calling:\n > %s" % cmd)
self._conanfile.run(cmd)

def make(self, target=None):
def make(self, target=None, args=None):
make_program = self._conanfile.conf.get("tools.gnu:make_program",
default="mingw32-make" if self._use_win_mingw() else "make")
str_args = self._make_args
str_extra_args = " ".join(args) if args else ""
jobs = ""
if "-j" not in str_args and "nmake" not in make_program.lower():
njobs = build_jobs(self._conanfile)
if njobs:
jobs = "-j{}".format(njobs)
command = join_arguments([make_program, target, str_args, jobs])
command = join_arguments([make_program, target, str_args, str_extra_args, jobs])
self._conanfile.run(command)

def _fix_osx_shared_install_name(self):
Expand All @@ -75,28 +76,26 @@ def _fix_install_name(lib_name, lib_folder):
lib_name))
self._conanfile.run(command)

def _is_modified_install_name(lib_name, lib_folder):
def _is_modified_install_name(lib_name, full_folder, libdir):
"""
Check that the user did not change the default install_name using the install_name
linker flag in that case we do not touch this field
"""
command = "otool -D {}".format(os.path.join(lib_folder, lib_name))
out = check_output_runner(command).strip().split(":")[1]
return False if str(os.path.join(lib_folder, shared_lib)) in out else True
command = "otool -D {}".format(os.path.join(full_folder, lib_name))
install_path = check_output_runner(command).strip().split(":")[1].strip()
default_path = str(os.path.join("/", libdir, shared_lib))
return False if default_path == install_path else True

libdirs = getattr(self._conanfile.cpp.package, "libdirs")
for folder in libdirs:
full_folder = os.path.join(self._conanfile.package_folder, folder)
for libdir in libdirs:
full_folder = os.path.join(self._conanfile.package_folder, libdir)
shared_libs = _osx_collect_dylibs(full_folder)
for shared_lib in shared_libs:
if not _is_modified_install_name(shared_lib, full_folder):
if not _is_modified_install_name(shared_lib, full_folder, libdir):
_fix_install_name(shared_lib, full_folder)

def install(self):
# FIXME: we have to run configure twice because the local flow won't work otherwise
# because there's no package_folder until the package step
self.configure()
self.make(target="install")
self.make(target="install", args=["DESTDIR={}".format(self._conanfile.package_folder)])

Choose a reason for hiding this comment

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

If one sets default_configure_install_args to False, they may not want the DESTDIR either.

Having used the tools in anger for a bit, having extra arguments added under the hood seems to be an anti pattern and hides the magic.

Personally, I would much prefer if autotoolstoolchain auto populated the autoreconf_args/configure_args/make_args and I could modify them as I see fit before generation.

Usage would then be:

    def generate(self) -> None:
        at = AutotoolsToolchain(self)
        print(at.autoreconf_args) # has the `--force` and `--install` arguments in
        print(at.configure_args) # has the `--prefix` and `--shared` arguments in
        print(at.make_args) # has `DESTDIR`  and `-j` in it
        # I know _exactly_ will be passed to `autoreconf`, `configure` and `make`

I'd even argue adding --enable-checking=fatal would be a good default that users can easily remove with:

        at.configure_args = [a for a in at.configure_args if a.startswith("--enable-checking")]

Then, if I have a really wonky package I can customise the arguments entirely. There are so many odd autotools packages in the wild.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for the review @mattyclarkson, I think something in that direction would make sense, I'll add some changes to see how it looks

if self._conanfile.settings.get_safe("os") == "Macos" and self._conanfile.options.get_safe("shared", False):
self._fix_osx_shared_install_name()

Expand Down
3 changes: 1 addition & 2 deletions conan/tools/gnu/autotoolstoolchain.py
Expand Up @@ -167,7 +167,7 @@ def generate(self, env=None, scope="build"):
self.generate_args()
VCVars(self._conanfile).generate(scope=scope)

def _shared_static_args(self):
def shared_flags(self):
args = []
if self._conanfile.options.get_safe("shared", False):
args.extend(["--enable-shared", "--disable-static"])
Expand All @@ -179,7 +179,6 @@ def _shared_static_args(self):

def generate_args(self):
configure_args = []
configure_args.extend(self._shared_static_args())
configure_args.extend(self.configure_args)
user_args_str = args_to_string(self.configure_args)
for flag, var in (("host", self._host), ("build", self._build), ("target", self._target)):
Expand Down
5 changes: 2 additions & 3 deletions conans/assets/templates/new_v2_autotools.py
Expand Up @@ -8,7 +8,6 @@
from conan import ConanFile
from conan.tools.gnu import AutotoolsToolchain, Autotools
from conan.tools.layout import basic_layout
from conan.tools.files import chdir


class {package_name}Conan(ConanFile):
Expand Down Expand Up @@ -38,6 +37,7 @@ def layout(self):

def generate(self):
at_toolchain = AutotoolsToolchain(self)
at_toolchain.configure_args = at_toolchain.shared_flags()
czoido marked this conversation as resolved.
Show resolved Hide resolved
at_toolchain.generate()

def build(self):
Expand Down Expand Up @@ -79,10 +79,9 @@ def package_info(self):
import os

from conan import ConanFile
from conan.tools.gnu import AutotoolsToolchain, Autotools, AutotoolsDeps
from conan.tools.gnu import Autotools
from conan.tools.layout import basic_layout
from conan.tools.build import cross_building
from conan.tools.files import chdir


class {package_name}TestConan(ConanFile):
Expand Down
56 changes: 56 additions & 0 deletions conans/test/functional/toolchains/gnu/autotools/test_basic.py
Expand Up @@ -245,3 +245,59 @@ def test_autotools_with_pkgconfigdeps():
assert re.search("I.*hello.*1.0.*include", str(client.out))
assert "-lhello" in client.out
assert re.search("L.*hello.*1.0.*package", str(client.out))


@pytest.mark.skipif(platform.system() not in ["Linux", "Darwin"], reason="Requires Autotools")
@pytest.mark.tool_autotools()
def test_autotools_option_checking():
# https://github.com/conan-io/conan/issues/11265
client = TestClient(path_with_spaces=False)
client.run("new mylib/1.0@ -m autotools_lib")
conanfile = textwrap.dedent("""
import os

from conan import ConanFile
from conan.tools.gnu import AutotoolsToolchain, Autotools
from conan.tools.layout import basic_layout
from conan.tools.build import cross_building
from conan.tools.files import chdir


class MylibTestConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
# VirtualBuildEnv and VirtualRunEnv can be avoided if "tools.env.virtualenv:auto_use" is defined
# (it will be defined in Conan 2.0)
generators = "AutotoolsDeps", "VirtualBuildEnv", "VirtualRunEnv"
apply_env = False
test_type = "explicit"

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

def generate(self):
at_toolchain = AutotoolsToolchain(self)
at_toolchain.configure_args = ['--enable-option-checking=fatal']
at_toolchain.generate()

def build(self):
autotools = Autotools(self)
autotools.autoreconf()
autotools.configure()
autotools.make()

def layout(self):
basic_layout(self)

def test(self):
if not cross_building(self):
cmd = os.path.join(self.cpp.build.bindirs[0], "main")
self.run(cmd, env="conanrun")
""")

client.save({"test_package/conanfile.py": conanfile})
client.run("create . -tf=None")

# check that the shared flags are not added to the exe's configure, making it fail
client.run("test test_package mylib/1.0@")
assert "configure: error: unrecognized options: --disable-shared, --enable-static, --with-pic" \
not in client.out
3 changes: 1 addition & 2 deletions conans/test/functional/toolchains/gnu/autotools/test_ios.py
Expand Up @@ -76,5 +76,4 @@ def build(self):

conanbuild = load_toolchain_args(client.current_folder)
configure_args = conanbuild["configure_args"]
assert configure_args == "'--disable-shared' '--enable-static' '--with-pic' " \
"'--host=aarch64-apple-ios' '--build=x86_64-apple-darwin'"
assert configure_args == "'--host=aarch64-apple-ios' '--build=x86_64-apple-darwin'"
6 changes: 4 additions & 2 deletions conans/test/unittests/tools/gnu/autotools_test.py
Expand Up @@ -17,10 +17,12 @@ def test_source_folder_works():
conanfile.folders.set_base_install(folder)
sources = "/path/to/sources"
conanfile.folders.set_base_source(sources)
autotools = Autotools(conanfile, build_script_folder="subfolder")
autotools.configure()
autotools = Autotools(conanfile)
autotools.default_configure_install_args = False
autotools.configure(build_script_folder="subfolder")
assert conanfile.command.replace("\\", "/") == '"/path/to/sources/subfolder/configure" -foo bar'

autotools = Autotools(conanfile)
autotools.default_configure_install_args = False
autotools.configure()
assert conanfile.command.replace("\\", "/") == '"/path/to/sources/configure" -foo bar'