From 3e7d3afdc725670ff007367ef1ce01c764bbff61 Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Wed, 22 Jul 2026 11:47:23 +0000 Subject: [PATCH 1/2] Revert "Expand version comparator test with multi-part version strings" This reverts commit b83d216c634618d3d92f7d9be11ca42b43fcb000. --- .../jdk/ToolchainDiscovererTest.java | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java b/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java index 4a0907f..2ab09d6 100644 --- a/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java +++ b/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java @@ -84,38 +84,4 @@ void testVersionComparator() { assertEquals("11", list.get(1).getProvides().getProperty("version")); assertEquals("8", list.get(2).getProvides().getProperty("version")); } - - @Test - void testVersionComparatorMultiPart() { - ToolchainDiscoverer discoverer = new ToolchainDiscoverer(); - - ToolchainModel v1 = new ToolchainModel(); - v1.setType("jdk"); - v1.addProvide("version", "11.0.1"); - - ToolchainModel v2 = new ToolchainModel(); - v2.setType("jdk"); - v2.addProvide("version", "11.0.31"); - - ToolchainModel v3 = new ToolchainModel(); - v3.setType("jdk"); - v3.addProvide("version", "17.0.1"); - - ToolchainModel v4 = new ToolchainModel(); - v4.setType("jdk"); - v4.addProvide("version", "1.8"); - - List list = new ArrayList<>(); - list.add(v1); - list.add(v2); - list.add(v3); - list.add(v4); - - list.sort(discoverer.version()); - - assertEquals("17.0.1", list.get(0).getProvides().getProperty("version")); - assertEquals("11.0.31", list.get(1).getProvides().getProperty("version")); - assertEquals("11.0.1", list.get(2).getProvides().getProperty("version")); - assertEquals("1.8", list.get(3).getProvides().getProperty("version")); - } } From 5128ab7e90cb6f392cbcbee3acda7c28a4a3a7be Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Wed, 22 Jul 2026 11:47:23 +0000 Subject: [PATCH 2/2] Revert "Fix version comparator: use numeric comparison instead of lexicographic string comparison (#166)" This reverts commit 5d385b6605c379f840718d40f0e23288f36c627f. --- .../toolchain/jdk/ToolchainDiscoverer.java | 25 +++++++------- .../jdk/ToolchainDiscovererTest.java | 33 ------------------- 2 files changed, 12 insertions(+), 46 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java b/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java index 3b671cc..fa9455d 100644 --- a/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java +++ b/src/main/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscoverer.java @@ -370,25 +370,24 @@ Comparator version() { String[] b = v2.split("\\."); int length = Math.min(a.length, b.length); for (int i = 0; i < length; i++) { - int oa = parseInt(a[i]); - int ob = parseInt(b[i]); - if (oa != ob) { - return Integer.compare(oa, ob); + String oa = a[i]; + String ob = b[i]; + if (!Objects.equals(oa, ob)) { + // A null element is less than a non-null element + if (oa == null || ob == null) { + return oa == null ? -1 : 1; + } + int v = oa.compareTo(ob); + if (v != 0) { + return v; + } } } - return Integer.compare(a.length, b.length); + return a.length - b.length; }) .reversed(); } - private static int parseInt(String s) { - try { - return Integer.parseInt(s); - } catch (NumberFormatException e) { - return 0; - } - } - private Set findJdks() { if (foundJdks == null) { synchronized (this) { diff --git a/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java b/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java index 2ab09d6..11657c0 100644 --- a/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java +++ b/src/test/java/org/apache/maven/plugins/toolchain/jdk/ToolchainDiscovererTest.java @@ -18,11 +18,7 @@ */ package org.apache.maven.plugins.toolchain.jdk; -import java.util.ArrayList; -import java.util.List; - import org.apache.maven.toolchain.model.PersistedToolchains; -import org.apache.maven.toolchain.model.ToolchainModel; import org.codehaus.plexus.util.xml.Xpp3Dom; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnJre; @@ -31,7 +27,6 @@ import org.slf4j.LoggerFactory; import static org.apache.maven.plugins.toolchain.jdk.ToolchainDiscoverer.CURRENT; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -56,32 +51,4 @@ void testDiscovery() { assertTrue(persistedToolchains.getToolchains().stream() .anyMatch(tc -> tc.getProvides().containsKey(CURRENT))); } - - @Test - void testVersionComparator() { - ToolchainDiscoverer discoverer = new ToolchainDiscoverer(); - - ToolchainModel jdk8 = new ToolchainModel(); - jdk8.setType("jdk"); - jdk8.addProvide("version", "8"); - - ToolchainModel jdk11 = new ToolchainModel(); - jdk11.setType("jdk"); - jdk11.addProvide("version", "11"); - - ToolchainModel jdk17 = new ToolchainModel(); - jdk17.setType("jdk"); - jdk17.addProvide("version", "17"); - - List list = new ArrayList<>(); - list.add(jdk8); - list.add(jdk17); - list.add(jdk11); - - list.sort(discoverer.version()); - - assertEquals("17", list.get(0).getProvides().getProperty("version")); - assertEquals("11", list.get(1).getProvides().getProperty("version")); - assertEquals("8", list.get(2).getProvides().getProperty("version")); - } }