versions icon indicating copy to clipboard operation
versions copied to clipboard

`display-dependency-updates` version comparison algorithm bug for `-pfd`

Open garretwilson opened this issue 1 year ago • 43 comments

I have the following managed dependency in my Maven POM:

<dependency>
  <groupId>javax.faces</groupId>
  <artifactId>javax.faces-api</artifactId>
  <version>2.3</version>
  <scope>provided</scope>
</dependency>

However when I invoke mvn versions:display-dependency-updates, Versions Maven Plugin shows me:

javax.faces:javax.faces-api ........................... 2.3 -> 2.3-pfd

Offhand I don't know what -pfd means (pre-final-deliverable?), but that's irrelevant. You can see on Maven Central that this version was released before v2.3.

The semantic versioning specification says that any -* suffix (which would include -pfd) is considered a "pre-release" version. Thus 2.3-pfd is considered a pre-release version of 2.3 and should not be listed as a new available version. (The specification then goes on to explain how to calculate version precedence for pre-release suffixes, but that's even not the case here, as any pre-release version should not appear greater than the release version.)

(On a related note, the specification also says that version metadata, that is suffixes beginning with +*, must be completely ignored when determining version precedence.)

garretwilson avatar Oct 07 '22 13:10 garretwilson

Confirmed

jarmoniuk avatar Oct 07 '22 14:10 jarmoniuk

Thank you, @ajarmoniuk . It's really so nice to see this project is getting quick and useful responses. I sincerely appreciate it. Have a good weekend.

garretwilson avatar Oct 07 '22 14:10 garretwilson

No worries! Looking into it. Have a nice weekend too!

jarmoniuk avatar Oct 07 '22 14:10 jarmoniuk

Apparently, the problem also exists in Maven itself -- looks like the class ComparableVersion got carried over to org.apache.maven.artifact.versioning.ComparableVersion there.

If you use a range specification like

<dependency>
  <groupId>javax.faces</groupId>
  <artifactId>javax.faces-api</artifactId>
  <version>[2.3,)</version>
</dependency>

your project will actually depend on version 2.3-pfd (you can check with e.g. dependency:tree). But this is indeed incorrect.

jarmoniuk avatar Oct 07 '22 15:10 jarmoniuk

@slachiewicz

jarmoniuk avatar Oct 07 '22 15:10 jarmoniuk

https://issues.apache.org/jira/browse/MNG-7559

jarmoniuk avatar Oct 07 '22 15:10 jarmoniuk

both javadoc and version policy proposal describe exactly this behaviour;

Unknown qualifiers are considered after known qualifiers, with lexical order (always case insensitive),

I have failed to find any Maven documentation that says Maven follows what semver describes.

https://octopus.com/blog/maven-versioning-explained provides a very readable summary

mprins avatar Oct 08 '22 09:10 mprins

On the other hand, https://www.mojohaus.org/versions-maven-plugin/version-rules.html and e.g. https://docs.oracle.com/middleware/1212/core/MAVEN/maven_version.htm#MAVEN401 seem to promote a more generic approach, conflicting with the one presented in the blog you mention, @mprins

The thing about this blog post is that it actually treats the class as a source of truth. But we kinda can read code anyway, can't we?

All versions with a qualifier are older than the same version without a qualifier (release version).

Unless we only consider "standard" qualifiers as qualifiers.

jarmoniuk avatar Oct 08 '22 10:10 jarmoniuk

I don't think linking to an oracle document from 2015 is helpful at all; it is clear from the maven versioning wiki that I pointed to (and that the mojohaus document you gave us links to in the first line) that unknown qualifiers added to a version are considered to be a higher version than no qualifier (eg. commonly used Final, GA by hibernate work fine as do things like jre8, jre17, atlassian-1, atlassian-2, redhat-2... https://mvnrepository.com/artifact/jaxen/jaxen?repo=redhat-ga )

mprins avatar Oct 08 '22 12:10 mprins

I'm questioning the implementation, but the blog you're citing provides that implementation in question as the authority...

Any reason to disqualify the Oracle document based on its age? Has something changed in the meanwhile?

Anyway, I'll wait for someone else to chime in.

jarmoniuk avatar Oct 08 '22 12:10 jarmoniuk

https://cwiki.apache.org/confluence/display/MAVENOLD/Versioning is our Maven reference, written and implemented before Semver 1.0.0 even existed Changing version comparison won't bring much value: whatever convention is written by someone, other people use other convention, then there are edge cases...

hboutemy avatar Oct 08 '22 15:10 hboutemy

Then this should be stated clearly in the documentation. I'll go ahead and add a link to this to the docs.

jarmoniuk avatar Oct 08 '22 15:10 jarmoniuk

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 "other people user other convention" problem @hboutemy mentioned. 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.

garretwilson avatar Oct 08 '22 15:10 garretwilson

Let me ask you this question: looking at Maven Central, does it appear that the authors of javax.faces:javax.faces-api intended the -pfd suffix to be interpreted as Maven interprets it? Or did they intend for it to be interpreted according to how Semantic Version interprets it? The answer is clear: they intended it to be interpreted as Semantic Versioning interprets it. They likely didn't even know this Maven versioning documentation existed.

Changing version comparison won't bring much value …

What is the value of furthering some noble but outdated versioning logic that the average developer doesn't even know about, when the average developer is instead publishing artifacts that follow Semantic Versioning?

Are there very many artifacts published using a non-well-known extension for which the developer intended the Maven rules to apply? In this case at least, they expected Semantic Version rules to apply.

garretwilson avatar Oct 08 '22 15:10 garretwilson

Agreed with Garrett. I understand that changing the established behaviour is not fortunate. Luckily, the impact of this is relatively small since custom qualifiers are not common.

Also, rules should be clearly stated by which I don't mean an internal wiki or the source code of a class imported from versions-maven-plugin. And a source code of a class is not the source of truth.

Lastly, it's worth noting that other similar systems (e.g. Gradle) consider custom qualifiers less major than release versions.

jarmoniuk avatar Oct 08 '22 21:10 jarmoniuk

https://maven.apache.org/pom.html#version-order-specification

pzygielo avatar Oct 10 '22 10:10 pzygielo

So Maven is not exactly in line with its own specification:

else ".qualifier" < "-qualifier" < "-number" < ".number"

jarmoniuk avatar Oct 10 '22 10:10 jarmoniuk

So Maven doesn't even respect its own specification:

else ".qualifier" < "-qualifier" < "-number" < ".number"

Not sure what you mean by that, as

$ java -jar maven-artifact-3.8.6.jar 2.3-1 2.3-pfd
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 2.3-1 -> 2.3-1; tokens: [2, 3, [1]]
   2.3-1 > 2.3-pfd
2. 2.3-pfd -> 2.3-pfd; tokens: [2, 3, [pfd]]
$ java -jar maven-artifact-3.8.6.jar 2.3-0 2.3-pfd
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 2.3-0 -> 2.3; tokens: [2, 3]
   2.3-0 < 2.3-pfd
2. 2.3-pfd -> 2.3-pfd; tokens: [2, 3, [pfd]]

pzygielo avatar Oct 10 '22 10:10 pzygielo

So Maven doesn't even respect its own specification:

else ".qualifier" < "-qualifier" < "-number" < ".number"

Not sure what you mean by that, as

$ java -jar maven-artifact-3.8.6.jar 2.3-1 2.3-pfd
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 2.3-1 -> 2.3-1; tokens: [2, 3, [1]]
   2.3-1 > 2.3-pfd
2. 2.3-pfd -> 2.3-pfd; tokens: [2, 3, [pfd]]
$ java -jar maven-artifact-3.8.6.jar 2.3-0 2.3-pfd
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 2.3-0 -> 2.3; tokens: [2, 3]
   2.3-0 < 2.3-pfd
2. 2.3-pfd -> 2.3-pfd; tokens: [2, 3, [pfd]]

2.3-pdf is considered > 2.3

java -jar maven-artifact-3.8.6.jar 2.3 2.3-pfd

1. 2.3 -> 2.3; tokens: [2, 3]
   2.3 < 2.3-pfd
2. 2.3-pfd -> 2.3-pfd; tokens: [2, 3, [pfd]]

which is not in line with the specificication you posted. That's the whole problem here. I pointed out that it should probably be otherwise, i.e. release versions should always be considered more major than versions with unknown qualifiers.

jarmoniuk avatar Oct 10 '22 11:10 jarmoniuk

This is exactly in line,

Empty tokens are replaced with "0"

and then 2.3-0 -> 2.3, which I shown as

2.3-0 < 2.3-pfd

I think the problem is in expectation not being matched.

pzygielo avatar Oct 10 '22 11:10 pzygielo

I think the specification is clear...

Me too.

As 2.3 has no qualifier, it's considered as 2.3-final for comparison, and pfd comes after all special tokens.

You can see on Maven Central that this version was released before v2.3.

Time of the release does not participate in the process. Only version string.

pzygielo avatar Oct 10 '22 11:10 pzygielo

Thanks. I missed the part about empty tokens becoming zeros. Thus, the current behaviour is indeed reflected in the specification.

jarmoniuk avatar Oct 10 '22 11:10 jarmoniuk

i also encountered versioning discrepancies with .RC2 < -RC1 and MR/RM/CR qualifiers and things like that. eager to help when i can.

sultan avatar Oct 16 '22 18:10 sultan

image

the order could be alpha beta rc severity level : low

sultan avatar Oct 16 '22 18:10 sultan

image

RC < MR < FINAL severity level : medium, label is wrongly placed and use latest mojos might not take the good one

sultan avatar Oct 16 '22 18:10 sultan

Time of the release does not participate in the process. Only version string.

I did not assert that the time of release participates in the comparison process or that it should. Rather I only mentioned this as one piece of evidence that the authors of javax.faces:javax.faces-api expected their version numbers to be interpreted according to rules similar to semantic versioning, and not according to some outdated versioning rules in an obscure Maven document that give special meaning to magic strings.

garretwilson avatar Oct 16 '22 19:10 garretwilson

FYI I did some checking and found out what "-pfd" means in this case: apparently it is a stage in the Java Community Process that means "Proposed Final Draft" (e.g. Enterprise Java Beans 3.0 Proposed Final Draft). In other words, they are using it in much the same way as other teams use the term "release candidate". This means all the Java Specification Requests will likely wind up with a -pfd artifact in Maven (which will be sorted incorrectly) before each final release.

garretwilson avatar Oct 16 '22 20:10 garretwilson

we are unlucky that someone thought it would be a good idea to use SP1 to qualify a patch release. we sure cant take every magic string into account especially if two originators used incompatible rules. but at least we can make our best to have a correct answer when asked for the latest version. currently it looks as if qualifiers after a "-" are treated as pre release and qualifier after a "." is treated as post release.

sultan avatar Oct 16 '22 21:10 sultan

@sultan any irregular qualifier is considered later/greater than the release version, not only by this plugin, but also by Maven. That's the whole point of this issue.

jarmoniuk avatar Oct 17 '22 04:10 jarmoniuk

OK! do you happen to know an artifact having a later qualifier (other than SP)? maybe this can be improved/discussed on the Maven git?

sultan avatar Oct 17 '22 06:10 sultan