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

Revert npm3, go back to npm2 #4939

Merged
merged 6 commits into from
Sep 17, 2015
Merged

Revert npm3, go back to npm2 #4939

merged 6 commits into from
Sep 17, 2015

Conversation

rashidkpc
Copy link
Contributor

This reverts #4768
Fixes #4842

It also effectively fixes this, which was caused by the change to NPM3: #4842

@@ -168,6 +168,6 @@
},
"engines": {
"node": "2.5",
"npm": "3.2"
"npm": ">=2"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still matches npm 3, no? We should probably lock in the version we expect, or at least use 2.x

@rashidkpc
Copy link
Contributor Author

This currently passes tests, but doesn't actually build

Error: Cannot find module 'regenerator/runtime'

@epixa epixa assigned epixa and unassigned BigFunger Sep 17, 2015
epixa and others added 2 commits September 17, 2015 16:28
The npm shrinkwrap that we generate during build in order to package
with distributions is now properly created based on the built
dependencies rather than the local dev install.
Create shrinkwrap from build
@epixa epixa assigned spalger and unassigned epixa Sep 17, 2015
@spalger
Copy link
Contributor

spalger commented Sep 17, 2015

For posterity:

npm install -g [email protected]
npm rebuild -g
rm -rf node_modules
npm install

@spalger
Copy link
Contributor

spalger commented Sep 17, 2015

@epixa the build now has a package.json and npm-shrinkwrap.json that match, but the actual node_modules directory does not.

Since we are running the clean:deepModules task after we npm shrinkwrap we invalidate the shrinkwrap file by modifying the node_modules directory.

To fix this I thought maybe npm would let us generate the shrinkwrap file using our modified node_modules directory, but that does not work according to my test:

grunt build
cd target
unzip kibana-4.2.0-snapshot-darwin-x64.zip
cd kibana-4.2.0-snapshot-darwin-x64
rm npm-shrinkwrap.json
npm shrinkwrap

output:
image

@epixa
Copy link
Contributor

epixa commented Sep 17, 2015

Good catch. I'm not sure if there is any practical solution to this using npm 2 at this point. I vote for removing the shrinkwrap from the build entirely. Any objections?

@spalger
Copy link
Contributor

spalger commented Sep 17, 2015

No objection here

We're not maintaining the shrinkwrap as part of the repo, and it doesn't
add any value when being generated automatically as part of our build
process. In its current form, it actually makes it so we cannot
successfully create new builds without manually setting up a local
shrinkwrap first.
@spalger spalger assigned epixa and unassigned spalger Sep 17, 2015
@epixa
Copy link
Contributor

epixa commented Sep 17, 2015

@rashidkpc I issued another PR to this branch to remove the shrinkwrap entirely rashidkpc#6

@epixa epixa assigned rashidkpc and unassigned epixa Sep 17, 2015
@spalger
Copy link
Contributor

spalger commented Sep 17, 2015

Travis had some trouble an hour ago and they say the queue to start new jobs is really backed up. This test was supposed to start 30 minutes ago but since the Jenkins one passed I'm calling it good (the previous failures were all build related, which is Jenkins only)

LGTM

spalger added a commit that referenced this pull request Sep 17, 2015
@spalger spalger merged commit 1e3dd08 into elastic:master Sep 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kibana <-> Plugin dependency sharing
5 participants