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

Fix version order specification #707

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vampire
Copy link

@Vampire Vampire commented Feb 24, 2025

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Your pull request should address just one issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
    Note that commits might be squashed by a maintainer on merge.
  • Run mvn site and examine output in target/site directory.
    Site will also be built on your pull request automatically and attached to GitHub Action result.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement 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
you have to acknowledge this by using the following check-box.

@Vampire Vampire force-pushed the fix-version-order-specification branch from 570bf77 to fb6af67 Compare February 25, 2025 00:22
except for the following tokens which come first in this order:

"<<<alpha>>>" \< "<<<beta>>>" \< "<<<milestone>>>" \< "<<<rc>>>" = "<<<cr>>>" \< "<<<snapshot>>>" \< "" = "<<<final>>>" = "<<<ga>>>" \< "<<<sp>>>"
"<<<alpha>>>" \< "<<<beta>>>" \< "<<<milestone>>>" \< "<<<rc>>>" = "<<<cr>>>" \< "<<<snapshot>>>" \< "" = "<<<final>>>" = "<<<ga>>>" = "<<<release>>>" \< "<<<sp>>>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this one need m too? I need to check on release

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understood it as these are the canonical names, next line says that you can abbreviate the first three if directly followed by a digit and then below you have the order including theses short-handles.
release is part, yes, I check all my corrections using the ComparableVersion.main helper.
Here it is defined: https://github.com/apache/maven/blob/master/compat/maven-artifact/src/main/java/org/apache/maven/artifact/versioning/ComparableVersion.java#L297


* else "<<<.qualifier>>>" = "<<<-qualifier>>>" \< "<<<-number>>>" \< "<<<.number>>>"

* <<<alpha>>> = <<<a>>> < <<<beta>>> = <<<b>>> < <<<milestone>>> = <<<m>>> < <<<rc>>> = <<<cr>>> < <<<snapshot>>> < '<<<>>>' = <<<final>>> = <<<ga>>> = <<<release>>> \< <<<sp>>>
* <<<alpha>>> \< <<<a1>>> \< <<<beta>>> \< <<<b1>>> \< <<<milestone>>> \< <<<m1>>> \< <<<rc>>> = <<<cr>>> \< <<<snapshot>>> \< <<<>>> = <<<final>>> = <<<ga>>> = <<<release>>> \< <<<sp>>>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why ml instead of m?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

m1, not m.
m is not special as well as a or b.
Only if followed by a digit.
This is described above and is also the actual behavior.


* "<<<1.ga>>>" = "<<<1-ga>>>" = "<<<1-0>>>" = "<<<1.0>>>" = "<<<1>>>" (removing of trailing "null" values)

* "<<<1-sp>>>" \> "<<<1-ga>>>"

* "<<<1-sp.1>>>" \> "<<<1-ga.1>>>"

* "<<<1-sp-1>>>" \> "<<<1-ga-1>>>"
* "<<<1-sp-1>>>" \< "<<<1-ga-1>>>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels wrong. Is this what the code does? If so, it might be a mistake. Do we have a unit test for this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, I just corrected the docs to show what the code does.

Due to the described and implemented algorithm the ga is stripped as it is followed by a hypen (instead of dot like in the line above it).

Then, starting from the end of the version, the trailing "null" values (0, "", "final", "ga") are trimmed. This process is repeated at each remaining hyphen from end to start.

So 1-ga-1 becomes 1-1 and 1-1 is greater than 1-sp-1.

@@ -479,31 +479,31 @@ mvn install:install-file -Dfile=non-maven-proj.jar -DgroupId=some.group -Dartifa

* "<<<1-foo2>>>" \< "<<<1-foo10>>>" (correctly automatically "switching" to numeric order)

* "<<<1.foo>>>" = "<<<1-foo>>>" \< "<<<1-1>>>" = "<<<1.1>>>"
* "<<<1.foo>>>" = "<<<1-foo>>>" \< "<<<1-1>>>" \< "<<<1.1>>>"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels wrong. Do we have a unit test for this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said, no idea.
But also here it is also following the documented algorithm higher on the page:

If the separator is the same, then compare the token:
[...]
else ".qualifier" = "-qualifier" < "-number" < ".number"

So this is the "-number" < ".number" case where -1 is less than .1 and thus 1-1 less than 1.1.


Note: Contrary to what was stated in some design documents, for version order, snapshots are not treated differently than releases or any other qualifier.

Note: As <<<2.0-rc1>>> \< <<<2.0>>>, the version requirement <<<[1.0,2.0)>>> excludes <<<2.0>>> but includes version <<<2.0-rc1>>>, which is contrary to
what most people expect. In addition, Gradle interprets it differently, resulting in different dependency trees for the same POM.
If the intention is to restrict it to <1.*> versions, the better version requirement is <<<[1,1.999999)>>>.
If the intention is to restrict it to <<<1.*>>> versions, the better version requirement is <<<[1,1.999999)>>> or <<<[1,2alpha)>>>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.2alpha might simply be the better choice here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it depends.
If only Maven consumers are involved, yes, as 2alpha or 2-alpha (maybe reads better) is the smallest possible version.
But as the text just talked about differences with Gradle, there 2a would be the smallest possible version and so the 1.999999 could also have its use-cases. 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The smallest possible version shouldn't differ between Maven and Gradle. If it does, that's likely a bug in Gradle.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, they do, and it is not a Gradle bug, but a different definition of version ordering.
Actually I was wrong, the lowest possible version in Gradle land is 2-dev while in Maven land it is 2-alpha.
For Gradle alpha, beta, and milestone are not in any way special tokens.
dev is special in that it is lower than any other non-numeric part.
And if that would not be the case, then the smallest would be 2-A, as in Gradle comparison is case-sensitive and uppercase is less than lowercase.

Also in Gradle 1-sp-1 would indeed be greater than 1-ga-1, as in the Gradle algorithm no tokens are omitted and the actual separator never has a significance.
And unlike Maven where 1.1 is equal to 1.1.0, in Gradle the former is less than the latter.

You can find the Gradle version ordering logic at https://docs.gradle.org/current/userguide/dependency_versions.html#sec:version-ordering

Also this document is aware of the differences if you just look one line up where it says "Gradle interprets it differently, resulting in different dependency trees for the same POM.".

If you think Gradle is buggy due to that, feel free to open a bug issue, but I'm pretty sure these differences were intentional decisions, just like Gradle also does not this nearest-wins thing Maven is doing, but does proper conflict resolution so that the most likely for all working version is used if nothing is controlled explicitly.

Really foolproof you never are, but you have to adapt to the versioning strategy the target of that range is following. 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO any tool that uses the Maven repository system should be following Maven rules here. I sort of wish the repo system were a little more independent of the rest of Maven, but that's the price of early success.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, even with the version ordering being equal you would not get the same results, as Gradle does conflict resolution different than the nearest-wins Maven does. (which imho makes much more sense how Gradle does it)
But that's really not related to this PR I'd say. :-)


Note: Contrary to what was stated in some design documents, for version order, snapshots are not treated differently than releases or any other qualifier.

Note: As <<<2.0-rc1>>> \< <<<2.0>>>, the version requirement <<<[1.0,2.0)>>> excludes <<<2.0>>> but includes version <<<2.0-rc1>>>, which is contrary to
what most people expect. In addition, Gradle interprets it differently, resulting in different dependency trees for the same POM.
If the intention is to restrict it to <1.*> versions, the better version requirement is <<<[1,1.999999)>>>.
If the intention is to restrict it to <<<1.*>>> versions, the better version requirement is <<<[1,1.999999)>>> or <<<[1,2alpha)>>>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The smallest possible version shouldn't differ between Maven and Gradle. If it does, that's likely a bug in Gradle.

@Vampire Vampire force-pushed the fix-version-order-specification branch from fb6af67 to 8d0aa2b Compare February 26, 2025 02:15
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

Successfully merging this pull request may close these issues.

2 participants