Use SemVer precedence when selecting release tags - #240
Conversation
|
Sorry this PR got ignored for so long. In order for this to be mergeable, could you add at least one test showing the improved behavior...? |
|
The extra commits I added are from #249 — once that PR goes through, we can rebase this and drop those commits. I wanted to see if they would help with the CI failure. |
|
the CI failure seems to be real, so it appears this needs further work |
976f014 to
612b212
Compare
| versions | ||
| .map(Version.parse) | ||
| .sortWith(_.compareTo(_) > 0) | ||
| .headOption |
There was a problem hiding this comment.
sbt has https://github.com/sbt/librarymanagement/blob/develop/core/src/test/scala/sbt/librarymanagement/SemanticSelectorSpec.scala, which I think can be used for sorting with a few more lines of code.
There was a problem hiding this comment.
@eed3si9n I didn't manage to use it, can you try it yourself?
There was a problem hiding this comment.
@eed3si9n I revisited your SemanticSelector suggestion and checked it against concrete SemVer cases. With librarymanagement 1.12.3, both of these return false, although SemVer requires true:
import sbt.librarymanagement.{SemanticSelector, VersionNumber}
SemanticSelector(">1.0.0-beta.2").matches(VersionNumber("1.0.0-beta.11"))
SemanticSelector("<1.0.0-alpha").matches(VersionNumber("1.0.0-BETA"))The first example also returns false with librarymanagement 1.5.3, used by our sbt 1.5.8 cross-build baseline. The selector implementation splits pre-release tags on hyphens and compares non-numeric tags case-insensitively. SemVer precedence instead compares dot-separated identifiers, numeric identifiers numerically, and text in ASCII order. Adapting the selector would therefore require additional comparison logic, beyond wrapping it in sortWith.
I have kept java-semver for that reason: its existing precedence comparator handles these cases without maintaining our own implementation. Version 0.10.2 supports Java 8 and has no transitive runtime dependencies (POM); the JAR is about 51 KiB.
I also corrected two problems in my original implementation: strict parsing rejected existing short versions such as 1.0, and compareTo includes build metadata. The update uses lenient parsing and compareToIgnoreBuildMetadata, preserving the original selected string. For non-SemVer formats, it retains versionsort for the entire candidate set rather than mixing comparators pair by pair. This does mean retaining both dependencies, deliberately, to preserve existing custom version formats.
@SethTisue I added ten unit tests and extended the existing find-tag-newest scripted test. On current unmodified main, the new tests reproduce two remaining failures: numeric pre-release overflow (beta.2147483648) and metadata affecting precedence. The updated implementation passes all 14 unit tests locally and all 11 scripted scenarios in both CI configurations (Java 8 / Scala 2.12 and Java 17 / Scala 3), including the formatting checks. I have also refreshed the branch onto current main and removed the unrelated changes from the diff.
The new GitHub Actions run is awaiting maintainer approval.
There was a problem hiding this comment.
I agree that 1.0.0-beta.11 getting sorted after 1.0.0-beta.2 is technically not compliant with the SemVer specification, but how is picking 1.0.0-beta.2 useful for the purpose of sbt-git when the user tagged 1.0.0-beta.11?
8320465 to
49e8978
Compare
|
Thanks for following up, and sorry for the confusion in my earlier explanation. Our original use case at work is to publish release candidates such as I have now verified that current The original problem that motivated this PR has therefore already been resolved. The remaining edge cases covered by my recent update are separate from our original need, so I am closing this PR rather than expanding its scope further. Thank you both for maintaining the plugin and taking the time to review and discuss this! |
When several version tags point to HEAD, release selection should use SemVer precedence. The original failure involved labeled tags (#192). Current
versionsorthandles ordinary pre-releases, but still throwsNumberFormatExceptionfor1.0.0-beta.2147483648and lets build metadata affect the selected version.Use
java-semver0.10.2 to compare SemVer versions, with lenient parsing for existing short versions such as1.0. Compare without build metadata and return the original version string, retaining its spelling and metadata before appending any uncommitted suffix. Equivalent versions retain their input order.Keep
versionsortfor compatibility when any candidate cannot be parsed as SemVer, for example1.2.3.4or1.0.0a. The fallback applies to the entire candidate set so that one consistent ordering is used. This intentionally retains the existing dependency alongsidejava-semver.Add ten unit tests covering SemVer precedence, numeric and ASCII pre-release ordering, metadata, short versions, custom tag conversions, suffixes, and legacy formats. Extend
git-versioning/find-tag-newestwith multiple pre-release and metadata tags on the same commit. Document the selection and fallback behavior in the README.Local validation:
main(7ef2ceb), the new unit suite fails for numeric pre-release overflow and metadata precedence.++2.12.x clean scalafmtSbtCheck scalafmtCheckAll test scripted.++3.x clean scalafmtCheckAll test scripted.The updated branch is based on current
main; the old maintenance commits and unrelatedtest-projectchange are no longer part of the diff.The new GitHub Actions run is awaiting maintainer approval.