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

Upgrade npm to version 5.0.3 (take 2) #8835

Merged
merged 23 commits into from Jun 27, 2017

Conversation

Projects
None yet
4 participants
@benjamn
Member

benjamn commented Jun 22, 2017

This PR takes the place of #8831, which was prematurely merged and closed by @benjamn.

This is a major update from npm 4.6.1, and should bring significant benefits in terms of speed and reproducibility of npm installs. Read the blog post for an introduction to those benefits.

To understand npm's new package-lock.json file format, I would highly recommend reading this documentation. Note especially that npm-shrinkwrap.json files are exactly the same as package-lock.json files, except that package-lock.json files cannot be published to npm.

For backwards compatibility, Meteor still works with npm-shrinkwrap.json files internally (for example, to manage Npm.depends-style dependencies), but that's perfectly fine because npm-shrinkwrap.json files have the same meaning as package-lock.json files. The distinction about publishing is moot because Meteor packages are not published to npm.

For application developers using meteor npm in their own apps, this update is a great opportunity to revisit your workflows to see if you can make better use of the new npm version. In particular, if you've been using yarn (or meteor yarn), you might consider switching back to meteor npm, since package-lock.json files are directly inspired by yarn.lock files.

Meteor remains completely agnostic about how you populate the node_modules directory in your application, so you shouldn't have to change your workflows at all if you don't want to. However, this new version of npm is a pretty big leap forward, so with any luck it will make your dependency management a little bit faster and more reliable.

benjamn and others added some commits Jun 21, 2017

Bump $BUNDLE_VERSION to 8.1.3 before rebuilding dev bundle.
Note that this version no longer corresponds exactly to the current Node
version, which is perfectly fine, yet ever so slightly disappointing.
Stop clearing the npm cache unnecessarily before tests.
Based on this warning:

npm ERR! As of npm@5, the npm cache self-heals from corruption issues and
npm ERR! data extracted from the cache is guaranteed to be valid. If you
npm ERR! want to make sure everything is consistent, use 'npm cache
npm ERR! verify' instead.
npm ERR!
npm ERR! If you're sure you want to delete the entire cache, rerun this
npm ERR! command with --force.
Don't put `node-gyp` into a tiered `node_modules` on Windows dev bundle.
This was preventing `node-gyp` from installing the Node header files on
Windows and was the reason that `minimatch` was not being found, as seen
in #8831.

The `minimatch` module was present, but it was just in `dev_bundle/lib`,
not in `dev_bundle/lib/node_modules/npm/node_modules`.

This expecation may have been expected from older versions of npm but is
no longer the case.  This replicates the behavior of the Unix
`generate_dev_bundle.sh` script, which also does not nest `node-gyp`.

/cc @benjamn
Account for different error messages in npm v5.
Much in the same way as these were changed from v3 to v4, they have been
changed again!

@benjamn benjamn added this to the Release 1.6 milestone Jun 22, 2017

@benjamn benjamn self-assigned this Jun 22, 2017

@benjamn benjamn requested review from hwillson and abernix Jun 22, 2017

Fix modules tests broken by the upgrade to npm 5.0.3.
The new version of npm no longer tolerates stray packages in node_modules
that are not mentioned in package.json, such as node_modules/repl.
@hwillson

Déjà vu - looks good! 🙂

@abernix

Lgtm! The tests that were failing before now pass—granted, it looks as if another unrelated (hot CSS push) will still fail.

@benjamn benjamn force-pushed the upgrade-to-npm-5 branch from 08f9bd6 to 0ca9794 Jun 22, 2017

@benjamn benjamn force-pushed the upgrade-to-npm-5 branch 2 times, most recently from 3405547 to fd6e0f4 Jun 22, 2017

benjamn added some commits Jun 22, 2017

Use URL versions for npm packages installed from URLs.
If a package has a semantic (x.y.z) version in npm-shrinkwrap.json, npm
appears to install it always from the npm registry, rather than the
original tarball URL (uncommon but used by the less and stylus Meteor
packages, among others).

@benjamn benjamn force-pushed the upgrade-to-npm-5 branch 2 times, most recently from ef1f026 to 214a972 Jun 23, 2017

@benjamn

This comment has been minimized.

Member

benjamn commented Jun 23, 2017

I have to admit I'm pretty baffled by the persistence of these test failures, as I can't imagine why the changes in this PR would affect the timing of those particular self-tests.

@hexagon6

This comment has been minimized.

hexagon6 commented Jun 25, 2017

@benjamn duplicate extension test of circleci Build 4351 (container 1) seems to be failing because of this line:

=> Client modified -- refreshing% but expects

this line 'Modified -- restarting'

1| => Client modified -- refreshing%
compiler-plugins: compiler plugin caching - local plugin ...
... ok (54854 ms)
compiler-plugins: compiler plugins - duplicate extension ...
... fail!
... retrying (2 tries remaining) ...
... fail!
... retrying (1 try remaining) ...
... fail!
=> match-timeout at ../../../tools/tests/compiler-plugins.js:294

I suspect the output of the Sandbox has changed in the meantime? I was not able to test this so I'm guessing and would expect changing the test matching line

run.match('Modified -- restarting');

To

run.match('Client modified -- refreshing');

Might not fail the test. But I guess you might know better :-)

@hexagon6

This comment has been minimized.

hexagon6 commented Jun 25, 2017

@benjamn @abernix While looking at the css hot code push test I saw this:

    // XXX: Remove me.  This shouldn't be needed, but sometimes
    // if we run too quickly on fast (or Linux?) machines, it looks
    // like there's a race and we see a weird state
utils.sleepMs(10000);

Might this be related to the failed tests we are seeing?

benjamn added some commits Jun 26, 2017

@benjamn

This comment has been minimized.

Member

benjamn commented Jun 27, 2017

@hexagon6 Thanks for looking into the test failures. As you can see, often the timing of tests, and the specific order of output, are the root causes of the failures (especially the sporadic ones). That utils.sleepMs(10000) call bothers me, too!

@benjamn benjamn merged commit cfc2c25 into release-1.6 Jun 27, 2017

3 of 4 checks passed

ci/circleci Your tests failed on CircleCI
Details
CLA Author has signed the Meteor CLA.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment