Skip to content

Commit 6c1ab62

Browse files
dgp1130mgechev
authored andcommitted
fix(@angular/cli): Prints out when a commit is made in ng update.
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.
1 parent 31127eb commit 6c1ab62

File tree

4 files changed

+109
-31
lines changed

4 files changed

+109
-31
lines changed

packages/angular/cli/commands/update-impl.ts

Lines changed: 106 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,9 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
139139
}
140140
}
141141

142+
/**
143+
* @return Whether or not the migrations were performed successfully.
144+
*/
142145
private async executeMigrations(
143146
packageName: string,
144147
collectionPath: string,
@@ -183,17 +186,22 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
183186
return false;
184187
}
185188

189+
this.logger.info(colors.green(`${colors.symbols.check} Migration succeeded.`));
190+
186191
// Commit migration
187192
if (commit) {
188-
let message = `${packageName} migration - ${migration.name}`;
189-
if (migration.description) {
190-
message += '\n' + migration.description;
193+
const commitPrefix = `${packageName} migration - ${migration.name}`;
194+
const commitMessage = migration.description
195+
? `${commitPrefix}\n${migration.description}`
196+
: commitPrefix;
197+
const committed = this.commit(commitMessage);
198+
if (!committed) {
199+
// Failed to commit, something went wrong. Abort the update.
200+
return false;
191201
}
192-
// TODO: Use result.files once package install tasks are accounted
193-
this.createCommit(message, []);
194202
}
195203

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

199207
return true;
@@ -556,7 +564,11 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
556564
});
557565

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

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

605+
/**
606+
* @return Whether or not the commit was successful.
607+
*/
608+
private commit(message: string): boolean {
609+
// Check if a commit is needed.
610+
let commitNeeded: boolean;
611+
try {
612+
commitNeeded = hasChangesToCommit();
613+
} catch (err) {
614+
this.logger.error(` Failed to read Git tree:\n${err.stderr}`);
615+
616+
return false;
617+
}
618+
619+
if (!commitNeeded) {
620+
this.logger.info(' No changes to commit after migration.');
621+
622+
return true;
623+
}
624+
625+
// Commit changes and abort on error.
626+
try {
627+
createCommit(message);
628+
} catch (err) {
629+
this.logger.error(
630+
`Failed to commit update (${message}):\n${err.stderr}`);
631+
632+
return false;
633+
}
634+
635+
// Notify user of the commit.
636+
const hash = findCurrentGitSha();
637+
const shortMessage = message.split('\n')[0];
638+
if (hash) {
639+
this.logger.info(` Committed migration step (${getShortHash(hash)}): ${
640+
shortMessage}.`);
641+
} else {
642+
// Commit was successful, but reading the hash was not. Something weird happened,
643+
// but nothing that would stop the update. Just log the weirdness and continue.
644+
this.logger.info(` Committed migration step: ${shortMessage}.`);
645+
this.logger.warn(' Failed to look up hash of most recent commit, continuing anyways.');
646+
}
647+
648+
return true;
649+
}
650+
593651
private checkCleanGit(): boolean {
594652
try {
595653
const topLevel = execSync('git rev-parse --show-toplevel', { encoding: 'utf8', stdio: 'pipe' });
@@ -614,24 +672,6 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
614672
return true;
615673
}
616674

617-
private createCommit(message: string, files: string[]) {
618-
try {
619-
execSync('git add -A ' + files.join(' '), { encoding: 'utf8', stdio: 'pipe' });
620-
621-
execSync(`git commit --no-verify -m "${message}"`, { encoding: 'utf8', stdio: 'pipe' });
622-
} catch (error) {}
623-
}
624-
625-
private findCurrentGitSha(): string | null {
626-
try {
627-
const result = execSync('git rev-parse HEAD', { encoding: 'utf8', stdio: 'pipe' });
628-
629-
return result.trim();
630-
} catch {
631-
return null;
632-
}
633-
}
634-
635675
/**
636676
* Checks if the current installed CLI version is older than the latest version.
637677
* @returns `true` when the installed version is older.
@@ -652,6 +692,47 @@ export class UpdateCommand extends Command<UpdateCommandSchema> {
652692
}
653693
}
654694

695+
/**
696+
* @return Whether or not the working directory has Git changes to commit.
697+
*/
698+
function hasChangesToCommit(): boolean {
699+
// List all modified files not covered by .gitignore.
700+
const files = execSync('git ls-files -m -d -o --exclude-standard').toString();
701+
702+
// If any files are returned, then there must be something to commit.
703+
return files !== '';
704+
}
705+
706+
/**
707+
* Precondition: Must have pending changes to commit, they do not need to be staged.
708+
* Postcondition: The Git working tree is committed and the repo is clean.
709+
* @param message The commit message to use.
710+
*/
711+
function createCommit(message: string) {
712+
// Stage entire working tree for commit.
713+
execSync('git add -A', { encoding: 'utf8', stdio: 'pipe' });
714+
715+
// Commit with the message passed via stdin to avoid bash escaping issues.
716+
execSync('git commit --no-verify -F -', { encoding: 'utf8', stdio: 'pipe', input: message });
717+
}
718+
719+
/**
720+
* @return The Git SHA hash of the HEAD commit. Returns null if unable to retrieve the hash.
721+
*/
722+
function findCurrentGitSha(): string | null {
723+
try {
724+
const hash = execSync('git rev-parse HEAD', {encoding: 'utf8', stdio: 'pipe'});
725+
726+
return hash.trim();
727+
} catch {
728+
return null;
729+
}
730+
}
731+
732+
function getShortHash(commitHash: string): string {
733+
return commitHash.slice(0, 9);
734+
}
735+
655736
function coerceVersionNumber(version: string | undefined): string | null {
656737
if (!version) {
657738
return null;

tests/legacy-cli/e2e/tests/update/update-1.0.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ export default async function() {
1010

1111
await useCIChrome('.');
1212
await expectToFail(() => ng('build'));
13-
// Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
14-
await ng('update', '@angular/cli', '-C');
13+
await ng('update', '@angular/cli');
1514
await useBuiltPackages();
1615
await silentNpm('install');
1716
await ng('update', '@angular/core', ...extraUpdateArgs);

tests/legacy-cli/e2e/tests/update/update-1.7-longhand.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ export default async function() {
99
await createProjectFromAsset('1.7-project');
1010

1111
await expectToFail(() => ng('build'));
12-
// Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
13-
await ng('update', '@angular/cli', '--migrate-only', '--from=1.7.1', '-C');
12+
await ng('update', '@angular/cli', '--migrate-only', '--from=1.7.1');
1413
await useBuiltPackages();
1514
await silentNpm('install');
1615
await ng('update', '@angular/core', ...extraUpdateArgs);

tests/legacy-cli/e2e/tests/update/update-1.7.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ export default async function() {
1212

1313
await useCIChrome('.');
1414
await expectToFail(() => ng('build'));
15-
// Turn off git commits ('-C') per migration to avoid breaking E2E cleanup process
16-
await ng('update', '@angular/cli', '-C');
15+
await ng('update', '@angular/cli');
1716
await useBuiltPackages();
1817
await silentNpm('install');
1918
await ng('update', '@angular/core', ...extraUpdateArgs);

0 commit comments

Comments
 (0)