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

Currently publishing fails if a pre-release is cut for an upcoming
major/minor. This is because the `next` release-train has some
special logic where its version is already bumped, but the
version has not been released. Our release notes logic currently
when building a pre-release for next, looks for a tag named
similar to the version in the `package.json`. This will always fail.

To fix this, we consult the release trains for building up
the Git range that is used for the release notes. We still
rely on tags, but we determine the tags from the active release
trains (which is more robust than relying on the package json value).

For the special `next` release-train situation, we build up the
release notes from the most recent patch to `HEAD` of next. While
doing this, we also dedupe any commits that have already landed in
patch, RC, or FF releases.

Author: devversion

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.png

@@ -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/get-commits-in-range.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/commits/unique-commit-id.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/notes/release-notes.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.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/branch-off-next-branch.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,
     );