fix pre-release checks in constraints #35
Conversation
|
This makes sense to me. Is this specified at all by Semver? (This lib doesn't strictly follow semver so it wouldn't be unprecedented to merge, just curious). Approved either way. |
|
AFAIK semver doesn't prescribe anything regarding constraint handling, just version comparison. Our comparisons seem to follow the spec from what I can see, so this PR only needed to check for the existence of pre-releases in constraint checks. |
constraint_test.go
Outdated
| @@ -62,6 +62,14 @@ func TestConstraintCheck(t *testing.T) { | |||
| {"~> 1.0.9.5", "1.0.9.6", true}, | |||
| {"~> 1.0.9.5", "1.0.9.5.0", true}, | |||
| {"~> 1.0.9.5", "1.0.9.5.1", true}, | |||
| {">=2.1.0-a", "2.1.1-beta", true}, | |||
There was a problem hiding this comment.
I'm not sure what rubygems does but my experience with other tools is that there is a further constraint here: if I request 2.1.0-a then that can match any prerelease with the base 2.1.0, but would not match a prerelease with another base such as 2.1.1-beta.
You can see this in npm's constraint syntax, for example.
It feels "right" to me that specifying >= 2.1.0-a would not match 2.1.1-beta for reasons similar to what is stated on that npm page, but that's definitely subjective and I'm certainly bringing my biases from other software here. If what you implemented here is rubygems behavior then I'd be fine with moving forward with it specified this way on that basis, since being consistent with something is better than going our own way.
There was a problem hiding this comment.
@apparentlymart,
Thanks! I think that is a good idea, and agree that it's a saner behavior.
jbardin
force-pushed
the
jbardin/pre-release-constraints
branch
from
9dd34d5
to
1ba082d
Compare
Pre-releases should not be automatically accepted as candidate versions for constraints without a pre-release component. This follows the logic documented for RubyGems which appears to be the inspiration for the constraint operators, and matches the behavior of other version packages. Adding a pre-release component to a pessimistic constraint will limit accepted versions to pre-releases only matching the base segments. Adding a pre-release to other constraint types will accept any comparable version, but pre-releases must still match the base segments.
jbardin
force-pushed
the
jbardin/pre-release-constraints
branch
from
1ba082d
to
fbe76cf
Compare
The go-version library changed the behavior of how pre-release constraints are checked in hashicorp/go-version#35
See: #35 Because of how go-version parses versions with so-called pre-release tags (see: hashicorp/go-version#35) and because not all packages have strict semver versions (especially ones we have taken from alpine), some packages end up being erroneously listed as uninstallable. This PR essentially tells vin to install the latest version of a package, even when that version is recognised as a pre-release
A pre-release can only be compared between pre-releases due to the limitations of the hashicorp/go-version libraries and will not behave as expected. hashicorp/go-version#35 On the other hand, the ApplyWithForce test is a legacy one that can only be tested before 1.1 and does not need to be tested on pre-releases 1.1+. So we can skip it if pre-release.
A pre-release can only be compared between pre-releases due to the limitations of the hashicorp/go-version libraries and will not behave as expected. hashicorp/go-version#35 On the other hand, the ApplyWithForce test is a legacy one that can only be tested before 1.1 and does not need to be tested on pre-releases 1.1+. So we can skip it if pre-release.
Pre-releases should not be automatically accepted as candidate versions
for constraints without a pre-release component. This follows the logic
documented for RubyGems which appears to be the inspiration for the
constraint operators, and matches the behavior of other version
packages.
Adding a pre-release component to a pessimistic constraint will limit
accepted versions to pre-releases only. Adding a pre-release to other
constraint types will accept any comparable version, including
pre-releases.