THRIFT-5920: modernize thrift-maven-plugin and add thread-safety regression proofs

[greggdonovan]

Why

This modernizes contrib/thrift-maven-plugin to current Maven plugin conventions (Maven 3.9.x metadata/API usage), addresses parallel-build thread-safety warnings, and adds explicit regression proof that core code generation behavior is unchanged.

What changed

1) Modernized plugin metadata and Maven API usage

• Migrated legacy mojo metadata to annotation-based metadata in:
  • contrib/thrift-maven-plugin/src/main/java/org/apache/thrift/maven/ThriftCompileMojo.java
  • contrib/thrift-maven-plugin/src/main/java/org/apache/thrift/maven/ThriftTestCompileMojo.java
  • contrib/thrift-maven-plugin/src/main/java/org/apache/thrift/maven/AbstractThriftMojo.java
• Marked goals as thread-safe via annotation metadata (threadSafe = true).
• Updated dependency resolution usage to modern APIs (project.getArtifacts()).
• Updated local repository handling to modern parameterized path usage.

2) Modernized module build setup

• Updated contrib/thrift-maven-plugin/pom.xml for modern Maven plugin best practices:
  • explicit modern plugin versions
  • maven-plugin-annotations
  • modern provided-scope Maven API/core dependencies
  • removed obsolete test dependency (mockito-all)

3) Added unit-level regression proofs

• Added descriptor metadata test:
  • contrib/thrift-maven-plugin/src/test/java/org/apache/thrift/maven/TestThreadSafePluginDescriptor.java
  • verifies generated plugin.xml contains thread-safe metadata for compile and testCompile goals.
• Added generated-output checksum regression test:
  • contrib/thrift-maven-plugin/src/test/java/org/apache/thrift/maven/TestThriftOutputRegression.java
  • baseline manifests:
    • contrib/thrift-maven-plugin/src/test/resources/expected/java.sha256
    • contrib/thrift-maven-plugin/src/test/resources/expected/java_private_members.sha256

4) Added Maven Invoker integration test suite

• Added run-its profile and invoker execution in contrib/thrift-maven-plugin/pom.xml.
• Added fixtures under contrib/thrift-maven-plugin/src/it/:
  • compile-output-equivalence: asserts generated main sources match expected checksums.
  • test-compile-output-equivalence: asserts generated test sources match expected checksums.
  • parallel-thread-safe: runs multi-module build with -T 2, verifies generated output and absence of the thread-safety warning in build.log.

Proof of no regression

Local validation run on this branch

• mvn -f contrib/thrift-maven-plugin/pom.xml clean test
• mvn -f contrib/thrift-maven-plugin/pom.xml -Prun-its clean verify

Both pass, including:

• unit tests (descriptor + checksum regression)
• invoker integration tests (real Maven executions in fixture projects)

Maintainer reproduction steps

Prereqs

• Maven 3.9.x
• thrift executable available on PATH

1) Unit tests

mvn -f contrib/thrift-maven-plugin/pom.xml clean test

2) Integration tests (invoker)

mvn -f contrib/thrift-maven-plugin/pom.xml -Prun-its clean verify

If thrift is not on PATH, use:

mvn -f contrib/thrift-maven-plugin/pom.xml -Prun-its clean verify \
  -Dthrift.executable.for.it=/absolute/path/to/thrift

3) Optional A/B comparison against released plugin

Generate the same IDLs with org.apache.thrift:thrift-maven-plugin:0.10.0 and this branch version, then diff -ru generated output directories. This branch includes in-repo checksum tests and invoker fixtures that already provide automated equivalence checks for representative IDLs.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR successfully modernizes the thrift-maven-plugin to Maven 3.9.x conventions with proper thread-safety support and comprehensive regression testing.

Key Changes:

• Migrated from legacy JavaDoc-style plugin metadata (@goal, @phase) to modern annotation-based metadata (@Mojo, @Parameter)
• Marked both compile and testCompile goals as threadSafe = true to enable parallel Maven builds
• Updated Maven API usage from deprecated maven-project APIs (getCompileArtifacts(), getTestArtifacts(), ArtifactRepository) to modern maven-core APIs (getArtifacts(), String-based repository path)
• Added resource management improvements (try-with-resources for JarFile)
• Removed obsolete test dependency (mockito-all)

Testing & Validation:

• Added unit test TestThreadSafePluginDescriptor to verify generated plugin.xml contains threadSafe=true metadata
• Added regression test TestThriftOutputRegression with SHA256 checksum validation to ensure codegen output is unchanged
• Added Maven Invoker integration tests including multi-module parallel build test (-T 2) that verifies absence of thread-safety warnings
• Integration tests validate output equivalence for both main and test source compilation

Observations:

• The API migration from getCompileArtifacts()/getTestArtifacts() to getArtifacts() is appropriate for Maven 3.9+ and both mojos now use the same unified artifact resolution
• The PR includes many unrelated commits (Ruby header protocol, C++ forward setter, dependency bumps), but the core Maven plugin changes are contained in commits 06704a7, 9cf5d47, 35cb7e2, and 41c7e6d
• All modernization changes follow current Maven plugin best practices and maintain backward compatibility

Confidence Score: 5/5

• This PR is safe to merge with high confidence
• The Maven plugin modernization is well-executed with proper migration to modern APIs, comprehensive test coverage (unit tests, regression tests, and integration tests), and explicit validation that codegen behavior is unchanged. The changes follow Maven plugin best practices and include proper resource management improvements.
• No files require special attention

Important Files Changed

Filename	Overview
contrib/thrift-maven-plugin/pom.xml	Updated to Maven 3.9 API with modern plugin versions, added maven-invoker-plugin profile for integration tests, and migrated from maven-project to maven-core dependency
contrib/thrift-maven-plugin/src/main/java/org/apache/thrift/maven/ThriftCompileMojo.java	Migrated from JavaDoc annotations to @mojo annotation with threadSafe=true, changed from getCompileArtifacts() to getArtifacts()
contrib/thrift-maven-plugin/src/main/java/org/apache/thrift/maven/ThriftTestCompileMojo.java	Migrated from JavaDoc annotations to @mojo annotation with threadSafe=true, changed from getTestArtifacts() to getArtifacts(), updated method signature from ArtifactRepository to String
contrib/thrift-maven-plugin/src/main/java/org/apache/thrift/maven/AbstractThriftMojo.java	Migrated from JavaDoc annotations to @Parameter/@component annotations, changed from ArtifactRepository to String localRepositoryPath, added try-with-resources for JarFile handling
contrib/thrift-maven-plugin/src/test/java/org/apache/thrift/maven/TestThreadSafePluginDescriptor.java	New test that validates generated plugin.xml contains threadSafe=true for compile and testCompile goals using XPath assertions
contrib/thrift-maven-plugin/src/test/java/org/apache/thrift/maven/TestThriftOutputRegression.java	New regression test that verifies thrift codegen output matches baseline SHA256 checksums for java and java:private_members generators

Sequence Diagram

sequenceDiagram
    participant Maven
    participant ThriftCompileMojo
    participant AbstractThriftMojo
    participant Thrift Compiler
    participant FileSystem

    Maven->>ThriftCompileMojo: execute() [compile goal]
    ThriftCompileMojo->>AbstractThriftMojo: execute()
    AbstractThriftMojo->>AbstractThriftMojo: checkParameters()
    AbstractThriftMojo->>AbstractThriftMojo: findThriftFilesInDirectory()
    AbstractThriftMojo->>FileSystem: scan for .thrift files
    FileSystem-->>AbstractThriftMojo: thrift files list
    AbstractThriftMojo->>AbstractThriftMojo: getDependencyArtifacts()
    Note over AbstractThriftMojo: Now uses project.getArtifacts()<br/>instead of getCompileArtifacts()
    AbstractThriftMojo->>AbstractThriftMojo: makeThriftPathFromJars()
    AbstractThriftMojo->>FileSystem: extract .thrift from JARs
    FileSystem-->>AbstractThriftMojo: extracted thrift files
    AbstractThriftMojo->>Thrift Compiler: compile(thriftFiles, generator, paths)
    Thrift Compiler-->>AbstractThriftMojo: exit status
    AbstractThriftMojo->>ThriftCompileMojo: attachFiles()
    ThriftCompileMojo->>Maven: addCompileSourceRoot(outputDirectory)
    Note over Maven: Parallel execution safe<br/>with threadSafe=true

Loading

_{Last reviewed commit: 06704a7}

[greptile-apps]

_{64 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}