zafarkhaja / jsemver

Java implementation of the SemVer Specification
MIT License
429 stars 82 forks source link

Ranges for release candidates fail to parse #24

Closed bripkens closed 5 months ago

bripkens commented 8 years ago

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.

zafarkhaja commented 8 years ago

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 commented 8 years ago

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"))); 
bripkens commented 8 years ago

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

zafarkhaja commented 8 years ago

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

bripkens commented 8 years ago

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
zafarkhaja commented 8 years ago

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.

eric-isakson commented 8 years ago

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.

zafarkhaja commented 8 years ago

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

jrmash commented 8 years ago

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!

zafarkhaja commented 8 years ago

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

jrmash commented 8 years ago

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

maxleiko commented 7 years ago

@zafarkhaja could you tell me the status on this?

Any help required?

zafarkhaja commented 5 months ago

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