From f68de59464d0974c1e69fd845f3514bb7c43e558 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 15 Aug 2017 23:01:46 +0300 Subject: [PATCH 1/2] Stop setting `NPM_CONFIG_PREFIX` in `getEnv`. This was causing problems with `npm@5`, but realistically it may not be be necessary anymore since `npm@5` has a much smarter global, self-healing `cacache`. --- tools/cli/dev-bundle-bin-helpers.js | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tools/cli/dev-bundle-bin-helpers.js b/tools/cli/dev-bundle-bin-helpers.js index 5a5bef6c3d..e99f534e60 100644 --- a/tools/cli/dev-bundle-bin-helpers.js +++ b/tools/cli/dev-bundle-bin-helpers.js @@ -86,13 +86,6 @@ exports.getEnv = function (options) { env.NPM_CONFIG_PREFIX = devBundleDir; } - // Make sure we don't try to use the global ~/.npm cache accidentally. - if (! env.NPM_CONFIG_CACHE) { - env.NPM_CONFIG_CACHE = path.join( - // If the user set NPM_CONFIG_PREFIX, respect that. - env.NPM_CONFIG_PREFIX, ".npm"); - } - if (env.METEOR_ALLOW_SUPERUSER) { // Note that env.METEOR_ALLOW_SUPERUSER could be "0" or "false", which // should propagate falsy semantics to NPM_CONFIG_UNSAFE_PERM. From fe9401a5efff92bd47e0acb48c225fcf9ea022e7 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 15 Aug 2017 17:53:39 +0300 Subject: [PATCH 2/2] Observe requirements for running `.cmd` scripts on Windows. The `child_process` documentation indicates that .cmd scripts (such as the `npm.cmd` script for npm) must be started within a shell, such as cmd.exe. While it was tempting to switch this to use `child_process`'s `spawn`, which supports a `shell` option, it would have required us to buffer our own stdout/stderr output via `data` event handlers. Ref: https://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows --- tools/isobuild/meteor-npm.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tools/isobuild/meteor-npm.js b/tools/isobuild/meteor-npm.js index 0ee8f039fd..ffed839430 100644 --- a/tools/isobuild/meteor-npm.js +++ b/tools/isobuild/meteor-npm.js @@ -830,9 +830,22 @@ Profile("meteorNpm.runNpmCommand", function (args, cwd) { isWindows ? "npm.cmd" : "npm" )); + // On Windows, `.cmd` and `.bat` files must be launched in a shell per: + // http://nodejs.org/api/child_process.html#child_process_spawning_bat_and_cmd_files_on_windows + // + // Additionally, the COMSPEC environment variable is meant to have the path to + // cmd.exe, but we'll use 'cmd.exe' if it's not set, in the same spirit as + // http://nodejs.org/api/child_process.html#child_process_shell_requirements. + + let commandToRun = npmPath; + if (isWindows) { + args = ['/c', npmPath, ...args]; + commandToRun = process.env.ComSpec || "cmd.exe"; + } + if (meteorNpm._printNpmCalls) { // only used by test-bundler.js - process.stdout.write('cd ' + cwd + ' && ' + npmPath + ' ' + + process.stdout.write('cd ' + cwd + ' && ' + commandToRun + ' ' + args.join(' ') + ' ...\n'); } @@ -853,7 +866,7 @@ Profile("meteorNpm.runNpmCommand", function (args, cwd) { return new Promise(function (resolve) { require('child_process').execFile( - npmPath, args, opts, function (err, stdout, stderr) { + commandToRun, args, opts, function (err, stdout, stderr) { if (meteorNpm._printNpmCalls) { process.stdout.write(err ? 'failed\n' : 'done\n'); }