jsemver icon indicating copy to clipboard operation
jsemver copied to clipboard

Ranges for release candidates fail to parse

Open bripkens opened this issue 8 years ago • 12 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.

bripkens avatar Nov 01 '15 12:11 bripkens

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.

zafarkhaja avatar Nov 01 '15 20:11 zafarkhaja

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"))); 

zafarkhaja avatar Nov 02 '15 11:11 zafarkhaja

Thanks for getting back to me @zafarkhaja. Unfortunately I am not creating the expression, but I am only consuming them.

bripkens avatar Nov 02 '15 12:11 bripkens

I just checked the node-semver ranges and it looks like they don't support pre-release versions either...

zafarkhaja avatar Nov 03 '15 06:11 zafarkhaja

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

bripkens avatar Nov 03 '15 07:11 bripkens

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.

zafarkhaja avatar Nov 03 '15 07:11 zafarkhaja

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-isakson avatar Feb 10 '16 21:02 eric-isakson

Eric, thank you for following up. Give me some time to digest this and I'll get back to you soon.

zafarkhaja avatar Feb 11 '16 06:02 zafarkhaja

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!

jrmash avatar Mar 25 '16 01:03 jrmash

@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 avatar Apr 27 '16 19:04 zafarkhaja

@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.

jrmash avatar Apr 27 '16 20:04 jrmash

@zafarkhaja could you tell me the status on this?

Any help required?

maxleiko avatar Apr 26 '17 08:04 maxleiko

Closing this issue in favor of #70, the progress can be followed from there.

zafarkhaja avatar Jan 14 '24 14:01 zafarkhaja