[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17725067#comment-17725067 ]
ASF GitHub Bot commented on MNG-7714: ------------------------------------- elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1200872369 ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -422,6 +436,106 @@ public String toString() { } } + /** + * Represents a combination in the version item list. + * It is usually a combination of a string and a number, with the string first and the number second + */ + private static class CombinationItem implements Item { + + StringItem stringValue; + + Item digitValue; + + CombinationItem(String v) { Review Comment: or version? ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -422,6 +436,106 @@ public String toString() { } } + /** + * Represents a combination in the version item list. + * It is usually a combination of a string and a number, with the string first and the number second + */ + private static class CombinationItem implements Item { + + StringItem stringValue; + + Item digitValue; + + CombinationItem(String v) { + int index = 0; + for (int i = 0; i < v.length(); i++) { + char c = v.charAt(i); + if (Character.isDigit(c)) { Review Comment: Please elaborate with reference to the docs. I don't think non-ASCII digits are included in maven's version comparison algorithm. Though now that I look at it I don't think we say that: https://maven.apache.org/pom.html We should probably bring this up on the dev mailing list to see if there are any unexpected consequences here of non-ASCII digits. ########## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ########## @@ -199,6 +202,7 @@ public int compareTo(Item item) { return -1; case STRING_ITEM: + case COMBINATION_ITEM: Review Comment: I don't see the change. Perhaps you need to push the branch. ########## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ########## @@ -147,10 +154,10 @@ void testVersionsEqual() { checkVersionsEqual("1.0.0x", "1-x"); // aliases - checkVersionsEqual("1ga", "1"); - checkVersionsEqual("1release", "1"); - checkVersionsEqual("1final", "1"); - checkVersionsEqual("1cr", "1rc"); + checkVersionsEqualOrder("1ga", "1"); Review Comment: still easier to follow if you add new tests without changing the existing test methods. > sp < final > ---------- > > Key: MNG-7714 > URL: https://issues.apache.org/jira/browse/MNG-7714 > Project: Maven > Issue Type: Bug > Reporter: Elliotte Rusty Harold > Assignee: Elliotte Rusty Harold > Priority: Major > Fix For: 4.0.0-alpha-6 > > > Ported from a comment on https://issues.apache.org/jira/browse/MNG-7701 > The claim is that sp < final, which if true is incorrect according to spec. > It is easy to demonstrate that this is not fixed and also not in line with > the spec, with just this one important example (yes this does break for us): > $ jbang org.apache.maven:maven-artifact:3.8.6 1.0.final-redhat-0001 > 1.0.sp1-redhat-0001 > Display parameters as parsed by Maven (in canonical form and as a list of > tokens) and comparison result: > 1. 1.0.final-redhat-0001 -> 1-redhat-1; tokens: [1, [redhat, [1]]] > 1.0.final-redhat-0001 < 1.0.sp1-redhat-0001 > 2. 1.0.sp1-redhat-0001 -> 1.0.sp-1-redhat-1; tokens: [1, 0, sp, [1, [redhat, > [1]]]] > versus > $ jbang org.apache.maven:maven-artifact:3.8.7 1.0.final-redhat-0001 > 1.0.sp1-redhat-0001 > Display parameters as parsed by Maven (in canonical form and as a list of > tokens) and comparison result: > 1. 1.0.final-redhat-0001 -> 1-redhat-1; tokens: [1, [redhat, [1]]] > 1.0.final-redhat-0001 > 1.0.sp1-redhat-0001 > 2. 1.0.sp1-redhat-0001 -> 1-sp-1-redhat-1; tokens: [1, [sp, [1, [redhat, > [1]]]]] > As you can see, our `sp` release is now ordered after our `final` release > despite this clear text in the "spec": > Non-numeric tokens ("qualifiers") have the alphabetical order, except for > the following tokens which come first in this order: "alpha" < "beta" < > "milestone" < "rc" = "cr" < "snapshot" < "" = "final" = "ga" < "sp" > It's clear that this tokenization isn't really correct by any reasonable > measurement, and breaking large amounts of (our) existing artifacts in the > wild is definitely not OK. -- This message was sent by Atlassian Jira (v8.20.10#820010)