From 314c8a1a3464e368aef4ce17172af83e11a52515 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Wed, 30 Apr 2014 11:45:30 -0700 Subject: [PATCH] We no longer need to pass --force to npm install (Also, make a test assertion useful: assert.equal's default truncation is horrible.) --- History.md | 3 +++ tools/meteor-npm.js | 20 ++++++++++---------- tools/tests/old/test-bundler-npm.js | 4 ++-- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/History.md b/History.md index 601bd33f80..5b9ca6c6ed 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,8 @@ ## v.NEXT +* Speed up updates of NPM modules by patching NPM to work around + https://github.com/npm/npm/issues/3265 instead of passing `--force`. + ## v0.8.1 #### Meteor Accounts diff --git a/tools/meteor-npm.js b/tools/meteor-npm.js index d4a9f670f7..2eab72a12e 100644 --- a/tools/meteor-npm.js +++ b/tools/meteor-npm.js @@ -459,17 +459,18 @@ var installNpmModule = function (name, version, dir) { // how to silence all output (specifically the installed tree which // is printed out with `console.log`) // - // We use --force, because the NPM cache is broken! See - // https://github.com/isaacs/npm/issues/3265 Basically, switching + // We used to use --force here, because the NPM cache is broken! See + // https://github.com/npm/npm/issues/3265 Basically, switching // back and forth between a tarball fork of version X and the real - // version X can confuse NPM. But the main reason to use tarball + // version X could confuse NPM. But the main reason to use tarball // URLs is to get a fork of the latest version with some fix, so - // it's easy to trigger this! So instead, always use --force. (Even - // with --force, we still WRITE to the cache, so we can corrupt the - // cache for other invocations of npm... ah well.) + // it was easy to trigger this! + // + // We now use a forked version of npm with our PR + // https://github.com/npm/npm/pull/5137 to work around this. var result = meteorNpm._execFileSync(path.join(files.getDevBundle(), "bin", "npm"), - ["install", "--force", installArg], + ["install", installArg], {cwd: dir}); if (! result.success) { @@ -498,11 +499,10 @@ var installFromShrinkwrap = function (dir) { ensureConnected(); - // `npm install`, which reads npm-shrinkwrap.json. See above for why - // --force. + // `npm install`, which reads npm-shrinkwrap.json. var result = meteorNpm._execFileSync(path.join(files.getDevBundle(), "bin", "npm"), - ["install", "--force"], {cwd: dir}); + ["install"], {cwd: dir}); if (! result.success) { // XXX include this in the buildmessage.error instead diff --git a/tools/tests/old/test-bundler-npm.js b/tools/tests/old/test-bundler-npm.js index bc67ad612c..7300035e15 100644 --- a/tools/tests/old/test-bundler-npm.js +++ b/tools/tests/old/test-bundler-npm.js @@ -84,7 +84,7 @@ var _assertCorrectPackageNpmDir = function (deps) { var expected = JSON.stringify({ dependencies: expectedMeteorNpmShrinkwrapDependencies}, null, /*indentation, the way npm does it*/2) + '\n'; - assert.equal(actual, expected); + assert.equal(actual, expected, actual + " == " + expected); assert.equal( fs.readFileSync(path.join(testPackageDir, ".npm", "package", ".gitignore"), 'utf8'), @@ -203,7 +203,7 @@ var runTest = function () { // just remove all of the .npm directory) var bareExecFileSync = meteorNpm._execFileSync; meteorNpm._execFileSync = function (file, args, opts) { - if (args.length > 2 && args[0] === 'install' && args[1] === '--force') + if (args.length > 1 && args[0] === 'install') assert.fail("shouldn't be installing specific npm packages: " + args[1]); return bareExecFileSync(file, args, opts); };