slachiewicz opened a new pull request, #1356:
URL: https://github.com/apache/maven-javadoc-plugin/pull/1356
The `maven-compat` dependency here carries a `@TODO remove!!` and nothing in
the plugin imports anything from it. Removing it alone does not work, and the
reason is not in this plugin.
### Why it could not simply be deleted
`maven-core` **3.6.3** ships `DefaultProjectBuildingHelper` with a field of
the legacy `org.apache.maven.repository.RepositorySystem`, whose only
implementation — `LegacyRepositorySystem` — lives in maven-compat. So any test
that looks up a Mojo needing `ProjectBuilder` fails without it, with zero
compat code in the plugin:
```
No implementation for org.apache.maven.repository.RepositorySystem was bound.
while locating org.apache.maven.project.DefaultProjectBuildingHelper
```
`maven-core` changed that field to
`org.apache.maven.bridge.MavenRepositorySystem`, which lives in maven-core
itself.
### The version is 3.9.12 exactly, not "3.9.x"
Worth being precise, since this sets a floor. Commit `3afbdb8f76` ("Use
MavenRepositorySystem in ProjectBuildingHelper instead of deprecated
RepositorySystem", #11358); `git tag --contains` gives `maven-3.9.12` as the
first release carrying it. Confirmed against the bytecode:
```
maven-core 3.9.11 org.apache.maven.repository.RepositorySystem
repositorySystem
maven-core 3.9.12 org.apache.maven.bridge.MavenRepositorySystem
repositorySystem
```
So `mavenVersion` moves to **3.9.12** — the earliest release where this
holds — rather than a rounder number that would raise the floor further than
necessary. `resolverVersion` moves to **1.9.25**, which is what Maven 3.9.12
itself ships, rather than a version chosen independently.
### This is a project decision, not just a refactor
`<prerequisites><maven>${mavenVersion}</maven></prerequisites>` reads that
property, so this **raises the minimum Maven for consumers from 3.6.3 to
3.9.12**. It wants a release-note entry and your agreement, not just a review
of the diff.
There is a legitimate alternative: **do nothing.** The dependency is
`test`-scoped, never ships, and costs users nothing. If the project is not
ready to move the floor, leaving it is defensible — but then the `@TODO
remove!!` should say so, rather than implying someone forgot. The precedent for
moving is `maven-plugin-plugin`, which removed its compat dependency in
MPLUGIN-519/520 by raising its prerequisite.
### Verification
`mvn verify`: **69 tests, 1 failure — identical to `master` under the same
command.** `JavadocReportTest.testJavadocResourcesWithExcludes` already fails
on an untouched checkout in my environment, and the failure set is the same
before and after, so this change is neutral to it.
Ran `verify` rather than `test` deliberately: on a sibling plugin, removing
maven-compat passed all tests while breaking the build, because a dependency
was reaching the compile classpath through it and only the dependency analysis
in `verify` catches that.
Part of a survey of which projects still declare `maven-compat` versus which
genuinely need it. This one needed only the baseline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]