From 97cb2c14a84e88644500c86707d245a48a6d3ad2 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Fri, 1 Apr 2016 17:09:03 +0300 Subject: [PATCH] "meteor npm/node" should return exit code from command Fixes #6673 The Meteor "dev bundle bin commands" which proxy through to the meteor version of npm/node was not returning the exit code from the command which it executed. This creates problems for things like `meteor npm run script-name` when the exit code is important. This comes into play when you run npm scripts which run tests, lint code, etc. This fix causes the meteor-tool to process.exit with the spawned process exit code. Windows Disclaimer: I used the same flush-buffers-on-exit-in-windows that the tool/cli/main.js uses because I would assume the same problem exists, however, I don't have the Windows environment to test or confirm that this code works at all. Also, couldn't find any tests that directly tested this dev bundle bin-command passing scenario (though hard to search through them all), so I created a barebones test app and tests. --- tools/index.js | 16 ++++++++++++- .../.meteor/.gitignore | 1 + .../dev-bundle-bin-commands/.meteor/packages | 8 +++++++ .../dev-bundle-bin-commands/.meteor/release | 1 + .../apps/dev-bundle-bin-commands/package.json | 12 ++++++++++ tools/tests/dev-bundle-bin-commands.js | 24 +++++++++++++++++++ 6 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 tools/tests/apps/dev-bundle-bin-commands/.meteor/.gitignore create mode 100644 tools/tests/apps/dev-bundle-bin-commands/.meteor/packages create mode 100644 tools/tests/apps/dev-bundle-bin-commands/.meteor/release create mode 100644 tools/tests/apps/dev-bundle-bin-commands/package.json create mode 100644 tools/tests/dev-bundle-bin-commands.js diff --git a/tools/index.js b/tools/index.js index 127312bb8d..0547137c72 100644 --- a/tools/index.js +++ b/tools/index.js @@ -1,4 +1,18 @@ -if (! require("./cli/dev-bundle-bin-commands.js").process) { +var spawnBinProcess = require('./cli/dev-bundle-bin-commands.js').process +if (spawnBinProcess) { + // On Node 0.10 on Windows, stdout and stderr don't get flushed when calling + // `process.exit`. We use a workaround for now, but this should be fixed on + // Node 0.12, so when we upgrade let's remember to remove this clause, and the + // file it requires. See https://github.com/joyent/node/issues/3584 + // This same comment and require is in ./cli/main.js + if (process.platform === 'win32') { + require('./tool-env/flush-buffers-on-exit-in-windows.js'); + } + + spawnBinProcess.on('exit', function (exitCode) { + process.exit(exitCode); + }); +} else { // Set up the Babel transpiler require('./tool-env/install-babel.js'); diff --git a/tools/tests/apps/dev-bundle-bin-commands/.meteor/.gitignore b/tools/tests/apps/dev-bundle-bin-commands/.meteor/.gitignore new file mode 100644 index 0000000000..4083037423 --- /dev/null +++ b/tools/tests/apps/dev-bundle-bin-commands/.meteor/.gitignore @@ -0,0 +1 @@ +local diff --git a/tools/tests/apps/dev-bundle-bin-commands/.meteor/packages b/tools/tests/apps/dev-bundle-bin-commands/.meteor/packages new file mode 100644 index 0000000000..df8d730183 --- /dev/null +++ b/tools/tests/apps/dev-bundle-bin-commands/.meteor/packages @@ -0,0 +1,8 @@ +# Meteor packages used by this project, one per line. +# Check this file (and the other files in this directory) into your repository. +# +# 'meteor add' and 'meteor remove' will edit this file for you, +# but you can also edit it by hand. + +meteor-base # Packages every Meteor app needs to have +ecmascript diff --git a/tools/tests/apps/dev-bundle-bin-commands/.meteor/release b/tools/tests/apps/dev-bundle-bin-commands/.meteor/release new file mode 100644 index 0000000000..621e94f0ec --- /dev/null +++ b/tools/tests/apps/dev-bundle-bin-commands/.meteor/release @@ -0,0 +1 @@ +none diff --git a/tools/tests/apps/dev-bundle-bin-commands/package.json b/tools/tests/apps/dev-bundle-bin-commands/package.json new file mode 100644 index 0000000000..ac77436ea2 --- /dev/null +++ b/tools/tests/apps/dev-bundle-bin-commands/package.json @@ -0,0 +1,12 @@ +{ + "name": "dev-bundle-bin-commands", + "private": true, + "scripts": { + "start": "meteor run", + "exit-with-status": "echo \"This script has an exit status\" && exit 1", + "exit-normally": "echo \"This script will exit normally\" && exit 0" + }, + "dependencies": { + "meteor-node-stubs": "~0.2.0" + } +} diff --git a/tools/tests/dev-bundle-bin-commands.js b/tools/tests/dev-bundle-bin-commands.js new file mode 100644 index 0000000000..7680019338 --- /dev/null +++ b/tools/tests/dev-bundle-bin-commands.js @@ -0,0 +1,24 @@ +var selftest = require('../tool-testing/selftest.js'); +var Sandbox = selftest.Sandbox; + +selftest.define("meteor npm run some-script-name - error returns exit status to shell", function () { + var s = new Sandbox(); + var run; + + s.createApp("myapp", "dev-bundle-bin-commands"); + s.cd("myapp"); + run = s.run("npm", "run", "exit-with-status"); + run.matchErr("This script has an exit status"); + run.expectExit(1); +}); + +selftest.define("meteor npm some-script-name - normal exit returns normal to shell", function () { + var s = new Sandbox(); + var run; + + s.createApp("myapp", "dev-bundle-bin-commands"); + s.cd("myapp"); + run = s.run("npm", "run", "exit-normally"); + run.match("This script will exit normally"); + run.expectExit(0); +}); \ No newline at end of file