Duplicate symbol issue with bazel and blacklisted_protos

[keith]

What version of protobuf and what language are you using?
Version: v3.10.1
Language: C++

What operating system (Linux, Windows, ...) and version?
macOS 10.14.6 targeting iOS 11.0

What runtime / compiler are you using
clang 11.0 (Xcode 11.2.1)

General description

When building envoy-mobile we discovered there's an issue with protobuf where symbols are duplicated in the binary that is produced. This started when we updated protobuf from v3.9.2 and I narrowed this down to these 2 BUILD file changes:

7b28278
a03d332

It appears that when using proto_library targets as your proto_lang_toolchain's blacklisted_protoss, it does not have the same effect as before (I've filed this bazel bug since I assume this is an issue there bazelbuild/bazel#10484).

What did you do?
Steps to reproduce the behavior:

1. Checkout a local copy of protobuf at v3.10.1
2. Apply the patch in envoy-mobile/envoy/bazel/protobuf.patch
3. In an envoy-mobile clone checked out at d27f0deb854d931f59effb0da6b9e2f8b9a6c4cc run bazel aquery 'deps(ios_dist)' --override_repository=com_google_protobuf=/path/to/protobuf --output=textproto > /tmp/after.proto
4. In your checkout of protobuf run git checkout v3.9.2 -- BUILD to get the older version of the BUILD file
5. In envoy-mobile run bazel aquery 'deps(ios_dist)' --override_repository=com_google_protobuf=/path/to/protobuf --output=textproto > /tmp/before.proto
6. Using aquery_differ in a bazel checkout run bazel run //tools/aquery_differ -- --before=/tmp/before.proto --after=/tmp/after.proto --input_type=textproto --attrs=inputs --attrs=cmdline | less
7. Notice that some actions have new inputs of the libraries from the wkt protos such as:

+bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild/bin/external/com_google_protobuf/libstruct_proto.a
+bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild/bin/external/com_google_protobuf/libtimestamp_proto.a
+bazel-out/ios-x86_64-min10.0-applebin_ios-ios_x86_64-fastbuild/bin/external/com_google_protobuf/libwrappers_proto.a

You can also use this aquery method to see that on the v3.10.1 tag removing blacklisted_protos entirely has no effect on the build, which isn't what we would expect. If you do the same test removing blacklisted_protos on the v3.9.2 tag you will see there's a similar difference as above where these libraries are included that shouldn't be.

What did you expect to see

The libraries such as libstruct_proto.a should not be inputs to the build.

What did you see instead?

These libraries are included and therefore can lead to duplicate symbols in the produced binary.

Possible fix

I believe we should revert the offending commits:

7b28278
a03d332

Until there is a clear fix upstream from bazel. Based on their original PRs I do not understand exactly what they were fixing though, so I assume that is not possible. Any other thoughts on how we could resolve this would be appreciated! It's possible we could go back to using a genrule for the blacklisted_protos while still keeping the other changes.

[jtattermusch]

Looks like gRPC is facing the same problem and it's preventing us from upgrading to protobuf 3.11.2.
@rafi-kamal

[UInt2048]

@jtattermusch Looks like gRPC got it sorted out.

[perezd]

We've also addressed this in protobuf.