ascheman commented on code in PR #12771:
URL: https://github.com/apache/maven/pull/12771#discussion_r3815750923
##########
impl/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java:
##########
@@ -270,6 +269,61 @@ private void selectVersion(DefaultPluginVersionResult
result, PluginVersionReque
}
}
+ /**
+ * Returns the newest version of {@code candidates} that passes {@link
#isCompatible}, or {@code null}.
+ */
+ private String selectCompatible(PluginVersionRequest request,
TreeSet<Version> candidates, String kind) {
+ if (candidates.isEmpty()) {
+ return null;
+ }
+ logger.info(
+ "Looking for compatible {} version of plugin {}:{}",
+ kind,
+ request.getGroupId(),
+ request.getArtifactId());
+ for (Version v : candidates) {
+ String ver = v.toString();
+ if (isCompatible(request, ver)) {
+ return ver;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Does the repository offer at least one version that is neither a
snapshot nor a pre-release?
+ */
+ private boolean hasStableVersion(Versions versions) {
+ for (String ver : versions.versions.keySet()) {
+ if (!ver.endsWith("-SNAPSHOT") && !isPreRelease(ver)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Is this a pre-release version such as {@code 4.0.0-beta-1} or {@code
1.0-alpha-2}?
+ * <p>
+ * Decided with the version scheme itself rather than with a list of
qualifier names: a pre-release
+ * qualifier sorts <em>before</em> the version it qualifies ({@code
1.0-beta-1 < 1.0}), while a build
+ * or vendor qualifier does not ({@code 1.0-jre > 1.0}).
+ */
+ private boolean isPreRelease(String version) {
+ int qualifier = version.indexOf('-');
+ if (qualifier <= 0) {
+ return false;
+ }
+ try {
+ return versionScheme
+ .parseVersion(version)
+
.compareTo(versionScheme.parseVersion(version.substring(0, qualifier)))
+ < 0;
+ } catch (InvalidVersionSpecificationException e) {
+ return false;
+ }
+ }
+
private boolean isCompatible(PluginVersionRequest request, String version)
{
Review Comment:
Option (b): the behaviour is deliberate, the Javadoc was not. `isPreRelease`
answers "does this version sort before the version it qualifies", which
snapshots satisfy too; every caller classifies snapshots first
(`endsWith("-SNAPSHOT")`), so they never reach the pre-release bucket.
Excluding them inside the predicate would duplicate that test and make the
method's name disagree with its own ordering semantics. The Javadoc now says
exactly this.
https://github.com/apache/maven/pull/12771/commits/0e1f5c84f0581d0da24b4bd91f22fceae4b150a0
##########
impl/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java:
##########
@@ -214,34 +232,15 @@ private void selectVersion(DefaultPluginVersionResult
result, PluginVersionReque
}
}
- if (!releases.isEmpty()) {
- logger.info(
- "Looking for compatible RELEASE version of plugin
{}:{}",
- request.getGroupId(),
- request.getArtifactId());
- for (Version v : releases) {
- String ver = v.toString();
- if (isCompatible(request, ver)) {
- version = ver;
- repo = versions.versions.get(version);
- break;
- }
- }
+ version = selectCompatible(request, releases, "RELEASE");
+ if (version == null) {
+ version = selectCompatible(request, preReleases,
"pre-release");
}
-
- if (version == null && !snapshots.isEmpty()) {
- logger.info(
- "Looking for compatible SNAPSHOT version of plugin
{}:{}",
- request.getGroupId(),
- request.getArtifactId());
- for (Version v : snapshots) {
- String ver = v.toString();
- if (isCompatible(request, ver)) {
- version = ver;
- repo = versions.versions.get(version);
- break;
- }
- }
+ if (version == null) {
+ version = selectCompatible(request, snapshots, "SNAPSHOT");
+ }
Review Comment:
Agreed — now `RELEASE` / `PRE-RELEASE` / `SNAPSHOT`.
https://github.com/apache/maven/pull/12771/commits/0e1f5c84f0581d0da24b4bd91f22fceae4b150a0
##########
impl/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java:
##########
@@ -270,6 +269,61 @@ private void selectVersion(DefaultPluginVersionResult
result, PluginVersionReque
}
}
+ /**
+ * Returns the newest version of {@code candidates} that passes {@link
#isCompatible}, or {@code null}.
+ */
+ private String selectCompatible(PluginVersionRequest request,
TreeSet<Version> candidates, String kind) {
+ if (candidates.isEmpty()) {
+ return null;
+ }
+ logger.info(
+ "Looking for compatible {} version of plugin {}:{}",
+ kind,
+ request.getGroupId(),
+ request.getArtifactId());
+ for (Version v : candidates) {
+ String ver = v.toString();
+ if (isCompatible(request, ver)) {
+ return ver;
+ }
+ }
+ return null;
+ }
+
+ /**
+ * Does the repository offer at least one version that is neither a
snapshot nor a pre-release?
+ */
+ private boolean hasStableVersion(Versions versions) {
+ for (String ver : versions.versions.keySet()) {
+ if (!ver.endsWith("-SNAPSHOT") && !isPreRelease(ver)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
Review Comment:
Good catch, that was a real inconsistency: an unparseable entry made
`hasStableVersion` claim a stable fallback exists, while the selection loop
(which parses) could never pick it. It now also requires the version to be
parseable by the configured `VersionScheme`.
https://github.com/apache/maven/pull/12771/commits/0e1f5c84f0581d0da24b4bd91f22fceae4b150a0
##########
impl/maven-core/src/main/java/org/apache/maven/plugin/version/internal/DefaultPluginVersionResolver.java:
##########
@@ -186,6 +186,21 @@ private void selectVersion(DefaultPluginVersionResult
result, PluginVersionReque
version = versions.latestVersion;
repo = versions.latestRepository;
}
+ // A pre-release (alpha/beta/milestone/rc) is a "release" as far as
repository metadata is
+ // concerned, but it is not what a user asking for an unversioned
plugin expects: such
+ // versions are typically built against unstable APIs. Prefer a stable
version whenever the
+ // repository offers one, and fall back to the pre-release only when
it does not.
+ if (version != null && isPreRelease(version) &&
hasStableVersion(versions)) {
+ logger.info(
+ "Metadata of plugin {}:{} points at pre-release version
{}, looking for a stable version",
+ request.getGroupId(),
+ request.getArtifactId(),
+ version);
+ version = null;
+ repo = null;
+ searchPerformed = true;
+ }
Review Comment:
Added five: a stable version wins over the pre-release the metadata names as
`<release>`, the newest stable is taken, a pre-release is used only when no
stable exists, a snapshot is the last resort, and a stable `<release>` is taken
as-is without a search. They drive `selectVersion` with a stubbed
`MavenPluginManager`, so the ordering is observed without the IT harness.
https://github.com/apache/maven/pull/12771/commits/0e1f5c84f0581d0da24b4bd91f22fceae4b150a0
--
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]
