[MNG-7559] ComparableVersion vs versions with custom qualifiers

[jira-importer]

Andrzej J. opened MNG-7559 and commented

Since I know that ComparableVersion was brought to Maven from versions-maven-plugin, it turns out the bug described here:

mojohaus/versions#744

also exists in maven, at least in 3.8.3.

According to the maven version spec, versions containing a qualifier should be treated as less major than the same versions without the qualifier.

Currently it's only the case for a few "standard" qualifiers, e.g. "-rc*", "-alpha", etc.

However, it looks like "2.3-pfd" is deemed less major than "2.3".

@Test
public void testComparableVersionWithCustomQualifier()
{
    assertThat( new ComparableVersion( "2.3" ).compareTo( new ComparableVersion( "2.3-pfd" ) ),
            greaterThan( 0 ) );
}

---

Affects: 3.8.3

Attachments:

• image-2022-10-22-18-22-11-591.png (95.76 kB)

Remote Links:

• GitHub Pull Request #331
  
• GitHub Pull Request #845
  
• GitHub Pull Request #887
  
• GitHub Pull Request #925
  
• GitHub Pull Request #929
  

0 votes, 5 watchers

[jira-importer]

Andrzej J. commented

So, the least intrusive "fix" (if I may, but I'm not really sure if the "business" logic is correct here) is simply inverting the outcome here (0 instead of array length):

        /**
         * Returns a comparable for a qualifier.
         * <p/>
         * This method both takes into account the ordering of known qualifiers as well as lexical ordering for unknown
         * qualifiers.
         * <p/>
         * just returning an Integer with the index here is faster, but requires a lot of if/then/else to check for -1
         * or QUALIFIERS.size and then resort to lexical ordering. Most comparisons are decided by the first character,
         * so this is still fast. If more characters are needed then it requires a lexical sort anyway.
         *
         * @param qualifier
         * @return
         */
        public static Comparable comparableQualifier( String qualifier )
        {
            int i = QUALIFIERS_LIST.indexOf( qualifier );

            return i == -1 ? "0-" + qualifier : String.valueOf( i );
        }

which makes unknown qualifiers compare as less major than releases, just like most known qualifiers.

It looks like this inversion will not trigger any errors in integration tests in versions-maven-plugin (which is understandable considering the fact that non-standard qualifiers are not common), but the question remains: what should the actual behaviour be here? And I think this behaviour must also be in line across the board, i.e. at least between versions-maven-plugin and maven itself.

[jira-importer]

Gwénaël Ruelland commented

indeed not a bug, but the spec could be improved to take more milestones qualifiers like the PREVIEW qualifiers used on jdbc drivers

as to make .RC1 < -RC2.

and MR as Milestone 

if there is room for change can we change it to enhancement?

[jira-importer]

Gwénaël Ruelland commented

!https://user-images.githubusercontent.com/5782559/196051187-1e733e4c-7c07-4d1d-8ce6-0420eee1be51.png|width=443,height=354!

!https://user-images.githubusercontent.com/5782559/196051274-d2ec8a1f-cd1d-4ae1-b1e8-5c7ff6299a30.png|width=444,height=233!

[jira-importer]

Gwénaël Ruelland commented

!image-2022-10-22-18-22-11-591.png!

[jira-importer]

Michael Osipov commented

The handling of this issue is a mess. Elliotte Rusty Harold, please clean up which PRs have been merged, close this and link the commit message in our canonical repos. I am not able to follow/understand what happened here.

[jira-importer]

Garret Wilson commented

indeed not a bug, but the spec could be improved to take more milestones qualifiers like the PREVIEW qualifiers used on jdbc drivers

With the decision to add even more "magic strings", is it any surprise this is turning into a "mess", as Michael Osipov called it?

Perhaps the time has come for Maven to simply follow Semantic Versioning, as everyone expects anyway?

As I commented on the original bug I filed:

It's not surprising that at one time Maven created some reasonable, internally consistent versioning schema, and it is notable and commendable that Maven actually created a specification for this at a time when so many projects were making versioning format decisions based upon arbitrary whims of the current developer. I would guess that Maven probably even had some influence on the codification of the Semantic Versioning specification.

But it is unquestionable that the software industry is now converging around Semantic Version, and the last thing that we need is a central component of the build pipeline to be marching to some other tune. Just because something was specified at one time doesn't mean it can't be improved in a later version. HTML 4 wasn't bad. We should not be using HTML 4 in 2022—we should be using HTML 5, because that good specification has been improved.

It is certainly illustrative that after I filed this bug, initially everyone expected it to be a bug until we found some little-known Maven versioning specification. This expectation illustrates that in 2022 developers expect to be using Semantic Versioning. Developer do not expect that certain suffixes are special-cased and treated differently based upon whether they are "well known" or not. Marking certain limited set of identifiers as "well-known" leads exactly to [the never-ending dilemma of which special strings to support and how to deal with them]. It is better to treat them all the same.

The difference in version number interpretation between Maven versions and Semantic Versions is small. It is time for Maven to be using Semantic Versioning, in the least regarding the comparison of pre-release version suffixes.

[jira-importer]

Michael Osipov commented

Garret Wilson, I agree with you that it should be SemVer from Maven 4+ IFF we exactly know what this might cause.

[jira-importer]

Michael Osipov commented

What am I supposed to say:

D:\Entwicklung\Projekte\maven [MNG-7590+MNG-7600 ≡ +15 ~0 -0 !]> git log --grep=MNG-7559 master
D:\Entwicklung\Projekte\maven [MNG-7590+MNG-7600 ≡ +15 ~0 -0 !]>
D:\Entwicklung\Projekte\maven [MNG-7590+MNG-7600 ≡ +15 ~0 -0 !]> git log --grep=MNG-7559 maven-3.9.x
D:\Entwicklung\Projekte\maven [MNG-7590+MNG-7600 ≡ +15 ~0 -0 !]>

[jira-importer]

Michael Osipov commented

Commit reverted. Let's start fresh and make it right.

[jira-importer]

Michael Osipov commented

Most important question: How relevant is this for Maven 3.8.x? In other words: Shall/should this be included in 3.8.7?

[elharo]

There is no room for further change without breaking existing projects and builds. If there are any remaining bugs in implementation, or inconsistencies or incompleteness in the spec, file them, one per issue.