From b13a7d80cf43c20fb617e59d48aa784eb790053b Mon Sep 17 00:00:00 2001
From: Laurent Goujon <laurentgo@users.noreply.github.com>
Date: Thu, 28 Mar 2024 06:06:21 -0700
Subject: [PATCH] GH-40843: [Java] Cleanup protobuf-maven-plugin usage (#40844)

### 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 <laurent@apache.org>
Signed-off-by: David Li <li.davidm96@gmail.com>
---
 java/dataset/pom.xml                          | 11 ++++------
 java/flight/flight-core/pom.xml               | 16 ++-------------
 .../src/test/{protobuf => proto}/perf.proto   |  0
 .../src/test/{protobuf => proto}/test.proto   |  0
 java/flight/pom.xml                           | 20 -------------------
 java/gandiva/pom.xml                          | 11 ++++------
 java/pom.xml                                  | 10 ++++++++++
 7 files changed, 20 insertions(+), 48 deletions(-)
 rename java/flight/flight-core/src/test/{protobuf => proto}/perf.proto (100%)
 rename java/flight/flight-core/src/test/{protobuf => proto}/test.proto (100%)

diff --git a/java/dataset/pom.xml b/java/dataset/pom.xml
index a003fd18068ec..43b913167390f 100644
--- a/java/dataset/pom.xml
+++ b/java/dataset/pom.xml
@@ -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>
diff --git a/java/flight/flight-core/pom.xml b/java/flight/flight-core/pom.xml
index 98491e7ba091e..830caf8a28246 100644
--- a/java/flight/flight-core/pom.xml
+++ b/java/flight/flight-core/pom.xml
@@ -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>
@@ -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>
diff --git a/java/flight/flight-core/src/test/protobuf/perf.proto b/java/flight/flight-core/src/test/proto/perf.proto
similarity index 100%
rename from java/flight/flight-core/src/test/protobuf/perf.proto
rename to java/flight/flight-core/src/test/proto/perf.proto
diff --git a/java/flight/flight-core/src/test/protobuf/test.proto b/java/flight/flight-core/src/test/proto/test.proto
similarity index 100%
rename from java/flight/flight-core/src/test/protobuf/test.proto
rename to java/flight/flight-core/src/test/proto/test.proto
diff --git a/java/flight/pom.xml b/java/flight/pom.xml
index 2f777ab42b756..5b9caafa82ef9 100644
--- a/java/flight/pom.xml
+++ b/java/flight/pom.xml
@@ -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>
diff --git a/java/gandiva/pom.xml b/java/gandiva/pom.xml
index 819baee11edec..0d2a23345f6ea 100644
--- a/java/gandiva/pom.xml
+++ b/java/gandiva/pom.xml
@@ -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>
diff --git a/java/pom.xml b/java/pom.xml
index add2823ccb0d2..659ccfca08c76 100644
--- a/java/pom.xml
+++ b/java/pom.xml
@@ -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>