New AutotoolsDeps, AutotoolsToolchain helpers in conan.tools.gnu #8457

memsharded · 2021-02-08T00:45:55Z

Changelog: Feature: New AutotoolsDeps, AutotoolsToolchain helpers in conan.tools.gnu
Docs: conan-io/docs#2057

Close #7070 (will invert the logic in _GLIBCXX_USE_CXX11_ABI)

SSE4 · 2021-02-08T05:21:06Z

conan/tools/gnu/autotools.py

+        if hasattr(self._conanfile, 'settings_build'):
+            os_build, arch_build = get_build_os_arch(self._conanfile)
+        else:
+            # FIXME: Why not use 'os_build' and 'arch_build' from conanfile.settings?


if it's new helper, can we avoid all these os_build/arch_build mess and just use modern two-profile approach?

Yes, definitely, it is very ongoing work, I plan to refactor everything and use the 2 profiles.

SSE4 · 2021-02-08T05:22:00Z

conan/tools/gnu/autotools.py

+            os_build, arch_build = get_build_os_arch(self._conanfile)
+        else:
+            # FIXME: Why not use 'os_build' and 'arch_build' from conanfile.settings?
+            os_build = detected_os() or platform.system()


I'd say, that may be going forward, avoid auto-detecting altogether and specify everything explicitly in profile/settings, to ensure it's reproducible?

100% agree, there will be no auto-detection here, only deterministic from profiles.

SSE4 · 2021-02-08T05:23:21Z

conan/tools/gnu/autotools.py

+        self._conanfile.run(cmd, win_bash=self._win_bash, subsystem=self.subsystem)
+
+    def _configure_help_output(self, configure_path):
+        from six import StringIO  # Python 2 and 3 compatible


for the new helper, I think we can avoid python 2 and six

Agree, ongoing work.

SSE4 · 2021-02-08T05:24:26Z

conan/tools/gnu/autotools.py

+        if platform.system() == "Windows":
+            cmd = "autotoolsdeps.bat && autotools.bat && {}".format(make_program)
+        else:
+            cmd = "bash -c 'source autotoolsdeps.sh "\


I think we shouldn't hard-code bash here

Unless it is the only way to make it work reproducibly. The autotoolsdeps.sh, etc are using bash. If we are supporting other shells, we need tests, I don't want to add things into the new codebase that cannot be tested.

the make command is not hard-coded and got from the config.
the same, more or less, should be done for the bash I believe.
also, maybe it's enough to just use plain sh or $SHELL?
I don't think we need advanced bash-specific features, and it would be nice to be portable and require only posix sh.
bash is not always available and not a default shell everywhere.

The make command is hardcoded, most of the times it doesn't come from config:

conan/conans/client/build/autotools_environment.py

Line 237 in 2ccc79c

make_program = os.getenv("CONAN_MAKE_PROGRAM") or make_program or "make"

I guess you are aware that many recipes in ConanCenter are doing make_program=which("make") or which("mingw32-make") to be able to work on Windows. No config here, but recipes hardcoding it.

Same as bash, hardcoded here:

conan/conans/client/tools/oss.py

Line 389 in 2ccc79c

return which("bash")

.

The fact that they have configs, doesn't mean that they are not hardcoded in the codebase. Same I am doing here, we can discuss some defaults as we gain knowledge, but I am not introducing new hardcoded things.

The new Environment files at the moment only work on bash. If we can generalize them to work on different shells, or to provide different generated files, for different shells, good for me.

I know there are recipes doing that, as I wrote some of them.
that might be a sign we're not managing things the best way.
in some cases, mingw32-make is valid defaults, in some cases valid default could be nmake or just plain make.
that might be the reason we need to provide helper to deduce sane default based on settings, e.g. it may return MinGW make, nmake, GNU make or BSD make depending on system and compiler.
but that we know already the logic doesn't believe to the AutoToolsBuildEnvironment or recipes.

Yes, agree, a helper would be the best. This helper can hardcode some defaults for some given configurations (settings, etc), and always have a [conf] to be able to override it.

But lets wait a bit and have a minimum set of build helpers, just the most common "AutotoolsXXX" ones, that work well and make recipes simpler for a majority of cases, before extracting new helpers like get_make_program(conanfile), when we gain more insights about these defaults, our new helpers will be closer to something stable.

SSE4

may we try to avoid complicated logic (e.g. platform-specific), at least in the first iteration?

SSE4 · 2021-02-08T05:27:18Z

conan/tools/gnu/autotoolsdeps.py

+        libs = ["-l%s" % library for library in self.libs]
+
+        # Ldflags
+        # TODO: Discuss, should the helper filter frameworks based on compiler?


I vote to keep the least possible logic in these helpers. if some filtering has to be done, it doesn't belong here. and IMO it's the responsibility of the recipe author to ensure the correct CppInfo is built for the given configuration.

SSE4 · 2021-02-08T05:30:04Z

conan/tools/gnu/autotools.py

+
+        make_program = self._conanfile.conf["tools.gnu"].make_program
+        if make_program is None:
+            make_program = "mingw32-make" if platform.system() == "Windows" else "make"


I also don't think it's always desired to enforce mingw32-make on Windows. it should go from the configuration.

It is probably a sane default in windows, the same as make is in other platforms. But we will see as we keep adding support for other subsystems, I am fine with removing it.

SSE4 · 2021-02-08T05:31:31Z

conan/tools/gnu/autotools.py

+
+        tmp_compilation_flags = copy.copy(self.flags)
+
+        if tools.is_apple_os(self._os):


I don't think it belongs here as well. let's keep all platform-specific code as minimal as possible, and isolated. the way we did it previously was proven to be fragile and doesn't scale well.

Agree, this code is not used at the moment, needs to be worked on.

SSE4 · 2021-02-08T05:33:19Z

conan/tools/env/environment.py

@@ -0,0 +1,138 @@
+import textwrap


maybe this one deserves its own first class generator? and new Virtual*Env generators might be based on that one?

Not yet. This could be a first class model, like self.env_info = Environment() in the recipes. This is separated in another branch and will have a separate PR once it is stabilized.

…ools

SSE4 · 2021-03-29T07:30:13Z

conan/tools/gnu/autotoolstoolchain.py

+    # TODO:  tools.XCRun(self._conanfile.settings).sdk_path
+    # TODO: "-arch", tools.to_apple_arch(self._arch)
+
+    def _cxx11_abi_define(self):


belongs to conan/tools/_compilers.py?

The idea is to factor out to a common place like _compilers.py only when there is a repetition/duplicated code needed somewhere else.

SSE4 · 2021-03-29T07:30:18Z

conan/tools/gnu/autotoolstoolchain.py

+            if libcxx == 'libstdc++':
+                return '_GLIBCXX_USE_CXX11_ABI=0'
+
+    def _libcxx(self):


belongs to conan/tools/_compilers.py?

SSE4 · 2021-03-29T07:32:18Z

conan/tools/gnu/autotoolsgen.py

+        # FIXME: Use settings, not platform Not always defined :(
+        # os_ = self._conanfile.settings_build.get_safe("os")
+        if platform.system() == "Windows":
+            build_env.save_bat("conanbuildenv.bat")


name is too generic? may overwrite other generators, if multiple are used together...

conanxxxx filenames are generated by Conan (probably we should document this more explicitly too).
This filename is automatically used by self.run() and is the same filename generated by VirtualEnv (to be automatically injected too). The idea is that this file is already the aggregation of the necessary build environments, in a single file, oriented also to make user life easier at the consumer flow.

Ideas welcome, any suggestion of other filename?

I am not sure if I got the idea. what if we add similar generator e.g. for premake, which will also need VirtualEnv.
and what if project uses both autotoolsgen and premake (e.g. it has to sub-directories with different build systems)
now both will try to create conanbuildenv.bat to override each other?

Autotools generators do not create conanbuildenv.bat, AutotoolsGen that wraps both Autotools and VirtualEnv does. In the same spirit, if you want to combine Autotools and Premake in an ordered, structured way, you can do it directly in the recipe, aggregating environments before saving conanbuildenv.bat or creating a generator that aggregates them and using it.

memsharded added 23 commits January 21, 2021 23:15

refactor autotools tests

a841c9b

autotools

062c3f9

Merge branch 'develop' into feature/tools_autotools

7330385

working

a92a2cb

Merge branch 'develop' into feature/tools_autotools

08e7144

working

e41792c

working .bat

03a4bbf

Merge branch 'develop' into feature/environment

74e9198

working in Windows and Linux

3292aec

Merge branch 'feature/environment' into feature/tools_autotools

087f030

test passing Linux

fd18ee9

working

b33cd92

Merge branch 'feature/environment' into feature/tools_autotools

cc5c8b3

working

9b997f8

improved environment

cf97406

Merge branch 'develop' into feature/tools_autotools

79675af

Merge branch 'feature/environment' into feature/tools_autotools

dfda73e

mingw test

c6226b6

working

c2b7e63

Merge branch 'develop' into feature/tools_autotools

36bf481

working

642b3fe

spaces

38b4dd3

working

96d0334

memsharded mentioned this pull request Feb 8, 2021

[feature] Test with msys2 mingw #8458

Closed

SSE4 reviewed Feb 8, 2021

View reviewed changes

SSE4 suggested changes Feb 8, 2021

View reviewed changes

SSE4 reviewed Feb 8, 2021

View reviewed changes

memsharded added 18 commits February 24, 2021 22:55

Merge branch 'develop' into feature/environment_propagate

7f86beb

Merge branch 'develop' into feature/tools_autotools

49dfb47

merging environment

7d98f08

working

7507507

merged develop

f77726b

reviewing merge

e4e280e

merge review

ab0375f

Merge branch 'develop' into feature/tools_autotools

b4fbb64

adding diamond visits and avoid repetitions

99a3ae9

Merge branch 'develop' into feature/tools_autotools

d3f065f

Merge branch 'fix/environment_diamond_visit' into feature/tools_autot…

0fe4307

…ools

test working again

59800d8

working

b1de1f4

Merge branch 'develop' into feature/tools_autotools

5aee77f

test passing

42c4dfc

working

fba1ede

merged develop

e0e9c9c

fix conan.tools.CMake for Msys/Cygwin

58776e5

memsharded marked this pull request as ready for review March 27, 2021 20:32

memsharded added this to the 1.35 milestone Mar 27, 2021

use correct make instead of mingw32-make in subsystems

67d2275

memsharded requested review from czoido, lasote and SSE4 March 27, 2021 22:57

SSE4 reviewed Mar 29, 2021

View reviewed changes

memsharded merged commit ea15adb into conan-io:develop Mar 29, 2021

memsharded deleted the feature/tools_autotools branch March 29, 2021 16:26

czoido mentioned this pull request Mar 30, 2021

autotools docs conan-io/docs#2057

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New AutotoolsDeps, AutotoolsToolchain helpers in conan.tools.gnu #8457

New AutotoolsDeps, AutotoolsToolchain helpers in conan.tools.gnu #8457

memsharded commented Feb 8, 2021 •

edited by czoido

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 left a comment

SSE4 Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Feb 8, 2021

memsharded Feb 8, 2021

SSE4 Mar 29, 2021

memsharded Mar 29, 2021

SSE4 Mar 29, 2021

SSE4 Mar 29, 2021

memsharded Mar 29, 2021

SSE4 Mar 29, 2021

memsharded Mar 29, 2021


		tmp_compilation_flags = copy.copy(self.flags)

		if tools.is_apple_os(self._os):

New AutotoolsDeps, AutotoolsToolchain helpers in conan.tools.gnu #8457

New AutotoolsDeps, AutotoolsToolchain helpers in conan.tools.gnu #8457

Conversation

memsharded commented Feb 8, 2021 • edited by czoido

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SSE4 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

memsharded commented Feb 8, 2021 •

edited by czoido