Questions on warnings in java_grpc_library bazel function

[Joey Bratton]

The java_grpc_library function emits a couple warnings that aren't totally clear and I was hoping to get some clarification on whether they were there for a reason, or if they were just vestigial from previous iterations of the function:

1. https://github.com/grpc/grpc-java/blob/master/java_grpc_library.bzl#L10 - The source proto must be in the same package as the consuming rule. It looks like this is no longer true with the change made in https://github.com/grpc/grpc-java/pull/3675.
2. https://github.com/grpc/grpc-java/blob/master/java_grpc_library.bzl#L96 - Support for multiple deps is deprecated. It's not really clear what the reasoning is behind that limitation.

Thanks!
Joey

[Eric Anderson]

On Wed, Dec 13, 2017 at 5:05 PM, Joey Bratton <jo...@joeyb.org> wrote:

1. https://github.com/grpc/grpc-java/blob/master/java_grpc_library.bzl#L10 - The source proto must be in the same package as the consuming rule. It looks like this is no longer true with the change made in https://github.com/grpc/grpc-java/pull/3675.

I'm a bit uncertain what we should do here. The basic problem is there needs to be a single java_grpc_library for each proto across the entire build system, lest you get duplicate classes and things can get a bit weird. The best way to do that is to keep java_grpc_library targets in the same file as the proto_library target it references. 

But there are cases where you need to build a proto from another repository which doesn't already have a java_grpc_library. That needs to be permitted, but if that other repository adds a java_grpc_library target itself, then that should be used instead.

2. https://github.com/grpc/grpc-java/blob/master/java_grpc_library.bzl#L96 - Support for multiple deps is deprecated. It's not really clear what the reasoning is behind that limitation.

The rule only lets you have a single entry in 'srcs'. That should mean you only need one entry in 'deps,' for the java_proto_library target. However... java_library's "Strict Deps" requires adding additional deps in certain cases. What we want to do here is disable strict deps for java_grpc_library at which point you will only need one entry in 'deps.' But until then adding additional entries may still be necessary.

In the internal version of this rule I did comment out the check (after the OSS version was copied from it). However, internally I can also "easily" update all users when I fix the strict deps problem and then start enforcing that len(deps) == 1. So I'd be okay with commenting it out in OSS as well, but the expectation is that I'd try to remove that ability in the future.