search

grpc / grpc-java

The Java gRPC implementation. HTTP/2 based RPC
https://grpc.io/docs/languages/java/
Apache License 2.0
11.42k stars 3.84k forks source link

grpc-bom should manage protoc-gen-grpc-java artifacts? #8936

Closed danielnorberg closed 2 years ago

danielnorberg commented 2 years ago

What version of gRPC-Java are you using?

1.44.1

What is your environment?

Any

What did you expect to see?

The grpc-bom managing the protoc-gen-grpc-java artifacts with exe type and os classifier.

https://repo1.maven.org/maven2/io/grpc/protoc-gen-grpc-java/1.44.1/

What did you see instead?

The grpc-bom managing protoc-gen-grpc-java with pom type. Is there a use case for this? It is effectively empty: https://repo1.maven.org/maven2/io/grpc/protoc-gen-grpc-java/1.44.1/protoc-gen-grpc-java-1.44.1.pom

Steps to reproduce the bug

ejona86 commented 2 years ago

Is this causing you a specific problem? Or did you just notice the oddity?

Yeah, that looks a bit broken. It doesn't matter much for Gradle right now because the gradle plugin can't be controlled by a BOM. For Maven, I'd doubt/wonder that dependencyManagement could impact the build; it seems like it'd just impact <dependencies>.

So I wonder if it makes sense to just remove the entry for protoc-gen-grpc-java from grpc-bom.

ejona86 commented 2 years ago

The type=pom was added later in https://github.com/grpc/grpc-java/pull/6220 . And the type in the pom is pom. Yeah... this is all weird. Let's remove it.

danielnorberg commented 2 years ago

I encountered this oddity when inspecting the output of our central dependency management manifest as part of exploring improvements to our dependency management tooling at Spotify. There was some confusion around the purpose of this managed dependency in the grpc-bom but afaict there's no direct negative impact apart from that. Our tooling is not dependent on this dependency being in grpc-bom and afaict neither is our fleet, but there's always Hyrum's Law and https://xkcd.com/1172/.