Cannot depend on two overlapping cpp_proto_library()'s

[fahhem]

If you do this, then bazel will link in two separate (merged) directories. At best, this will cause ODR violations at link time; at worst, it gets caught at runtime (I found it when compiling with ASAN to debug a segfault caused by it).

Essentially, if I have a cc_binary that depends on a rule at //path:proto_one_cc, which is a cpp_proto_library(), and another rule at //path:proto_two_cc, and they both depend on a cpp_proto_library(name="common"), then I'll get two instances of every linked object from the common proto. Typically, this will include things harmless like the default instance of the protos, but also various static objects like descriptors that are compared by pointer (because why would you have two descriptors for the same proto in your binary?).

One way to avoid this is to make the cpp_proto_library() also an aspect, not just the cpp_proto_compile(), and switch to merge_directories=False. That aspect would be able to grab the includes from all its dependencies, transitively, and add those to itself. It would then return CcInfo to be part of a C++ compilation natively.

[aaliddell]

Would you be able to provide a failing test workspace for this? Also, does the cpp_proto_library in the protobuf repo correctly handle this?

Your proposed solution looks like it's the proper way of doing this, so I'll add it to the TODO list.

[toddlipcon]

I threw together a test repo here: https://github.com/toddlipcon/rules_proto_grpc/tree/overlapping-deps/test_workspaces/common_cpp_library

The test fails at runtime due to multiple definitions of a field extension (this is the issue I'm hitting in my project due to a more complex dependency graph including the file which has the extensions multiple times)

[aaliddell]

Thanks, that's a great help!

[aaliddell]

This should be fixed in 3.0.0 release, where there is a non-transitive compilation mode that will avoid the common ancestor's definitions appearing multuple times.