Hey @dwijnand, I think you meant to tag @tanishiking on this. :-)

Thank you for your reply @dwijnand.
Sure, your refactoring (#221 and #222) looks great to me and I'm happy to rebase my work onto them after they are done!

cool, thanks. I'll ping you when (if) they're accepted.

However in semantic-versioning we can separate identifiers in pre-release version by hyphens or dots, this VersionNumber#(un)apply seems not support separation by dots. Is this deliberately? Or should we implement it? (Or am I misunderstanding semantic-versioning?)

first off, my interpretation is that in SemVer pre-release identifiers cannot be hyphen-separated, only dot-separated.

looking at VersionNumber it's quite problematic because VersionNumber doesn't, for instance, allow numbers or dots its tags fields.. it becomes part of the extras strings..

I'm not sure what we should do to fix this, but I would highlight that pre-release version ordering is pretty niche, not mentioned in sbt/sbt#3710, and in my opinion never going to work anyways because from my experience people (sbt included!) don't dot-separate their prerelease identifiers, e.g sbt version 1.0.0-RC4 rather than 1.0.0-RC.4..

In comparing identifiers in pre-releases, we compare identifiers case-insensitively for convenience. Is this acceptable? Should we compare them case-sensitively?

hehehe I looked the history here and found a comment from me in 2014 :)

semver/semver#176

I agree with you: we should compare case-insensitively.

I wonder though whether we should change the implementation of VersionNumber equals (and therefore hashCode) to also ignore case differences in strings.

going to review your implementation next

Thank you for reviewing!

first off, my interpretation is that in SemVer pre-release identifiers cannot be hyphen-separated, only dot-separated.

Oh, sorry I misunderstood SemVer. After reading https://semver.org/#spec-item-11 again, indeed it is described.

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows:

in my opinion never going to work anyways because from my experience people (sbt included!) don't dot-separate their prerelease identifiers, e.g sbt version 1.0.0-RC4 rather than 1.0.0-RC.4..

I agree with you! OK, in this PR, I'll leave it :)

hehehe I looked the history here and found a comment from me in 2014 :)
semver/semver#176
I agree with you: we should compare case-insensitively.

Awesome, thanks!

Hmm... looking at discussion in semver/semver#176 I think we should change the implementation of VersionNumber equals (and therefore hashCode) to also ignore case differences in strings. (If we compare VersionNumber ignoring case differences).

Hmm... looking at discussion in semver/semver#176 I think we should change the implementation of VersionNumber equals (and therefore hashCode) to also ignore case differences in strings. (If we compare VersionNumber ignoring case differences).

looking again this PR is introducing an Ordering for VersionNumber based on the SemVer specification, in implicit scope. therefore I think we have to either (a) make equals (and hashCode) ignore case differences, or (b) make ordering case insensitive.

I'm not 100% sure because I don't know what it means to have methods based on Ordering on VersionNumber that are based on an implicit scope Ordering, with regards to its equals. does it need to be consistent? I think probably yes if it's so automatic (i.e no need for an import). need to do more research.

and if it needs to be consistent we should enforce that it is (with testing).

I changed the VersionNumber's equals method to ignore case difference 3182e4d. I'll squash these commits after rebasing this PR onto refactors :)

I'm not 100% sure because I don't know what it means to have methods based on Ordering on VersionNumber that are based on an implicit scope Ordering, with regards to its equals

Sorry I couldn't catch your feedback. Which do you mean by "consistent"?

• VersionNumber#equals's behavior should be "consistent" even if any instance of Ordering[VersionNumber] is in implicit scope.
  • If so, yes Ordering[VersionNumber] has no effects to VersionNumber's equal method.
• VersionNumber#equals's behavior should be "consistent" with definition of Ordering[VersionNumber]
  • In other words, VersionNumber should have property like (v1 == v2) == (v1 <= v2 && v1 >= v2) for all VersionNumber.
  • To do so, we need to ignore extras in equals in addition to ignore case difference, because in SemVer we ignore extras for comparing their versions.
• Or other?

Ordering extends Equiv, so it has an equiv method. if we're introducing an implicit scope Ordering[NumberVersion] then NumberVersion's definition of equals should match that ordering's definition of equiv.

I think we should just delegate equals to implicitly[Ordering[NumberVersion]].equiv(this, that).

Thank you for the explanation. I understood!

I think we should just delegate equals to implicitly[Ordering[NumberVersion]].equiv(this, that).

I agree with you (and tried to implement it e745970) but to do that we have to either

• ignore extra field in Ordering[VersionNumber]#compare and equals
  • this follows semantic-versioning's rule.

    • Build metadata SHOULD be ignored when determining version precedence. Thus two versions that differ only in the build metadata, have the same precedence. https://semver.org/#spec-item-10

  • but I think it is not unintuitive to ignore extra in equals and this change will break the backword compatiblity for VersionNumber#equals.
  • if implement this way, I think we should remain the VersionNumber.Strict#isCompatible to compare extra field.
• do not ignore extra field in Ordering[VersionNumber]#compare (and equals).
  • like e745970
  • this breaks the semantic-versioning's rule.
  • but I think it is pretty niche to compare extra field.

I prefer the latter one. what do you think about this?

what do you think about this?

I'm torn.

to add to your list of options there is option 3: don't put the SemVer-honoring in implicit scope, but make it opt-in.

we should evaluate in terms of the effect it would be have on the use case presented in #3710

meanwhile my refactoring in #222 is rebased and green. but @eed3si9n is currently on holiday until next week.

@tanishiking please note that #222 is now merged.

Thank you for ping (and sorry for my late reply), rebased.

But, after considering the use case presented in sbt/sbt#3710 some days, I came to realize that we should implement an api like semantic version selectors which is given by some package managers like npm (e.g. VersionNumber("2.12.2").satisfies(">=2.12")), instead of this (semver-honoring) ordering api.

The reason why implementing VersionNumber's ordering wouldn't be adequate to resolve sbt/sbt#3710 is below.

• I also think VersionNumber's ordering should be consistent with its equality(VersionNumber should satisfy symmetry rule)(∀a,b ∈ VersionNumber, (a <= b && b <= a) <=> a == b).
  • Considering the use case given in Version model and Ordering sbt#3710, i think,
    • v >= VersionNumber("2.12") expects v = "x.y.z" | (x > 2) or (x == 2 and y >= 12)
    • v > VersionNumber("2.12") expects v = "x.y.z" | (x > 2) or (x == 2 and y > 12)
    • v <= VersionNumber("2.12") expects v = "x.y.z" | (x < 2) or (x == 2 and y <= 12)
    • v < VersionNumber("2.12") expects v = "x.y.z" | (x < 2) or (x == 2 and y < 12)
      • I don't know how VersionNumber should behave if v's minor and patch versions are omitted.
  • SemVer doesn't seem to specify how should we handle the version that doesn't have minor or patch versions like 2.12 or 2. (refer Optional minor and patch versions semver/semver#237 ), and this PR's semver-honoring ordering api doesn't satisfy above expectation.
  • We can change (or make it opt-in) the behavior of the VersionNumber's ordering so that it satisfies the expectation above (ignoring semver specification). But we can't avoid breaking the VersionNumber's symmetry rule if VersionNumber satisfies the expectation.
    • in addition, I think it is confusing to implement a ordering which isn't commonly specified (unlike semver).

On the other hand, semantic version selectors realize the use case presented in #3710 without introducing a ordering that is not commonly specified or would breaks symmetry rule.

@tanishiking I'm sorry for leaving you hanging. In retrospect I realise that I should have forewarned you that I (and Eugene) were going to be busy and afk all of last week.

I think you're right and we should go for something like those selectors to avoid conflicting concerns.

Please let me know if you need a hand.

@dwijnand It's no problem :)

I think you're right and we should go for something like those selectors to avoid conflicting concerns.
Please let me know if you need a hand.

OK, I'll try to implement something like that selector and close this PR, thank you for discussing about this!