[ 
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)

Reply via email to