Feat: Improve matching of Toolchains versions by cstamas · Pull Request #11786 · apache/maven

cstamas · 2026-03-11T15:42:42Z

This feature was totally off from what it was documented. Have to note, that in this case versions (and hence, their matching) reflect exactly the version that Toolchains discovered (ie there is no "automatic" handling of 1.8 vs 8, JDK reported version is used as is).

Doco: https://maven.apache.org/plugins/maven-invoker-plugin/examples/selector-conditions.html

Expressions and changes:

"11" (did equality) is now "starts with 11." (as per doco, as range [1.5,) is equivalent of this)
"11+" (unsure what this did) is now "contained in [11,)" (as per doco)
"11-" (unsure what this did) is now "contained in (,11)" (as per doco)
otherwise parse it as range and check for contains

This feature was totally off from what it was documented.

nielsbasjes · 2026-03-11T17:28:52Z

I say

"11" is "[11,12)"
"11+" is "[11,)"

cstamas · 2026-03-11T17:34:09Z

I think I captured both: the latter "11+" is becoming "[11,)", and former is again same (but implemented as "starts with 11. -- DOT appended, so matches 11, 11.0, 11.999 but not 12.0) -- oohh "11" is not matched.

Edit: right, "11" is NOT matched.
Edit2: no, sorry, IS matched. Adding UT

HannesWell · 2026-03-11T19:35:33Z

maven-core/src/main/java/org/apache/maven/toolchain/RequirementMatcherFactory.java

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


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.

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

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.

For possible 1.8 support, see this similar case: c2e4be2

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".

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.

HannesWell · 2026-03-11T19:36:35Z

maven-core/src/main/java/org/apache/maven/toolchain/RequirementMatcherFactory.java

+            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);
+            }


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.

HannesWell · 2026-03-12T07:00:08Z

"11+" (unsure what this did) is now "contained in [11,)" (as per doco)

"11-" (unsure what this did) is now "contained in (,11)" (as per doco)

I have to admit that I wasn't aware this notation is possible and maybe for others it's the same.
And since this 'style' is not common in Maven, I want to ask the basic question if it's really worth the effort to support it, also with respect to the questions of older Java versions from #11786 (comment)?
Since one does not type version requirements all the time and the range syntax is well known in Maven (and similar to mathematical formulations), I'd argue that it is sufficient and the two characters saved are not worth it.
Instead the abbreviation could just be removed from the documentation. If it has never worked, it shouldn't be a problem.

But then then this is close to #11770, isn't it?

nielsbasjes · 2026-03-12T10:06:51Z

I think I captured both: the latter "11+" is becoming "[11,)", and former is again same (but implemented as "starts with 11. -- DOT appended, so matches 11, 11.0, 11.999 but not 12.0) -- oohh "11" is not matched.

Edit: right, "11" is NOT matched. Edit2: no, sorry, IS matched. Adding UT

I was primarily reacting to this line

"11" (did equality) is now "starts with 11." (as per doco, as range [1.5,) is equivalent of this)

The 'starts with 11' is not the same as [1.5,) (or [11,) in this example).

nielsbasjes · 2026-03-12T10:17:28Z

"11+" (unsure what this did) is now "contained in [11,)" (as per doco)

"11-" (unsure what this did) is now "contained in (,11)" (as per doco)

I have to admit that I wasn't aware this notation is possible and maybe for others it's the same. And since this 'style' is not common in Maven, I want to ask the basic question if it's really worth the effort to support it, also with respect to the questions of older Java versions from [#11786 (comment)]

I have only seen this notation in the invoker plugin where you can use this for the invoker.properties to specify a need for a specific java version to be active to run a specific integration test. I hit my head around this a week ago because this syntax is bot supported for the requested toolchain in the same file regarding the same kind of requirement.

From my perspective I would

Make the matching consistent across all places where you can do version matching like this.
Have a specific one for java/jdk versions where the "1.8"/"8" issue is handled.
Isolate the "+"/"-" syntax of the invoker plugin purely to that plugin and make it simply translate that syntax into something like '[11,)' / '(,11)' and send it to the generic code from there. Also give a deprecation warning message in the output if this is used.
A few versions later drop this old syntax from invoker and then the syntax is consistent across the plugins.

cstamas · 2026-03-12T17:19:36Z

But then then this is close to #11770, isn't it?

Well, not quite. This PR is about Toolchains, so about key invoker.toolchain.jdk.version in Invoker properties.

But alas, invoker does behave as documented for keys like invoker.java.version.

So we will end up with two ways to control/select/match "java version"?

This was the crux and motivation for this PR, to align the two (hopefully all).

Feat: Improve matching of Toolchains versions

b8e39bf

This feature was totally off from what it was documented.

cstamas self-assigned this Mar 11, 2026

cstamas mentioned this pull request Mar 11, 2026

feat: Improve the matching of required versions #11770

Open

8 tasks

cstamas mentioned this pull request Mar 11, 2026

Introduce a majorVersion property apache/maven-toolchains-plugin#142

Open

Add UT

31bfd36

HannesWell reviewed Mar 11, 2026

View reviewed changes

Move in the logic

c4b4641

Conversation

cstamas commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nielsbasjes commented Mar 11, 2026

Uh oh!

cstamas commented Mar 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HannesWell Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

cstamas Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

cstamas Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

cstamas Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

cstamas Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

HannesWell Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

HannesWell Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

HannesWell commented Mar 12, 2026

Uh oh!

nielsbasjes commented Mar 12, 2026

Uh oh!

nielsbasjes commented Mar 12, 2026

Uh oh!

cstamas commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cstamas commented Mar 11, 2026 •

edited

Loading

cstamas commented Mar 11, 2026 •

edited

Loading

cstamas commented Mar 12, 2026 •

edited

Loading