Skip to content

Commit

Permalink
fix(@angular/cli): Prints out when a commit is made in ng update.
Browse files Browse the repository at this point in the history
Fixes #16060.

Any time a `git commit` is made, the CLI now prints out the hash and short message. For migrations, the message is simply the first line of the commit. For schematics, the commit message isn't all that helpful, so I used the list of packages instead.
  • Loading branch information
dgp1130 authored and mgechev committed Nov 12, 2019
1 parent 31127eb commit 6c1ab62
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 31 deletions.
131 changes: 106 additions & 25 deletions packages/angular/cli/commands/update-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}
}

/**
* @return Whether or not the migrations were performed successfully.
*/
private async executeMigrations(
packageName: string,
collectionPath: string,
Expand Down Expand Up @@ -183,17 +186,22 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
return false;
}

this.logger.info(colors.green(`${colors.symbols.check} Migration succeeded.`));

// Commit migration
if (commit) {
let message = `${packageName} migration - ${migration.name}`;
if (migration.description) {
message += '\n' + migration.description;
const commitPrefix = `${packageName} migration - ${migration.name}`;
const commitMessage = migration.description
? `${commitPrefix}\n${migration.description}`
: commitPrefix;
const committed = this.commit(commitMessage);
if (!committed) {
// Failed to commit, something went wrong. Abort the update.
return false;
}
// TODO: Use result.files once package install tasks are accounted
this.createCommit(message, []);
}

this.logger.info(colors.green(`${colors.symbols.check} Migration succeeded.\n`));
this.logger.info(''); // Extra trailing newline.
}

return true;
Expand Down Expand Up @@ -556,7 +564,11 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
});

if (success && options.createCommits) {
this.createCommit('Angular CLI update\n' + packagesToUpdate.join('\n'), []);
const committed = this.commit(
`Angular CLI update for packages - ${packagesToUpdate.join(', ')}`);
if (!committed) {
return 1;
}
}

// This is a temporary workaround to allow data to be passed back from the update schematic
Expand Down Expand Up @@ -590,6 +602,52 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
return success ? 0 : 1;
}

/**
* @return Whether or not the commit was successful.
*/
private commit(message: string): boolean {
// Check if a commit is needed.
let commitNeeded: boolean;
try {
commitNeeded = hasChangesToCommit();
} catch (err) {
this.logger.error(` Failed to read Git tree:\n${err.stderr}`);

return false;
}

if (!commitNeeded) {
this.logger.info(' No changes to commit after migration.');

return true;
}

// Commit changes and abort on error.
try {
createCommit(message);
} catch (err) {
this.logger.error(
`Failed to commit update (${message}):\n${err.stderr}`);

return false;
}

// Notify user of the commit.
const hash = findCurrentGitSha();
const shortMessage = message.split('\n')[0];
if (hash) {
this.logger.info(` Committed migration step (${getShortHash(hash)}): ${
shortMessage}.`);
} else {
// Commit was successful, but reading the hash was not. Something weird happened,
// but nothing that would stop the update. Just log the weirdness and continue.
this.logger.info(` Committed migration step: ${shortMessage}.`);
this.logger.warn(' Failed to look up hash of most recent commit, continuing anyways.');
}

return true;
}

private checkCleanGit(): boolean {
try {
const topLevel = execSync('git rev-parse --show-toplevel', { encoding: 'utf8', stdio: 'pipe' });
Expand All @@ -614,24 +672,6 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
return true;
}

private 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) {}
}

private findCurrentGitSha(): string | null {
try {
const result = execSync('git rev-parse HEAD', { encoding: 'utf8', stdio: 'pipe' });

return result.trim();
} catch {
return null;
}
}

/**
* Checks if the current installed CLI version is older than the latest version.
* @returns `true` when the installed version is older.
Expand All @@ -652,6 +692,47 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
}
}

/**
* @return Whether or not the working directory has Git changes to commit.
*/
function hasChangesToCommit(): boolean {
// List all modified files not covered by .gitignore.
const files = execSync('git ls-files -m -d -o --exclude-standard').toString();

// If any files are returned, then there must be something to commit.
return files !== '';
}

/**
* Precondition: Must have pending changes to commit, they do not need to be staged.
* Postcondition: The Git working tree is committed and the repo is clean.
* @param message The commit message to use.
*/
function createCommit(message: string) {
// Stage entire working tree for commit.
execSync('git add -A', { encoding: 'utf8', stdio: 'pipe' });

// Commit with the message passed via stdin to avoid bash escaping issues.
execSync('git commit --no-verify -F -', { encoding: 'utf8', stdio: 'pipe', input: message });
}

/**
* @return The Git SHA hash of the HEAD commit. Returns null if unable to retrieve the hash.
*/
function findCurrentGitSha(): string | null {
try {
const hash = execSync('git rev-parse HEAD', {encoding: 'utf8', stdio: 'pipe'});

return hash.trim();
} catch {
return null;
}
}

function getShortHash(commitHash: string): string {
return commitHash.slice(0, 9);
}

function coerceVersionNumber(version: string | undefined): string | null {
if (!version) {
return null;
Expand Down
3 changes: 1 addition & 2 deletions tests/legacy-cli/e2e/tests/update/update-1.0.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ export default async function() {

await useCIChrome('.');
await expectToFail(() => ng('build'));
// Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
await ng('update', '@angular/cli', '-C');
await ng('update', '@angular/cli');
await useBuiltPackages();
await silentNpm('install');
await ng('update', '@angular/core', ...extraUpdateArgs);
Expand Down
3 changes: 1 addition & 2 deletions tests/legacy-cli/e2e/tests/update/update-1.7-longhand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ export default async function() {
await createProjectFromAsset('1.7-project');

await expectToFail(() => ng('build'));
// 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 ng('update', '@angular/cli', '--migrate-only', '--from=1.7.1');
await useBuiltPackages();
await silentNpm('install');
await ng('update', '@angular/core', ...extraUpdateArgs);
Expand Down
3 changes: 1 addition & 2 deletions tests/legacy-cli/e2e/tests/update/update-1.7.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ export default async function() {

await useCIChrome('.');
await expectToFail(() => ng('build'));
// Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
await ng('update', '@angular/cli', '-C');
await ng('update', '@angular/cli');
await useBuiltPackages();
await silentNpm('install');
await ng('update', '@angular/core', ...extraUpdateArgs);
Expand Down

0 comments on commit 6c1ab62

Please sign in to comment.