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

Consuming Abseil via Protobuf as transitive dependency is fragile #249

Open
uilianries opened this issue Jun 3, 2024 · 4 comments
Open

Comments

@uilianries
Copy link
Contributor

Hello!

We have packaged libprotobuf-mutator in Conan (see PR #24163). During this process, we noticed that the Abseil library is not only used by Protobuf but also directly by libprotobuf-mutator. For instance, the absl::StrCat function is used without a direct reference, such as a header include or CMake target.

This creates a fragile situation where the functionality may depend on Protobuf’s use of Abseil, leading to potential issues during linking. It would be beneficial to refactor the CMake configuration of this project to explicitly consume both Protobuf and Abseil. This can be achieved not only by using CMake targets but also by utilizing the official FindProtobuf module to avoid additional overhead.

I am willing to submit a PR with these changes, but I would first like to hear your thoughts on this matter.

@vitalybuka
Copy link
Collaborator

vitalybuka commented Jun 5, 2024

I am not sure how FindProtobuf will work if it's looking for system lib. For best effect we need proto library instrumented with coverage and sanitizer.

Explicit Abseil LGTM.

@georgthegreat
Copy link
Contributor

Actually, NixOS build of protobuf-mutator is broken by this issue, as NixOS hides transitive dependencies from the user.

@uilianries, @vitalybuka, do you have a fix / workaround for the issue by any chance?

@uilianries
Copy link
Contributor Author

@georgthegreat There is no straightforward workaround, even if you use CMAKE_PROJECT_TOP_LEVEL_INCLUDES to inject find_package(absl), you still need to update the project's CMake file in order to consume the required targets from Absiel.

@georgthegreat
Copy link
Contributor

@uilianries, any chance you do you have a patch to achieve this?

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

No branches or pull requests

3 participants