You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/20 17:43:09 UTC

[GitHub] [maven] sultan opened a new pull request, #930: [MNG-7644] Fix version comparison ( .RC1 < -RC2 )

sultan opened a new pull request, #930:
URL: https://github.com/apache/maven/pull/930

   The current version parser does not treat .RC and -RC correctly:
   actual : 1.0.0.RC1 > 1.0.0-RC2
   expected : 1.0.0.RC1 < 1.0.0-RC2
    
   because RC1 < RC2
   how to fix : place a list item before qualifier
   ---
   Following this checklist to help us incorporate your
   contribution quickly and easily:
   
    - [x] Make sure there is a [JIRA issue](https://issues.apache.org/jira/browse/MNG) filed
          for the change (usually before you start working on it).  Trivial changes like typos do not
          require a JIRA issue. Your pull request should address just this issue, without
          pulling in other changes.
    - [x] Each commit in the pull request should have a meaningful subject line and body.
    - [x] Format the pull request title like `[MNG-XXX] SUMMARY`,
          where you replace `MNG-XXX` and `SUMMARY` with the appropriate JIRA issue.
    - [x] Also format the first line of the commit message like `[MNG-XXX] SUMMARY`.
          Best practice is to use the JIRA issue title in both the pull request title and in the first line of the commit message.
    - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
    - [ ] Run `mvn clean verify` to make sure basic checks pass. A more thorough check will
          be performed on your pull request automatically.
    - [ ] You have run the [Core IT][core-its] successfully.
   
   If your pull request is about ~20 lines of code you don't need to sign an
   [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf) if you are unsure
   please ask on the developers list.
   
   To make clear that you license your contribution under
   the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   you have to acknowledge this by using the following check-box.
   
    - [ ] I hereby declare this contribution to be licenced under the [Apache License Version 2.0, January 2004](http://www.apache.org/licenses/LICENSE-2.0)
   
    - [ ] In any other case, please file an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
   
   [core-its]: https://maven.apache.org/core-its/core-it-suite/
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1362187894

   Merged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1363834337

   > > @michael-o we may want to consider this a major change and reverting this one
   > 
   > Then docs need to be reverted as well, no?
   
   the docs may need an update to explain how 3.x and 4.x treats the versions order differently


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on a diff in pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #930:
URL: https://github.com/apache/maven/pull/930#discussion_r1054191144


##########
maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java:
##########
@@ -337,4 +337,20 @@ public void testReuse()
 
         assertEquals( "reused instance should be equivalent to new instance", c1, c2 );
     }
+
+    /**
+     * Test <a href="https://issues.apache.org/jira/browse/MNG-7644">MNG-7644</a> edge cases
+     * 1.0.0.RC1 < 1.0.0-RC2
+     */
+    public void testMng7644()
+    {
+        for ( String x : new String[]{ "alpha", "a", "beta", "b", "milestone", "m", "RC" } ) {

Review Comment:
   You should add at least two arbitrary one as well.



##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -53,7 +53,8 @@
  *     </ul>
  *     Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive),
  *   </li>
- * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li>
+ * <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
+ *   1.0.RC2 &lt; 1.0-RC3 &lt; 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </li>

Review Comment:
   This needs to be generalized as well.



##########
maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java:
##########
@@ -337,4 +337,20 @@ public void testReuse()
 
         assertEquals( "reused instance should be equivalent to new instance", c1, c2 );
     }
+
+    /**
+     * Test <a href="https://issues.apache.org/jira/browse/MNG-7644">MNG-7644</a> edge cases
+     * 1.0.0.RC1 < 1.0.0-RC2

Review Comment:
   This needs to be generalized as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on a diff in pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #930:
URL: https://github.com/apache/maven/pull/930#discussion_r1054197217


##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -53,7 +53,8 @@
  *     </ul>
  *     Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive),
  *   </li>
- * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li>
+ * <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
+ *   1.0.RC2 &lt; 1.0-RC3 &lt; 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </li>

Review Comment:
   yes



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o closed pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o closed pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch
URL: https://github.com/apache/maven/pull/930


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #930: [MNG-7644] Fix version comparison ( .RC1 < -RC2 ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1360241555

   > > Does this really only apply to RC or anything else which has the pattern `dot/hyphen{item}{digit/number}`?
   > 
   > it is intended to apply to all string qualifiers: dot/hyphen{string item}
   
   Then please generalize the issue summary as well as the description and add more tests which depict that is general and not specific. Let's make this complete for master first and then I consent, then you can back port and save time.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on a diff in pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #930:
URL: https://github.com/apache/maven/pull/930#discussion_r1054215565


##########
maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java:
##########
@@ -337,4 +337,20 @@ public void testReuse()
 
         assertEquals( "reused instance should be equivalent to new instance", c1, c2 );
     }
+
+    /**
+     * Test <a href="https://issues.apache.org/jira/browse/MNG-7644">MNG-7644</a> edge cases
+     * 1.0.0.RC1 < 1.0.0-RC2

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1363838716

   > > > @michael-o we may want to consider this a major change and reverting this one
   > > 
   > > 
   > > Then docs need to be reverted as well, no?
   > 
   > the docs may need an update to explain how 3.x and 4.x treats the versions order differently
   
   Let's do the following: Keep the change with RC1/RC2 even 3.8.x with the docs in place and see whether will be reports due to the change in behavior. If really someone complains I'd be willing to fix this regression and release 3.8.8 for this. Otherwise this edge case fix makes life much easier for others.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #930: [MNG-7644] Fix version comparison ( .RC1 < -RC2 ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1360225328

   Does this really only apply to RC or anything else which has the pattern dot/hyphen{item}{digit/number}?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] michael-o commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1363820772

   > @michael-o we may want to consider this a major change and reverting this one
   
   Then docs need to be reverted as well, no?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1363858907

   > > > > @michael-o we may want to consider this a major change and reverting this one
   > > > 
   > > > 
   > > > Then docs need to be reverted as well, no?
   > > 
   > > 
   > > the docs may need an update to explain how 3.x and 4.x treats the versions order differently
   > 
   > Let's do the following: Keep the change with RC1/RC2 even 3.8.x with the docs in place and see whether will be reports due to the change in behavior. If really someone complains I'd be willing to fix this regression and release 3.8.8 for this. Otherwise this edge case fix makes life much easier for others.
   
   Nice! thanks Michael
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on pull request #930: [MNG-7644] Fix version comparison ( .RC1 < -RC2 ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1360236991

   > Does this really only apply to RC or anything else which has the pattern `dot/hyphen{item}{digit/number}`?
   
   it is intended to apply to all string qualifiers dot/hyphen{string item}
   
   it is not intended to apply to digit/number because 1.2.3 != 1.2-3
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1361083577

   > > > Does this really only apply to RC or anything else which has the pattern `dot/hyphen{item}{digit/number}`?
   > > 
   > > 
   > > it is intended to apply to all string qualifiers: dot/hyphen{string item}
   > 
   > Then please generalize the issue summary as well as the description and add more tests which depict that is general and not specific. Let's make this complete for master first and then I consent, then you can back port and save time.
   
   it's fair @michael-o, i updated the Jira and PRs/commits.
   i hope its all ok now.
   thank you very much for your time!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on a diff in pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #930:
URL: https://github.com/apache/maven/pull/930#discussion_r1054206213


##########
maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java:
##########
@@ -337,4 +337,20 @@ public void testReuse()
 
         assertEquals( "reused instance should be equivalent to new instance", c1, c2 );
     }
+
+    /**
+     * Test <a href="https://issues.apache.org/jira/browse/MNG-7644">MNG-7644</a> edge cases
+     * 1.0.0.RC1 < 1.0.0-RC2
+     */
+    public void testMng7644()
+    {
+        for ( String x : new String[]{ "alpha", "a", "beta", "b", "milestone", "m", "RC" } ) {

Review Comment:
   i suppose you mean like abc and def. will do



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on a diff in pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on code in PR #930:
URL: https://github.com/apache/maven/pull/930#discussion_r1054215083


##########
maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java:
##########
@@ -53,7 +53,8 @@
  *     </ul>
  *     Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive),
  *   </li>
- * <li>a hyphen usually precedes a qualifier, and is always less important than something preceded with a dot.</li>
+ * <li>a hyphen usually precedes a qualifier, and is always less important than digits/number, for example
+ *   1.0.RC2 &lt; 1.0-RC3 &lt; 1.0.1 ; but prefer '1.0.0-RC1' over '1.0.0.RC1' </li>

Review Comment:
   done



##########
maven-artifact/src/test/java/org/apache/maven/artifact/versioning/ComparableVersionTest.java:
##########
@@ -337,4 +337,20 @@ public void testReuse()
 
         assertEquals( "reused instance should be equivalent to new instance", c1, c2 );
     }
+
+    /**
+     * Test <a href="https://issues.apache.org/jira/browse/MNG-7644">MNG-7644</a> edge cases
+     * 1.0.0.RC1 < 1.0.0-RC2
+     */
+    public void testMng7644()
+    {
+        for ( String x : new String[]{ "alpha", "a", "beta", "b", "milestone", "m", "RC" } ) {

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven] sultan commented on pull request #930: [MNG-7644] Fix version comparison ( .X1 < -X2 for any string qualifier x ), 3.8.x branch

Posted by GitBox <gi...@apache.org>.
sultan commented on PR #930:
URL: https://github.com/apache/maven/pull/930#issuecomment-1363815374

   @michael-o we should consider this a major change and reverting this one


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org