You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Ross Goldberg (Jira)" <ji...@apache.org> on 2020/01/01 16:56:00 UTC
[jira] [Comment Edited] (MNG-6420) ComparableVersion incorrectly
parses certain version strings
[ https://issues.apache.org/jira/browse/MNG-6420?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17006452#comment-17006452 ]
Ross Goldberg edited comment on MNG-6420 at 1/1/20 4:55 PM:
------------------------------------------------------------
[~elharo] Thanks for the info.
"2.0" / "2.0.0.0" work fine in both ComparableVersion & ComparableVersionTest, I just referenced them to better illustrate the problems with "2.0" < "2.0.a" < "2.0.0.a".
"a" is only equivalent to "alpha" if it is the beginning of a token and directly followed by a digit. Since this usage isn't directly followed by a digit, it's just an ad-hoc qualifier "a", not "alpha". A better example would be "2.0" < "2.0.q" < "2.0.0.q". Here, "q" is just a random qualifier, like "a" in the original example, but switching "a" to "q" removes the possibility for confusion with "alpha".
I asked about performance benchmarks because a) I don't want to cause regressions, and b) many of the recent commits to ComparableVersion were for performance reasons. I haven't optimized for performance, and wasn't going to unless I actually identified bottlenecks via performance tests. The theoretical better performance of my code should be a side benefit of changes that I made to the code to streamline it and to improve correctness, not specifically to improve performance.
Immutable collections (immutable anything really) are still useful for me even if not in the public API, as they prevent bugs from accidental modification of data that should be immutable. If you don't use any, however, I'll just wrap with unmodifiable wrappers.
Will leave ComparableVersion#parseVersion there, and will open another issue to deprecate it.
The only outstanding question is: do you agree that the current ordering of "2.0" < "2.0.q" < "2.0.0.q" is non-sensical, and should be changed to "2.0" < "2.0.0.q" < "2.0.q"? If so, do you agree that my proposed spec revision will achieve that, and not cause any other problems?
(though any performance testing info would still be useful, if anyone has any)
Thanks again.
was (Author: xdr):
[~elharo] Thanks for the info.
"2.0" / "2.0.0.0" work fine in both ComparableVersion & ComparableVersionTest, I just referenced them to better illustrate the problems with "2.0" < "2.0.a" < "2.0.0.a".
"a" is only equivalent to "alpha" if it is the beginning of a token and directly followed by a digit. Since this usage isn't directly followed by a digit, it's just an ad-hoc qualifier "a", not "alpha". A better example would be "2.0" < "2.0.q" < "2.0.0.q". Here, "q" is just a random qualifier, like "a" in the original example, but switching "a" to "q" removes the possibility for confusion with "alpha".
I asked about performance benchmarks because a) I don't want to cause regressions, and b) many of the recent commits to ComparableVersion were for performance reasons. I haven't optimized for performance, and wasn't going to unless I actually identified bottlenecks via performance tests. The theoretical better performance of my code should be a side benefit of changes that I made to the code to streamline it and to improve correctness, not specifically to improve performance.
Immutable collections (immutable anything really) are still useful for me even if not in the public API, as they prevent bugs from accidental modification of data that should be immutable. If you don't use any, however, I'll just wrap with unmodifiable wrappers.
Will leave ComparableVersion#parseVersion there, and will open another issue to deprecate it.
So, the outstanding question is: do you agree that the current ordering of "2.0" < "2.0.q" < "2.0.0.q" is non-sensical, and should be changed to "2.0" < "2.0.0.q" < "2.0.q"? If so, do you agree that my proposed spec revision will achieve that, and not cause any other problems?
Thanks again.
> ComparableVersion incorrectly parses certain version strings
> ------------------------------------------------------------
>
> Key: MNG-6420
> URL: https://issues.apache.org/jira/browse/MNG-6420
> Project: Maven
> Issue Type: Bug
> Components: core
> Affects Versions: 3.5.3
> Reporter: Ross Goldberg
> Priority: Major
>
> For certain version strings, ComparableVersion doesn't follow the Maven version order spec (https://maven.apache.org/pom.html#Version_Order_Specification).
> To improve the code & fix the following bugs, I completely rewrote the version parser (using Java 10 & Google Guava 25.1). It is attached & passes all of the tests from ComparableVersionTest.
> Bug 1:
>
> java -jar /usr/local/Cellar/maven/3.5.3/libexec/lib/maven-artifact-3.5.3.jar 1-0.3 1
>
> Outputs:
>
> 1. 1-0.3 == 1-0.3
> 1-0.3 == 1
> 2. 1 == 1
>
> This probleem stems from:
>
> [https://github.com/apache/maven/blob/b8c06e61ab73cd9e25a5b2c93d9e5077b2196751/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java#L295-L296]
>
>
>
> Bug 2:
>
> java -jar /usr/local/Cellar/maven/3.5.3/libexec/lib/maven-artifact-3.5.3.jar 1-0-2 1-0.1
>
> 1. 1-0-2 == 1-2
> 1-0-2 < 1-0.1
> 2. 1-0.1 == 1-0.1
>
> This problem stems from retaining ListItems that, after normalization, have no children other than the subsequent ListItem. Removing the unnecessary ListItem should fix this (i.e. remove the extraneous ListItem (named extraneous) from its parent ListItem (named parent), then add the only child of extraneous to parent).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)