Skip to content

Feat: Improve matching of Toolchains versions #11786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
cstamas wants to merge 3 commits into
base: maven-3.9.x
Choose a base branch
from
Draft

Feat: Improve matching of Toolchains versions #11786

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8e39bf
Feat: Improve matching of Toolchains versions
cstamas Mar 11, 2026
31bfd36
Add UT
cstamas Mar 11, 2026
c4b4641
Move in the logic
cstamas Mar 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 69 additions & 18 deletions maven-core/src/main/java/org/apache/maven/toolchain/RequirementMatcherFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
*/
package org.apache.maven.toolchain;

import org.apache.maven.artifact.versioning.DefaultArtifactVersion;
import org.apache.maven.artifact.versioning.InvalidVersionSpecificationException;
import org.apache.maven.artifact.versioning.VersionRange;
import java.util.Locale;
import java.util.Objects;

import org.eclipse.aether.util.version.GenericVersionScheme;
import org.eclipse.aether.version.InvalidVersionSpecificationException;
import org.eclipse.aether.version.VersionScheme;

/**
*
Expand All @@ -38,16 +41,17 @@ public static RequirementMatcher createVersionMatcher(String provideValue) {
}

private static final class ExactMatcher implements RequirementMatcher {

private String provides;
private final String provides;

private ExactMatcher(String provides) {
this.provides = provides;
}

@Override
public boolean matches(String requirement) {
return provides.equalsIgnoreCase(requirement);
return Objects.equals(
provides != null ? provides.toLowerCase(Locale.ENGLISH) : null,
requirement != null ? requirement.toLowerCase(Locale.ENGLISH) : null);
}

@Override
Expand All @@ -57,31 +61,78 @@ public String toString() {
}

private static final class VersionMatcher implements RequirementMatcher {
DefaultArtifactVersion version;
private final String version;

private VersionMatcher(String version) {
this.version = new DefaultArtifactVersion(version);
this.version = version;
}

@Override
public boolean matches(String requirement) {
try {
VersionRange range = VersionRange.createFromVersionSpec(requirement);
if (range.hasRestrictions()) {
return range.containsVersion(version);
String r = requirement != null ? requirement.toLowerCase(Locale.ENGLISH) : null;
String v = version != null ? version.toLowerCase(Locale.ENGLISH) : null;
if (v == null && r == null) {
return true; // null == null
}
if (v == null || r == null) {
return false; // null != non-null
}
if (v.equals(r)) {
return true; // str == str (ignoring case)
}
return matchesRequirement(v, r);
}

private static final VersionScheme VERSION_SCHEME = new GenericVersionScheme();

private static boolean matchesRequirement(String version, String requirement) {
boolean interval = false;
boolean included = false;
if (requirement.endsWith("+")) {
interval = true;
included = true;
requirement = requirement.substring(0, requirement.length() - 1);
} else if (requirement.endsWith("-")) {
interval = true;
requirement = requirement.substring(0, requirement.length() - 1);
}

@HannesWell HannesWell Mar 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't this code be moved into the if block that has checked that the requirement isn't a version-range? In case of the latter this seems to be ignored any-ways.

final String req = requirement;

// if requirement is not a version range itself
if (!req.contains("[") && !req.contains("(") && !req.contains(",")) {
if (!interval) {
return version.startsWith(req + "."); // "11" -> "11.xxx"

@HannesWell HannesWell Mar 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to propose that logic as well for #11770, i.e. when one just specifies a version requirement that the actual version should just start with the requirement to handle cases like

  • 11
  • 11.1
  • 11.1.1

Of course one has to handle the Java-8 and older versions. But maybe this can be done by removing a leading 1. to 'norm' the versions to all start with their, what's now called, feature version of the JDK.

In this case I wonder how it works if one requires 1.5.2 and has a version 1.5.2 since a trailing dot is added? But I see this case covered in tests.
Just in case that still has slipped, one could test for equals or starts with dot suffix, i.e. the current code.

@cstamas cstamas Mar 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this call the normalized strings are tested for equality. This is what I missed in my comment above as well. So "1.5.2".equalsIgnoreCase("1.5.2") will yield true

@cstamas cstamas Mar 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re 1.8 support (or older): IMHO, user must write versions as reported by JDK, especially as toolchains XML may be generated today. And if user consistently writes 1.8 there is no problem. The only problem would be 8+ that does not include Java 1.8... unsure about this.

@cstamas cstamas Mar 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For possible 1.8 support, see this similar case: c2e4be2

@cstamas cstamas Mar 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legacy code for sure wrote "1.4", "1.6" or "1.8"... and given toolchain does not have "feature version" to match against (nor could?) I'd say we are fine IF users use versions "as reported by Java".

@HannesWell HannesWell Mar 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this call the normalized strings are tested for equality.

Oversaw that, thanks.

I'd say we are fine IF users use versions "as reported by Java".

Sounds good. I would try to keep it simple and as long as there is a way to handle it, it's fine.
Also given that the number of users at <=1.8 is hopefully declining, this should also become less of a problem.

} else {
return range.getRecommendedVersion().compareTo(version) == 0;
try {
if (included) {
return VERSION_SCHEME
.parseVersionRange("[" + req + ",)")
.containsVersion(VERSION_SCHEME.parseVersion(version)); // "11+" -> "[11,)"
} else {
return VERSION_SCHEME
.parseVersionRange("(," + req + ")")
.containsVersion(VERSION_SCHEME.parseVersion(version)); // "11-" -> "(,11)"
}
} catch (InvalidVersionSpecificationException e) {
// nope; GenericVersionScheme never throes but we need to make compiler happy
throw new RuntimeException(e);
}
}
} else {
try {
return VERSION_SCHEME
.parseVersionRange(req)
.containsVersion(VERSION_SCHEME.parseVersion(version)); // "range" -> "range"
} catch (InvalidVersionSpecificationException e) {
// nope; GenericVersionScheme never throes but we need to make compiler happy
throw new RuntimeException(e);
}
} catch (InvalidVersionSpecificationException ex) {
// TODO error reporting
ex.printStackTrace();
return false;
}
}

@Override
public String toString() {
return version.toString();
return version;
}
}
}
45 changes: 44 additions & 1 deletion maven-core/src/test/java/org/apache/maven/toolchain/RequirementMatcherFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
*/
package org.apache.maven.toolchain;

import java.util.Arrays;
import java.util.Collection;
import java.util.List;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -50,17 +54,56 @@ public void testCreateExactMatcher() {
public void testCreateVersionMatcher() {
RequirementMatcher matcher;
matcher = RequirementMatcherFactory.createVersionMatcher("1.5.2");
assertFalse(matcher.matches("1.5"));
assertTrue(matcher.matches("1"));
assertTrue(matcher.matches("1.5"));
assertTrue(matcher.matches("1.5.2"));
assertFalse(matcher.matches("[1.4,1.5)"));
assertFalse(matcher.matches("[1.5,1.5.2)"));
assertFalse(matcher.matches("(1.5.2,1.6)"));
assertTrue(matcher.matches("(1.4,1.5.2]"));
assertTrue(matcher.matches("(1.5,)"));

assertTrue(matcher.matches("1.5+"));
assertFalse(matcher.matches("1.5-"));

assertFalse(matcher.matches("1.6+"));
assertTrue(matcher.matches("1.6-"));

assertEquals("1.5.2", matcher.toString());

// Ensure it is not printed as 1.5.0
matcher = RequirementMatcherFactory.createVersionMatcher("1.5");
assertEquals("1.5", matcher.toString());
}

@Test
public void testCreateVersionMatcherWithJavaVersions() {
RequirementMatcher java25 = RequirementMatcherFactory.createVersionMatcher("25.0.2");
RequirementMatcher java21 = RequirementMatcherFactory.createVersionMatcher("21.0.10");
RequirementMatcher java17 = RequirementMatcherFactory.createVersionMatcher("17.0.18");
RequirementMatcher java11 = RequirementMatcherFactory.createVersionMatcher("11.0.30");
RequirementMatcher java8 = RequirementMatcherFactory.createVersionMatcher("1.8.0_482");
List<RequirementMatcher> matchers = Arrays.asList(java25, java21, java17, java11, java8);

testMatch("11", matchers, java11);
testMatch("11+", matchers, java25, java21, java17, java11);
testMatch("11-", matchers, java8);
testMatch("[11,21)", matchers, java17, java11);
testMatch("1.8", matchers, java8);
testMatch("1.8+", matchers, java25, java21, java17, java11, java8);
testMatch("8", matchers);
testMatch("8+", matchers, java25, java21, java17, java11);
}

private static void testMatch(
String requirement, Collection<RequirementMatcher> allMatchers, RequirementMatcher... requiredMatchers) {
int matches = 0;
for (RequirementMatcher matcher : allMatchers) {
if (matcher.matches(requirement)) {
matches++;
assertTrue(Arrays.asList(requiredMatchers).contains(matcher));
}
}
assertEquals(matches, requiredMatchers.length);
}
}