Ranges for release candidates fail to parse
bripkens opened this issue · 13 comments
Test
ExpressionParser.newInstance().parse("<=2.0.0-rc7")Exception
Caused by: Illegal character near 'rc7'
at com.github.zafarkhaja.semver.expr.Lexer.tokenize(Lexer.java:218)
at com.github.zafarkhaja.semver.expr.ExpressionParser.parse(ExpressionParser.java:86)
at com.github.zafarkhaja.semver.expr.ExpressionParser.parse(ExpressionParser.java:43)
Expected
Release candidates can be compared. For instance 2.0.0-rc8 does not satisfy the expression <=2.0.0-rc7.
According to the grammar the pre-release versions and build metadata are not supported. Yet.
Let's keep this issue as a feature request as I was planning to implement it. Unfortunately, though, it's not easy with the current parser implementation because there's a grammar ambiguity we need to work around; there's a conflict between Hyphen ranges and pre-release version separator.
By the way, in the meantime you can use the internal DSL for this if it suits you.
Version v = Version.valueOf("2.0.0-rc8");
boolean result = v.satisfies(lte("2.0.0-rc7"))); Thanks for getting back to me @zafarkhaja. Unfortunately I am not creating the expression, but I am only consuming them.
I just checked the node-semver ranges and it looks like they don't support pre-release versions either...
pre-release versions seem to be supported, but only match other pre-release
> require('semver').satisfies('2.0.0', '<=2.0.0-rc7')
false
> require('semver').satisfies('2.0.0-rc8', '<=2.0.0-rc7')
false
> require('semver').satisfies('2.0.0-rc1', '<=2.0.0-rc7')
true
> require('semver').satisfies('2.0.0-rc2', '<=2.0.0-rc7')
true
Well, JS was never my strong suit and the user documentation is not clear about that.
Yeah, it seems to support pre-release versions pretty well. I'm not sure what you mean by "but only match other pre-release", but all the examples in your comment look quite correct to me.
node-semver doesn't have the - ambiguity as they require space before and after the - operator. Their BNF is here: https://github.com/npm/node-semver/blob/master/range.bnf
hyphen ::= partial ' - ' partial
I just ran into this today and also need support for range expresssions that can be applied to prerelease versions. I don't really want it implemented they way node-semver chose to. To be more precise about "but only match other pre-release", node-semver defines it this way:
https://github.com/npm/node-semver#prerelease-tags
Essentially, for node-semver, when you express a range part with a prerelease value, you are saying the major.minor.patch of any candidate version with a prelease must match (at least according to the doc). So given candidates:
1.2.3-alpha.0
1.2.3-beta.0
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4-alpha.0
1.2.4
1.3.0-alpha.0
1.3.0
2.0.0
An expression of '>=1.2.3-rc.1' would be satisfied by:
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4
1.3.0
2.0.0
An expression of '~1.2.3-rc.1' would be satisfied by:
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4
An expression of '^1.2.3-rc.1' would be satisfied by:
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4
1.3.0
Notice that in each case, the only prerelease versions that match have the exact major, minor, patch as the version used in the range expression. I think node-semver prerelease handling is implemented somewhat ambiguously though and contradicts their doc. Check this out:
$ node -e "console.log(require('semver').satisfies('1.4.5', '>=1.0.5-0 <=2.0.0-0'))"
true
// makes sense
$ node -e "console.log(require('semver').satisfies('1.4.5-rc.1', '>=1.0.5-0 <=2.0.0-0'))"
false
// is about dependency management, not what the spec defines as > and < according to precedence
$ node -e "console.log(require('semver').satisfies('1.0.5-rc.1', '>=1.0.5-0 <=2.0.0-0'))"
true
// makes sense but doesn't follow the rules I expect for an "and" expression between these two given the rule that I shouldn't match for a prerelease that is not in my x.y.z in this case 2.0.0
$ node -e "console.log(require('semver').satisfies('1.0.5-rc.1', '<=2.0.0-0'))"
false
// ?? oh, it doesn't start with 2.0.0...so why did the previous one work?
$ node -e "console.log(require('semver').satisfies('1.0.5', '<=2.0.0-0'))"
true
// yep
$ node -e "console.log(require('semver').satisfies('2.0.0-0', '>=1.0.5-0 <=2.0.0-0'))"
true
// um? isn't this an and expression?
$ node -e "console.log(require('semver').satisfies('2.0.0-0', '>=1.0.5-0'))"
false
// ugh this one follow the node-semver doc but has nothing to do with what I expect of >= w.r.t. the semver spec
for an expression like '>=1.2.3-rc.1 <=1.3.0-alpha.0' which should never match a prerelease since either side of the expression should filter out the other side's prereleases according to the doc but apparently it isn't implemented quite that way.
Personally, I think the node-semver implementation is really about dependency resolution and making some reasonable assertion about what people want to do when they fetch a dependency. This is great and has practical application in its domain.
For a pure semver library though, I would rather see an implementation that supports prerelease ranges that is purely about the sort precedence of the versions and leave the dependency resolution details out of the range expression implementation. Something like:
An expression of '>=1.2.3-rc.1' would be satisfied by:
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4-alpha.0
1.2.4
1.3.0-alpha.0
1.3.0
2.0.0
An expression of '~1.2.3-rc.1' would be equivalent to '>=1.2.3-rc.1 & <1.3.0-0' (prerelease 0 is the lowest possible prerelease) and satisfied by:
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4-alpha.0
1.2.4
An expression of '^1.2.3-rc.1' would be equivalent to '>=1.2.3-rc.1 & <2.0.0-0' and satisfied by:
1.2.3-rc.1
1.2.3-rc.2
1.2.3
1.2.4-alpha.0
1.2.4
1.3.0-alpha.0
1.3.0
I can then present those to my dependency management tooling and make a decision on whether or not to accept a given prerelease version without clouding the semver range expression syntax with decisions that are driven by a requirement that may not hold in all contexts.
If you want to do filtering the way node-semver does, you would probably need access to the range expression AST to make the right decisions.
Eric, thank you for following up. Give me some time to digest this and I'll get back to you soon.
I have a use-case for this as well, so I'm curious what your thoughts are on it? Also, I've noticed that the 'satisfies' functionality doesn't seem to work on versions with a tilde - Is that a related issue?
Thanks for the insights!
@eric-isakson @jrmash thank you guys for your feedback and suggestions. I must say that I totally agree with what Eric has proposed in his last comment. I'll be shipping this feature with the next release (0.10.0).
@jrmash I don't think the tilde ranges are somehow related to this issue unless you're using pre-release versions, other than that the tilde ranges should work as expected. If you believe otherwise, please create a separate issue and provide an example of what's not working for you and I'll take a look. Thank you.
@zafarkhaja I should have been clearer with my inquiry, but yes I am/was using tilde ranges with pre-release versions and the 'satisfies' feature.
@zafarkhaja could you tell me the status on this?
Any help required?
Closing this issue in favor of #70, the progress can be followed from there.