fix(ng-dev): publishing fails if pre-release is cut for new minor/major

WORKSPACE

@@ -44,3 +44,9 @@ browser_repositories()
 load("@npm//@bazel/esbuild:esbuild_repositories.bzl", "esbuild_repositories")
 
 esbuild_repositories()
+
+register_toolchains(
+    "//tools/git-toolchain:git_linux_toolchain",
+    "//tools/git-toolchain:git_macos_toolchain",
+    "//tools/git-toolchain:git_windows_toolchain",
+)

ng-dev/commit-message/parse.ts

@@ -142,10 +142,9 @@ function parseInternal(fullText: string | Buffer): CommitFromGitLog | Commit {
   // Extract the commit message notes by marked types into their respective lists.
   commit.notes.forEach((note: ParsedCommit.Note) => {
     if (note.title === NoteSections.BREAKING_CHANGE) {
-      return breakingChanges.push(note);
-    }
-    if (note.title === NoteSections.DEPRECATED) {
-      return deprecations.push(note);
+      breakingChanges.push(note);
+    } else if (note.title === NoteSections.DEPRECATED) {
+      deprecations.push(note);
     }
   });
 

ng-dev/release/notes/cli.ts

@@ -12,13 +12,12 @@ import {SemVer} from 'semver';
 import {Arguments, Argv, CommandModule} from 'yargs';
 
 import {info} from '../../utils/console';
-import {GitClient} from '../../utils/git/git-client';
 
 import {ReleaseNotes} from './release-notes';
 
 /** Command line options for building a release. */
 export interface ReleaseNotesOptions {
-  from?: string;
+  from: string;
   to: string;
   outFile?: string;
   releaseVersion: SemVer;
@@ -36,7 +35,7 @@ function builder(argv: Argv): Argv<ReleaseNotesOptions> {
     .option('from', {
       type: 'string',
       description: 'The git tag or ref to start the changelog entry from',
-      defaultDescription: 'The latest semver tag',
+      demandOption: true,
     })
     .option('to', {
       type: 'string',
@@ -58,11 +57,8 @@ function builder(argv: Argv): Argv<ReleaseNotesOptions> {
 
 /** Yargs command handler for generating release notes. */
 async function handler({releaseVersion, from, to, outFile, type}: Arguments<ReleaseNotesOptions>) {
-  // Since `yargs` evaluates defaults even if a value as been provided, if no value is provided to
-  // the handler, the latest semver tag on the branch is used.
-  from = from || GitClient.get().getLatestSemverTag().format();
   /** The ReleaseNotes instance to generate release notes. */
-  const releaseNotes = await ReleaseNotes.fromRange(releaseVersion, from, to);
+  const releaseNotes = await ReleaseNotes.forRange(releaseVersion, from, to);
 
   /** The requested release notes entry. */
   const releaseNotesEntry = await (type === 'changelog'

ng-dev/release/notes/commits/get-commits-in-range.ts

@@ -0,0 +1,94 @@
+/**
+ * @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
+ */
+
+import {GitClient} from '../../../utils/git/git-client';
+import {
+  CommitFromGitLog,
+  gitLogFormatForParsing,
+  parseCommitFromGitLog,
+} from '../../../commit-message/parse';
+import {computeUniqueIdFromCommitMessage} from './unique-commit-id';
+
+/**
+ * Gets all commits the head branch contains, but the base branch does not include.
+ * This follows the same semantics as Git's double-dot revision range.
+ *
+ * i.e. `<baseRef>..<headRef>` revision range as per Git.
+ * https://git-scm.com/book/en/v2/Git-Tools-Revision-Selection.
+ *
+ * Branches in the Angular organization are diverging quickly due to multiple factors
+ * concerning the versioning and merging. i.e. Commits are cherry-picked into branches,
+ * resulting in different SHAs for each branch. Additionally, branches diverge quickly
+ * because changes can be made only for specific branches (e.g. a master-only change).
+ *
+ * In order to allow for comparisons that follow similar semantics as Git's double-dot
+ * revision range syntax, the logic re-implementing the semantics need to account for
+ * the mentioned semi-diverged branches. We achieve this by excluding commits in the
+ * head branch which have a similarly-named commit in the base branch. We cannot rely on
+ * SHAs for determining common commits between the two branches (as explained above).
+ *
+ * More details can be found in the `get-commits-in-range.png` file which illustrates a
+ * scenario where commits from the patch branch need to be excluded from the main branch.
+ */
+export function getCommitsForRangeWithDeduping(
+  client: GitClient,
+  baseRef: string,
+  headRef: string,
+): CommitFromGitLog[] {
+  const commits: CommitFromGitLog[] = [];
+  const commitsForHead = fetchCommitsForRevisionRange(client, `${baseRef}..${headRef}`);
+  const commitsForBase = fetchCommitsForRevisionRange(client, `${headRef}..${baseRef}`);
+
+  // Map that keeps track of commits within the base branch. Commits are
+  // stored with an unique id based on the commit message. If a similarly-named
+  // commit appears multiple times, the value number will reflect that.
+  const knownCommitsOnlyInBase = new Map<string, number>();
+
+  for (const commit of commitsForBase) {
+    const id = computeUniqueIdFromCommitMessage(commit);
+    const numSimilarCommits = knownCommitsOnlyInBase.get(id) ?? 0;
+    knownCommitsOnlyInBase.set(id, numSimilarCommits + 1);
+  }
+
+  for (const commit of commitsForHead) {
+    const id = computeUniqueIdFromCommitMessage(commit);
+    const numSimilarCommits = knownCommitsOnlyInBase.get(id) ?? 0;
+
+    // If there is a similar commit in the base branch, the current commit in the head branch
+    // needs to be skipped. We keep track of the number of similar commits so that we do not
+    // accidentally "dedupe" a commit. e.g. consider a case where commit `X` lands in the
+    // patch branch and next branch. Then a similar similarly named commits lands only in the
+    // next branch. We would not want to omit that one as it is not part of the patch branch.
+    if (numSimilarCommits > 0) {
+      knownCommitsOnlyInBase.set(id, numSimilarCommits - 1);
+      continue;
+    }
+
+    commits.push(commit);
+  }
+
+  return commits;
+}
+
+/** Fetches commits for the given revision range using `git log`. */
+export function fetchCommitsForRevisionRange(
+  client: GitClient,
+  revisionRange: string,
+): CommitFromGitLog[] {
+  const splitDelimiter = '-------------ɵɵ------------';
+  const output = client.run([
+    'log',
+    `--format=${gitLogFormatForParsing}${splitDelimiter}`,
+    revisionRange,
+  ]);
+
+  return output.stdout
+    .split(splitDelimiter)
+    .filter((entry) => !!entry.trim())
+    .map((entry) => parseCommitFromGitLog(Buffer.from(entry, 'utf-8')));
+}

ng-dev/release/notes/commits/unique-commit-id.ts

@@ -0,0 +1,28 @@
+/**
+ * @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
+ */
+
+import {Commit} from '../../../commit-message/parse';
+
+/**
+ * Fields from a `Commit` to incorporate when building up an unique
+ * id for a commit message.
+ *
+ * Note: The header incorporates the commit type, scope and message
+ * but lacks information for fixup, revert or squash commits..
+ */
+const fieldsToIncorporateForId: (keyof Commit)[] = ['header', 'isFixup', 'isRevert', 'isSquash'];
+
+/**
+ * Computes an unique id for the given commit using its commit message.
+ * This can be helpful for comparisons, if commits differ in SHAs due
+ * to cherry-picking.
+ */
+export function computeUniqueIdFromCommitMessage(commit: Commit): string {
+  // Join all resolved fields with a special character to build up an id.
+  return fieldsToIncorporateForId.map((f) => commit[f]).join('ɵɵ');
+}

ng-dev/release/notes/release-notes.ts

@@ -9,19 +9,21 @@ import {render} from 'ejs';
 import * as semver from 'semver';
 import {CommitFromGitLog} from '../../commit-message/parse';
 
-import {getCommitsInRange} from '../../commit-message/utils';
 import {promptInput} from '../../utils/console';
 import {GitClient} from '../../utils/git/git-client';
 import {DevInfraReleaseConfig, getReleaseConfig, ReleaseNotesConfig} from '../config/index';
 import {RenderContext} from './context';
 
 import changelogTemplate from './templates/changelog';
 import githubReleaseTemplate from './templates/github-release';
+import {getCommitsForRangeWithDeduping} from './commits/get-commits-in-range';
 
 /** Release note generation. */
 export class ReleaseNotes {
-  static async fromRange(version: semver.SemVer, startingRef: string, endingRef: string) {
-    return new ReleaseNotes(version, startingRef, endingRef);
+  static async forRange(version: semver.SemVer, baseRef: string, headRef: string) {
+    const client = GitClient.get();
+    const commits = getCommitsForRangeWithDeduping(client, baseRef, headRef);
+    return new ReleaseNotes(version, commits);
   }
 
   /** An instance of GitClient. */
@@ -30,19 +32,10 @@ export class ReleaseNotes {
   private renderContext: RenderContext | undefined;
   /** The title to use for the release. */
   private title: string | false | undefined;
-  /** A promise resolving to a list of Commits since the latest semver tag on the branch. */
-  private commits: Promise<CommitFromGitLog[]> = this.getCommitsInRange(
-    this.startingRef,
-    this.endingRef,
-  );
   /** The configuration for release notes. */
   private config: ReleaseNotesConfig = this.getReleaseConfig().releaseNotes;
 
-  protected constructor(
-    public version: semver.SemVer,
-    private startingRef: string,
-    private endingRef: string,
-  ) {}
+  protected constructor(public version: semver.SemVer, private commits: CommitFromGitLog[]) {}
 
   /** Retrieve the release note generated for a Github Release. */
   async getGithubReleaseEntry(): Promise<string> {
@@ -75,7 +68,7 @@ export class ReleaseNotes {
   private async generateRenderContext(): Promise<RenderContext> {
     if (!this.renderContext) {
       this.renderContext = new RenderContext({
-        commits: await this.commits,
+        commits: this.commits,
         github: this.git.remoteConfig,
         version: this.version.format(),
         groupOrder: this.config.groupOrder,
@@ -86,12 +79,8 @@ export class ReleaseNotes {
     return this.renderContext;
   }
 
-  // These methods are used for access to the utility functions while allowing them to be
-  // overwritten in subclasses during testing.
-  protected async getCommitsInRange(from: string, to?: string) {
-    return getCommitsInRange(from, to);
-  }
-
+  // These methods are used for access to the utility functions while allowing them
+  // to be overwritten in subclasses during testing.
   protected getReleaseConfig(config?: Partial<DevInfraReleaseConfig>) {
     return getReleaseConfig(config);
   }

ng-dev/release/publish/actions.ts

@@ -27,6 +27,7 @@ import {changelogPath, packageJsonPath, waitForPullRequestInterval} from './cons
 import {invokeReleaseBuildCommand, invokeYarnInstallCommand} from './external-commands';
 import {findOwnedForksOfRepoQuery} from './graphql-queries';
 import {getPullRequestState} from './pull-request-state';
+import {getReleaseTagForVersion} from '../versioning/version-tags';
 
 /** Interface describing a Github repository. */
 export interface GithubRepo {
@@ -377,33 +378,44 @@ export abstract class ReleaseAction {
   /**
    * Creates a commit for the specified files with the given message.
    * @param message Message for the created commit
-   * @param files List of project-relative file paths to be commited.
+   * @param files List of project-relative file paths to be committed.
    */
   protected async createCommit(message: string, files: string[]) {
+    // Note: `git add` would not be needed if the files are already known to
+    // Git, but the specified files could also be newly created, and unknown.
+    this.git.run(['add', ...files]);
     this.git.run(['commit', '-q', '--no-verify', '-m', message, ...files]);
   }
 
   /**
-   * Stages the specified new version for the current branch and creates a
-   * pull request that targets the given base branch.
+   * Stages the specified new version for the current branch and creates a pull request
+   * that targets the given base branch. Assumes the staging branch is already checked-out.
+   *
+   * @param newVersion New version to be staged.
+   * @param compareVersionForReleaseNotes Version used for comparing with the current
+   *   `HEAD` in order build the release notes.
+   * @param pullRequestTargetBranch Branch the pull request should target.
    * @returns an object describing the created pull request.
    */
   protected async stageVersionForBranchAndCreatePullRequest(
     newVersion: semver.SemVer,
-    pullRequestBaseBranch: string,
+    compareVersionForReleaseNotes: semver.SemVer,
+    pullRequestTargetBranch: string,
   ): Promise<{releaseNotes: ReleaseNotes; pullRequest: PullRequest}> {
-    /**
-     * The current version of the project for the branch from the root package.json. This must be
-     * retrieved prior to updating the project version.
-     */
-    const currentVersion = this.git.getMatchingTagForSemver(await this.getProjectVersion());
-    const releaseNotes = await ReleaseNotes.fromRange(newVersion, currentVersion, 'HEAD');
+    const releaseNotesCompareTag = getReleaseTagForVersion(compareVersionForReleaseNotes);
+
+    // Fetch the compare tag so that commits for the release notes can be determined.
+    this.git.run(['fetch', this.git.getRepoGitUrl(), `refs/tags/${releaseNotesCompareTag}`]);
+
+    // Build release notes for commits from `<releaseNotesCompareTag>..HEAD`.
+    const releaseNotes = await ReleaseNotes.forRange(newVersion, releaseNotesCompareTag, 'HEAD');
+
     await this.updateProjectVersion(newVersion);
     await this.prependReleaseNotesToChangelog(releaseNotes);
     await this.waitForEditsAndCreateReleaseCommit(newVersion);
 
     const pullRequest = await this.pushChangesToForkAndCreatePullRequest(
-      pullRequestBaseBranch,
+      pullRequestTargetBranch,
       `release-stage-${newVersion}`,
       `Bump version to "v${newVersion}" with changelog.`,
     );
@@ -417,15 +429,25 @@ export abstract class ReleaseAction {
   /**
    * Checks out the specified target branch, verifies its CI status and stages
    * the specified new version in order to create a pull request.
+   *
+   * @param newVersion New version to be staged.
+   * @param compareVersionForReleaseNotes Version used for comparing with `HEAD` of
+   *   the staging branch in order build the release notes.
+   * @param stagingBranch Branch within the new version should be staged.
    * @returns an object describing the created pull request.
    */
   protected async checkoutBranchAndStageVersion(
     newVersion: semver.SemVer,
+    compareVersionForReleaseNotes: semver.SemVer,
     stagingBranch: string,
   ): Promise<{releaseNotes: ReleaseNotes; pullRequest: PullRequest}> {
     await this.verifyPassingGithubStatus(stagingBranch);
     await this.checkoutUpstreamBranch(stagingBranch);
-    return await this.stageVersionForBranchAndCreatePullRequest(newVersion, stagingBranch);
+    return await this.stageVersionForBranchAndCreatePullRequest(
+      newVersion,
+      compareVersionForReleaseNotes,
+      stagingBranch,
+    );
   }
 
   /**
@@ -481,7 +503,7 @@ export abstract class ReleaseAction {
     versionBumpCommitSha: string,
     prerelease: boolean,
   ) {
-    const tagName = releaseNotes.version.format();
+    const tagName = getReleaseTagForVersion(releaseNotes.version);
     await this.git.github.git.createRef({
       ...this.git.remoteParams,
       ref: `refs/tags/${tagName}`,

ng-dev/release/publish/actions/branch-off-next-branch.ts

@@ -11,8 +11,10 @@ import * as semver from 'semver';
 import {green, info, yellow} from '../../../utils/console';
 import {semverInc} from '../../../utils/semver';
 import {ReleaseNotes} from '../../notes/release-notes';
-import {ActiveReleaseTrains} from '../../versioning/active-release-trains';
-import {computeNewPrereleaseVersionForNext} from '../../versioning/next-prerelease-version';
+import {
+  computeNewPrereleaseVersionForNext,
+  getReleaseNotesCompareVersionForNext,
+} from '../../versioning/next-prerelease-version';
 import {ReleaseAction} from '../actions';
 import {
   getCommitMessageForExceptionalNextVersionBump,
@@ -42,6 +44,10 @@ export abstract class BranchOffNextBranchBaseAction extends ReleaseAction {
   }
 
   override async perform() {
+    const compareVersionForReleaseNotes = await getReleaseNotesCompareVersionForNext(
+      this.active,
+      this.config,
+    );
     const newVersion = await this._computeNewVersion();
     const newBranch = `${newVersion.major}.${newVersion.minor}.x`;
 
@@ -53,6 +59,7 @@ export abstract class BranchOffNextBranchBaseAction extends ReleaseAction {
     // created branch instead of re-fetching from the upstream.
     const {pullRequest, releaseNotes} = await this.stageVersionForBranchAndCreatePullRequest(
       newVersion,
+      compareVersionForReleaseNotes,
       newBranch,
     );
 

ng-dev/release/publish/actions/cut-lts-patch.ts

@@ -30,8 +30,11 @@ export class CutLongTermSupportPatchAction extends ReleaseAction {
   override async perform() {
     const ltsBranch = await this._promptForTargetLtsBranch();
     const newVersion = semverInc(ltsBranch.version, 'patch');
+    const compareVersionForReleaseNotes = ltsBranch.version;
+
     const {pullRequest, releaseNotes} = await this.checkoutBranchAndStageVersion(
       newVersion,
+      compareVersionForReleaseNotes,
       ltsBranch.name,
     );