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

bom: Remove protoc-gen-grpc-java #9020

Merged
merged 1 commit into from Mar 28, 2022
Merged

Conversation

ejona86
Copy link
Member

@ejona86 ejona86 commented Mar 25, 2022

protoc-gen-grpc-java isn't useful in a BOM because the Gradle plugin
doesn't support BOMs (google/protobuf-gradle-plugin#412) and it seems
unlikely the Maven plugin would as well.

Right now the type is pom, which is useless; nobody cares about the pom
itself. We'd need to use a type of exe and repeat it for each platform
(classifier). Given that it is a strange case and we can't actually test
the results and having it in the bom might lead a user to trying to make
it work, let's just remove it for now until it provides value.

Fixes #8936

protoc-gen-grpc-java isn't useful in a BOM because the Gradle plugin
doesn't support BOMs (google/protobuf-gradle-plugin#412) and it seems
unlikely the Maven plugin would as well.

Right now the type is pom, which is useless; nobody cares about the pom
itself. We'd need to use a type of exe and repeat it for each platform
(classifier). Given that it is a strange case and we can't actually test
the results and having it in the bom might lead a user to trying to make
it work, let's just remove it for now until it provides value.

Fixes grpc#8936
@danielnorberg
Copy link
Contributor

danielnorberg commented Mar 25, 2022

👍

This would break the build for maven projects that have a (useless) dependency on protoc-gen-grpc-java and relies on it being managed:

<dependencyManagement>
  <dependencies>
    <dependency>
      <groupId>io.grpc</groupId>
      <artifactId>grpc-bom</artifactId>
      <version>1.45.0</version>
      <scope>import</scope>
      <type>pom</type>
    </dependency>
  </dependencies>
</dependencyManagement>

<dependencies>
  <dependency>
    <groupId>io.grpc</groupId>
    <artifactId>protoc-gen-grpc-java</artifactId>
    <type>pom</type>
  </dependency>
</dependencies>

They would then get the following error if we remove protoc-gen-grpc-java from grpc-bom:

[ERROR]     'dependencies.dependency.version' for io.grpc:protoc-gen-grpc-java:pom is missing. @ line X, column Y

Hopefully this is rare in the wild. I did not find any such users in our internal index.

@ejona86
Copy link
Member Author

ejona86 commented Mar 25, 2022

Yeah, that should be rare and should only break on a version upgrade, at which point the solution is "delete the useless dependency that does nothing." So I'm not very concerned.

@ejona86 ejona86 merged commit 4a84c6f into grpc:master Mar 28, 2022
@ejona86 ejona86 deleted the bom-no-protoc branch March 28, 2022 21:39
@ST-DDT
Copy link
Contributor

ST-DDT commented May 8, 2022

FFR: I used the version from the bom in my gradle project and v1.46.0 broke the ci build.
I will add it there now to fix the issue:

IMO its a bug in the maven plugin that it doesn't use the version from the bom.

ST-DDT added a commit to grpc-ecosystem/grpc-spring that referenced this pull request May 8, 2022
@ejona86
Copy link
Member Author

ejona86 commented May 9, 2022

@ST-DDT, I mention I didn't think it worked for gradle as well and linked to an issue. Looks like you are using dependencyManagement in buildscript. Interesting. We'll revert this.

ejona86 added a commit to ejona86/grpc-java that referenced this pull request May 9, 2022
This reverts commit 4a84c6f. The BOM
was usable for protoc-gen-grpc-java using dependencyManagement for the
buildscript. See conversation on grpc#9020.
ejona86 added a commit that referenced this pull request May 9, 2022
This reverts commit 4a84c6f. The BOM
was usable for protoc-gen-grpc-java using dependencyManagement for the
buildscript. See conversation on #9020.
ejona86 added a commit to ejona86/grpc-java that referenced this pull request May 9, 2022
This reverts commit 4a84c6f. The BOM
was usable for protoc-gen-grpc-java using dependencyManagement for the
buildscript. See conversation on grpc#9020.
ejona86 added a commit that referenced this pull request May 9, 2022
This reverts commit 4a84c6f. The BOM
was usable for protoc-gen-grpc-java using dependencyManagement for the
buildscript. See conversation on #9020.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

grpc-bom should manage protoc-gen-grpc-java artifacts?
4 participants