From 5947239f89fa26a4494eec408dbec4e233aaadc5 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 4 May 2021 17:09:51 +0200 Subject: [PATCH] fix(dev-infra): do not set lts dist tag on packages from release-candidate train (#41946) Currently if a major release-train in the `release-candidate`/`feature-freeze` phase becomes `latest`, we intend to set the NPM LTS dist tag for all packages of the previous major (as the old release train in `latest` moves into LTS phase). The logic for this exists but the release tool sets the NPM dist tag for all packages of the new major. This means that the script might error if a new package is part of the new major; or it could cause a deleted package to not receive the LTS tag properly. PR Close #41946 --- dev-infra/ng-dev.js | 7 ++++--- dev-infra/release/publish/actions/cut-stable.ts | 7 ++++--- dev-infra/release/publish/test/cut-stable.spec.ts | 7 +++++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index fdaa7c2df6..92d0715da2 100755 --- a/dev-infra/ng-dev.js +++ b/dev-infra/ng-dev.js @@ -6500,16 +6500,17 @@ class CutStableAction extends ReleaseAction { // If a new major version is published and becomes the "latest" release-train, we need // to set the LTS npm dist tag for the previous latest release-train (the current patch). if (isNewMajor) { - const previousPatchVersion = this.active.latest.version; - const ltsTagForPatch = getLtsNpmDistTagOfMajor(previousPatchVersion.major); + const previousPatch = this.active.latest; + const ltsTagForPatch = getLtsNpmDistTagOfMajor(previousPatch.version.major); // Instead of directly setting the NPM dist tags, we invoke the ng-dev command for // setting the NPM dist tag to the specified version. We do this because release NPM // packages could be different in the previous patch branch, and we want to set the // LTS tag for all packages part of the last major. It would not be possible to set the // NPM dist tag for new packages part of the released major, nor would it be acceptable // to skip the LTS tag for packages which are no longer part of the new major. + yield this.checkoutUpstreamBranch(previousPatch.branchName); yield invokeYarnInstallCommand(this.projectDir); - yield invokeSetNpmDistCommand(ltsTagForPatch, previousPatchVersion); + yield invokeSetNpmDistCommand(ltsTagForPatch, previousPatch.version); } yield this.cherryPickChangelogIntoNextBranch(newVersion, branchName); }); diff --git a/dev-infra/release/publish/actions/cut-stable.ts b/dev-infra/release/publish/actions/cut-stable.ts index dee7e88ee8..0db17c0bba 100644 --- a/dev-infra/release/publish/actions/cut-stable.ts +++ b/dev-infra/release/publish/actions/cut-stable.ts @@ -39,8 +39,8 @@ export class CutStableAction extends ReleaseAction { // If a new major version is published and becomes the "latest" release-train, we need // to set the LTS npm dist tag for the previous latest release-train (the current patch). if (isNewMajor) { - const previousPatchVersion = this.active.latest.version; - const ltsTagForPatch = getLtsNpmDistTagOfMajor(previousPatchVersion.major); + const previousPatch = this.active.latest; + const ltsTagForPatch = getLtsNpmDistTagOfMajor(previousPatch.version.major); // Instead of directly setting the NPM dist tags, we invoke the ng-dev command for // setting the NPM dist tag to the specified version. We do this because release NPM @@ -48,8 +48,9 @@ export class CutStableAction extends ReleaseAction { // LTS tag for all packages part of the last major. It would not be possible to set the // NPM dist tag for new packages part of the released major, nor would it be acceptable // to skip the LTS tag for packages which are no longer part of the new major. + await this.checkoutUpstreamBranch(previousPatch.branchName); await invokeYarnInstallCommand(this.projectDir); - await invokeSetNpmDistCommand(ltsTagForPatch, previousPatchVersion); + await invokeSetNpmDistCommand(ltsTagForPatch, previousPatch.version); } await this.cherryPickChangelogIntoNextBranch(newVersion, branchName); diff --git a/dev-infra/release/publish/test/cut-stable.spec.ts b/dev-infra/release/publish/test/cut-stable.spec.ts index 5383b8f300..8a740b7316 100644 --- a/dev-infra/release/publish/test/cut-stable.spec.ts +++ b/dev-infra/release/publish/test/cut-stable.spec.ts @@ -70,6 +70,13 @@ describe('cut stable action', () => { latest: new ReleaseTrain('10.0.x', parse('10.0.3')), }); + // Ensure that the NPM dist tag is set only for packages that were available in the previous + // major version. A spy has already been installed on the function. + (externalCommands.invokeSetNpmDistCommand as jasmine.Spy).and.callFake(() => { + expect(action.gitClient.head.ref?.name).toBe('10.0.x'); + return Promise.resolve(); + }); + await expectStagingAndPublishWithCherryPick(action, '11.0.x', '11.0.0', 'latest'); expect(externalCommands.invokeSetNpmDistCommand).toHaveBeenCalledTimes(1); expect(externalCommands.invokeSetNpmDistCommand)