[feature] Implement tools.build:install_strip for Autotools

[pbs3141]

Changelog: Feature: Implement tools.build:install_strip for Autotools.
Docs: conan-io/docs#4211

The tools.build:install_strip config is supposed to allow shared libraries to be stripped. This was implemented for CMake in #14167, Meson in #18429, but not yet implemented for Autotools - which this PR does.

• I've read the Contributing guide.
• I've followed the PEP8 style guides for Python code.
• I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

[CLAassistant]

All committers have signed the CLA.

[memsharded]

Hi @pbs3141
Thanks for your contribution!

It would be necessary to add at least some integration tests (in the integration folder), that means, those tests shouldn't really compile or install-strip anything, but at least cover that the install() method works as expected.
This can be easily done in a similar way to test test_autotools_make_parameters, by defining a custom def run() method in the recipe, that will receive the resulting call.

[memsharded]

Is it possible that there are projects/packages out there that do not define a install-strip target at all?
And then this would break for those projects/packages?
Or it is expected that every autotools project that calls autotools.install() must have this install-strip target?

[pbs3141]

According to this automake manual the install-strip target always exists,

Automake also generates rules for targets uninstall, installdirs, and install-strip.

though I'm no expert with autotools, so I could have somehow misinterpreted this documentation.

[pbs3141]

I've added a unit test based on those for the linked PRs.

However, I haven't yet run it locally - in fact, I was rather hoping CI could do it for me!

[pbs3141]

I tried rebuilding all my dependencies with this option, and only libiconv failed, due to what appears to be a bug in the libiconv recipe, though I don't know what it is yet.

Here is the output at the point where it diverges, which happens at line 4:

Before PR (good):

libtool: warning: library '/root/.conan2/p/b/libicdbb1e7b8e81df/p//lib/libiconv.la' was moved.
libtool: warning: library '/root/.conan2/p/b/libicdbb1e7b8e81df/p//lib/libiconv.la' was moved.
libtool: link: /usr/lib/android-sdk/ndk/28.1.13356709/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android31-clang -O3 iconv.o -o iconv  ../srclib/libicrt.a /root/.conan2/p/b/libicdbb1e7b8e81df/p//lib/libiconv.so -L/root/.conan2/p/b/libicdbb1e7b8e81df/p//lib -L/root/.conan2/p/b/libicdbb1e7b8e81df/p//lib
/bin/bash ../libtool --mode=install /usr/bin/install -c iconv /root/.conan2/p/b/libicdbb1e7b8e81df/p//bin/iconv
libtool: install: /usr/bin/install -c iconv /root/.conan2/p/b/libicdbb1e7b8e81df/p//bin/iconv
make[1]: Leaving directory '/root/.conan2/p/b/libicdbb1e7b8e81df/b/build-release/src'

After PR (fails):

libtool: warning: library '/root/.conan2/p/b/libic653990c05fb7e/p//lib/libiconv.la' was moved.
libtool: warning: library '/root/.conan2/p/b/libic653990c05fb7e/p//lib/libiconv.la' was moved.
libtool: link: /usr/lib/android-sdk/ndk/28.1.13356709/toolchains/llvm/prebuilt/linux-x86_64/bin/aarch64-linux-android31-clang -O3 iconv.o -o iconv  ../srclib/libicrt.a /root/.conan2/p/b/libic653990c05fb7e/p//lib/libiconv.so -L/root/.conan2/p/b/libic653990c05fb7e/p//lib -L/root/.conan2/p/b/libic653990c05fb7e/p//lib
/bin/bash ../libtool --mode=install /bin/bash /root/.conan2/p/b/libic653990c05fb7e/b/src/build-aux/install-sh -c -s iconv /root/.conan2/p/b/libic653990c05fb7e/p//bin/iconv
libtool: install: /bin/bash /root/.conan2/p/b/libic653990c05fb7e/b/src/build-aux/install-sh -c -s iconv /root/.conan2/p/b/libic653990c05fb7e/p//bin/iconv
strip: Unable to recognise the format of the input file `/root/.conan2/p/b/libic653990c05fb7e/p//bin/_inst.61817_'

Note that the problematic file (/root/.conan2/p/b/libic653990c05fb7e/p//bin/iconv) shouldn't even be getting packaged, as it is an executable not a library.

[memsharded]

I tried rebuilding all my dependencies with this option, and only libiconv failed, due to what appears to be a bug in the libiconv recipe, though I don't know what it is yet.

Thanks very much for checking and testing this, very appreciated!

This is exactly the type of risk that we want to be aware, and understand the tradeoffs before moving something like this forward, very often changes looking as evident as this one do actually break different users and use cases out there in the wild.

Note that the problematic file (/root/.conan2/p/b/libic653990c05fb7e/p//bin/iconv) shouldn't even be getting packaged, as it is an executable not a library.

This shouldn't be a problem, there are several packages in ConanCenter, that even they are library-like, for example protobuf, containing the libprotobuf library, they also contain some executable, for example the protoc executable. These packages can be used simultaneously as requires and tool_requires as seen in this example https://docs.conan.io/2/examples/graph/tool_requires/using_protobuf.html, allowing to use those executables as build-tools by other packages, even in cross-building scenarios.

So the solution wouldn't be to remove the iconv executable from the package. It would be great to understand why this is happening in the first place, maybe tomorrow @jcar87 or some other expert from ConanCenter can give further insights about this.

[memsharded]

I think this is what is failing in the tests in CI

[pbs3141]

The issue with libiconv can't simply be due to the fact it's installing an executable, because my autotools libpotrace recipe also does this but works fine with the PR.

[memsharded]

I think it would be better to at least fix iconv for this use case before merging this, as this could break the builds of some users, even if conceptually this PR looks "correct", the risk of breaking is clear, and iconv is used by tons of users.

[memsharded]

The added test was broken, I have submitted a fix for the test.

This would still need the fix for iconv in ConanCenter first, I'll try to have a look

[memsharded]

The failed tests are unrelated, they come from recent updates of github runners for OSX that broke some of our tests, we are trying to fix those tests.

I have tried building libiconv/1.18 with:

conan create . --version=1.18 -c tools.files.download:verify=False -c tools.build:install_strip=True

In my Ubuntu, but it didn't fail. Up to my knowledge, it didn't get any recent change to fix this.

It seems this could fail only for an Android build? Can you please give more details on your seen errors and how to reproduce?

[jcar87]

The implementation itself LGTM.

however I'm not sure what will work in practice if enabled wholesale

The tools.build:install_strip config is supposed to allow shared libraries to be stripped.

The docs currently say this:

tools.build:install_strip: (boolean) Strip the binaries when installing them with CMake and Meson

Which "binaries" are stripped are up to the relevant tool. I'm fairly certain that the autotools target only strips executables, not libraries. And I've seen some cases where install-strip doesn't work, or where the strip configuration is different (its provided as a flag to ./configure).

So while the implementation is fine, I feel like this makes very few guarantees compared to what meson and CMake offer.

[memsharded]

Merged, this will go in next 2.20. Thanks again for your contribution!