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

SemVer 2.0.0 support #2422

Closed
ghost opened this issue Nov 14, 2013 · 19 comments
Closed

SemVer 2.0.0 support #2422

ghost opened this issue Nov 14, 2013 · 19 comments

Comments

@ghost
Copy link

ghost commented Nov 14, 2013

Any chance we'll see support for SemVer 2.0.0, especially build numbers soon?

@Seldaek
Copy link
Member

Seldaek commented Nov 14, 2013

Yeah it has been reported recently and should indeed be fixed, if you would like to help there is more info on the issue I linked above. the VersionParser class should be updated to just ignore them I think according to the spec.

@stof
Copy link
Contributor

stof commented Nov 14, 2013

the comparison of pre-release versions should also be checked to verify it adheres to semver. I'm not sure it is the case currently

@Seldaek
Copy link
Member

Seldaek commented Nov 14, 2013

@RobertKosten did you just delete your last comment or is github acting weird? I can't see it here. But anyway yes v prefix is already ignored. As for the rest, I'm not too likely to accept using an external lib unless it's as fast or faster than the code we have. Comparisons especially have to be fast and having the code inlined here is probably sensible. That said if we have any blatant semver violations we should try and fix them as long as we don't break BC too much by doing so, but I think we are fairly compliant for most simple cases at least.

@ghost
Copy link
Author

ghost commented Nov 14, 2013

Github acting weird, but y browser cache is my friend ;-)

Yup, build numbers are to be ignored (a shame, if you ask me, but that's the spec and it's even a "MUST NOT"), the other interesting change is the removal of the optional "v" prefix, which quite a few people use in their tags, so I would suggest simply ignoring it. I'll have a look at the pre-release precedence bug mentioned next week and try to fix it.

A couple months ago I had actually started to develop a stand-alone semver parser for 1.0.0 and 2.0.0 (yeah, I'm the kind of person who enjoys reading RFCs lying at the beach), if you're interested I could revive that small project sometime mid-December and provide a patch once it's stable. That way it'd become one less worry for composer.

@ghost
Copy link
Author

ghost commented Nov 14, 2013

It's not exactly a complicated spec, so blatantly violating it is pretty hard ;-)

I'll have a look at the current VersionParser next week (Renovating new apartment this week, so really no time to do it earlier) and see whether I can bring it up to 2.0.0, without breaking BC of course.

@Seldaek
Copy link
Member

Seldaek commented Nov 14, 2013

Ok cool. No rush on my end. The build thing should be easy enough, I didn't
read the 2.0 spec in details yet so I'm not sure if there is anything else
to do.

@ghost
Copy link
Author

ghost commented Nov 14, 2013

Looking at the VersionParser there may be a problem with pre-release versions: The spec allows pretty arbitrary strings and the code looks like it's limited to a set ("alpha", "beta", etc.), I'll have to verify.

Another thing may be stability. According to the spec both major version 0.x.z and all pre-release versions re to be considered unstable...

I'll go through things thoroughly and will include explanations and unit test with all changes so you can see whether you want them :-)

@stof
Copy link
Contributor

stof commented Nov 14, 2013

yeah, for the build thing, it is probably just a matter of tweaking a bit the normalization in it. It does not affect other parts.

For the pre-release stuff, I think 2.0-alpha.1.2.beta should be supported according to the spec (this example is insane, I know. an example coming from the spec itself is 1.0.0-alpha.beta).
These cases may not be parsed properly, and the comparison would then need to be checked. But they should be quite uncommon though.
there is another difference on the comparison of pre-release versions too, but I'm not sure we want to break BC there by being stricter to match the spec.

Composer: 1.0-beta1 === 1.0.beta.1 < 1.0-beta11 === 1.0-beta.11 < 1.0-beta2 === 1.0.beta.2
Semver: 1.0.beta.1 < 1.0.beta.2 < 1.0-beta.11 < 1.0-beta1 < 1.0-beta11 < 1.0-beta2

this is because Semver explodes the pre-release version only on dots and then compare segment by segment, considering that a segment which is not numeric should be match alphabetically (hence beta < beta1 < beta11 < beta2)

@stof
Copy link
Contributor

stof commented Nov 14, 2013

@RobertKosten indeed, composer does not consider 0.x as unstable (pre-release version are considered as unstable already).
However, I think a 0.x release should still be considered as stable in composer (even if the user should not consider the API as stable), otherwise it would be far to complex to install pre-1.0 versions with composer because of the minimum-stability setting.

@ghost
Copy link
Author

ghost commented Nov 14, 2013

I agree that 0.x.z should probably still be considered stable, as the stability concepts used by composer and semver are slightly different, though maybe a new level "strict" could be added for spec-nuts like me...

for the pre-release precedence I'd want to argue for getting closer to the spec, even if it does change behaviour, as the current behaviour feels more arbitrary to me.

@glenjamin
Copy link

While working on #2489 I noticed that in my build OPcache gives version "7.0.2FE", which currently get normalised to "0" due to the strict pre-release version checking done by composer.

I was going to add arbitrary suffix support, but due to the normalisation composer does, this is a bit tricky.

Do you have any guidance on the suggested approach? I can probably knock up a PR (either separately or as part of #2489)

My current suggestion would be:

  • Continue to normalise version specifiers
  • Also normalise unrecognised suffixes in the same way
  • Compare lexically, as in the current behaviour
  • Treat unrecognised suffixes as "stable"

I think it's unlikely that a project would mix unrecognised suffixes with standard ones in a way that would break against this approach.

@Seldaek
Copy link
Member

Seldaek commented Dec 16, 2013

@glenjamin I'm not sure what your suggestion would exactly entail in terms of code and matching performance (this is kinda important). I think though that it's worth reporting this as a bug on opcache because it'd be great if the php core stuff would stop using random garbage as versions.

@glenjamin
Copy link

@Seldaek I think performance should not be adversely affected, but I'd have to test it out.

It could be handled by keeping the fixed list of normalised strings in the regex, but also including "|[0-9a-z-]" on the end.

Semver 1.0 and 2.0 both specify that the "pre-release" version MAY be denoted by a dot separated list of arbitrary strings, so it seems restrictive to me for composer to not allow this.

A pre-release version MAY be denoted by appending a hyphen and a series of dot separated identifiers immediately following the patch version. Identifiers MUST comprise only ASCII alphanumerics and hyphen [0-9A-Za-z-]. Identifiers MUST NOT be empty.

@thers
Copy link

thers commented Apr 15, 2014

Any updates here? Semver 2.0 allows versions like 2.2.0-dev+20140414 but composer throws an UnexpectedValueException "Invalid version string"

@asgrim
Copy link
Contributor

asgrim commented Apr 15, 2014

+1 this affects us - we have a feature browscap/browscap#27 we'd like to implement which depends on build metadata in the version, e.g. 1.0.0+5027.

@Mihailoff
Copy link

Such notation 2.4.0-stable.2014061800 is accepted in Composer version e77435c 2014-07-02 15:44:54

@asgrim
Copy link
Contributor

asgrim commented Jul 10, 2014

This is actually called "precedence" and is part of SemVer 2.0.0, but a
full implementation should include "build metadata" which is denoted by a
plus, followed by arbitrary characters in [a-zA-Z0-9-].

Precedence should typically be just a number, rather than a date. Examples
of each:

1.0.0-stable.1
1.0.0-stable.2
1.0.0-stable.1+20140709
1.0.0-stable.1+5034

Note the format allows both:

major.minor.patch-stability.precedence+metadata

Metadata should be ignored when comparing versions (i.e. determining
precedence) but if package/a requires package/b version 1.0.0+5034 then it
should not be given 1.0.0+5033 as I think they should be distinct versions.
On 10 Jul 2014 17:30, "George Mihailov" notifications@github.com wrote:

Such notation 2.4.0-stable.2014061800 is accepted in Composer version e77435c
e77435c
2014-07-02 15:44:54

Reply to this email directly or view it on GitHub
#2422 (comment).

@Mihailoff
Copy link

In my particular case the "date" 2014061800 is plugin version but not a build number. Major/minor/patch are taken from core release version. So this is kind of semver mimic and attempt to take some advantages of composer version definitions like ~2.4@stable.

@asgrim
Copy link
Contributor

asgrim commented Jul 10, 2014

Yeah... That's not SemVer ;) at a push you could claim the "date" was a
precedence, but otherwise, it doesn't conform to the SemVer standard - have
a read of http://semver.org/
On 10 Jul 2014 21:11, "George Mihailov" notifications@github.com wrote:

In my particular case the "date" 2014061800 is plugin version but not a
build number. Major/minor/patch are taken from core release version. So
this is kind of semver mimic and attempt to take some advantages of
composer version definitions like ~2.4@stable.

Reply to this email directly or view it on GitHub
#2422 (comment).

@Seldaek Seldaek closed this as completed in 92f4c1f Dec 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants