slachiewicz opened a new pull request, #650: URL: https://github.com/apache/maven-project-info-reports-plugin/pull/650
Removes `maven-compat`. No baseline change — PIR is already on maven 3.9.16 / resolver 1.9.27, and every API used here is present as far back as maven-core 3.6.3, so `<prerequisites>` is untouched. ### The constructor parameter is removed, not retyped The legacy `org.apache.maven.repository.RepositorySystem` was a constructor parameter of `AbstractProjectInfoReport` and therefore of all 17 report Mojos — but the field was used in **none** of them. Only three call sites needed it, so rather than change the type in 17 places the parameter is deleted and those three go through PIR's own `RepositoryUtils`, which was already an injectable `@Named @Singleton` holding the resolver `RepositorySystem`. This is a source- and binary-incompatible change to a `protected` constructor. That is unavoidable either way: the parameter's type is the thing being removed, so anyone subclassing `AbstractProjectInfoReport` has to move regardless. ### Not via `MavenRepositorySystem` — that does not work in a plugin The obvious replacement for the legacy `RepositorySystem` is `org.apache.maven.bridge.MavenRepositorySystem`. **It cannot be used from a plugin.** maven-core does not export `org.apache.maven.bridge` to plugin class realms — it appears in no `<exportedPackages>` entry of `META-INF/maven/extension.xml` in 3.9.16 or 4.0.0-rc-5, while `org.apache.maven.artifact` does. That swap was tried first. It compiled, all 30 unit tests passed, and it failed only in an integration test: ``` A required class was missing while executing maven-site-plugin:3.22.0:site: org/apache/maven/bridge/MavenRepositorySystem ``` The plugin-testing harness puts everything on one flat classpath, so realm isolation does not exist there and `mvn verify` stays green. Worth recording for anyone else doing this work. What is used instead: `ArtifactHandlerManager` — which *is* exported, recursively — plus `DefaultArtifact` constructed directly, which is what `MavenRepositorySystem.createArtifact` does internally. ### One behaviour change, and it needs a release note `MavenMetadataSource.retrieveAvailableVersions` returned every version in `maven-metadata.xml`, snapshots included, and Maven's `VersionRange.containsVersion` accepts snapshots — so **a SNAPSHOT could win a version range**. Resolver's `DefaultVersionRangeResolver` requests `RELEASE`-nature metadata unless a bound of the range is itself a snapshot. Concretely: for `[1.0,)` on an artifact with `2.2.0-SNAPSHOT` published, the Dependency Management report used to be able to render the snapshot's URL and licences; it now renders 2.1.0's. That aligns the report with how Maven's own dependency resolution treats ranges, so it reads as a fix — but it is user-visible. Two things were deliberately held constant so the change stays on that one axis: the repository set is unchanged (`buildingRequest.getRemoteRepositories()` through `RepositoryUtils.toRepos`, so mirrors, auth and proxies come out exactly as core resolved them), and version *ordering* is unchanged (resolver `Version`s are converted back to `DefaultArtifactVersion` so Maven's comparator still picks the winner). ### Verification, and what it does not cover `mvn verify` 30/30 and `-Prun-its` 22/22, identical on both sides, against a clean baseline worktree. Beyond counts: **all 529 generated HTML pages across the 22 ITs are byte-identical**, baseline versus migrated. **The unit tests do not cover the changed path** — `DependencyManagementReportTest` pins a plain version, so the range branch is skipped. The `full-pom` IT does cover it, and its execution was confirmed from the build log rather than inferred: ``` Resolving range for DependencyManagement on org.apache.maven.doxia:doxia-sink-api:jar:[1.0,) DependencyManagement resolved: org.apache.maven.doxia:doxia-sink-api:jar:2.1.0 ``` **What is still not covered: a range whose matches include snapshots.** No fixture in the tree has one, so the behaviour change above is reasoned from the resolver's implementation plus a release-only IT — not observed. If that should be nailed down before merge, it needs a new IT with a snapshot repository. Maven 4 was checked statically — every API used is present and exported in 4.0.0-rc-5 — but the ITs were not run against a Maven 4 install. Given how the `bridge` failure surfaced, I would not call Maven 4 proven without it. -- 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]
