Remove maven-compat, which needed three things rather than one - #158
Open
slachiewicz wants to merge 1 commit into
Open
Remove maven-compat, which needed three things rather than one#158slachiewicz wants to merge 1 commit into
slachiewicz wants to merge 1 commit into
Conversation
Nothing here imports maven-compat, but removing it took more than deleting the line. First, the baseline. maven-core 3.6.3 ships DefaultProjectBuildingHelper with a field of the legacy org.apache.maven.repository.RepositorySystem, implemented only in maven-compat, so Mojo lookups fail without it. maven-core 3.9.12 moved that field to MavenRepositorySystem in maven-core -- commit 3afbdb8f76, first released in 3.9.12. So mavenVersion moves there, and resolverVersion to the 1.9.25 that ships with it. Second, the test built its local repository through ArtifactRepositoryFactory, which lives only in maven-compat. MavenRepositorySystem.createArtifactRepository is a static method with the same five parameters, so the call is unchanged apart from the receiver. Third, and least obvious: the test loads maven-javadoc-plugin to exercise the executor, and pinned 3.4.0. That version needs RepositoryMetadataManager, whose only implementation -- DefaultRepositoryMetadataManager -- is also in maven-compat, so the removal failed on a component this project never mentions. Current javadoc-plugin does not use it, so the pin moves to 3.12.0. Tests: 4, 0 failures, same as master under mvn verify.
There was a problem hiding this comment.
Pull request overview
This PR removes the test-scoped maven-compat dependency by updating the project’s Maven/Resolver baselines and adjusting tests to use APIs available without maven-compat, including updating the pinned maven-javadoc-plugin used in tests.
Changes:
- Bump
mavenVersionto 3.9.12 and alignresolverVersionto 1.9.25, then removeorg.apache.maven:maven-compatfrom test dependencies. - Update the test’s local-repo construction to use
MavenRepositorySystem.createArtifactRepository(...)instead ofArtifactRepositoryFactory. - Update the test-pinned
maven-javadoc-pluginfrom 3.4.0 to 3.12.0 to avoidmaven-compat-only Plexus components.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/test/java/org/apache/maven/reporting/exec/TestDefaultMavenReportExecutor.java | Updates test setup to avoid maven-compat APIs and bumps the test-loaded javadoc plugin version. |
| pom.xml | Raises Maven/Resolver version properties and removes the test dependency on maven-compat. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nothing here imports
maven-compat, but removing it took three changes rather than one. The third is the interesting one.1. The baseline
maven-core3.6.3 shipsDefaultProjectBuildingHelperwith a field of the legacyorg.apache.maven.repository.RepositorySystem, whose only implementation lives in maven-compat — so Mojo lookups fail without it.maven-coremoved that field toMavenRepositorySystemin 3.9.12 exactly (commit3afbdb8f76;git tag --containsgivesmaven-3.9.12, and the bytecode differs between 3.9.11 and 3.9.12).mavenVersionmoves there, andresolverVersionto the 1.9.25 that Maven 3.9.12 itself ships, rather than a number chosen separately.2. The local repository in the test
It was built through
ArtifactRepositoryFactory, which exists only in maven-compat.MavenRepositorySystem.createArtifactRepositoryis astaticmethod with the same five parameters, so only the receiver changes.3. The part that is not visible from this project at all
After both of those, the tests still failed:
The test loads maven-javadoc-plugin to exercise the executor, and pinned 3.4.0 — from 2022. That version needs
RepositoryMetadataManager, whose interface is in maven-core but whose only implementation,DefaultRepositoryMetadataManager, is in maven-compat. So the removal was blocked by a component this project never mentions, reached through a plugin the test happens to load.Current maven-javadoc-plugin does not use it, so the pin moves to 3.12.0.
Worth flagging for the wider effort: this is the same shape as the
RepositorySystemproblem — interface in maven-core, sole implementation in maven-compat — but it was not fixed in 3.9.12, so raising the baseline does not help with it. Anywhere a test loads an older plugin, that plugin's own compat needs become yours.Verification
mvn verify: 4 tests, 0 failures — same as master. Run asverifyrather thantestdeliberately: on a sibling project, removing maven-compat passed every test while breaking the build, because a dependency was arriving on the compile classpath through it and only the dependency analysis inverifycatches that.This raises the prerequisite
<prerequisites><maven>${mavenVersion}</maven></prerequisites>reads that property, so the minimum Maven for consumers goes from 3.6.3 to 3.9.12. That is a project decision — apache/maven#12709 is open to settle it across the plugins and shared components, with the survey data behind it. If the answer is "not yet", this should be closed and the dependency annotated as blocked rather than left looking like an oversight.