Skip to content

Switch gRPC plugin to io.github.ascopes:protobuf-maven-plugin #1805

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dsyer
Copy link
Contributor

@dsyer dsyer commented May 22, 2025

The samples at Spring gRPC have all been switched to this other plugin because the old one has fallen out of maintenance. It works on the command line right now, but Eclipse/VSCode users will want the hints to drive the build. See spring-io/initializr#1650

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 22, 2025
@mhalbritter mhalbritter added type: enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels May 26, 2025
@mhalbritter
Copy link
Contributor

A simple demo project with this fails with:

[INFO] Ignoring provided path /home/mhalbritter/Downloads/demo/src/main/protobuf as it does not appear to actually exist
[ERROR] No protobuf sources found. If this is unexpected, check your configuration and try again.
[ERROR] Generation failed - NO SOURCES: No valid protobuf sources were found. Check the build logs above for more details.

We need to add this to the configuration:

<sourceDirectories>
  <sourceDirectory>src/main/proto</sourceDirectory>
</sourceDirectories>

or we can change the generated empty directory from src/main/proto to src/main/protobuf. Which do you prefer?

And i think the whole dance with kr.motd.maven:os-maven-plugin is no longer necessary. If you remove binaryMavenPlugins.binaryMavenPlugin.classifier from the protobuf-maven-plugin configuration it still appears to work. Can we then remove that?

@mhalbritter mhalbritter added the status: waiting-for-feedback We need additional information before we can continue label May 26, 2025
@mhalbritter mhalbritter changed the title Switch gRPC plugins for Maven builds Switch gRPC plugin to io.github.ascopes:protobuf-maven-plugin May 26, 2025
@dsyer
Copy link
Contributor Author

dsyer commented May 26, 2025

I forgot about the proto file path. I think we should use the default, so (since we don’t ship samples from initializr with proto files) there’s nothing to do? Or can we create the empty directory as a hint?

I didn’t know the os classifier wasn’t needed. Maybe we should verify on all platforms? It seems to work for me on Linux.

UPDATE: I pushed some changes to align with comments above.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels May 26, 2025
@dsyer
Copy link
Contributor Author

dsyer commented May 26, 2025

N.B. I don't want to merge this yet until we either add an <?m2e ...?> hint (requires spring-io/initializr#1650) or the plugin gets a new release (3.2.4 will hopefully have the metadata OOTB).

@dsyer dsyer marked this pull request as draft May 26, 2025 10:35
@mhalbritter
Copy link
Contributor

spring-io/initializr#1650 is in.

@dsyer
Copy link
Contributor Author

dsyer commented May 26, 2025

So we can use that new API?

@mhalbritter
Copy link
Contributor

Yeah, we can add processing instructions until the plugin authors release their new version.

@dsyer dsyer marked this pull request as ready for review May 28, 2025 05:55
@mhalbritter
Copy link
Contributor

The maven plugin now uses src/main/protobuf, while the Gradle plugin (which is the offical com.google.protobuf one) expects the proto files at src/main/proto. Given that the existing documentation also uses src/main/proto, I suggest that we switch the Maven plugin to src/main/proto. I can take care of that change. OK for you?

@dsyer
Copy link
Contributor Author

dsyer commented May 28, 2025

We switched the project samples already and I think the gradle builds still work. I’ll have to check. The docs can be changed of course.

UPDATE: it's a big mess. I will fix the samples in the gRPC project. I think you are right but I'm having some issues convincing the ascopes plugin to follow its configuration.

@mhalbritter
Copy link
Contributor

mhalbritter commented May 28, 2025

This is what I currently have:

<configuration>
	<sourceDirectories>
		<sourceDirectory>src/main/proto</sourceDirectory>
	</sourceDirectories>
	<failOnMissingSources>false</failOnMissingSources>
	<protocVersion>${protobuf-java.version}</protocVersion>
	<binaryMavenPlugins>
		<binaryMavenPlugin>
			<groupId>io.grpc</groupId>
			<artifactId>protoc-gen-grpc-java</artifactId>
			<version>${grpc.version}</version>
			<options>jakarta_omit,@generated=omit</options>
		</binaryMavenPlugin>
	</binaryMavenPlugins>
</configuration>

@dsyer
Copy link
Contributor Author

dsyer commented May 28, 2025

That should work for a standalone sample but it breaks if the app is not the top level in the Maven reactor. We can see if it can be fixed in the plugin (I opened an issue). I prefer to just use the default here for now,

@dsyer
Copy link
Contributor Author

dsyer commented May 28, 2025

I added a new commit that changes the configuration in Gradle instead. You can decide how to do it.

@dsyer dsyer force-pushed the main branch 2 times, most recently from 5ee5eac to f6e0129 Compare May 29, 2025 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants