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

Fix CI: non-semver compliant Yarn versions #5362

Merged
merged 1 commit into from
Oct 8, 2018
Merged

Conversation

Timer
Copy link
Contributor

@Timer Timer commented Oct 8, 2018

/cc @arcanis

> require('semver').gte('v1.13.0-a20181006.0202', '1.1.0')
TypeError: Invalid Version: v1.13.0-a20181006.0202
    at new SemVer (/Users/joe/Documents/Development/OSS/create-react-app/node_modules/semver/semver.js:305:11)
    at compare (/Users/joe/Documents/Development/OSS/create-react-app/node_modules/semver/semver.js:578:10)
    at Function.gte (/Users/joe/Documents/Development/OSS/create-react-app/node_modules/semver/semver.js:627:10)
> require('semver').gte('v1.13.0-a201810060202', '1.1.0')
true
> require('semver').gte('v1.13.0-a20181006+0202', '1.1.0')
true
> require('semver').gte('v1.13.0-a20181006-0202', '1.1.0')
true

@Timer Timer added this to the 2.0.5 milestone Oct 8, 2018
@Timer Timer merged commit f80d27b into facebook:master Oct 8, 2018
@arcanis
Copy link
Contributor

arcanis commented Oct 8, 2018

cc @Daniel15 maybe we should replace the dot by a dash on nightlies?

@Timer Timer deleted the fix-ci branch October 8, 2018 22:49
@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2018

@arcanis Yarn's current nightly versioning is semver compliant. See section 9 of the semver spec:

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 [...] Examples: 1.0.0-alpha, 1.0.0-alpha.1, 1.0.0-0.3.7, 1.0.0-x.7.z.92.

Date and time are separate identifiers, so they're correctly separated by dots.

@Timer Where does that version number come from? I don't think the Yarn version numbers start with v or have an a in them. It should look like 1.13.0-20181006.0202.

@Timer
Copy link
Contributor Author

Timer commented Oct 9, 2018

@Daniel15 this is coming from running yarnpkg --version

@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2018

@Timer I can't repro:

C:\temp
λ curl https://nightly.yarnpkg.com/latest.js -L -o yarn.js
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100 4365k    0 4365k    0     0  3592k      0 --:--:--  0:00:01 --:--:-- 13.4M

C:\temp
λ node .\yarn.js --version
1.13.0-20181009.1747

Which the semver module says is valid:

λ node
> require('semver').valid('1.13.0-20181009.1747')
'1.13.0-20181009.1747'

@arcanis
Copy link
Contributor

arcanis commented Oct 9, 2018

@Timer is right, the gte function throws an exception even if valid passes 🤷‍♀️

Cf https://runkit.com/arcanis/5bbd1c8c1b4bc50012da3645

@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2018 via email

@arcanis
Copy link
Contributor

arcanis commented Oct 9, 2018

Oooh I found the issue. It's even better than I thought. It's not the . that's at fault, nor the a. It's the 0 that follows the dot. That's why @Timer got the issue: his nightly at the time he made the PR ended with 0202. After he upgraded later on he got 1747, and when you tried to reproduce with this version it worked. Check this:

Works:

console.log(require('semver').gte('1.13.0-20181009.1747', '1.1.0'))

Breaks:

console.log(require('semver').gte('1.13.0-20181006.0202', '1.1.0'))

Similarly, if you check 0202 with valid, you'll get null:

console.log(require('semver').valid('1.13.0-20181006.0202'))

@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2018

Hahahaha what. Is that a bug in the semver package? I don't see why that'd be invalid.

@arcanis
Copy link
Contributor

arcanis commented Oct 9, 2018

Looking at the grammar, it seems invalid. Notice how <numeric identifier> cannot start by 0 unless it's the only digit. Very unintuitive:

<pre-release> ::= <dot-separated pre-release identifiers>

<dot-separated pre-release identifiers> ::= <pre-release identifier>
                                          | <pre-release identifier> "." <dot-separated pre-release identifiers>

<pre-release identifier> ::= <alphanumeric identifier>
                           | <numeric identifier>

<numeric identifier> ::= "0"
                       | <positive digit>
                       | <positive digit> <digits>

<positive digit> ::= "1" | "2" | "3" | "4" | "5" | "6" | "7" | "8" | "9"

@Daniel15
Copy link
Member

Daniel15 commented Oct 9, 2018

Where is that grammar from?

I don't see this documented at https://semver.org/ :/

@arcanis
Copy link
Contributor

arcanis commented Oct 9, 2018

On the github repo: https://github.com/semver/semver/blob/master/semver.md#backusnaur-form-grammar-for-valid-semver-versions

Funnily, if the faulty 0202 was followed by any alpha character, it would become an alphanumeric identifier and would become valid. So we could make it 02h02m for example 😄

@iansu
Copy link
Contributor

iansu commented Oct 10, 2018

Is this maybe because a number that starts with 0 is considered to be octal? That's all I can come up with. 🤷‍♂️

@lock lock bot locked and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants