[bug] Problems with CMakeDeps generator

[gouriano]

Describe the bug

Environment:
Linux
cmake 3.21.2
conan 2.6.0

I have a problem linking against gRPC, but the real problem seems to be in CMakeDeps generator.
It fails to generate proper library settings for dependent packages (in this case, abseil and protobuf)

How to reproduce it

Take test_package.cpp source file from abseil recipe
https://github.com/conan-io/conan-center-index/blob/master/recipes/abseil/all/test_package/test_package.cpp

Create CMakeLists.txt

cmake_minimum_required(VERSION 3.15)
project(test_package LANGUAGES CXX)
find_package(absl REQUIRED)
add_executable(test test_package.cpp)
target_link_libraries(test abseil::abseil)

and create the following conanfile.txt:

[requires]
grpc/1.50.1
[options]
abseil*:shared=True
grpc*:shared=True
protobuf*:shared=True
[generators]
CMakeDeps
[layout]
cmake_layout

Yes, require grpc (which requires abseil, as we know)
Now run

conan install . --build missing -s build_type=Release

and look at build/Release/generators/absl-release-x86_64-data.cmake
we see:
set(abseil_LIBS_RELEASE )
The list is empty.

Now delete build directory, add abseil into conanfile.txt, and do "conan install" again

[requires]
abseil/20230802.1
grpc/1.50.1
[options]
abseil*:shared=True
grpc*:shared=True
protobuf*:shared=True
[generators]
CMakeDeps
[layout]
cmake_layout

Now in absl-release-x86_64-data.cmake we see

set(abseil_LIBS_RELEASE absl_flags_parse absl_log_flags .. and the long list of libraries)

I believe, it is a bug?

I do not know what is abseil and I do not care.
What I need is grpc. I request grpc, I "find_package(gRPC)", and then linking fails because abseil libraries are missing

The same story is with protobuf.
If it is absent in the list of requires, protobuf_LIBS_RELEASE list in protobuf-release-x86_64-data.cmake file is empty.
If present, protobuf_LIBS_RELEASE list is not empty.
It is so in "shared" mode only. in "shared=False", everything works as expected - LIBS_RELEASE lists are never empty

[memsharded]

This seems connected to this: conan-io/conan-center-index#24806, it is possible that it is a recipe issuel, will investigate

[memsharded]

As described in #16898 (comment), the fact that set(abseil_LIBS_RELEASE ) shouldnt be an issue if abseil is not a direct dependency.

We are trying to reproduce with a full minimal example, but so far can't.

[gouriano]

but so far can't.

Interesting... With the described steps, I reproduce it easily, all the time.
If you need any additional info about my environment, let me know.

[memsharded]

Yes, we would need a full example that produces the linking errors that other users are seeing. I think it is related, but the above setup is not enough:

• The target_link_libraries(test abseil::abseil) is expected to fail. abseil is not a direct dependency, and as such it shouldn't be directly referenced or linked in consumer build scripts or code. To make this possible, it is necessary to add abseil as a direct requires in the conanfile.
• The set(abseil_LIBS_RELEASE ) is totally expected for a transitive shared libraries, which are not part of the linked libraries, but part of the INTERFACE_LINK_DIRECTORIES only
• The rational for this is as follows: if grpc would be shared library that links abseil as a static library, then abseil would become an internal implementation detail of grpc, what we call the binary "embed" mode. Then, consumers of grpc should not link directly abseil, because they don't really depend on it.

So what is needed is a full repro case that:

• Declares only grpc as dependency
• Links only with grpc in the build scripts
• Produce linking errors

So having the full consumer conanfile, CMakeLists.txt and .cpp files, together with the exact Conan commands is what would help. I have been trying to reproduce that with the test_package of grpc, which already contain such files, but so far I haven't been able to reproduce.

[SpaceIm]

Actually I'm quite surprised now that I'm reading git history of grpc recipe. I've added transitive_libs for abseil in conan-io/conan-center-index#17284 (it was already there for protobuf), but it has been removed later on for abseil & protobuf by conan-io/conan-center-index#24215 (while keeping transitive_headers, which is quite weird, those libs are not header-only so if headers are public, libs are also public). Why? I even provided a link conan-io/conan-center-index#17284 (comment) which was quite self explanatory for the reason why transitive_libs was required.
abseil and protobuf are public dependencies of grpc.

So actually it might be a recipe issue.

Moreover test package has been so simplified in conan-io/conan-center-index#24215 that it can't properly test anymore this kind of link issue.

[gouriano]

I hope the following is a better example.

Take two files - test_package.cpp and helloworld.proto, from the previous version of grpc recipe
https://github.com/conan-io/conan-center-index/tree/1bdbd1323367c1d36cd078bf5a122c719d857448/recipes/grpc/all/test_package

Add CMakeLists.txt:

cmake_minimum_required(VERSION 3.15)
project(testapp)
find_package(gRPC REQUIRED)
add_executable(testapp test_package.cpp)
target_link_libraries(testapp gRPC::grpc++)
protobuf_generate(TARGET testapp PROTOS helloworld.proto 
    PROTOC_OUT_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
protobuf_generate(TARGET testapp PROTOS helloworld.proto 
    PROTOC_OUT_DIR "${CMAKE_CURRENT_SOURCE_DIR}"
    LANGUAGE grpc GENERATE_EXTENSIONS .grpc.pb.h .grpc.pb.cc
    PLUGIN protoc-gen-grpc=$<TARGET_FILE:gRPC::grpc_cpp_plugin>)

Add conanfile.txt:

[requires]
grpc/1.50.1
[options]
*:shared=True
[generators]
CMakeDeps
CMakeToolchain
VirtualRunEnv
[layout]
cmake_layout

Configure

conan install . --build missing -s build_type=Release
cd build/Release
cmake ../../ --preset conan-release

Build:

make
...
[100%] Linking CXX executable testapp
/usr/bin/ld: CMakeFiles/testapp.dir/helloworld.pb.cc.o: undefined reference to symbol '_ZN6google8protobuf8internal14ArenaStringPtr12ClearToEmptyEv'
//export/home/gouriano/Conan/.conan2/p/b/protob24d2b536231b/p/lib/libprotobuf.so.32: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

Add protobuf into conanfile.txt:

[requires]
grpc/1.50.1
protobuf/3.21.12
[options]
*:shared=True
[generators]
CMakeDeps
CMakeToolchain
VirtualRunEnv
[layout]
cmake_layout

configure, build. The output now says

[ 66%] Linking CXX executable testapp
/usr/bin/ld: CMakeFiles/testapp.dir/helloworld.grpc.pb.cc.o: undefined reference to symbol '_ZN4absl12lts_202308025MutexD1Ev'
//export/home/gouriano/Conan/.conan2/p/b/absei9f439b2f74c73/p/lib/libabsl_synchronization.so.2308.0.0: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status

[jcar87]

Apologies for the grpc/abseil case - this is a regression that I introduced when adding the new version. fix here: https://github.com/conan-io/conan-center-index/pull/25087/files

if headers are public, libs are also public

Arguably, that is an overarching generalisation which does not apply to all cases.
for a library A that has transitive_headers=True on library B - transitive_libs=True would only be needed if the headers of library A contain implementation causing the consumer to require symbols/references that are only in library B. This is not the case all the time - in a lot of cases the transitive headers are require to know types, or a macro, or only reference "pointers" of things - but do not cause the consumer to reference symbols. in other cases, it may be "fully public" dependency, but one such that it is impossible for the consumer to be "unaware" that another library is transitively being used - to the point where the consumer should also be depending on the transitive dependency.

The case at hand (grpc) is different - the references are mostly in generated code (by protoc) in the consumer side - so it's neither of the cases above - in order to determine what is transitive we need to check the generated code too.

transitive_headers=True and transitive_libs=True are NOT equivalents to cmake "PUBLIC" modifier.

CMake's target_link_libraries(A PRIVATE B) translates to transitive_libs=False if B is shared, else transitive_libs=True, but transitive_headers=False.

Whereas Conan's transitive_libs=True(vs leaving it undefined) means they are always propagated when linking against "this" - regardless of library types. We tend to discourage this on purpose so that users can retain the flexibility to have a shared library that statically links a transitive dependency, causing the symbols to be provided by the shared library to consumers (such that the static library does not propagate beyond "this"). If transitive_libs=True, and "this" recipe is shared, and the transitive dependency is static, we'll end up with duplicated symbols.

From what I can see, equivalent functionality was requested in CMake here

• https://discourse.cmake.org/t/add-only-library-headers-during-target-link-libraries/2973

And was implemented in CMake 3.27 as the COMPILE_ONLY genex: https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#id30

thus transitive_headers=True and leaving transitive_libs undefined, is a very valid case and is the intention in Conan 2 to support it.

I do like CMake's PRIVATE/PUBLIC/INTERFACE, but in practice, they do not map 1:1 with the Conan 2 traits, they map to the low-level behaviour of CMake (LINK_ONLY, COMPILE_ONLY).

Perhaps there is a case for Conan to give the ability to express in the recipe that consumers will reference symbols from a transitive dependency that may require propagating a linking library, depending on the specific combination of shared/static that we have.

Moreover test package has been so simplified in conan-io/conan-center-index#24215 that it can't properly test anymore this kind of link issue.

I remember testing the new recipe revision against the old test_package, as well as other consumers that existed in Conan Center, manually, precisely to avoid this kind of regression. Bad on my part, as I clearly did not test the specific platforms where this issue arises.

[jcar87]

Thanks @gouriano for providing more details. The abseil build error is being fixed in https://github.com/conan-io/conan-center-index/pull/25087/files which will be merged today.

As for the protobuf errors, if you are calling protobuf_generate in CMake, and are adding the generated sources to a target, then you should directly depend on protobuf - not only in the conanfile.py, but also in target_link_libraries - which is what the old test package was actually doing.

Abseil, on the other hand is implicit and unknown to the consumer - hence the need for the fix.

[gouriano]

I disagree. I do not see why protobuf is different than abseil.
protobuf_generate uses a code generator, while linking uses libraries.
grpc requires protobuf, always, unconditionally. When I "require" grpc, I should get correct settings for protobuf.
When I define IMPORTED library in CMake, I am not supposed to know what does that require. Maybe, it requires packages X and Y, whatever. I get them all automatically. Same thing here. I link against grpc. grpc requires protobuf, I should get it automatically.
Anyway, I now know a workaround, I know how to handle this. To fix it or not, is your job, to make it work - mine.

[jcar87]

Using protobuf_generate on the consumer side means that the consumer is using protobuf directly and not implicitly - both at the cmake level, and the C++ level - thus, protobuf is a direct dependency and should be expressed as such. This is in line with the practices recommended by:

• include what you use: https://github.com/include-what-you-use/include-what-you-use
• link what you use: https://cmake.org/cmake/help/latest/prop_tgt/LINK_WHAT_YOU_USE.html
• the error the linker gave you: "error adding symbols: DSO missing from command line" - which translates to "I know where this symbol is, and know who provides it and know how to resolve it, but I haven't been told to link it directly, so I won't"

Linking directly with libprotobuf when using gRPC C++ with protobuf generate, is also what all the grpc examples do, which is what was missing from your code

• https://github.com/grpc/grpc/blob/v1.66.0/examples/cpp/route_guide/CMakeLists.txt#L71
• https://github.com/grpc/grpc/blob/v1.66.0/examples/cpp/route_guide/CMakeLists.txt#L87
• https://github.com/grpc/grpc/blob/70b3562b824893ac3a7033fd3077ac4664d6c233/examples/android/helloworld/app/CMakeLists.txt#L68
• You will see this behaviour in most examples in the grpc repo that invoke protobuf

As well as explicit calls to both find_package(Protobuf) and find_package(gRPC):

• https://github.com/grpc/grpc/blob/70b3562b824893ac3a7033fd3077ac4664d6c233/examples/cpp/cmake/common.cmake#L100-L101
• https://github.com/grpc/grpc/blob/70b3562b824893ac3a7033fd3077ac4664d6c233/examples/cpp/cmake/common.cmake#L113

This is also what we had in in the original test package:

• https://github.com/conan-io/conan-center-index/blob/d19ae905cc348712c882bf3c23603efa7573762c/recipes/grpc/all/test_package/CMakeLists.txt#L14

What we are advocating for and supporting in the recipe is to use gRPC in line with how it is in the gRPC examples and documentation. Abseil is different because it is used with (almost) zero visibility on the consumer side.

[vakatov]

grpc requires protobuf, always, unconditionally

FWIW, if I remember it correctly, theoretically gRPC can use other protocols (not protobuf). Not sure if anybody does that nowadays though.

[memsharded]

Any further question here @gouriano?
Now that the grpc recipe has been fixed and the rationale has been clarified above, I think this ticket can be closed.