Merge pull request microsoft#1283 from microsoft/octogonz/rush-issue1282

octogonz · web-flow · commit 2f707b9265d8 · 2019-05-26T11:37:18.000-07:00
[rush] Fix an issue where "rush build" ignored changes for projects with an empty build script
diff --git a/apps/rush-lib/src/logic/taskRunner/ITask.ts b/apps/rush-lib/src/logic/taskRunner/ITask.ts
@@ -21,6 +21,12 @@ export interface ITaskDefinition {
    */
   isIncrementalBuildAllowed: boolean;
 
+  /**
+   * Assigned by execute().  True if the build script was an empty string.  Operationally an empty string is
+   * like a shell command that succeeds instantly, but e.g. it would be odd to report build time statistics for it.
+   */
+  hadEmptyScript: boolean;
+
   /**
    * Method to be executed for the task.
    */
diff --git a/apps/rush-lib/src/logic/taskRunner/ProjectTask.ts b/apps/rush-lib/src/logic/taskRunner/ProjectTask.ts
@@ -42,6 +42,7 @@ export class ProjectTask implements ITaskDefinition {
   }
 
   public isIncrementalBuildAllowed: boolean;
+  public hadEmptyScript: boolean = false;
 
   private _hasWarningOrError: boolean;
   private _rushProject: RushConfigurationProject;
@@ -64,6 +65,9 @@ export class ProjectTask implements ITaskDefinition {
   public execute(writer: ITaskWriter): Promise<TaskStatus> {
     try {
       const taskCommand: string = this._getScriptToRun();
+      if (!taskCommand) {
+        this.hadEmptyScript = true;
+      }
       const deps: IPackageDependencies | undefined = this._getPackageDependencies(taskCommand, writer);
       return this._executeTask(taskCommand, writer, deps);
     } catch (error) {
@@ -128,9 +132,15 @@ export class ProjectTask implements ITaskDefinition {
         FileSystem.deleteFile(currentDepsPath);
 
         if (!taskCommand) {
-          // tslint:disable-next-line:max-line-length
-          writer.writeLine(`The task command ${this._commandToRun} was registered in the package.json but is blank, so no action will be taken.`);
-          return Promise.resolve(TaskStatus.Skipped);
+          writer.writeLine(`The task command ${this._commandToRun} was registered in the package.json but is blank,`
+            + ` so no action will be taken.`);
+
+          // Write deps on success.
+          if (currentPackageDeps) {
+            JsonFile.save(currentPackageDeps, currentDepsPath);
+          }
+
+          return Promise.resolve(TaskStatus.Success);
         }
 
         // Run the task
@@ -166,7 +176,7 @@ export class ProjectTask implements ITaskDefinition {
 
         return new Promise((resolve: (status: TaskStatus) => void, reject: (error: TaskError) => void) => {
           task.on('close', (code: number) => {
-              this._writeLogsToDisk(writer);
+            this._writeLogsToDisk(writer);
 
             if (code !== 0) {
               reject(new TaskError('error', `Returned error code: ${code}`));
diff --git a/apps/rush-lib/src/logic/taskRunner/TaskRunner.ts b/apps/rush-lib/src/logic/taskRunner/TaskRunner.ts
@@ -272,8 +272,13 @@ export class TaskRunner {
    * Marks a task as being completed, and removes it from the dependencies list of all its dependents
    */
   private _markTaskAsSuccess(task: ITask): void {
-    this._terminal.writeLine(Colors.green(`${this._getCurrentCompletedTaskString()}`
+    if (task.hadEmptyScript) {
+      this._terminal.writeLine(Colors.green(`${this._getCurrentCompletedTaskString()}`
+      + `[${task.name}] had an empty script`));
+    } else {
+      this._terminal.writeLine(Colors.green(`${this._getCurrentCompletedTaskString()}`
       + `[${task.name}] completed successfully in ${task.stopwatch.toString()}`));
+    }
     task.status = TaskStatus.Success;
 
     task.dependents.forEach((dependent: ITask) => {
@@ -417,7 +422,7 @@ export class TaskRunner {
           case TaskStatus.SuccessWithWarning:
           case TaskStatus.Blocked:
           case TaskStatus.Failure:
-            if (task.stopwatch) {
+            if (task.stopwatch && !task.hadEmptyScript) {
               const time: string = task.stopwatch.toString();
               this._terminal.writeLine(headingColor(`${task.name} (${time})`));
             } else {
diff --git a/apps/rush-lib/src/logic/taskRunner/test/TaskRunner.test.ts b/apps/rush-lib/src/logic/taskRunner/test/TaskRunner.test.ts
@@ -14,7 +14,8 @@ function createDummyTask(name: string, action?: () => void): ITaskDefinition {
         action();
       }
       return Promise.resolve(TaskStatus.Success);
-    }
+    },
+    hadEmptyScript: false
   };
 }
 
@@ -95,7 +96,8 @@ describe('TaskRunner', () => {
           writer.write('Build step 1' + EOL);
           writer.writeError('Error: step 1 failed' + EOL);
           return Promise.resolve(TaskStatus.Failure);
-        }
+        },
+        hadEmptyScript: false
       });
       return taskRunner
         .execute()
@@ -117,7 +119,8 @@ describe('TaskRunner', () => {
           writer.write('Build step 1' + EOL);
           writer.write('Error: step 1 failed' + EOL);
           return Promise.resolve(TaskStatus.Failure);
-        }
+        },
+        hadEmptyScript: false
       });
       return taskRunner
         .execute()
@@ -139,7 +142,8 @@ describe('TaskRunner', () => {
             writer.write(` - unit #${i};${EOL}`);
           }
           return Promise.resolve(TaskStatus.Failure);
-        }
+        },
+        hadEmptyScript: false
       });
       return taskRunner
         .execute()
@@ -162,7 +166,8 @@ describe('TaskRunner', () => {
             writer.writeError(` - error #${i};  ${EOL}`);
           }
           return Promise.resolve(TaskStatus.Failure);
-        }
+        },
+        hadEmptyScript: false
       });
       return taskRunner
         .execute()
diff --git a/common/changes/@microsoft/rush/octogonz-rush-issue1282_2019-05-19-19-55.json b/common/changes/@microsoft/rush/octogonz-rush-issue1282_2019-05-19-19-55.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "Fix an issue where \"rush build\" ignored changes to a project with an empty build script (GitHub #1282)",
+      "packageName": "@microsoft/rush",
+      "type": "none"
+    }
+  ],
+  "packageName": "@microsoft/rush",
+  "email": "4673363+octogonz@users.noreply.github.com"
+}
