From 7fb8527e1efddd6fc2a80ee051635891bb179526 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 14 May 2021 20:23:52 +0200 Subject: [PATCH] refactor(dev-infra): remove invoke bazel clean command from release tool (#42101) Currently the ng-dev release tool always run `bazel clean` before calling the configured build release function. The clean is necessary to ensure the release output is actually built; and not restored from previous builds which could have different bazel workspace status variables (which provide the NPM package version). Instead of doing this as part of the release tool, the actual script running to build the release output should run the `bazel clean`. The release tool does not intend to know about details on how the release output is built. This is necessary because the build setup could vary between version branches (especially for older ones; such as LTS version branches). PR Close #42101 --- dev-infra/ng-dev.js | 20 ------------------- dev-infra/release/publish/actions.ts | 3 +-- .../release/publish/external-commands.ts | 17 ---------------- dev-infra/release/publish/test/test-utils.ts | 1 - scripts/build/package-builder.js | 10 ++++++++++ 5 files changed, 11 insertions(+), 40 deletions(-) diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index 9a73ddbc31..9778612953 100755 --- a/dev-infra/ng-dev.js +++ b/dev-infra/ng-dev.js @@ -5623,25 +5623,6 @@ function invokeYarnInstallCommand(projectDir) { } }); } -/** - * Invokes the `yarn bazel clean` command in order to clean the output tree and ensure new artifacts - * are created for builds. - */ -function invokeBazelCleanCommand(projectDir) { - return tslib.__awaiter(this, void 0, void 0, function* () { - try { - // Note: No progress indicator needed as that is the responsibility of the command. - // TODO: Consider using an Ora spinner instead to ensure minimal console output. - yield spawnWithDebugOutput('yarn', ['bazel', 'clean'], { cwd: projectDir }); - info(green(' ✓ Cleaned bazel output tree.')); - } - catch (e) { - error(e); - error(red(' ✘ An error occurred while cleaning the bazel output tree.')); - throw new FatalReleaseActionError(); - } - }); -} /** * @license @@ -6297,7 +6278,6 @@ class ReleaseAction { // created in the `next` branch. The new package would not be part of the patch branch, // so we cannot build and publish it. yield invokeYarnInstallCommand(this.projectDir); - yield invokeBazelCleanCommand(this.projectDir); const builtPackages = yield invokeReleaseBuildCommand(); // Verify the packages built are the correct version. yield this._verifyPackageVersions(releaseNotes.version, builtPackages); diff --git a/dev-infra/release/publish/actions.ts b/dev-infra/release/publish/actions.ts index 5be6919a19..95aa24eaf2 100644 --- a/dev-infra/release/publish/actions.ts +++ b/dev-infra/release/publish/actions.ts @@ -21,7 +21,7 @@ import {runNpmPublish} from '../versioning/npm-publish'; import {FatalReleaseActionError, UserAbortedReleaseActionError} from './actions-error'; import {getCommitMessageForRelease, getReleaseNoteCherryPickCommitMessage} from './commit-message'; import {changelogPath, packageJsonPath, waitForPullRequestInterval} from './constants'; -import {invokeBazelCleanCommand, invokeReleaseBuildCommand, invokeYarnInstallCommand} from './external-commands'; +import {invokeReleaseBuildCommand, invokeYarnInstallCommand} from './external-commands'; import {findOwnedForksOfRepoQuery} from './graphql-queries'; import {getPullRequestState} from './pull-request-state'; import {getLocalChangelogFilePath, ReleaseNotes} from './release-notes/release-notes'; @@ -463,7 +463,6 @@ export abstract class ReleaseAction { // created in the `next` branch. The new package would not be part of the patch branch, // so we cannot build and publish it. await invokeYarnInstallCommand(this.projectDir); - await invokeBazelCleanCommand(this.projectDir); const builtPackages = await invokeReleaseBuildCommand(); // Verify the packages built are the correct version. diff --git a/dev-infra/release/publish/external-commands.ts b/dev-infra/release/publish/external-commands.ts index 2892bc47da..3c9cfbe8f7 100644 --- a/dev-infra/release/publish/external-commands.ts +++ b/dev-infra/release/publish/external-commands.ts @@ -90,20 +90,3 @@ export async function invokeYarnInstallCommand(projectDir: string): Promise { - try { - // Note: No progress indicator needed as that is the responsibility of the command. - // TODO: Consider using an Ora spinner instead to ensure minimal console output. - await spawnWithDebugOutput('yarn', ['bazel', 'clean'], {cwd: projectDir}); - info(green(' ✓ Cleaned bazel output tree.')); - } catch (e) { - error(e); - error(red(' ✘ An error occurred while cleaning the bazel output tree.')); - throw new FatalReleaseActionError(); - } -} diff --git a/dev-infra/release/publish/test/test-utils.ts b/dev-infra/release/publish/test/test-utils.ts index 0cffd5b8ff..77dd64410f 100644 --- a/dev-infra/release/publish/test/test-utils.ts +++ b/dev-infra/release/publish/test/test-utils.ts @@ -97,7 +97,6 @@ export function setupReleaseActionForTesting( spyOn(npm, 'runNpmPublish').and.resolveTo(); spyOn(externalCommands, 'invokeSetNpmDistCommand').and.resolveTo(); spyOn(externalCommands, 'invokeYarnInstallCommand').and.resolveTo(); - spyOn(externalCommands, 'invokeBazelCleanCommand').and.resolveTo(); spyOn(externalCommands, 'invokeReleaseBuildCommand').and.resolveTo([ {name: '@angular/pkg1', outputPath: `${testTmpDir}/dist/pkg1`}, {name: '@angular/pkg2', outputPath: `${testTmpDir}/dist/pkg2`} diff --git a/scripts/build/package-builder.js b/scripts/build/package-builder.js index 6c735a3cf0..9081eb70b4 100644 --- a/scripts/build/package-builder.js +++ b/scripts/build/package-builder.js @@ -81,6 +81,16 @@ function buildTargetPackages(destDir, enableIvy, description, isRelease = false) bazelCmd} query --output=label "attr('tags', '\\[.*release-with-framework.*\\]', //packages/...) intersect kind('ng_package|pkg_npm', //packages/...)"`; const targets = exec(getTargetsCmd, true).split(/\r?\n/); + // If we are in release mode, run `bazel clean` to ensure the execroot and action cache + // are not populated. This is necessary because targets using `npm_package` rely on + // workspace status variables for the package version. Such NPM package targets are not + // rebuilt if only the workspace status variables change. This could result in accidental + // re-use of previously built package output with a different `version` in the `package.json`. + if (isRelease) { + console.info('Building in release mode. Resetting the Bazel execroot and action cache..'); + exec(`${bazelCmd} clean`); + } + // Use either `--config=snapshot` or `--config=release` so that builds are created with the // correct embedded version info. exec(`${bazelCmd} build --config=${isRelease ? 'release' : 'snapshot'} --config=${