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

[question] Conan 2.x Migration - transitive_headers and transitive_libs Trait Defaults #17505

Open
1 task done
radonish opened this issue Dec 18, 2024 · 11 comments
Open
1 task done

Comments

@radonish
Copy link

radonish commented Dec 18, 2024

What is your question?

Hello, I've read over the requirements traits documentation and want to ensure the behavior I'm seeing is as desired.

Let's say liba requires libx and liby. Let's also say that all 3 libraries are built shared. liba conanfile.py has:

def requirements(self):
    self.requires(libx/1.0.0)
    self.requires(liby/1.0.0)

In migrating the test_package for liba my first attempt was to simply have the following in test_package's conanfile.py:

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

The link of test_package for liba fails due to symbols from libx and liby. (Edit: Note, the test_package is just an empty main() - no actual code/references to the transitive dependencies.)

If I update liba's conanfile.py as follows, test_package can link:

def requirements(self):
    self.requires(libx/1.0.0, transitive_headers=True, transitive_libs=True)
    self.requires(liby/1.0.0, transitive_headers=True, transitive_libs=True)

I think I would have expected the transitive_headers and transitive_libs to be True by default since the libraries are shared. Any insight on the behavior/my assumptions would be appreciated. Thank you!

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded memsharded self-assigned this Dec 18, 2024
@memsharded
Copy link
Member

Hi @radonish

Thanks for your question.

First, the transitive_headers=True are only necessary if the public headers of the package is #including the headers of the libx/1.0 and liby/1.0 respectively. This is not considered the default, as the default is try to hide as an implementation detail the transitive dependencies and expose to the consumers just the public interface of the current package. But some libraries, for example a "mathlib" that uses "eigen" as a base library might want to include eigen in the public headers and expose the eigen matrix types to the end consumer. For this case transitive_headers=True is what needs to be done

transitive_libs=True should be way more exceptional. Conan already manages correctly the propagation of transitive linkage requirements ,taking into account the package_type = shared-library/static-library/header-library. This is done by explicitly defining the package_type or by defining a shared option (the package_type will be implicitly deduced from the option value). With this, Conan can propagate things. You can easily check with:

$ mkdir liba && cd liba && conan new cmake_lib -d name=liba
$ conan create .
$ cd ..
$ mkdir libb && cd libb && conan new cmake_lib -d name=libb -d requires=liba/0.1
$ conan create .

This also works for shared libraries, you can add -o "*:shared=True" to the commands above and it will still work.

So the only case where transitive_libs=True is necessary is when transitive_headers=True and also those header require some linkage of code that is not already propagated automatically by the transitive linkage.

@radonish
Copy link
Author

Hello @memsharded, thank you for the quick response.

I ran the commands you gave and do see the expected behavior for those test packages. I'm comparing the generated recipes with the actual recipes my team is using in order to determine what is going wrong.

One key observation when looking at the linker command line for the link failure that occurs in the test_package for our library is that the library paths are there (-L parameters for transitive dependencies are contained) but the specific transitive library path/names are missing. That makes me wonder if it is a package_info problem where the libs for the transitive dependency packages aren't being properly set/advertised.

(In one example our proprietary library - let's call it libx - is simply dependent on the Conan Center's POCO library package. libx builds and links fine, but its test_package fails with linker errors that it cannot find POCO. As noted above, the linker command line contains the -L parameter to the Conan cache for POCO but I do not see any of the POCO libraries called out.)

@radonish
Copy link
Author

radonish commented Dec 19, 2024

@memsharded, the issue appears to occur when cross-compiling. An example cross-compile host profile that fails with your liba/libb test is this aarch64 one below:

$ cat ~/.conan2/profiles/profile-aarch64-buildroot-release
{% set TARGET_HOST = "aarch64-buildroot-linux-gnu" %}
{% set SYSROOT = "/tools/toolchains/aarch64-buildroot-linux-gnu_sdk-buildroot/aarch64-buildroot-linux-gnu/sysroot" %}

[settings]
os=Linux
arch=armv8
compiler=gcc
compiler.version=8.4
compiler.libcxx=libstdc++11
compiler.cppstd=gnu11
build_type=Release

[options]

[buildenv]
CONAN_CMAKE_SYSTEM_NAME=Linux
CONAN_CMAKE_SYSTEM_PROCESSOR=aarch64
PATH=+(path)/tools/git_2.42.0/bin:/tools/git_lfs_3.4.0/bin:/tools/python_3.11.4_venv/company_2.0.0/bin:/tools/cmake_3.30.5/bin:/tools/toolchains/aarch64-buildroot-linux-gnu_sdk-buildroot/bin
CC={{ TARGET_HOST }}-gcc
CXX={{ TARGET_HOST }}-g++
STRIP={{ TARGET_HOST }}-strip
LD={{ TARGET_HOST }}-ld
CFLAGS=--sysroot={{ SYSROOT }} -O3 -fPIC
CXXFLAGS=--sysroot={{ SYSROOT }} -O3 -fPIC -std=gnu++11
LDFLAGS=--sysroot={{ SYSROOT }} -L{{ SYSROOT }}/usr/lib -s

It works with a host profile that matches the build profile:

[settings]
os=Linux
arch=x86_64
compiler=gcc
compiler.version=4.8
compiler.libcxx=libstdc++
compiler.cppstd=gnu11
build_type=Release

[options]

[buildenv]
COMPANY_BUILD_HOST=True
CONAN_CMAKE_SYSTEM_NAME=Linux
CONAN_CMAKE_SYSTEM_PROCESSOR=x86_64
PATH=+(path)/tools/git_2.42.0/bin:/tools/git_lfs_3.4.0/bin:/tools/python_3.11.4_venv/company_2.0.0/bin:/tools/cmake_3.30.5/bin
CC=gcc
CXX=g++
CFLAGS=-O3 -s -fPIC
CXXFLAGS=-O3 -s -fPIC -std=gnu++11
LDFLAGS=-s

@radonish
Copy link
Author

Here is a build log that shows:

cd liba
conan create . -pr:b profile-x86_64-rhel-release -pr:h profile-aarch64-buildroot-release
cd ../libb
conan create . -pr:b profile-x86_64-rhel-release -pr:h profile-aarch64-buildroot-release

(Changed them to shared libraries within the conanfile.py itself.)

@radonish
Copy link
Author

radonish commented Dec 20, 2024

@memsharded, from the log above, the linker command that fails to link libb to liba for the libb test_package is:

/tools/toolchains/aarch64-buildroot-linux-gnu_sdk-buildroot/bin/aarch64-buildroot-linux-gnu-g++ --sysroot=/tools/toolchains/aarch64-buildroot-linux-gnu_sdk-buildroot/aarch64-buildroot-linux-gnu/sysroot -O3 -fPIC -std=gnu++11 -O3 -DNDEBUG --sysroot=/tools/toolchains/aarch64-buildroot-linux-gnu_sdk-buildroot/aarch64-buildroot-linux-gnu/sysroot -L/tools/toolchains/aarch64-buildroot-linux-gnu_sdk-buildroot/aarch64-buildroot-linux-gnu/sysroot/usr/lib -s CMakeFiles/example.dir/src/example.cpp.o -o example   -L/conantest/conan_data2/b/libb859c6370e04f5/p/lib  -L/conantest/conan_data2/b/liba29989e5cd9bde/p/lib  -Wl,-rpath,/conantest/conan_data2/b/libb859c6370e04f5/p/lib:/conantest/conan_data2/b/liba29989e5cd9bde/p/lib /conantest/conan_data2/b/libb859c6370e04f5/p/lib/liblibb.so

I think the problem is related to how the linker command is being constructed. My understanding is we want to use 1 of 2 options:

  1. Specify the shared library paths with -L and then reference the desired shared libraries with -l or explicit full paths
  2. Use the linker -rpath-link option

Instead the linker command being generated is adding a -L for each package (liba and libb) but no -l or full shared library path is included for liba. The linker is passed the -rpath option but I believe it should be -rpath-link. In fact, I manually executed the linker command above and changed -rpath to -rpath-link and the link succeeded. Note, we'd still want the -rpath flag for runtime use case.

If I change the Conan-generated libb/test_package/conanfile.py from:

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

to

def requirements(self):
    self.requires(self.tested_reference_str)
    self.requires("liba/0.1")

The link succeeds because it causes the addition of the full path to liba in the cache to the linker command line.

I haven't dug into why this works OK when I'm not cross-compiling and the build and host profiles are just the default RHEL build tools. Perhaps it has something to do with GCC 4 (build tool version) vs. GCC 8 (host tool version for our buildroot-based cross toolchains) and linker nuances.

Either way, it's starting to feel like this is a cross-compilation test_package linker bug when the dependency tree is made up of shared libraries. It seems to me that the test_package linker command should include the full path to the shared libraries of the transitive dependencies (as is the case for the actual package linker command).

Thank you

@radonish
Copy link
Author

radonish commented Jan 6, 2025

@memsharded, following up - hope you had a nice holiday season!

Wanted to share that an easy way to reproduce the problem is as follows

$ mkdir liba && cd liba && conan new cmake_lib -d name=liba
$ conan create . -pr:b <build profile> -pr:h <cross host profile> -o "*:shared=True"
$ cd ..
$ mkdir libb && cd libb && conan new cmake_lib -d name=libb -d requires=liba/0.1
$ conan create . -pr:b <build profile> -pr:h <cross host profile> -o "*:shared=True"

# libb create command fails within test package; skip test package with -tf option
$ conan create . -pr:b <build profile> -pr:h <cross host profile> -o "*:shared=True" -tf=""

$ cd ..
$ mkdir appx && cd appx && conan new cmake_exe -d name=appx -d requires=libb/0.1
$ conan create . -pr:b <build profile> -pr:h <cross host profile> -o "*:shared=True"
# appx create command fails

Again, I'm using a buildroot-generated aarch64 host profile to get the failure above. If I use a buildroot-generated x86_64 host profile (that is relatively close to the RHEL toolchain installed on the build machine) it succeeds just as the default profile succeeds - so it's not something specific to buildroot-generated toolchains.

If I update libb/conanfile.py to pass transitive_libs=True for the liba requirement the libb test package failure and appx failure go away. Per all the discussion here, it seems that transitive_libs=True should not be necessary and that there is an issue with automatic transitive lib propagation when shared libs are being used.

@memsharded
Copy link
Member

Hi @radonish

I hope you had some nice holiday season too!

Thanks for following up and the feedback. I hope you had some nice holiday season too!

It is indeed possible that sysroots cause some issues in the find_package() resolution paths, @jcar87 is also aware of this. The way the dependencies xxx-config.cmake are found when there are cross-compilation and sysroot is a bit challenging, and we are aware that the current CMakeDeps might have some issues resolving those, due to the way CMake prioritizes the paths to search for those files.

One of the goals of the new incubating CMakeDeps in https://docs.conan.io/2/incubating.html is that it helps to reduce this issue with direct definition of dependencies locations, but it is a bit too early stage. Still if you would like to give it a try, it would be very valuable feedback.

It is not that straightforward for me to have a buildroot-generated aarch64, maybe @jcar87 could help with this.

@radonish
Copy link
Author

radonish commented Jan 8, 2025

Thanks for the response, @memsharded and for pulling in @jcar87.

May I ask what version of GCC you're using in your non-cross-compile testing? Our local build machines are using GCC 4 whereas our cross-compilation buildroot-generated toolchains are GCC 8.

In this case it's not obvious that the sysroot is actually causing a problem itself. When breaking down a simplified version of what gets put on the link stage command line for the liba/libb/appx example, line by line:

g++
CMakeFiles/appx.dir/src/appx.cpp.o CMakeFiles/appx.dir/src/main.cpp.o
-o appx
-L<liba in Conan cache>
-L<libb in Conan cache>
-Wl,-rpath,<libb in Conan cache>:<liba in Conan cache>
<libb directory in Conan cache>/liblibb.so

It seems like it should work.

For GCC 4, it's clear that the shared library dependency paths are being used from the rpath option passed to the linker as I can delete the -L options and it still links.

For GCC 8 it's not clear why it fails to find libliba.so with the same core command line options. As stated earlier, I can get it to work in GCC 8 if I add rpath-link to the above options (-Wl,-rpath-link,<libb in Conan cache>:<liba in Conan cache>).

I'm trying to understand if there is a GCC version behavior delta that needs to be taken into account when generating that link command string. Is that a Conan or CMake responsibility?

EDIT: I recalled that one of our target cross-compilation toolchains was also GCC 4 (for an old ARM target). I verified that the cross-compilation liba/libb/appx example is successful with that GCC 4 ARM toolchain. So, it does indeed appear to be a GCC 4 vs. GCC 8/modern GCC issue. After some quick research I do see references to changes made in modern GCC versions in regards to how the rpath option is handled - rpath apparently now only applies to the current binary being processed, not its dependencies.

Thank you

@memsharded
Copy link
Member

May I ask what version of GCC you're using in your non-cross-compile testing? Our local build machines are using GCC 4 whereas our cross-compilation buildroot-generated toolchains are GCC 8.

Sure, I typically use something like Ubuntu 22, with a gcc 11.

I'm trying to understand if there is a GCC version behavior delta that needs to be taken into account when generating that link command string. Is that a Conan or CMake responsibility?

Yes, it would seem something more related to CMake than to Conan, it is CMake the one building the compile commands, not Conan, and Conan doesn't do anything different for different gcc compiler versions (in general Conan tries to be very agnostic about this, the knowledge that Conan build-system integrations like CMakeToolchain or CMakeDeps have regarding compiler versions is very minimal, none if we are talking about gcc up to my knowledge).

Regarding other possible -Wl,-rpath-link issues, the new incubating CMakeDeps generator brings completely new definition of targets, with more information about imported targets types, so that might still help to solve issues even if it is not related to cross-build toolchains or sysroots.

@radonish
Copy link
Author

radonish commented Jan 9, 2025

Thank you, @memsharded.

I have found a few CMake threads that essentially echo this same issue I've created in the liba/libb/appx sandbox. One good one is CMake issue #22678. I've not digested the discussions across the CMake threads well enough to know what could be done differently in the Conan-generated CMake files (via CMakeDeps) to trigger CMake to include -Wl,-rpath-link.

I did try the incubating CMakeDeps and the liba/libb/appx example worked with my GCC 8-based cross-compile toolchains that would otherwise fail. When inspecting the linker command line I can see that the behavior change in the incubating CMakeDeps is the explicit reference in the linker command of each transitive dependency library within the Conan cache. The new approach effectively works around the missing -Wl,-rpath-link option problem and avoids having to have GCC version-specific logic in the CMake generation.

I'm working on updating 80+ recipes to Conan 2.x. This incubating CMakeDeps avoids lots of needless references to transitive shared library dependencies within the conanfile.py and CMakeLists.txt files - but it's documented as something that should be used for testing only.

I'm at a fork in the road: litter conanfile.py and CMakeLists.txt files with what should be implicit transitive shared library dependencies or enable the experimental feature. The incubating page notes it "is limited to xxx-config.cmake files" - if that's all that we need, do you think it's safer to use?

@memsharded
Copy link
Member

I'm at a fork in the road: litter conanfile.py and CMakeLists.txt files with what should be implicit transitive shared library dependencies or enable the experimental feature. The incubating page notes it "is limited to xxx-config.cmake files" - if that's all that we need, do you think it's safer to use?

The new incubating CMakeDeps is definitely the future, but it might be a bumpy road at the moment, so it is almost guaranteed that it will break, so not safe to use at all. But it has already achieved the goal, if you reported that it solved your problem, that is gold feedback for us to keep working on it and get it out of the "incubating" and at that point it will be much safer.
The thing is that even if it solves your issue at the moment, the incubating CMakeDeps seems that it is still doing some overlinking in some cases, and #17459 is trying to solve it. Maybe if this is solved your problem comes back, so lets try to wait a bit, at least until this is fixed.

What is your urgency? Even if it is incubating now, it is not that our timelines are too large, it might be just a matter of a couple of months, maybe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants