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

Make npm install work again with NPM 5 #30759

Merged
merged 2 commits into from
Jul 17, 2017

Conversation

ivanz
Copy link
Contributor

@ivanz ivanz commented Jul 15, 2017

Two issues fixed:

  1. I think there was a version string mismatch which was causing the mainstream v8-profiler to be installed instead of the vscode electron-compatible fork in the shrinkwrap. This was causing the node-gyp step for v8-profiler package to fail with a build error

  2. Once the above was fixed then the extract of some electron bits started failing. Updating the yauzl package used for unzipping stuff fixed that. I suspect something was happening under the hood that resulted in a version mismatch between extract-zip and yauzl. The future proof fix here will be to update to NPM5 package lock files I guess.

Relates to #30134

ivanz added 2 commits July 15, 2017 11:30
… causing the wrong version of v8-profiler to be installed and the node-gyp build to fail.
…rors. Possibly due to a dependency chain mismatch pulling in an incompatible extract-zip version.
@mention-bot
Copy link

@ivanz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @joaomoreno and @bpasero to be potential reviewers.

@jrieken
Copy link
Member

jrieken commented Jul 17, 2017

Thanks!

@jrieken jrieken merged commit 5377a84 into microsoft:master Jul 17, 2017
@joaomoreno
Copy link
Member

This completely broke the shared process somehow. Didn't look into why. Reverted it: #30923

cc @sandy081 @jrieken

@joaomoreno
Copy link
Member

joaomoreno commented Jul 18, 2017

NPM5 support: #30134

@ghost
Copy link

ghost commented Jul 22, 2017

I'm running npm 5.3.0 same error after your merge @joaomoreno

$ ./scripts/npm.sh install --arch=x64

> [email protected] preinstall /home/grey/workspace/vscode
> node build/npm/preinstall.js

npm WARN read-shrinkwrap This version of npm is compatible with lockfileVersion@1, but npm-shrinkwrap.json was generated for lockfileVersion@0. I'll try to do my best with it!
npm WARN deprecated [email protected]: Use uuid module instead
npm WARN deprecated [email protected]: Jade has been renamed to pug, please install the latest version of pug instead of jade
npm WARN deprecated [email protected]: to-iso-string has been deprecated, use @segment/to-iso-string instead.

> [email protected] preinstall /home/grey/workspace/vscode/node_modules/v8-profiler
> node -e 'process.exit(0)'


> [email protected] install /home/grey/workspace/vscode/node_modules/electron-mksnapshot
> node ./download-mksnapshot.js

/home/grey/workspace/vscode/node_modules/extract-zip/index.js:35
      zipfile.readEntry()
              ^

TypeError: zipfile.readEntry is not a function
    at /home/grey/workspace/vscode/node_modules/extract-zip/index.js:35:15
    at /home/grey/workspace/vscode/node_modules/yauzl/index.js:31:7
    at /home/grey/workspace/vscode/node_modules/yauzl/index.js:96:14
    at /home/grey/workspace/vscode/node_modules/yauzl/index.js:342:5
    at /home/grey/workspace/vscode/node_modules/fd-slicer/index.js:32:7
    at FSReqWrap.wrapper [as oncomplete] (fs.js:681:17)
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules/fsevents):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: wanted {"os":"darwin","arch":"any"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules/windows-mutex):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: wanted {"os":"win32","arch":"any"} (current: {"os":"linux","arch":"x64"})
npm WARN optional SKIPPING OPTIONAL DEPENDENCY: [email protected] (node_modules/windows-foreground-love):
npm WARN notsup SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for [email protected]: wanted {"os":"win32","arch":"any"} (current: {"os":"linux","arch":"x64"})

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] install: `node ./download-mksnapshot.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/grey/.electron-gyp/.npm/_logs/2017-07-22T23_46_40_930Z-debug.log

UPDATE:
did npm install [email protected]
then reran script and is worked fine now :)

@ivanz
Copy link
Contributor Author

ivanz commented Jul 23, 2017

I am in holiday atm, but saw that the second change in this PR caused pains/breakage (sorry about that!), so the PR was reverted, which included change 1 for the electron build fix. Basically the version string in package.json doesn't match the shrinkwrap file, so the shrinkwrap doesn't get used and NPM installs the wrong v8-profiler package missing electron compatability patches.

I won't be able to open a PR in the next week, but if someone were to just move the first commit into its own PR - that should do the trick I think.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants