fix(ng-dev/release): do not error when yarn version of publish branch…

… is older

Currently there is one surprising issue within the release tool that
breaks the release of LTS versions.

If the ng-dev release tool is invoked through Yarn. e.g. `yarn ng-dev`,
Yarn will create a temporary directory and create wrappers for a binary
called `yarn` that will directly point to the checked-in Yarn file that
originally invoked the `ng-dev` tool. This means that child processes
calling to `yarn` will actually depend on the checked-in Yarn version
that invoked the `ng-dev` tool.

This logic works fine for most scripts/commands, but it breaks if the
release tool checks out an LTS branch and runs `yarn`. The checked-in
Yarn file from the original invocation may not exist anymore.

We fix this by resolving Yarn directly within the project, falling back
to the NPM global bin, or ultimately by leaving to the process `PATH`
standard resolution. This prevents this issue in all of our Angular
repositories where we check-in Yarn and have a `yarnrc` configuration.

ng-dev/misc/update-yarn/cli.ts

@@ -15,6 +15,7 @@ import {error, info, red} from '../../utils/console';
 import {Spinner} from '../../utils/spinner';
 import {AuthenticatedGitClient} from '../../utils/git/authenticated-git-client';
 import {addGithubTokenOption} from '../../utils/git/github-yargs';
+import {getYarnPathFromNpmGlobalBinaries} from '../../utils/resolve-yarn-bin';
 
 async function builder(yargs: Argv) {
   return addGithubTokenOption(yargs);
@@ -33,10 +34,16 @@ const skipHuskyEnv = {
 };
 
 async function handler() {
-  /** Directory where node binary are globally installed. */
-  const npmBinDir = spawnSync('npm', ['bin', '--global', 'yarn']).stdout.trim();
-  /** The full path to the globally installed yarn binary. */
-  const yarnBin = `${npmBinDir}/yarn`;
+  /**
+   * Process command that refers to the global Yarn installation.
+   *
+   * Note that we intend to use the global Yarn command here as this allows us to let Yarn
+   * respect the `.yarnrc` file, allowing us to check if the update has completed properly.
+   * Just using `yarn` does not necessarily resolve to the global Yarn version as Yarn-initiated
+   * sub-processes will have a modified `process.env.PATH` that directly points to the Yarn
+   * version that spawned the sub-process.
+   */
+  const yarnGlobalBin = (await getYarnPathFromNpmGlobalBinaries()) ?? 'yarn';
   /** Instance of the local git client. */
   const git = AuthenticatedGitClient.get();
   /** The main branch name of the repository. */
@@ -62,10 +69,10 @@ async function handler() {
     readdirSync(yarnReleasesDir).forEach((file) => unlinkSync(join(yarnReleasesDir, file)));
 
     spinner.update('Updating yarn version.');
-    spawnSync(yarnBin, ['policies', 'set-version', 'latest']);
+    spawnSync(yarnGlobalBin, ['policies', 'set-version', 'latest']);
 
     spinner.update('Confirming the version of yarn was updated.');
-    const newYarnVersion = spawnSync(yarnBin, ['-v'], {env: useYarnPathEnv}).stdout.trim();
+    const newYarnVersion = spawnSync(yarnGlobalBin, ['-v'], {env: useYarnPathEnv}).stdout.trim();
     if (git.run(['status', '--porcelain']).stdout.length === 0) {
       spinner.complete();
       error(red('Yarn already up to date'));

ng-dev/release/publish/actions.ts

@@ -80,26 +80,13 @@ export abstract class ReleaseAction {
    */
   abstract perform(): Promise<void>;
 
-  /** Cached found fork of the configured project. */
-  private _cachedForkRepo: GithubRepo | null = null;
-
   constructor(
     protected active: ActiveReleaseTrains,
     protected git: AuthenticatedGitClient,
     protected config: ReleaseConfig,
     protected projectDir: string,
   ) {}
 
-  /** Retrieves the version in the project top-level `package.json` file. */
-  private async getProjectVersion() {
-    const pkgJsonPath = join(this.projectDir, workspaceRelativePackageJsonPath);
-    const pkgJson = JSON.parse(await fs.readFile(pkgJsonPath, 'utf8')) as {
-      version: string;
-      [key: string]: any;
-    };
-    return new semver.SemVer(pkgJson.version);
-  }
-
   /** Updates the version in the project top-level `package.json` file. */
   protected async updateProjectVersion(newVersion: semver.SemVer) {
     const pkgJsonPath = join(this.projectDir, workspaceRelativePackageJsonPath);
@@ -599,7 +586,7 @@ export abstract class ReleaseAction {
     // publish branch. e.g. consider we publish patch version and a new package has been
     // created in the `next` branch. The new package would not be part of the patch branch,
     // so we cannot build and publish it.
-    const builtPackages = await invokeReleaseBuildCommand();
+    const builtPackages = await invokeReleaseBuildCommand(this.projectDir);
 
     // Verify the packages built are the correct version.
     await this._verifyPackageVersions(releaseNotes.version, builtPackages);

ng-dev/release/publish/actions/cut-stable.ts

@@ -69,7 +69,7 @@ export class CutStableAction extends ReleaseAction {
       await this.checkoutUpstreamBranch(previousPatch.branchName);
       await this.installDependenciesForCurrentBranch();
 
-      await invokeSetNpmDistCommand(ltsTagForPatch, previousPatch.version);
+      await invokeSetNpmDistCommand(this.projectDir, ltsTagForPatch, previousPatch.version);
     }
 
     await this.cherryPickChangelogIntoNextBranch(releaseNotes, branchName);

ng-dev/release/publish/actions/tag-recent-major-as-latest.ts

@@ -35,7 +35,7 @@ export class TagRecentMajorAsLatest extends ReleaseAction {
     await this.updateGithubReleaseEntryToStable(this.active.latest.version);
     await this.checkoutUpstreamBranch(this.active.latest.branchName);
     await this.installDependenciesForCurrentBranch();
-    await invokeSetNpmDistCommand('latest', this.active.latest.version);
+    await invokeSetNpmDistCommand(this.projectDir, 'latest', this.active.latest.version);
   }
 
   /**

ng-dev/release/publish/external-commands.ts

@@ -15,6 +15,7 @@ import {BuiltPackage} from '../config/index';
 import {NpmDistTag} from '../versioning';
 
 import {FatalReleaseActionError} from './actions-error';
+import {resolveYarnScriptForProject, YarnCommandInfo} from '../../utils/resolve-yarn-bin';
 
 /*
  * ###############################################################
@@ -37,17 +38,31 @@ import {FatalReleaseActionError} from './actions-error';
  * Invokes the `ng-dev release set-dist-tag` command in order to set the specified
  * NPM dist tag for all packages in the checked out branch to the given version.
  */
-export async function invokeSetNpmDistCommand(npmDistTag: NpmDistTag, version: semver.SemVer) {
+export async function invokeSetNpmDistCommand(
+  projectDir: string,
+  npmDistTag: NpmDistTag,
+  version: semver.SemVer,
+) {
+  // Note: We cannot use `yarn` directly as command because we might operate in
+  // a different publish branch and the current `PATH` will point to the Yarn version
+  // that invoked the release tool. More details in the function description.
+  const yarnCommand = await resolveYarnScriptForProject(projectDir);
+
   try {
     // Note: No progress indicator needed as that is the responsibility of the command.
-    await spawn('yarn', [
-      '--silent',
-      'ng-dev',
-      'release',
-      'set-dist-tag',
-      npmDistTag,
-      version.format(),
-    ]);
+    await spawn(
+      yarnCommand.binary,
+      [
+        ...yarnCommand.args,
+        '--silent',
+        'ng-dev',
+        'release',
+        'set-dist-tag',
+        npmDistTag,
+        version.format(),
+      ],
+      {cwd: projectDir},
+    );
     info(green(`  ✓   Set "${npmDistTag}" NPM dist tag for all packages to v${version}.`));
   } catch (e) {
     error(e);
@@ -60,14 +75,24 @@ export async function invokeSetNpmDistCommand(npmDistTag: NpmDistTag, version: s
  * Invokes the `ng-dev release build` command in order to build the release
  * packages for the currently checked out branch.
  */
-export async function invokeReleaseBuildCommand(): Promise<BuiltPackage[]> {
+export async function invokeReleaseBuildCommand(projectDir: string): Promise<BuiltPackage[]> {
+  // Note: We cannot use `yarn` directly as command because we might operate in
+  // a different publish branch and the current `PATH` will point to the Yarn version
+  // that invoked the release tool. More details in the function description.
+  const yarnCommand = await resolveYarnScriptForProject(projectDir);
   const spinner = new Spinner('Building release output.');
+
   try {
     // Since we expect JSON to be printed from the `ng-dev release build` command,
     // we spawn the process in silent mode. We have set up an Ora progress spinner.
-    const {stdout} = await spawn('yarn', ['--silent', 'ng-dev', 'release', 'build', '--json'], {
-      mode: 'silent',
-    });
+    const {stdout} = await spawn(
+      yarnCommand.binary,
+      [...yarnCommand.args, '--silent', 'ng-dev', 'release', 'build', '--json'],
+      {
+        cwd: projectDir,
+        mode: 'silent',
+      },
+    );
     spinner.complete();
     info(green('  ✓   Built release output for all packages.'));
     // The `ng-dev release build` command prints a JSON array to stdout
@@ -86,10 +111,19 @@ export async function invokeReleaseBuildCommand(): Promise<BuiltPackage[]> {
  * the configured project with the currently checked out revision.
  */
 export async function invokeYarnInstallCommand(projectDir: string): Promise<void> {
+  // Note: We cannot use `yarn` directly as command because we might operate in
+  // a different publish branch and the current `PATH` will point to the Yarn version
+  // that invoked the release tool. More details in the function description.
+  const yarnCommand = await resolveYarnScriptForProject(projectDir);
+
   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 spawn('yarn', ['install', '--frozen-lockfile', '--non-interactive'], {cwd: projectDir});
+    await spawn(
+      yarnCommand.binary,
+      [...yarnCommand.args, 'install', '--frozen-lockfile', '--non-interactive'],
+      {cwd: projectDir},
+    );
     info(green('  ✓   Installed project dependencies.'));
   } catch (e) {
     error(e);

ng-dev/release/publish/test/BUILD.bazel

@@ -2,6 +2,7 @@ load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")
 
 ts_library(
     name = "test_lib",
+    testonly = True,
     srcs = glob([
         "**/*.ts",
     ]),
@@ -17,8 +18,6 @@ ts_library(
         "//ng-dev/utils",
         "//ng-dev/utils/testing",
         "@npm//@octokit/plugin-rest-endpoint-methods",
-        "@npm//@types/jasmine",
-        "@npm//@types/node",
         "@npm//@types/semver",
         "@npm//@types/yargs",
         "@npm//nock",

ng-dev/release/publish/test/common.spec.ts

@@ -181,7 +181,7 @@ describe('common release action logic', () => {
     const forkBranchName = `changelog-cherry-pick-${version}`;
 
     it('should prepend the changelog to the next branch', async () => {
-      const {repo, fork, instance, testTmpDir} = setupReleaseActionForTesting(
+      const {repo, fork, instance, projectDir} = setupReleaseActionForTesting(
         TestAction,
         baseReleaseTrains,
       );
@@ -199,7 +199,7 @@ describe('common release action logic', () => {
       await instance.testCherryPickWithPullRequest(version, branchName);
 
       const changelogContent = readFileSync(
-        join(testTmpDir, workspaceRelativeChangelogPath),
+        join(projectDir, workspaceRelativeChangelogPath),
         'utf8',
       );
       expect(changelogContent).toMatch(changelogPattern`

ng-dev/release/publish/test/cut-next-prerelease.spec.ts

@@ -62,7 +62,7 @@ describe('cut next pre-release action', () => {
       await expectStagingAndPublishWithoutCherryPick(action, 'master', '10.2.0-next.0', 'next');
 
       const pkgJsonContents = readFileSync(
-        join(action.testTmpDir, workspaceRelativePackageJsonPath),
+        join(action.projectDir, workspaceRelativePackageJsonPath),
         'utf8',
       );
       const pkgJson = JSON.parse(pkgJsonContents) as {version: string; [key: string]: any};

ng-dev/release/publish/test/cut-stable.spec.ts

@@ -112,6 +112,7 @@ describe('cut stable action', () => {
     await expectStagingAndPublishWithCherryPick(action, '11.0.x', '11.0.0', 'next');
     expect(externalCommands.invokeSetNpmDistCommand).toHaveBeenCalledTimes(1);
     expect(externalCommands.invokeSetNpmDistCommand).toHaveBeenCalledWith(
+      action.projectDir,
       'v10-lts',
       matchesVersion('10.0.3'),
     );

ng-dev/release/publish/test/tag-recent-major-as-latest.spec.ts

@@ -111,7 +111,7 @@ describe('tag recent major as latest action', () => {
   );
 
   it('should re-tag the version in the NPM registry and update the Github release', async () => {
-    const {instance, gitClient, releaseConfig, repo} = setupReleaseActionForTesting(
+    const {instance, gitClient, projectDir, releaseConfig, repo} = setupReleaseActionForTesting(
       TagRecentMajorAsLatest,
       new ActiveReleaseTrains({
         releaseCandidate: null,
@@ -140,6 +140,7 @@ describe('tag recent major as latest action', () => {
 
     expect(externalCommands.invokeSetNpmDistCommand).toHaveBeenCalledTimes(1);
     expect(externalCommands.invokeSetNpmDistCommand).toHaveBeenCalledWith(
+      projectDir,
       'latest',
       matchesVersion('10.0.0'),
     );

ng-dev/release/publish/test/test-utils/test-action.ts

@@ -40,7 +40,7 @@ export interface TestReleaseAction<
   instance: T;
   repo: GithubTestingRepo;
   fork: GithubTestingRepo;
-  testTmpDir: string;
+  projectDir: string;
   githubConfig: GithubConfig;
   releaseConfig: ReleaseConfig;
   gitClient: O['useSandboxGitClient'] extends true ? SandboxGitClient : VirtualGitClient;

ng-dev/release/publish/test/test-utils/test-utils.ts

@@ -33,6 +33,7 @@ export function setupReleaseActionForTesting<T extends ReleaseAction, O extends
   // Reset existing HTTP interceptors.
   nock.cleanAll();
 
+  const projectDir = testTmpDir;
   const {githubConfig, releaseConfig} = getTestConfigurationsForAction();
   const repo = new GithubTestingRepo(githubConfig.owner, githubConfig.name);
   const fork = new GithubTestingRepo('some-user', 'fork');
@@ -52,9 +53,18 @@ export function setupReleaseActionForTesting<T extends ReleaseAction, O extends
     testOptions.useSandboxGitClient,
   );
 
-  const action = new actionCtor(active, gitClient, releaseConfig, testTmpDir);
+  const action = new actionCtor(active, gitClient, releaseConfig, projectDir);
 
-  return {instance: action, active, repo, fork, testTmpDir, githubConfig, releaseConfig, gitClient};
+  return {
+    instance: action,
+    active,
+    repo,
+    fork,
+    projectDir,
+    githubConfig,
+    releaseConfig,
+    gitClient,
+  };
 }
 
 /** Parses the specified version into Semver. */

ng-dev/utils/BUILD.bazel

@@ -22,13 +22,16 @@ ts_library(
         "@npm//@types/inquirer",
         "@npm//@types/node",
         "@npm//@types/semver",
+        "@npm//@types/which",
         "@npm//@types/yargs",
         "@npm//@types/yarnpkg__lockfile",
         "@npm//@yarnpkg/lockfile",
         "@npm//chalk",
         "@npm//inquirer",
         "@npm//semver",
         "@npm//typed-graphqlify",
+        "@npm//which",
+        "@npm//yaml",
         "@npm//yargs",
     ],
 )

ng-dev/utils/nodejs-errors.ts

@@ -0,0 +1,18 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * 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
+ */
+
+/**
+ * Type narrowing function that becomes true if the given error is of type `T`.
+ * The narrowed type will include the NodeJS `ErrnoException` properties.
+ */
+export function isNodeJSWrappedError<T extends new (...args: any) => Error>(
+  value: Error | unknown,
+  errorType: T,
+): value is InstanceType<T> & NodeJS.ErrnoException {
+  return value instanceof errorType;
+}