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

Nightly builds don't work with npm packages that specify minimum node version #2223

Closed
kornelski opened this issue Jul 22, 2015 · 23 comments
Closed
Labels
npm Issues and PRs related to the npm client dependency or the npm registry.

Comments

@kornelski
Copy link

npm i [email protected]
npm ERR! Darwin 14.4.0
npm ERR! argv "/usr/local/bin/iojs" "/usr/local/bin/npm" "i" "[email protected]"
npm ERR! node v2.3.5-nightly20150715f95f9ef6ea
npm ERR! npm v2.12.1
npm ERR! code ENOTSUP

npm ERR! notsup Unsupported
npm ERR! notsup Not compatible with your version of node/npm: [email protected]
npm ERR! notsup Required: {"node":">= 0.10"}
npm ERR! notsup Actual: {"npm":"2.12.1","node":"2.3.5-nightly20150715f95f9ef6ea"}

I guess this is because npm doesn't understand versions such as "2.3.5-nightly20150715f95f9ef6ea". The problem does not occur in iojs release version.

@Fishrock123 Fishrock123 added the npm Issues and PRs related to the npm client dependency or the npm registry. label Jul 22, 2015
@Fishrock123
Copy link
Contributor

cc @isaacs any idea what's up here since you maintain node-semver (which I presume this is coming from)?

@silverwind
Copy link
Contributor

">= 0.10" looks like an invalid semver range, there should be no space. Thought not sure if this is the cause for your issue.

@evanlucas
Copy link
Contributor

Could be related to https://github.com/npm/node-semver#prerelease-tags.

> semver.satisfies('v2.3.5-nightly20150715f95f9ef6ea', '>=0.10')
false
> semver.satisfies('v2.3.5-nightly20150715f95f9ef6ea', '>=1')
false
> semver.satisfies('v2.3.5-nightly20150715f95f9ef6ea', '>=2')
false
> semver.satisfies('v2.3.5-nightly20150715f95f9ef6ea', '>=3')
false
> semver.satisfies('v2.3.5-nightly20150715f95f9ef6ea', '>=4')
false

@targos
Copy link
Member

targos commented Jul 22, 2015

@pornel Have you enabled the engine-strict option in your npm config ? AFAIK without it you should just get a warning.

@silverwind
Copy link
Contributor

Was able to reproduce it, and can confirm that the change from ">= 0.10" to ">=0.10" fixes the issue. It's likely a bug in either npm oder node-semver, or working as intended if the space is not considered valid, but there is at least one occurence in package.json docs where the space is used, so I'm unsure.

@silverwind
Copy link
Contributor

Or rather, scrap that. I was testing the latest release. [email protected] still has an engineStrict in package.json, which will fail unfortunately. @pornel until npm@3 resolves this through removal of engineStrict, you could use this hack from npm/npm#7171:

npm config set node-version 2.3.5

@isaacs
Copy link
Contributor

isaacs commented Jul 23, 2015

Working as designed.

"Pre-release" versions are assumed invalid for meeting ranges unless you specify a prerelease version of the same (major,minor,patch) tuple in the range itself.

This is to prevent users from surprisingly receiving a broken thing without opting into perhaps getting not-fully-baked stuff.

engineStrict in package.json will no longer be an error in npm@3 as @silverwind points out, because that has always been a bit obnoxious and confusing.

Manually setting the node-version in the npm config is totally fine, but make sure to npm config unset node-version later, or you may have some weird behavior!

@rvagg
Copy link
Member

rvagg commented Jul 23, 2015

engineStrict in package.json will no longer be an error in npm@3 as

WOOHOO!

That's all.

@Fishrock123
Copy link
Contributor

This is to prevent users from surprisingly receiving a broken thing without opting into perhaps getting not-fully-baked stuff.

Sure when installing things, but it's a little different for node core which the user would have had to explicitly install, isn't it?

At least it won't error anymore, but doesn't it still log lots of warnings? Seems great for the rest of npm but maybe not so much for "engines".

@silverwind
Copy link
Contributor

Yeah, those warnings are annoying, you usually get a few screens full of them on packages with big dependency trees.

@rvagg
Copy link
Member

rvagg commented Jul 23, 2015

Note also that we're going to be seeing more use of nightlies and RCs as we enable proper node-gyp support for them, that's been one of the biggest hassles.

As long as the errors are gone I'm fine with the errors for now but would expect over time that this is annoying for enough people that it needs to change. I don't believe the current behaviour of semver ranges wrt prerelease tags makes sense when applied Node versions but am happy to let that discussion play out over the long run.

@jkrems
Copy link
Contributor

jkrems commented Sep 6, 2015

This makes testing RC builds of 4.0.0 a lot harder than it would need to be. E.g. we have multiple internal packages that specify >=0.10 in the engine field. It's not quite obvious why they stop installing without spending a decent amount of time digging. Maybe worth a note in the 4.0.0 RC description/announcement, especially since upgrading npm isn't actually an option for those IIRC?

Is the only option right now to run npm config set node-version 4.0.0 after switching to the RC version and to remember to unset it whenever switching node versions?

P.S.: It also seems to be possible to circumvent the engine check via npm i --force but I'm not sure what other side effects that would have.

@rvagg
Copy link
Member

rvagg commented Sep 7, 2015

this is out of our hands except that we'd like to make non-release builds more accessible and more commonly used which will in turn put pressure on npm to do something a bit more reasonable about this

@isaacs
Copy link
Contributor

isaacs commented Sep 7, 2015

Crossposted from krakenjs/lusca#47 (comment)

According to the SemVer specification, prerelease versions are not to be used by general public, and are considered "unstable", carrying a higher than normal risk of unintended breaking changes. In particular, they may be missing features that are expected to exist in the official release, so even very broad ranges are liable to violate expectations if prerelease versions match.

In consideration of this, the semver npm module does not match any version range against a version with a prerelease tag, unless the range specifier itself contains a prerelease of the same tuple, indicating that the consumer is specifically opting into this prerelease version family, and acknowledges the higher level of risk.

When there is an official 4.0.0 release, it will match against >=0.8.0. However, until then, the warnings are doing their job, and alerting the user to a not-yet-blessed version of the platform.

In the fast and furious world of small module authorship, this restriction is important and necessary in practice. However, since Node moves much more deliberately, and is extremely conservative with respect to API change, so the restriction feels overly cautious. Engines are not like normal dependencies.

That is the reason why npm v3 removed the author-specified engineStrict field in package.json; it is virtually impossible for the author of a module to know whether or not their module will work with future versions of Node, and while a warning may be indicated, failing to install makes it overly difficult to even determine if the module works on the new platform prior to official launch!

@jkrems
Copy link
Contributor

jkrems commented Sep 7, 2015

Okay, given node v5 shipping with npm v3 (assumption), this should only be an issue for the next couple of days / at most weeks until 4.0 ships. So as long as 4.0.0-rc.x doesn't become the new 0.11.x, this whole thing should become a non-issue very soon. Until then people testing the RCs should just use --force. @isaacs Does that sound correct?

P.S.: I'm assuming that upgrading to npm v3 isn't an option right now because node-gyp will break.

@cjihrig
Copy link
Contributor

cjihrig commented Sep 7, 2015

Okay, given node v5 shipping with npm v3 (assumption),

If by v5, you mean v4, I believe the plan is still to ship with the 2.x npm line.

@Fishrock123
Copy link
Contributor

@izs this will get interesting once we have NEXTMAJOR.0.0-next.x releases to coincide with v8.

Fwiw, those will be stable enough to use without native modules, and I suspect a good deal of tooling-usage people will want to use them. And we'll probably even like, encourage it, more or less.

I'd personally like if npm wouldn't spew warnings when used with -rc.x and -next.x versions. Again, as said, we don't move like a module. At least the -next.x ones will be effectively blessed in some capacity. I can't seem to find a good reason to check this the same as a module, even if we are using what is essentially semver. (And probably more than most modules use semver hahaha)

cc @domenic too, since I figure he / google probably plan(s) on using these.

@rvagg
Copy link
Member

rvagg commented Sep 7, 2015

This interpretation of the semver spec is taking some liberties, all the spec actually has to say about pre-releases is:

Pre-release versions have a lower precedence than the associated normal version. A pre-release version indicates that the version is unstable and might not satisfy the intended compatibility requirements as denoted by its associated normal version.

The spec is more concerned with precedence than the notion of "unstable" and what it might mean.

So a minor tangent:

"enginesStrict" being removed is a good thing, and the warnings don't bother me so much because it's not stopping anyone from installing. It's the break in functionality that does. node-semver's approach of completely removing pre-releases from semver.satisfies() and other operators like semver.gte() are arguably going against the spec because pre-release versions fit within a very clear chain of precedence, after the previous version but before the current version. This introduces problems like https://github.com/nodejs/node/issues/2719—actual broken functionality, and forces hacks like this (i.e. unable to use semver.satisfies(version, '>=1.0.0 <4.0.0') and having to resort to versionSemver.major >= 1 & versionSemver.major < 4); and you need to know about the sublties of this behavior in order to implement these workarounds, the casual user is going to assume that the operators are working according to semver's precedence specifications, which they are not.

@domenic
Copy link
Contributor

domenic commented Sep 8, 2015

@Fishrock123 we're mostly just disappointed that the up-to-date V8 releases are getting shunted into prerelease space, which as @isaacs points out is not meant for general use. From our point of view it's the outdated ("stable") releases that should not be used, and the "prereleases" (up-to-date versions) should be used exclusively.

@Fishrock123
Copy link
Contributor

From our point of view it's the outdated ("stable") releases that should not be used, and the "prereleases" (up-to-date versions) should be used exclusively.

This is just not how reality works. :/

@rvagg
Copy link
Member

rvagg commented Sep 8, 2015

indeed, give us a stable ABI and we may play to the same tune

@Fishrock123
Copy link
Contributor

Also we could patch node-semver in the version npm with ship with if this causes people too many problems. (I hope this isn't hostile :s)

@mgol
Copy link
Contributor

mgol commented Sep 15, 2015

Also we could patch node-semver in the version npm with ship with if this causes people too many problems. (I hope this isn't hostile :s)

I don't think patching npm in node is a good idea unless necessary (as was before node-gyp started to play nice with io.js etc.). This is just going to be extremely surprising that behavior changes when you update npm, even if you later downgrade to the "same" version that's included in Node.

phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 27, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Nov 28, 2017
phated pushed a commit to gulpjs/vinyl-fs that referenced this issue Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Issues and PRs related to the npm client dependency or the npm registry.
Projects
None yet
Development

No branches or pull requests