slachiewicz commented on PR #650:
URL:
https://github.com/apache/maven-project-info-reports-plugin/pull/650#issuecomment-5227045208
I need to withdraw most of this, with apologies to @belingueres.
This overlaps #628 ("[MPIR-477] Replace ArtifactMetadataSource by Resolver
API") almost completely — **19 of my 20 changed files are the same files**, and
the only one that is mine alone is `pom.xml`. Worse, we independently arrived
at the same solution: replace
`ArtifactMetadataSource.retrieveAvailableVersions` with resolver's
`VersionRangeRequest`, and build the artifact from `ArtifactHandlerManager` +
`DefaultArtifact` rather than anything in `org.apache.maven.bridge`.
#628 predates this by a long way, it is an outside contribution, and it
should land first. I did not check for an existing PR before opening this,
which is my mistake.
**Proposal:** leave #628 to merge on its own terms. Once it does, what
remains for maven-compat here is small — #628 already removes the legacy
`org.apache.maven.repository.RepositorySystem` from `AbstractProjectInfoReport`
and the 17 report Mojos, so the leftover is essentially the `pom.xml`
dependency block. I will rebase this branch onto #628 and reduce it to that, or
simply close it and open a one-line follow-up, whichever the maintainers prefer.
Two things from my verification that may be useful to #628 regardless, since
they concern the same code path:
**A behaviour change worth a release note.** `retrieveAvailableVersions`
returned every version in `maven-metadata.xml`, snapshots included, and Maven's
`VersionRange.containsVersion` accepts snapshots — so a SNAPSHOT could win a
range. Resolver's `DefaultVersionRangeResolver` requests `RELEASE`-nature
metadata unless a bound of the range is itself a snapshot. So 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 and now renders
2.1.0's. That reads as a fix — it aligns the report with Maven's own resolution
— but it is user-visible.
**A gap in test coverage for exactly that change.**
`DependencyManagementReportTest` pins a plain version, so
`range.getRecommendedVersion()` is non-null and the whole version-range branch
is skipped; a green unit run proves nothing about it. The `full-pom` IT does
exercise it — `doxia-sink-api:[1.0,)` — and its log shows the resolution
happening:
```
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
```
But no fixture anywhere has a range whose matches *include* a snapshot, so
the behaviour difference above is reasoned from the resolver's implementation
rather than observed. If it should be pinned down, it needs a new IT with a
snapshot repository.
Also worth knowing if anyone tries the obvious alternative:
`org.apache.maven.bridge.MavenRepositorySystem` **cannot be used from a
plugin**. maven-core does not export `org.apache.maven.bridge` to plugin class
realms — it is 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` is. It compiles, all unit tests pass, `mvn verify`
stays green because the plugin-testing harness uses one flat classpath, and it
fails only under `-Prun-its`. #628's `ArtifactHandlerManager` route avoids this
correctly.
--
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]
