feat(@angular/cli): update with migrate only creates commit per migration (#15414)

clydin · mgechev · commit 891fe527d74e · 2019-08-26T11:48:43.000-07:00
* feat(@angular/cli): update with migrate only creates commit per migration

* refactor(@angular/cli): simplify update command schematic execution
diff --git a/packages/angular/cli/commands/update-impl.ts b/packages/angular/cli/commands/update-impl.ts
@@ -5,12 +5,17 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
+import { normalize, virtualFs } from '@angular-devkit/core';
+import { NodeJsSyncHost } from '@angular-devkit/core/node';
+import { UnsuccessfulWorkflowExecution } from '@angular-devkit/schematics';
+import { NodeWorkflow, validateOptionsWithSchema } from '@angular-devkit/schematics/tools';
 import { execSync } from 'child_process';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as semver from 'semver';
-import { Arguments, Option } from '../models/interface';
-import { SchematicCommand } from '../models/schematic-command';
+import { Command } from '../models/command';
+import { Arguments } from '../models/interface';
+import { colors } from '../utilities/color';
 import { getPackageManager } from '../utilities/package-manager';
 import {
   PackageIdentifier,
@@ -28,11 +33,156 @@ const npa = require('npm-package-arg');
 
 const oldConfigFileNames = ['.angular-cli.json', 'angular-cli.json'];
 
-export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
+export class UpdateCommand extends Command<UpdateCommandSchema> {
   public readonly allowMissingWorkspace = true;
 
-  async parseArguments(_schematicOptions: string[], _schema: Option[]): Promise<Arguments> {
-    return {};
+  private workflow: NodeWorkflow;
+
+  async initialize() {
+    this.workflow = new NodeWorkflow(
+      new virtualFs.ScopedHost(new NodeJsSyncHost(), normalize(this.workspace.root)),
+      {
+        packageManager: await getPackageManager(this.workspace.root),
+        root: normalize(this.workspace.root),
+      },
+    );
+
+    this.workflow.engineHost.registerOptionsTransform(
+      validateOptionsWithSchema(this.workflow.registry),
+    );
+  }
+
+  async executeSchematic(
+    collection: string,
+    schematic: string,
+    options = {},
+  ): Promise<{ success: boolean; files: Set<string> }> {
+    let error = false;
+    const logs: string[] = [];
+    const files = new Set<string>();
+
+    const reporterSubscription = this.workflow.reporter.subscribe(event => {
+      // Strip leading slash to prevent confusion.
+      const eventPath = event.path.startsWith('/') ? event.path.substr(1) : event.path;
+
+      switch (event.kind) {
+        case 'error':
+          error = true;
+          const desc = event.description == 'alreadyExist' ? 'already exists' : 'does not exist.';
+          this.logger.error(`ERROR! ${eventPath} ${desc}.`);
+          break;
+        case 'update':
+          logs.push(`${colors.whiteBright('UPDATE')} ${eventPath} (${event.content.length} bytes)`);
+          files.add(eventPath);
+          break;
+        case 'create':
+          logs.push(`${colors.green('CREATE')} ${eventPath} (${event.content.length} bytes)`);
+          files.add(eventPath);
+          break;
+        case 'delete':
+          logs.push(`${colors.yellow('DELETE')} ${eventPath}`);
+          files.add(eventPath);
+          break;
+        case 'rename':
+          logs.push(`${colors.blue('RENAME')} ${eventPath} => ${event.to}`);
+          files.add(eventPath);
+          break;
+      }
+    });
+
+    const lifecycleSubscription = this.workflow.lifeCycle.subscribe(event => {
+      if (event.kind == 'end' || event.kind == 'post-tasks-start') {
+        if (!error) {
+          // Output the logging queue, no error happened.
+          logs.forEach(log => this.logger.info(log));
+        }
+      }
+    });
+
+    // TODO: Allow passing a schematic instance directly
+    try {
+      await this.workflow
+        .execute({
+          collection,
+          schematic,
+          options,
+          logger: this.logger,
+        })
+        .toPromise();
+
+      reporterSubscription.unsubscribe();
+      lifecycleSubscription.unsubscribe();
+
+      return { success: !error, files };
+    } catch (e) {
+      if (e instanceof UnsuccessfulWorkflowExecution) {
+        this.logger.error('The update failed. See above.');
+      } else {
+        this.logger.fatal(e.message);
+      }
+
+      return { success: false, files };
+    }
+  }
+
+  async executeMigrations(
+    packageName: string,
+    collectionPath: string,
+    range: semver.Range,
+    commit = false,
+  ) {
+    const collection = this.workflow.engine.createCollection(collectionPath);
+
+    const migrations = [];
+    for (const name of collection.listSchematicNames()) {
+      const schematic = this.workflow.engine.createSchematic(name, collection);
+      const description = schematic.description as typeof schematic.description & {
+        version?: string;
+      };
+      if (!description.version) {
+        continue;
+      }
+
+      if (semver.satisfies(description.version, range, { includePrerelease: true })) {
+        migrations.push(description as typeof schematic.description & { version: string });
+      }
+    }
+
+    if (migrations.length === 0) {
+      return true;
+    }
+
+    const startingGitSha = this.findCurrentGitSha();
+
+    migrations.sort((a, b) => semver.compare(a.version, b.version) || a.name.localeCompare(b.name));
+
+    for (const migration of migrations) {
+      this.logger.info(
+        `** Executing migrations for version ${migration.version} of package '${packageName}' **`,
+      );
+
+      const result = await this.executeSchematic(migration.collection.name, migration.name);
+      if (!result.success) {
+        if (startingGitSha !== null) {
+          const currentGitSha = this.findCurrentGitSha();
+          if (currentGitSha !== startingGitSha) {
+            this.logger.warn(`git HEAD was at ${startingGitSha} before migrations.`);
+          }
+        }
+
+        return false;
+      }
+
+      // Commit migration
+      if (commit) {
+        let message = `migrate workspace for ${packageName}@${migration.version}`;
+        if (migration.description) {
+          message += '\n' + migration.description;
+        }
+        // TODO: Use result.files once package install tasks are accounted
+        this.createCommit(message, []);
+      }
+    }
   }
 
   // tslint:disable-next-line:no-big-function
@@ -112,9 +262,9 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
       this.workspace.configFile &&
       oldConfigFileNames.includes(this.workspace.configFile)
     ) {
-          options.migrateOnly = true;
-          options.from = '1.0.0';
-        }
+      options.migrateOnly = true;
+      options.from = '1.0.0';
+    }
 
     this.logger.info('Collecting installed dependencies...');
 
@@ -125,19 +275,15 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
 
     if (options.all || packages.length === 0) {
       // Either update all packages or show status
-      return this.runSchematic({
-        collectionName: '@schematics/update',
-        schematicName: 'update',
-        dryRun: !!options.dryRun,
-        showNothingDone: false,
-        additionalOptions: {
-          force: options.force || false,
-          next: options.next || false,
-          verbose: options.verbose || false,
-          packageManager,
-          packages: options.all ? Object.keys(rootDependencies) : [],
-        },
+      const { success } = await this.executeSchematic('@schematics/update', 'update', {
+        force: options.force || false,
+        next: options.next || false,
+        verbose: options.verbose || false,
+        packageManager,
+        packages: options.all ? Object.keys(rootDependencies) : [],
       });
+
+      return success ? 0 : 1;
     }
 
     if (options.migrateOnly) {
@@ -153,6 +299,13 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
         return 1;
       }
 
+      const from = coerceVersionNumber(options.from);
+      if (!from) {
+        this.logger.error(`"from" value [${options.from}] is not a valid version.`);
+
+        return 1;
+      }
+
       if (options.next) {
         this.logger.warn('"next" option has no effect when using "migrate-only" option.');
       }
@@ -230,20 +383,18 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
         }
       }
 
-      return this.runSchematic({
-        collectionName: '@schematics/update',
-        schematicName: 'migrate',
-        dryRun: !!options.dryRun,
-        force: false,
-        showNothingDone: false,
-        additionalOptions: {
-          package: packageName,
-          collection: migrations,
-          from: options.from,
-          verbose: options.verbose || false,
-          to: options.to || packageNode.package.version,
-        },
-      });
+      const migrationRange = new semver.Range(
+        '>' + from + ' <=' + (options.to || packageNode.package.version),
+      );
+
+      const result = await this.executeMigrations(
+        packageName,
+        migrations,
+        migrationRange,
+        !options.skipCommits,
+      );
+
+      return result ? 1 : 0;
     }
 
     const requests: {
@@ -287,7 +438,9 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
       try {
         // Metadata requests are internally cached; multiple requests for same name
         // does not result in additional network traffic
-        metadata = await fetchPackageMetadata(packageName, this.logger, { verbose: options.verbose });
+        metadata = await fetchPackageMetadata(packageName, this.logger, {
+          verbose: options.verbose,
+        });
       } catch (e) {
         this.logger.error(`Error fetching metadata for '${packageName}': ` + e.message);
 
@@ -334,18 +487,14 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
       return 0;
     }
 
-    return this.runSchematic({
-      collectionName: '@schematics/update',
-      schematicName: 'update',
-      dryRun: !!options.dryRun,
-      showNothingDone: false,
-      additionalOptions: {
-        verbose: options.verbose || false,
-        force: options.force || false,
-        packageManager,
-        packages: packagesToUpdate,
-      },
+    const { success } = await this.executeSchematic('@schematics/update', 'update', {
+      verbose: options.verbose || false,
+      force: options.force || false,
+      packageManager,
+      packages: packagesToUpdate,
     });
+
+    return success ? 0 : 1;
   }
 
   checkCleanGit() {
@@ -366,9 +515,46 @@ export class UpdateCommand extends SchematicCommand<UpdateCommandSchema> {
           return false;
         }
       }
-
-    } catch { }
+    } catch {}
 
     return true;
   }
+
+  createCommit(message: string, files: string[]) {
+    try {
+      execSync('git add -A ' + files.join(' '), { encoding: 'utf8', stdio: 'pipe' });
+
+      execSync(`git commit --no-verify -m "${message}"`, { encoding: 'utf8', stdio: 'pipe' });
+    } catch (error) {}
+  }
+
+  findCurrentGitSha(): string | null {
+    try {
+      const result = execSync('git rev-parse HEAD', { encoding: 'utf8', stdio: 'pipe' });
+
+      return result.trim();
+    } catch {
+      return null;
+    }
+  }
+}
+
+function coerceVersionNumber(version: string): string | null {
+  if (!version.match(/^\d{1,30}\.\d{1,30}\.\d{1,30}/)) {
+    const match = version.match(/^\d{1,30}(\.\d{1,30})*/);
+
+    if (!match) {
+      return null;
+    }
+
+    if (!match[1]) {
+      version = version.substr(0, match[0].length) + '.0.0' + version.substr(match[0].length);
+    } else if (!match[2]) {
+      version = version.substr(0, match[0].length) + '.0' + version.substr(match[0].length);
+    } else {
+      return null;
+    }
+  }
+
+  return semver.valid(version);
 }
diff --git a/packages/angular/cli/commands/update.json b/packages/angular/cli/commands/update.json
@@ -62,6 +62,12 @@
           "description": "Display additional details about internal operations during execution.",
           "type": "boolean",
           "default": false
+        },
+        "skipCommits": {
+          "description": "Do not create source control commits for updates and migrations.",
+          "type": "boolean",
+          "default": false,
+          "aliases": ["C"]
         }
       }
     }
diff --git a/tests/legacy-cli/e2e/tests/update/update-1.0.ts b/tests/legacy-cli/e2e/tests/update/update-1.0.ts
@@ -10,7 +10,8 @@ export default async function() {
 
   await useCIChrome('.');
   await expectToFail(() => ng('build'));
-  await ng('update', '@angular/cli');
+  // Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
+  await ng('update', '@angular/cli', '-C');
   await useBuiltPackages();
   await silentNpm('install');
   await ng('update', '@angular/core', ...extraUpdateArgs);
diff --git a/tests/legacy-cli/e2e/tests/update/update-1.7-longhand.ts b/tests/legacy-cli/e2e/tests/update/update-1.7-longhand.ts
@@ -9,7 +9,8 @@ export default async function() {
   await createProjectFromAsset('1.7-project');
 
   await expectToFail(() => ng('build'));
-  await ng('update', '@angular/cli', '--migrate-only', '--from=1.7.1');
+  // Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
+  await ng('update', '@angular/cli', '--migrate-only', '--from=1.7.1', '-C');
   await useBuiltPackages();
   await silentNpm('install');
   await ng('update', '@angular/core', ...extraUpdateArgs);
diff --git a/tests/legacy-cli/e2e/tests/update/update-1.7.ts b/tests/legacy-cli/e2e/tests/update/update-1.7.ts
@@ -12,7 +12,8 @@ export default async function() {
 
   await useCIChrome('.');
   await expectToFail(() => ng('build'));
-  await ng('update', '@angular/cli');
+  // Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
+  await ng('update', '@angular/cli', '-C');
   await useBuiltPackages();
   await silentNpm('install');
   await ng('update', '@angular/core', ...extraUpdateArgs);

Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,12 @@`
`62`	`62`	`"description": "Display additional details about internal operations during execution.",`
`63`	`63`	`"type": "boolean",`
`64`	`64`	`"default": false`
	`65`	`+ },`
	`66`	`+ "skipCommits": {`
	`67`	`+ "description": "Do not create source control commits for updates and migrations.",`
	`68`	`+ "type": "boolean",`
	`69`	`+ "default": false,`
	`70`	`+ "aliases": ["C"]`
`65`	`71`	`}`
`66`	`72`	`}`
`67`	`73`	`}`