search

conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.3k stars 986 forks source link

[question] Conan2 INTERFACE_LINK_DIRS for static libs #17197

Closed darakelian closed 1 month ago

darakelian commented 1 month ago

What is your question?

Is there a reason that the generated target files in conan2 CMakeDeps set the INTERFACE_LINK_DIRECTORIES even if the library is a static lib? for example if my project depends on zlib being built as a static lib while using the Ninja generator, I am observing that the -Wl,-rpath option is showing up in the LINK_FLAGS with the directory being set to the lib dir of the zlib package in my conan cache. It seems like meson had an issue reported about this https://github.com/mesonbuild/meson/issues/5191 and it was fixed. This doesn't actually seem to be affecting our builds but another dev noticed it while reviewing one of our projects

It seems like we should not be doing this for static libs and I'm curious if there was a conscious decision to do it this way or if the team would be open to some sort of opt-out behavior?

Have you read the CONTRIBUTING guide?

memsharded commented 1 month ago

Hi @darakelian

Thanks for your question.

Yes, there is a reason. With the current CMakeDeps generator and current cpp_info, Conan does not have enough information to know if it is a STATIC or SHARED imported target, and then, defining the INTERFACE_LINK_DIRS becomes necessary when there are transitive shared libraries in Linux. As it doesn't seem to have any other unwanted side effects, it is always defined.

We are working in a new CMakeDeps generator in https://github.com/conan-io/conan/pull/16964 that will always define targets as SHARED, STATIC or INTERFACE, and so far it seems the definition of INTERFACE_LINK_DIRS is not necessary in this case (but still it is a bit too early, this new generator will need more testing and usage).

darakelian commented 1 month ago

Does conan not have access to the shared option when these target files get generated? As it stands, the current behavior I'd argue is technically wrong (though truthfully I'm not an expert in if passing these options to the linker would ever negatively impact the compilation/linking or the run-time of these libraries) since we don't need to pass these options to the linker if we aren't using them.

memsharded commented 1 month ago

It doesn't have information of the full location it needs, and it can also produce the wrong assumptions when there are packages with one package_type but it might contain different libraries and not all of them matching the package_type.

We tried to change it, but it was too risky and it will certainly be breaking behavior for many users, this is why we are proposing a new CMakeDeps that is fully opt-in to be able to improve these things.

I'm not an expert in if passing these options to the linker would ever negatively impact the compilation/linking or the run-time of these libraries)

In practice, as long as the -l<libraryname is not being passed, it doesn't have a negative impact, up to our understanding.

darakelian commented 1 month ago

OK, I appreciate you taking the time to explain the current implementation choices. I'll close this issue and if we do ever run into a negative impact I'll circle back and we can cross that bridge if needed!

memsharded commented 1 month ago

Great, testing and feedback on the new CMakeDeps once we start to share it (track https://github.com/conan-io/conan/pull/16964, hopefully next release) would be very appreciated too.