[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735713#comment-17735713 ] ASF GitHub Bot commented on MNG-7714: - elharo merged PR #1099: URL: https://github.com/apache/maven/pull/1099 > 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-7 > > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735259#comment-17735259 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1235155236 ## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ## @@ -353,15 +359,15 @@ public int getType() { @Override public boolean isNull() { -return (comparableQualifier(value).compareTo(RELEASE_VERSION_INDEX) == 0); +return value == null || value.isEmpty(); } /** * Returns a comparable value for a qualifier. - * + * * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical * ordering. - * + * * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1 Review Comment: Done. > 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-7 > > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735224#comment-17735224 ] ASF GitHub Bot commented on MNG-7714: - elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1235040466 ## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ## @@ -353,15 +359,15 @@ public int getType() { @Override public boolean isNull() { -return (comparableQualifier(value).compareTo(RELEASE_VERSION_INDEX) == 0); +return value == null || value.isEmpty(); } /** * Returns a comparable value for a qualifier. - * + * * This method takes into account the ordering of known qualifiers then unknown qualifiers with lexical * ordering. - * + * * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1 Review Comment: From here to the end is implementation detail. It's a good comment but it isn't needed in API doc. Move this part to a regular comment. ## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ## @@ -422,6 +439,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 Review Comment: nit: period at end > 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-7 > > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735152#comment-17735152 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1234788016 ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -383,4 +412,15 @@ void testMng7644() { checkVersionsEqual("2.0." + x, "2.0.0." + x); // previously ordered, now equals } } + +@Test +public void testMng7714() { +String f = "1.0.final-redhat"; Review Comment: Done. > 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-7 > > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735151#comment-17735151 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1234787833 ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -162,21 +173,39 @@ void testVersionsEqual() { checkVersionsEqual("1A", "1a"); checkVersionsEqual("1B", "1b"); checkVersionsEqual("1M", "1m"); -checkVersionsEqual("1Ga", "1"); -checkVersionsEqual("1GA", "1"); -checkVersionsEqual("1RELEASE", "1"); -checkVersionsEqual("1release", "1"); -checkVersionsEqual("1RELeaSE", "1"); -checkVersionsEqual("1Final", "1"); -checkVersionsEqual("1FinaL", "1"); -checkVersionsEqual("1FINAL", "1"); checkVersionsEqual("1Cr", "1Rc"); checkVersionsEqual("1cR", "1rC"); checkVersionsEqual("1m3", "1Milestone3"); checkVersionsEqual("1m3", "1MileStone3"); checkVersionsEqual("1m3", "1MILESTONE3"); } +@Test +void testVersionsEqualOrderAndObjectInequality() { +checkVersionsEqualOrder("1ga", "1"); +checkVersionObjectInequality("1ga", "1"); Review Comment: It was a special behavior before the change, but now that it's not a special behavior, I've removed the test case. > 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-7 > > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17735149#comment-17735149 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1234786802 ## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ## @@ -422,6 +439,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; Review Comment: That's a good idea. I changed it. > 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-7 > > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17734146#comment-17734146 ] ASF GitHub Bot commented on MNG-7714: - elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1233935475 ## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ## @@ -422,6 +439,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; Review Comment: This confused since I initially thought these were different representations of the same value. How about stringPart and digitPart? ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -162,21 +173,39 @@ void testVersionsEqual() { checkVersionsEqual("1A", "1a"); checkVersionsEqual("1B", "1b"); checkVersionsEqual("1M", "1m"); -checkVersionsEqual("1Ga", "1"); -checkVersionsEqual("1GA", "1"); -checkVersionsEqual("1RELEASE", "1"); -checkVersionsEqual("1release", "1"); -checkVersionsEqual("1RELeaSE", "1"); -checkVersionsEqual("1Final", "1"); -checkVersionsEqual("1FinaL", "1"); -checkVersionsEqual("1FINAL", "1"); checkVersionsEqual("1Cr", "1Rc"); checkVersionsEqual("1cR", "1rC"); checkVersionsEqual("1m3", "1Milestone3"); checkVersionsEqual("1m3", "1MileStone3"); checkVersionsEqual("1m3", "1MILESTONE3"); } +@Test +void testVersionsEqualOrderAndObjectInequality() { Review Comment: testVersionsHaveSameOrderButAreNotEqual ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +102,21 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { +ComparableVersion c1 = new ComparableVersion(v1); +ComparableVersion c2 = new ComparableVersion(v2); +assertEquals(0, c1.compareTo(c2), "expected " + v1 + " == " + v2); +assertEquals(0, c2.compareTo(c1), "expected " + v2 + " == " + v1); +} + +private void checkVersionObjectInequality(String v1, String v2) { Review Comment: checkVersionsAreNotEqual ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -383,4 +412,15 @@ void testMng7644() { checkVersionsEqual("2.0." + x, "2.0.0." + x); // previously ordered, now equals } } + +@Test +public void testMng7714() { +String f = "1.0.final-redhat"; Review Comment: Make these variables ComparableVersions, not strings, and then inline checkVersionsOrder in this method so it's easier to understand in one place what is being tested. `checkVersionsOrder(f, sp1)` is unclear because it does not indicate whether f is expected to be less than, equal to, or greater than sp1. ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { Review Comment: I see what's happening here now, but only because I've been over this a few times. Consider renaming this method to checkVersionsHaveSameOrder or checkVersionsAreOrderedTheSame ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -162,21 +173,39 @@ void testVersionsEqual() { checkVersionsEqual("1A", "1a"); checkVersionsEqual("1B", "1b"); checkVersionsEqual("1M", "1m"); -checkVersionsEqual("1Ga", "1"); -checkVersionsEqual("1GA", "1"); -checkVersionsEqual("1RELEASE", "1"); -checkVersionsEqual("1release", "1"); -checkVersionsEqual("1RELeaSE", "1"); -checkVersionsEqual("1Final", "1"); -checkVersionsEqual("1FinaL", "1"); -checkVersionsEqual("1FINAL", "1"); checkVersionsEqual("1Cr", "1Rc"); checkVersionsEqual("1cR", "1rC"); checkVersionsEqual("1m3", "1Milestone3"); checkVersionsEqual("1m3", "1MileStone3"); checkVersionsEqual("1m3", "1MILESTONE3"); } +@Test +void testVersionsEqualOrderAndObjectInequality() { +checkVersionsEqualOrder("1ga", "1"); +checkVersionOb
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17733986#comment-17733986 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1233451989 ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { Review Comment: @elharo I have modified it, please review again. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17733091#comment-17733091 ] ASF GitHub Bot commented on MNG-7714: - elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1231081124 ## 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"); +checkVersionsEqualOrder("1release", "1"); +checkVersionsEqualOrder("1final", "1"); Review Comment: OK, I think I understand you now. The behavior does change. The use of checkVersonsEquals and checkVersionsEqualOrder hides this though. Consider adding a test that the versions are no equalk. ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { Review Comment: Yes, what's unreadable is that I don't know which is greater and which is lesser. Or if it's doing something else then I am being confused by the name. Perhaps a clearer name would help, but it'ds also OK to just inline it. ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -383,4 +390,15 @@ void testMng7644() { checkVersionsEqual("2.0." + x, "2.0.0." + x); // previously ordered, now equals } } + +@Test +public void testMng7714() { +String f = "1.0.final-redhat"; +String sp1 = "1.0-sp1-redhat"; +String sp2 = "1.0-sp-1-redhat"; +String sp3 = "1.0-sp.1-redhat"; +checkVersionsOrder(f, sp1); Review Comment: only new tests please. Do not change existing tests unless the behavior has changed. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17732834#comment-17732834 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1230384026 ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { Review Comment: The `checkVersionsEqualOrder` method is only used to check whether two versions are in the same order. Is there anything unreadable about this? Do you think newComparable affects the semantics of this method? > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17732833#comment-17732833 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1230381847 ## 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: This code change will cause checkVersionsEqual("1ga", "1") to fail, and I think you can take a closer look at the specific changes. The difference between `checkVersionsEqualOrder` and the `checkVersionsEqual` method is that `checkVersionsEqualOrder` only compares the order of versions, `checkVersionsEqual` compares Comparable HashCodes. As for why I changed this test, I think I have explained above, because the previous logic was to directly change the contents of `1ga` to `1`, but now I will let `1ga` still stand for `1ga`, which will cause the hashcode in `checkVersionsEqual` to fail. This requires a new `checkVersionsEqualOrder` method to test that the order between `1ga` and `1` is consistent. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17729266#comment-17729266 ] ASF GitHub Bot commented on MNG-7714: - elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1217854060 ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { Review Comment: The name of this method does not make it obvious what it does. It probably isn;t needed at all. Inlining it would likely be easier to read. ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -101,6 +101,13 @@ private void checkVersionsEqual(String v1, String v2) { assertEquals(c2, c1, "expected " + v2 + ".equals( " + v1 + " )"); } +private void checkVersionsEqualOrder(String v1, String v2) { +Comparable c1 = newComparable(v1); +Comparable c2 = newComparable(v2); Review Comment: Inlining newComparable is probably also a good idea. Reading it now, I don't know what this method does. ## 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"); +checkVersionsEqualOrder("1release", "1"); +checkVersionsEqualOrder("1final", "1"); Review Comment: What you seem to have done is remove existing tests and replace them with new ones. Instead, leave the existing tests in place and unchanged. Then. add new tests in new methods. We need to be really sure that existing behavior doesn't change unless we want it to. ## 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: Does checkVersionsEqual("1ga", "1") now fail? This is important. If it does, then I need to dig into this much more carefully to understand what's going on. If it doesn't, then the code shouldn't change here. It should be line by line identical. New cases not previously tested should be in completely new test methods. related: Existing utility methods should not change. New ones can (but probably shouldn't) be added. Assume the existing tests are correct unless you're deliberately changing behavior, and then call that out. However, do not assume the existing tests are well written examples of good testing practice that should be imitated. They aren't. The existing code makes a number of novice mistakes (testing more than one case in a single method; unclear method names, hiding test behavior in utility methods) that should not be copied in new code. There is no benefit to being consistent with bad practices. > 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 compari
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17729179#comment-17729179 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1217499346 ## 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: I did not think of a particularly good way to ensure that the original test does not change, because the difference between checkVersionsEqualOrder and checkVersionsEqual is that the hash is no longer the same, let `1ga` be `1ga` itself, I think this is a good improvement. What do you think? > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17729178#comment-17729178 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1217499128 ## 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: I looked at the documentation and it doesn't seem to conflict with what the documentation says . This is just for combinations, like sp.1, sp-1, and so on. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ 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#8
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17724926#comment-17724926 ] ASF GitHub Bot commented on MNG-7714: - CrazyHZM commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1200430828 ## 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: Done. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17720573#comment-17720573 ] ASF GitHub Bot commented on MNG-7714: - elharo commented on code in PR #1099: URL: https://github.com/apache/maven/pull/1099#discussion_r1187611327 ## 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: include a return for every case, as relying on fallthrough is error-prone ## 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: I don't think we want to include non-ASCII digits here. ## 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: v --> value ## maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java: ## @@ -619,7 +748,13 @@ public final void parseVersion(String version) { } private static Item parseItem(boolean isDigit, String buf) { -if (isDigit) { +return parseItem(false, isDigit, buf); +} + +private static Item parseItem(boolean isCombination, boolean isDigit, String buf) { Review Comment: What is "buf"? It's not a StringBuffer. Is there a more descriptive name for this variable? ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -162,14 +169,14 @@ void testVersionsEqual() { checkVersionsEqual("1A", "1a"); checkVersionsEqual("1B", "1b"); checkVersionsEqual("1M", "1m"); -checkVersionsEqual("1Ga", "1"); -checkVersionsEqual("1GA", "1"); -checkVersionsEqual("1RELEASE", "1"); -checkVersionsEqual("1release", "1"); -checkVersionsEqual("1RELeaSE", "1"); -checkVersionsEqual("1Final", "1"); -checkVersionsEqual("1FinaL", "1"); -checkVersionsEqual("1FINAL", "1"); +checkVersionsEqualOrder("1Ga", "1"); Review Comment: These really belong in a new test method. There's too much in this test method already. ## maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java: ## @@ -383,4 +390,15 @@ void testMng7644() { checkVersionsEqual("2.0." + x, "2.0.0." + x); // previously ordered, now equals } } + +@Test +public void testMng7714() { +String f = "1.0.final-redhat"; +String sp1 = "1.0-sp1-redhat"; +String sp2 = "1.0-sp-1-redhat"; +String sp3 = "1.0-sp.1-redhat"; +checkVersionsOrder(f, sp1); Review Comment: This is unclear since there's nothing in the method call to say which is expected to be greater. If these can be tested directly, it would be easier to follow. That is, show the comparisons between f and sp1, etc. ## 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: I'd be more comfortable if existing tests didn't change > 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: Majo
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17718772#comment-17718772 ] Zhongming Hua commented on MNG-7714: [~elharo] Please review the PR: https://github.com/apache/maven/pull/1099 > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17717626#comment-17717626 ] Elliotte Rusty Harold commented on MNG-7714: Yes, I think that's true. What I haven't figured out yet is what the correct logic to fix this is. Any suggestions or PRs you might have are appreciated. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17717556#comment-17717556 ] Zhongming Hua commented on MNG-7714: [~elharo] I find that this problem is related to the following logic. Because treat *.x* as {*}-x{*}, 1.0.sp is resolved as [1,[sp]], not [1,sp]. {code:java} // 1.0.0.X1 < 1.0.0-X2 // treat .X as -X for any string qualifier X if (!list.isEmpty()) { list.add(list = new ListItem()); stack.push(list); } {code} > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17712734#comment-17712734 ] Elliotte Rusty Harold commented on MNG-7714: No, this bug still exists unless someone else has fixed it. > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17712732#comment-17712732 ] ASF GitHub Bot commented on MNG-7714: - slachiewicz commented on PR #1074: URL: https://github.com/apache/maven/pull/1074#issuecomment-1509990399 please close jira > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17705141#comment-17705141 ] ASF GitHub Bot commented on MNG-7714: - elharo merged PR #1074: URL: https://github.com/apache/maven/pull/1074 > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17704995#comment-17704995 ] ASF GitHub Bot commented on MNG-7714: - elharo opened a new pull request, #1074: URL: https://github.com/apache/maven/pull/1074 A minor detail while trying to resolve an underlying bug. The doc comment here does not accurately describe how the string 1.0alpha1 is actually parsed, and this is significant. > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17704989#comment-17704989 ] Elliotte Rusty Harold commented on MNG-7714: ``` @Test public void testMng7714() { ComparableVersion lesser = new ComparableVersion("1.0.final-redhat"); ComparableVersion greater = new ComparableVersion("1.0.sp1-redhat"); assertTrue(greater.compareTo( lesser ) > 0); } ``` > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17696441#comment-17696441 ] Elliotte Rusty Harold commented on MNG-7714: This is proving quite ugly. I can see the issue, but I can't easily fix it, much less without breaking other things. It seems to have to do with 1.0.sp getting parsed into [1, [sp]] instead of [1, sp]. Meanwhile 1.0.final-redhat gets parsed into [1, [redhat]] so redhat gets compared to sp. > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17696438#comment-17696438 ] Elliotte Rusty Harold commented on MNG-7714: https://github.com/apache/maven/pull/929 is likely involved > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17695704#comment-17695704 ] Elliotte Rusty Harold commented on MNG-7714: somehow in the failing cases we end up comparing redhat on the left to sp on the right, like the iteration is off. (Assuming I'm reading the debugger correctly) > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17695697#comment-17695697 ] Elliotte Rusty Harold commented on MNG-7714: "1.0.final" < "1.0.sp1" However 1.0.final-redhat > "1.0.sp1-redhat" // incorrect > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17695690#comment-17695690 ] Elliotte Rusty Harold commented on MNG-7714: also correctly handled: "1.0.final-redhat-0001" < "1.0.sp.1-redhat-0001" "1.0.final-redhat-0001" < "1.0.sp-1-redhat-0001" so it might be something in the splitting that screws this up. E.g. maybe sp1 is getting parsed as "sp1" instead of "sp-1" > 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 > > 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)
[jira] [Commented] (MNG-7714) sp < final
[ https://issues.apache.org/jira/browse/MNG-7714?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17695687#comment-17695687 ] Elliotte Rusty Harold commented on MNG-7714: I can reproduce this. Specifically, "final" < "sp" // correct "final" < "sp1" // correct "1.0.final-redhat-0001" < "1.0.sp-redhat-0001" // correct "1.0.final-redhat-0001" > "1.0.sp1-redhat-0001" // likely incorrect, investigating > 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 > > 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)