Skip to content

Commit

Permalink
GH-40843: [Java] Cleanup protobuf-maven-plugin usage (#40844)
Browse files Browse the repository at this point in the history
### Rationale for this change

`protobuf-maven-plugin` usage in Arrow codebase does not follow plugins best practices like sharing the same output directory for different execution or not using test goals for generating test classes

### What changes are included in this PR?

* Add protobuf-maven-plugin plugin to top level pom.xml under pluginManagement to define version and common configuration for all modules
* Remove unnecessary executions of test-compile goal when no test protobufs are present
* Remove use of outputDirectory and clearOutputDirectory and let the plugin choose it for each execution (the default output directory is based on the phase (main vs test) and the language/plugin-id)
* Replace use of compile/compile-custom goals with test-compile/test-compile-custom when generating test protobufs

### Are these changes tested?

As those changes are in the build system, they are covered by the build framework and tests run as part of the build

### Are there any user-facing changes?
None
* GitHub Issue: #40843

Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
  • Loading branch information
laurentgo authored Mar 28, 2024
1 parent 6cecbab commit a9b2cc2
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 48 deletions.
11 changes: 4 additions & 7 deletions java/dataset/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -177,18 +177,15 @@
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.6.1</version>
<configuration>
<protocArtifact>com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
</protocArtifact>
<protoSourceRoot>../../cpp/src/jni/dataset/proto</protoSourceRoot>
</configuration>
<executions>
<execution>
<id>src</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
</goals>
<configuration>
<protoSourceRoot>../../cpp/src/jni/dataset/proto</protoSourceRoot>
</configuration>
</execution>
</executions>
</plugin>
Expand Down
16 changes: 2 additions & 14 deletions java/flight/flight-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -228,19 +228,11 @@
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.6.1</version>
<configuration>
<protocArtifact>com.google.protobuf:protoc:${dep.protobuf-bom.version}:exe:${os.detected.classifier}</protocArtifact>
<clearOutputDirectory>false</clearOutputDirectory>
<pluginId>grpc-java</pluginId>
<pluginArtifact>io.grpc:protoc-gen-grpc-java:${dep.grpc-bom.version}:exe:${os.detected.classifier}</pluginArtifact>
</configuration>
<executions>
<execution>
<id>src</id>
<configuration>
<protoSourceRoot>${basedir}/../../../format/</protoSourceRoot>
<outputDirectory>${project.build.directory}/generated-sources/protobuf</outputDirectory>
</configuration>
<goals>
<goal>compile</goal>
Expand All @@ -249,13 +241,9 @@
</execution>
<execution>
<id>test</id>
<configuration>
<protoSourceRoot>${basedir}/src/test/protobuf</protoSourceRoot>
<outputDirectory>${project.build.directory}/generated-test-sources//protobuf</outputDirectory>
</configuration>
<goals>
<goal>compile</goal>
<goal>compile-custom</goal>
<goal>test-compile</goal>
<goal>test-compile-custom</goal>
</goals>
</execution>
</executions>
Expand Down
20 changes: 0 additions & 20 deletions java/flight/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,6 @@
<module>flight-integration-tests</module>
</modules>

<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.6.1</version>
<configuration>
<protocArtifact>
com.google.protobuf:protoc:${dep.protobuf-bom.version}:exe:${os.detected.classifier}
</protocArtifact>
<pluginId>grpc-java</pluginId>
<pluginArtifact>io.grpc:protoc-gen-grpc-java:${dep.grpc-bom.version}:exe:${os.detected.classifier}
</pluginArtifact>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>

<profiles>
<profile>
<id>pin-mockito-jdk8</id>
Expand Down
11 changes: 4 additions & 7 deletions java/gandiva/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -132,18 +132,15 @@
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.6.1</version>
<configuration>
<protocArtifact>com.google.protobuf:protoc:${protobuf.version}:exe:${os.detected.classifier}
</protocArtifact>
<protoSourceRoot>proto</protoSourceRoot>
</configuration>
<executions>
<execution>
<id>src</id>
<goals>
<goal>compile</goal>
<goal>test-compile</goal>
</goals>
<configuration>
<protoSourceRoot>proto</protoSourceRoot>
</configuration>
</execution>
</executions>
</plugin>
Expand Down
10 changes: 10 additions & 0 deletions java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,16 @@
</gradleEnterprise>
</configuration>
</plugin>
<plugin>
<groupId>org.xolstice.maven.plugins</groupId>
<artifactId>protobuf-maven-plugin</artifactId>
<version>0.6.1</version>
<configuration>
<protocArtifact>com.google.protobuf:protoc:${dep.protobuf-bom.version}:exe:${os.detected.classifier}</protocArtifact>
<pluginId>grpc-java</pluginId>
<pluginArtifact>io.grpc:protoc-gen-grpc-java:${dep.grpc-bom.version}:exe:${os.detected.classifier}</pluginArtifact>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
Expand Down

0 comments on commit a9b2cc2

Please sign in to comment.