GlobalScriptAction.ts now invokes the script and correctly returns its exit code

pgonzal · pgonzal · commit 5de315137c4e · 2018-06-08T15:36:20.000-07:00
diff --git a/apps/rush-lib/src/cli/RushCommandLineParser.ts b/apps/rush-lib/src/cli/RushCommandLineParser.ts
@@ -229,6 +229,10 @@ export class RushCommandLineParser extends CommandLineParser {
     // performs nontrivial work that can throw an exception.  Either the Rush class would need
     // to handle reporting for those exceptions, or else _populateActions() should be moved
     // to a RushCommandLineParser lifecycle stage that can handle it.
-    process.exit(1);
+    if (process.exitCode > 0) {
+      process.exit(process.exitCode);
+    } else {
+      process.exit(1);
+    }
   }
 }
diff --git a/apps/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts b/apps/rush-lib/src/cli/scriptActions/GlobalScriptAction.ts
@@ -1,10 +1,13 @@
 // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
 // See LICENSE in the project root for license information.
 
+import * as colors from 'colors';
 import * as fsx from 'fs-extra';
 import * as os from 'os';
 
 import { BaseScriptAction, IBaseScriptActionOptions } from './BaseScriptAction';
+import { Utilities } from '../../utilities/Utilities';
+import { AlreadyReportedError } from '../../../lib/utilities/AlreadyReportedError';
 
 /**
  * Constructor parameters for GlobalScriptAction.
@@ -34,19 +37,40 @@ export class GlobalScriptAction extends BaseScriptAction {
   }
 
   public run(): Promise<void> {
-    if (!fsx.existsSync(this.rushConfiguration.rushLinkJsonFilename)) {
-      throw new Error(`File not found: ${this.rushConfiguration.rushLinkJsonFilename}` +
-        `${os.EOL}Did you run "rush link"?`);
-    }
+    return Promise.resolve().then(() => {
+      if (!fsx.existsSync(this.rushConfiguration.rushLinkJsonFilename)) {
+        throw new Error(`File not found: ${this.rushConfiguration.rushLinkJsonFilename}` +
+          `${os.EOL}Did you run "rush link"?`);
+      }
 
-    // Collect all custom parameter values
-    const customParameterValues: string[] = [];
+      // Collect all custom parameter values
+      const customParameterValues: string[] = [];
 
-    for (const customParameter of this.customParameters) {
-      customParameter.appendToArgList(customParameterValues);
-    }
+      for (const customParameter of this.customParameters) {
+        customParameter.appendToArgList(customParameterValues);
+      }
 
-    return Promise.resolve();
+      let shellCommand: string = this._scriptPath;
+      if (customParameterValues.length > 0) {
+        shellCommand += ' ' + customParameterValues.join(' ');
+      }
+
+      const exitCode: number = Utilities.executeLifecycleCommand(shellCommand,
+        {
+          workingDirectory: this.rushConfiguration.rushJsonFolder,
+          initCwd: this.rushConfiguration.commonTempFolder,
+          handleOutput: false
+        }
+      );
+
+      if (exitCode > 0) {
+        console.log(os.EOL + colors.red(`The script failed with exit code ${exitCode}`));
+      }
+
+      process.exitCode = exitCode;
+
+      throw new AlreadyReportedError();
+    });
   }
 
   protected onDefineParameters(): void {
diff --git a/apps/rush-lib/src/cli/test/repo/common/config/rush/command-line.json b/apps/rush-lib/src/cli/test/repo/common/config/rush/command-line.json
@@ -12,7 +12,7 @@
       "commandKind": "global",
       "summary": "Deploys the build",
       "description": "Uploads all the built assets to the CDN",
-      "scriptPath": "common/scripts/deploy.js"
+      "scriptPath": "node common/scripts/deploy.js"
     }
   ],
 
diff --git a/apps/rush-lib/src/cli/test/repo/common/scripts/deploy.js b/apps/rush-lib/src/cli/test/repo/common/scripts/deploy.js
@@ -0,0 +1,6 @@
+console.log('Executing common/scripts/deploy.js');
+console.log('ARGV: ' + JSON.stringify(process.argv));
+console.log('CWD: ' + process.cwd());
+console.log('NPM_INITCWD: ' + process.env['INIT_CWD']);
+
+process.exit(123)

Original file line number	Diff line number	Diff line change
`@@ -229,6 +229,10 @@ export class RushCommandLineParser extends CommandLineParser {`
`229`	`229`	`// performs nontrivial work that can throw an exception. Either the Rush class would need`
`230`	`230`	`// to handle reporting for those exceptions, or else _populateActions() should be moved`
`231`	`231`	`// to a RushCommandLineParser lifecycle stage that can handle it.`
`232`		`- process.exit(1);`
	`232`	`+ if (process.exitCode > 0) {`
	`233`	`+ process.exit(process.exitCode);`
	`234`	`+ } else {`
	`235`	`+ process.exit(1);`
	`236`	`+ }`
`233`	`237`	`}`
`234`	`238`	`}`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@`
`12`	`12`	`"commandKind": "global",`
`13`	`13`	`"summary": "Deploys the build",`
`14`	`14`	`"description": "Uploads all the built assets to the CDN",`
`15`		`- "scriptPath": "common/scripts/deploy.js"`
	`15`	`+ "scriptPath": "node common/scripts/deploy.js"`
`16`	`16`	`}`
`17`	`17`	`],`
`18`	`18`