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

Inconsistency when patch version is leaved out in VersionReq #128

Closed
Frederick888 opened this issue Oct 20, 2017 · 10 comments
Closed

Inconsistency when patch version is leaved out in VersionReq #128

Frederick888 opened this issue Oct 20, 2017 · 10 comments

Comments

@Frederick888
Copy link

Frederick888 commented Oct 20, 2017

If I have a requirement like 0.30, it would match 0.30.0. However if the requirement is 0.30-beta.1, it does not match 0.30.0-beta.1.

I just went through the SemVer documentation without finding much related info. Actually I doubt whether such a requirement is really allowed?

Playground

extern crate semver;

use semver::{Version, VersionReq};

fn main() {
    let req = VersionReq::parse("0.30").unwrap();
    let ver = Version::parse("0.30.0").unwrap();
    println!("{}", req.matches(&ver)); // true

    let req = VersionReq::parse("0.30-beta.1").unwrap();
    let ver = Version::parse("0.30.0-beta.1").unwrap();
    println!("{}", req.matches(&ver)); // false
}
@steveklabnik
Copy link
Contributor

if the requirement is 0.30-beta.1,

Yeah, I'm not sure this is a well-formed requirement. Hmmmm

@Frederick888
Copy link
Author

@steveklabnik You meant 0.30-beta.1 is not well-formed but 0.30 is? I reckon to maintain consistency they should be both either legal or illegal.

But since it's not well-documented and actually requirements like 0.30 have already been used from time to time, what about applying similar rules to 0.30-beta.1 as well so that it can match 0.30.0-beta.1?

@steveklabnik
Copy link
Contributor

The semver spec doesn't discuss matchers at all, so there's more freedom here. We should investigate what matchers bundler and npm accept.

@Frederick888
Copy link
Author

Version constraint 0.30-beta is

...invalid in npm:

const semver = require('semver')
semver.satisfies('^0.30-beta', '0.30.0-beta') // TypeError: Invalid Version: ^0.30-beta

...valid in composer: https://getcomposer.org/doc/articles/versions.md#stability-constraints

use Composer\Semver\Semver;
var_dump(Semver::satisfies('0.30.0-beta', '^0.30-beta')); // bool(true)

...valid in gem: http://guides.rubygems.org/patterns/#pessimistic-version-constraint

puts Gem::Dependency.new('', '~> 0.30-beta').match?('', '0.31.1'); // true

@dhardy
Copy link

dhardy commented Dec 11, 2017

Actually Semver does specify this, in an indirect way:

If a version has a prerelease tag (for example, 1.2.3-alpha.3) then it will only be allowed to satisfy comparator sets if at least one comparator with the same [major, minor, patch] tuple also has a prerelease tag.

This seems to imply that the the specification must have a patch version number for a version with pre-release tag to be compatible. That seems to implies that either requirements like ^0.30-beta are invalid or ^0.30-beta should not match 0.30.0-beta.

Of course trying to interpret the specifications as literally as possible is not necessarily useful. But if we are to allow rules like ^0.30-beta then we need to decide:

  1. Should ^0.30-beta match 0.30.0-beta2? Probably yes.
  2. Should ^0.30-beta match 0.30.1-beta2? Questionable (quote above says pre-tags should not match if patch number differs). I'm going to say the only sensible answer, however, is yes.
  3. Should ^0.30 >=0.30.0-beta match 0.30.1-beta2? I don't know. Intuitively it looks like it should, while the rule quoted above will imply it cannot; version numbers are not intuitive!

Personally I'd like to simplify the rules by:

  • Allowing prerelease tags without minor/patch version numbers (like 0-beta, 0.5-pr1)
  • Remove the quoted rule and instead require prerelease checking in every =, ^ and ~ number (so the pre_tag_is_compatible function can be removed and is_exact, matches_tilde and is_compatible all check pre_is_compatible (or pre_is_equal) at the start of the function)
  • Allow a prerelease tag wildcard, e.g. ^0.30-*

Note that the above implies >=1.0 would now match 1.0-pre1. I'm not sure if we should fix this; if so we'd either need to add the quoted rule back in special cases or we'd need to break ordering (so !(1.0 <= 1.0-pre1) and !(1.0 >= 1.0-pre1)) or we'd need some weird ordering rules (IIRC Debian would consider 1.0-pre1 < 1.0 or something like that).

Then there's the question of how many project builds would get broken by changing the rules now. I suspect it wouldn't be too bad since prerelease tags aren't used very much and most version specifications are of the form x / x.y / x.y.z which semver interprets as ^x etc., in which case prerelease tags would not be interpreted differently aside from the weird cases mentioned above.

@steveklabnik I hope you are okay making some minor changes here? I'm quite happy to make a PR but we need to make some decisions first.

@steveklabnik
Copy link
Contributor

I am okay with making some changes, as long as they're going towards the spec and not away from it. :) In general, getting us to be compatible with npm's implementation is my goal. I actually wanted to make a comparison between major implementations, but hadn't had the time yet...

I'm super busy at the moment (just showed up at the work week) so I can't fully commit to a direction here, but it seems like we should change this; that is, in line with the principle above.

@dhardy
Copy link

dhardy commented Dec 12, 2017

It seems like prerelease tags are only expected when a patch version is present, making 0.30-beta invalid.

Regarding the other issue, it's not very clear whether NPM considers 1.2.3-beta to come semantically before or after 1.2.3; prerelease tags are discussed right next to range selection but the two issues don't intersect. Does 1.2.3 match >= 1.2.3-beta for example?

@steveklabnik
Copy link
Contributor

@isaacs if you have a chance, any comment on ☝️

@isaacs
Copy link

isaacs commented Dec 13, 2017

/me is summoned

There's some confusion here about what spec governs semver ranges. SemVer.org has detailed specifications for semver versions, but says nothing about ranges. (In the 2.0 version of the spec, the word "match" was struck entirely, specifically because it is ambiguous and confusing to talk about "matching" without treating ranges and versions separately. Versions don't match/satisfy one another; versions satisfy ranges, ranges match versions.)

The most popular existing implementation of semver ranges (by usage) is node-semver. So I'll treat that as if it's a spec, and talk about how it handles ranges and specifically, ranges with prerelease tags.

In node-semver, 0.30-beta is just not a valid range designation:

> semver.validRange('0.30-beta')
null

You could do something like ^0.30.0-beta or ~0.30.0-beta. In both cases, it would match any 0.30.0 version with a prerelease at or above beta (in an alphabetical sort). But, it would not match 0.30.1-beta, because the prerelease tag is attached to the {0, 30, 0} tuple, not the {0, 30, 1} tuple.

There is no way with node-semver ranges to specify "any version at or above 0.30.0, which has a prerelease tag at or above -beta". As others have pointed out, this is confusing, since 0.30.1-alpha may be "later" than 0.30.0, but is it ready? Should it match? Does it conform to the SemVer semantics of only containing non-breaking non-additive changes?

Prerelease tags are a separate universe that lives in a pre-committed state. It's "allowed" by the SemVer specification to have a 1.2.3-beta version that breaks entirely from 1.2.2. It's assumed to be a mistake, but mistakes are common and expected in prereleases (one might argue that's what betas are for, after all). If you wish to opt into beta testing, you have to specify that in the range, but you aren't opted into beta-testing future versions without opting in again. Because that would be weird.

Does 1.2.3 match >= 1.2.3-beta for example?

Yes, it does:

> semver.satisfies('1.2.3', '>=1.2.3-beta')
true

According to the SemVer spec, a prerelease tagged version is lower in the sort ordering than its non-prerelease-tagged counterpart.

$ semver 1.2.4 1.2.4-beta 1.2.2 1.2.5 1.2.3
1.2.2
1.2.3
1.2.4-beta
1.2.4
1.2.5

However, note that >= and <= will still omit prerelease tagged versions unless the named version in the range has a prerelease tag on it:

$ semver 1.2.4 1.2.4-beta 1.2.2 1.2.5 1.2.3 -r '>=1.2.3'
1.2.3
1.2.4
1.2.5

$ semver 1.2.4 1.2.4-beta 1.2.2 1.2.5 1.2.3 -r '>=1.2.4'
1.2.4
1.2.5

$ semver 1.2.4 1.2.4-beta 1.2.2 1.2.5 1.2.3 -r '>=1.2.4-alpha'
1.2.4-beta
1.2.4
1.2.5

$ semver 1.2.4 1.2.4-beta 1.2.2 1.2.5 1.2.3 -r '>=1.2.4-alpha <1.3 || 1.2'
1.2.2
1.2.3
1.2.4-beta
1.2.4
1.2.5

There's no law of the universe that says that this implementation of semver in Rust necessarily must match the implementation of semver in JavaScript, but if that is the goal, this is the answer. And if y'all go a different way on this, it's probably worth mentioning in the docs or something, so users of npm and cargo don't get confused :)

@dtolnay
Copy link
Owner

dtolnay commented May 25, 2021

This is fixed in 1.0.0 — 0.30-beta.1 is not considered a valid VersionReq.

This aligns with the grammar described in https://github.com/semver/semver/blob/efcff2c838c9945f79bfd21c1df0073271bcc29c/ranges.md.

@dtolnay dtolnay closed this as completed May 25, 2021
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

No branches or pull requests

5 participants