Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VersionScheme.MAVEN version comparison fails #333

Open
gastaldi opened this issue Jul 19, 2024 · 8 comments
Open

VersionScheme.MAVEN version comparison fails #333

gastaldi opened this issue Jul 19, 2024 · 8 comments

Comments

@gastaldi
Copy link
Contributor

gastaldi commented Jul 19, 2024

Given that 3.8.0.redhat-00001 is less than 3.8.0.SP1-redhat-00001, the following snippet returns 1 (expected is -1):

VersionScheme.MAVEN.compare("3.8.0.redhat-00001", "3.8.0.SP1-redhat-00001")

Originally posted by @gastaldi in #330 (comment)

@dmlloyd
Copy link
Collaborator

dmlloyd commented Jul 22, 2024

I posted this in the other thread:

I believe that custom qualifiers come after predefined qualifiers (like SP) according to the Maven rules.

$ jbang --main=org.eclipse.aether.util.version.GenericVersionScheme org.apache.maven.resolver:maven-resolver-util:1.9.6 1.redhat 1.sp
Display parameters as parsed by Maven Resolver (in canonical form and as a list of tokens) and comparison result:
1. 1.redhat -> 1.redhat; tokens: [1, redhat]
   1.redhat > 1.sp
2. 1.sp -> 1.sp; tokens: [1, 1]
$ jbang --main=org.eclipse.aether.util.version.GenericVersionScheme org.apache.maven.resolver:maven-resolver-util:1.9.6 3.8.0.redhat-00001 3.8.0.SP1-redhat-00001
Display parameters as parsed by Maven Resolver (in canonical form and as a list of tokens) and comparison result:
1. 3.8.0.redhat-00001 -> 3.8.0.redhat-00001; tokens: [3, 8, redhat, 1]
   3.8.0.redhat-00001 > 3.8.0.SP1-redhat-00001
2. 3.8.0.SP1-redhat-00001 -> 3.8.0.SP1-redhat-00001; tokens: [3, 8, 1, 1, redhat, 1]

This was also verified using 1.9.7 and 1.9.8. In light of this fact, I think this is not a bug, WDYT?

@gastaldi
Copy link
Contributor Author

I don't know, I believe that the absence of a predefined qualifier should assume Final or RELEASE for comparison purposes.

Any thoughts @aloubyansky?

@dmlloyd
Copy link
Collaborator

dmlloyd commented Jul 22, 2024

Well, that would be nice. :-)

However, Maven doesn't have any way to know whether you intend to put Final/0/etc. before a qualifier. It only has rules for leading zeros and trailing zeros. Additionally, it appears to be the intent of the Maven developers that . and - (and the empty separator) will become equivalent.

What we are doing, when we add our redhat qualifier, is we're basically saying "here is our special version that comes immediately after the version we qualify". But syntactically, this is not actually what happens. The only way to do such a thing is to add 0.0.0.0.0.... (to infinity) in between the original version number and our qualifier, which is not ideal. Maven general qualifiers were not really intended for this situation, hence our current troubles.

Addressing a syntactic weakness like this is actually very difficult, because if you do change the rules for this use case, then you will likely break some (or many) other use cases. Thus we have to look at bugs such as this one as "we made a wrong assumption about how this works" rather than "it doesn't work the way we want, so it should be fixed".

In practice, I think adding a 0 or Final when productizing versions of things which do not have (standard) qualifiers before redhat is probably the right solution.

@aloubyansky
Copy link
Member

Here is an implementation from the Maven resolver, to compare.

    public static void main(String[] args) throws Exception {
        compare("3.8.5", "3.8.5.redhat-00005");
        compare("3.8.5.SP1", "3.8.5.redhat-00005");
        compare("3.8.5.SP1", "3.8.5.SP1-redhat-00005");
        compare("3.8.5.SP2", "3.8.5.SP1-redhat-00005");
    }

    private static void compare(String v1, String v2) throws InvalidVersionSpecificationException {
        System.out.print(v1);
        var versionScheme = new org.eclipse.aether.util.version.GenericVersionScheme();
        final Version version1 = versionScheme.parseVersion(v1);
        final Version version2 = versionScheme.parseVersion(v2);
        final int result = version1.compareTo(version2);
        if(result < 0) {
            System.out.print(" < ");
        } else if(result == 0) {
            System.out.print(" = ");
        } else {
            System.out.print(" > ");
        }
        System.out.println(v2);
    }

results in

3.8.5 < 3.8.5.redhat-00005
3.8.5.SP1 < 3.8.5.redhat-00005
3.8.5.SP1 < 3.8.5.SP1-redhat-00005
3.8.5.SP2 > 3.8.5.SP1-redhat-00005

Where 3.8.5.SP1 < 3.8.5.redhat-00005 is not correct.

@aloubyansky
Copy link
Member

From the two implementations, I'd probably pick org.eclipse.aether.util.version.GenericVersionScheme. At least it seems to pick the latest downstream version.

@dmlloyd
Copy link
Collaborator

dmlloyd commented Jul 23, 2024

To be perfectly clear, my jbang examples above were also from the Maven resolver, same class. So we agree that Maven presently considers 3.8.5.SP1 < 3.8.5.redhat-00005, and the SmallRye version implementation does also? Thus I would consider this "not a bug", though we might also consider it "not very nice" as well.

@gastaldi
Copy link
Contributor Author

I'd say that Maven is wrong and so is Smallrye for not considering an empty milestone as a release during comparison 😀

@aloubyansky
Copy link
Member

Thank you both :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants