build: snapshot builds incorrectly modify semver versions (#20053)

An interesting case that came up in v10.0.0 with the docs-content. The
snapshot release package and docs-content output had various
`@breaking-change` notes in the source code referring to `v10.0.0` as
certain changes are planned to be made at that point.

The snapshot deploy scripts pick up the version from the package.json
file and replace it in the output packages with a more concrete version
that includes the SHA. This meant that we accidentally also overwrote
versions as in the `@breaking-change` notes (ultimately making it
difficult for us to use the latest docs-content for the v10 release as
it was incorrect).

Example snapshot commit:
angular/material2-docs-content@e624c71

We fix this by not including the SHA as part of the deployment, but
rather including the SHA when building the NPM packages. At that point,
we can safely just replace instances of the `0.0.0-PLACEHOLDER` without
having to worry about accidental version overriding.

To achieve this, we update our release stamping script to have two
modes. i.e. snapshot build mode and release mode. Framework does this
by checking the Git tag history but that seems less ideal as it makes
the release output building reliant on external factors while our
stamping is self-contained within the checked out code revision.

(cherry picked from commit 33b31b1)

Author: devversion

.bazelrc

@@ -37,16 +37,20 @@ query --output=label_kind
 # By default, failing tests don't print any output, it goes to the log file
 test --test_output=errors
 
-#################################
-# Release configuration.        #
-# Run with "--config=release"   #
-#################################
+####################################
+# Stamping configurations.         #
+# Run with "--config=release" or   #
+# "--config=snapshot-build".       #
+####################################
 
 # Configures script to do version stamping.
 # See https://docs.bazel.build/versions/master/user-manual.html#flag--workspace_status_command
 build:release --workspace_status_command="node ./tools/bazel-stamp-vars.js"
 build:release --stamp
 
+build:snapshot-build --workspace_status_command="node ./tools/bazel-stamp-vars.js --snapshot"
+build:snapshot-build --stamp
+
 ################################
 # View Engine / Ivy toggle     #
 ################################

.circleci/config.yml

@@ -359,7 +359,12 @@ jobs:
       - *setup_bazel_binary
 
       - run: yarn build
-      - run: yarn check-release-output
+      - run:
+          name: Checking release output
+          command: |
+            pkg_json_version=$(node -pe "require('./package.json').version")
+            expected_version="${pkg_json_version}-sha-$(git rev-parse --short HEAD)"
+            yarn check-release-output ${expected_version}
 
       # TODO(devversion): replace this with bazel tests that run Madge. This is
       # cumbersome and doesn't guarantee no circular deps for other entry-points.
@@ -435,7 +440,7 @@ jobs:
       # The components examples package is not a release package, but we publish it
       # as part of this job to the docs-content repository. It's not contained in the
       # attached release output, so we need to build it here.
-      - run: bazel build src/components-examples:npm_package --config=release
+      - run: bazel build src/components-examples:npm_package --config=snapshot-build
 
       # Ensures that we do not push the snapshot artifacts upstream until all previous
       # snapshot build jobs have completed. This helps avoiding conflicts when multiple

scripts/build-packages-dist.js

@@ -27,28 +27,39 @@ const queryPackagesCmd =
     `${bazelCmd} query --output=label "attr('tags', '\\[.*${releaseTargetTag}.*\\]', //src/...) ` +
     `intersect kind('.*_package', //src/...)"`;
 
+/** Path for the default distribution output directory. */
+const defaultDistPath = join(projectDir, 'dist/releases');
+
 // Export the methods for building the release packages. These
 // can be consumed by the release tool.
-exports.buildReleasePackages = buildReleasePackages;
-exports.defaultBuildReleasePackages = defaultBuildReleasePackages;
+exports.performNpmReleaseBuild = performNpmReleaseBuild;
+exports.performDefaultSnapshotBuild = performDefaultSnapshotBuild;
 
 if (module === require.main) {
-  defaultBuildReleasePackages();
+  // We always build as a snapshot bu8ild, unless the script is invoked directly by the
+  // release publish script. The snapshot release configuration ensures that the current
+  // Git `HEAD` sha is included for the version placeholders.
+  performDefaultSnapshotBuild();
+}
+
+/** Builds the release packages for NPM. */
+function performNpmReleaseBuild() {
+  buildReleasePackages(false, defaultDistPath, /* isSnapshotBuild */ false);
 }
 
 /**
- * Builds the release packages with the default compile mode and
- * output directory.
+ * Builds the release packages as snapshot build. This means that the current
+ * Git HEAD SHA is included in the version (for easier debugging and back tracing).
  */
-function defaultBuildReleasePackages() {
-  buildReleasePackages(false, join(projectDir, 'dist/releases'));
+function performDefaultSnapshotBuild() {
+  buildReleasePackages(false, defaultDistPath, /* isSnapshotBuild */ true);
 }
 
 /**
  * Builds the release packages with the given compile mode and copies
  * the package output into the given directory.
  */
-function buildReleasePackages(useIvy, distPath) {
+function buildReleasePackages(useIvy, distPath, isSnapshotBuild) {
   console.log('######################################');
   console.log('  Building release packages...');
   console.log(`  Compiling with Ivy: ${useIvy}`);
@@ -60,6 +71,12 @@ function buildReleasePackages(useIvy, distPath) {
   const bazelBinPath = exec(`${bazelCmd} info bazel-bin`, true);
   const getOutputPath = pkgName => join(bazelBinPath, 'src', pkgName, 'npm_package');
 
+  // Build with "--config=release" or `--config=snapshot-build` so that Bazel
+  // runs the workspace stamping script. The stamping script ensures that the
+  // version placeholder is populated in the release output.
+  const stampConfigArg = `--config=${isSnapshotBuild ? 'snapshot-build' : 'release'}`;
+  const ivySwitchConfigArg = `--config=${useIvy ? 'ivy' : 'view-engine'}`;
+
   // Walk through each release package and clear previous "npm_package" outputs. This is
   // a workaround for: https://github.com/bazelbuild/rules_nodejs/issues/1219. We need to
   // do this to ensure that the version placeholders are properly populated.
@@ -71,9 +88,7 @@ function buildReleasePackages(useIvy, distPath) {
     }
   });
 
-  // Build with "--config=release" so that Bazel runs the workspace stamping script. The
-  // stamping script ensures that the version placeholder is populated in the release output.
-  exec(`${bazelCmd} build --config=release --config=${useIvy ? 'ivy' : 'view-engine'} ${targets.join(' ')}`);
+  exec(`${bazelCmd} build ${stampConfigArg} ${ivySwitchConfigArg} ${targets.join(' ')}`);
 
   // Delete the distribution directory so that the output is guaranteed to be clean. Re-create
   // the empty directory so that we can copy the release packages into it later.

scripts/deploy/publish-build-artifacts.sh

@@ -45,11 +45,6 @@ publishPackage() {
   commitAuthorEmail=$(git --no-pager show -s --format='%ae' HEAD)
   commitMessage=$(git log --oneline -n 1)
 
-  # Note that we cannot store the commit SHA in its own version segment
-  # as it will not comply with the semver specification. For example:
-  # 1.0.0-00abcdef will break since the SHA starts with zeros. To fix this,
-  # we create a new version segment with the following format: "1.0.0-sha-00abcdef".
-  # See issue: https://jubianchi.github.io/semver-check/#/^8.0.0/8.2.2-0462599
   buildVersionName="${buildVersion}-sha-${commitSha}"
   buildTagName="${branchName}-${commitSha}"
   buildCommitMessage="${branchName} - ${commitMessage}"
@@ -99,12 +94,6 @@ publishPackage() {
     exit 0
   fi
 
-  # Replace the version in every file recursively with a more specific version that also includes
-  # the SHA of the current build job. Normally this "sed" call would just replace the version
-  # placeholder, but the version placeholders have been replaced by "npm_package" already.
-  escapedVersion=$(echo ${buildVersion} | sed 's/[.[\*^$]/\\&/g')
-  sed -i "s/${escapedVersion}/${buildVersionName}/g" $(find . -type f -not -path '*\/.*')
-
   echo "Updated the build version in every file to include the SHA of the latest commit."
 
   # Prepare Git for pushing the artifacts to the repository.

scripts/deploy/publish-docs-content.sh

@@ -41,11 +41,6 @@ commitAuthorName=$(git --no-pager show -s --format='%an' HEAD)
 commitAuthorEmail=$(git --no-pager show -s --format='%ae' HEAD)
 commitMessage=$(git log --oneline -n 1)
 
-# Note that we cannot store the commit SHA in its own version segment
-# as it will not comply with the semver specification. For example:
-# 1.0.0-00abcdef will break since the SHA starts with zeros. To fix this,
-# we create a new version segment with the following format: "1.0.0-sha-00abcdef".
-# See issue: https://jubianchi.github.io/semver-check/#/^8.0.0/8.2.2-0462599
 buildVersionName="${buildVersion}-sha-${commitSha}"
 buildTagName="${branchName}-${commitSha}"
 buildCommitMessage="${branchName} - ${commitMessage}"
@@ -95,12 +90,6 @@ if [[ $(git ls-remote origin "refs/tags/${buildTagName}") ]]; then
   exit 0
 fi
 
-# Replace the version in every file recursively with a more specific version that also includes
-# the SHA of the current build job. Normally this "sed" call would just replace the version
-# placeholder, but the version placeholders have been replaced by "npm_package" already.
-escapedVersion=$(echo ${buildVersion} | sed 's/[.[\*^$]/\\&/g')
-sed -i "s/${escapedVersion}/${buildVersionName}/g" $(find . -type f -not -path '*\/.*')
-
 # Setup the Git configuration
 git config user.name "$commitAuthorName"
 git config user.email "$commitAuthorEmail"

tools/bazel-stamp-vars.js

@@ -9,14 +9,13 @@
 
 const spawnSync = require('child_process').spawnSync;
 const packageJson = require('../package');
-
-const currentCommitSha = getCurrentCommitSha();
+const isSnapshotStamp = process.argv.slice(2).includes('--snapshot');
 
 // The "BUILD_SCM_VERSION" will be picked up by the "npm_package" and "ng_package"
 // rule in order to populate the "0.0.0-PLACEHOLDER". Note that the SHA will be only
-// appended for snapshots builds from within the "publish-build-artifacts.sh" script.
-console.log(`BUILD_SCM_VERSION ${packageJson.version}`);
-console.log(`BUILD_SCM_COMMIT_SHA ${currentCommitSha}`);
+// appended for snapshots builds (if the `--snapshot` flag has been passed to this script).
+console.log(`BUILD_SCM_VERSION ${getBuildVersion()}`);
+console.log(`BUILD_SCM_COMMIT_SHA ${getCurrentCommitSha()}`);
 console.log(`BUILD_SCM_BRANCH ${getCurrentBranchName()}`);
 console.log(`BUILD_SCM_USER ${getCurrentGitUser()}`);
 
@@ -25,6 +24,11 @@ function getCurrentCommitSha() {
   return spawnSync('git', ['rev-parse', 'HEAD']).stdout.toString().trim();
 }
 
+/** Returns the abbreviated SHA for the current git HEAD of the project. */
+function getAbbreviatedCommitSha() {
+  return spawnSync('git', ['rev-parse', '--short', 'HEAD']).stdout.toString().trim();
+}
+
 /** Returns the name of the currently checked out branch of the project. */
 function getCurrentBranchName() {
   return spawnSync('git', ['symbolic-ref', '--short', 'HEAD']).stdout.toString().trim();
@@ -37,3 +41,17 @@ function getCurrentGitUser() {
 
   return `${userName} <${userEmail}>`;
 }
+
+/** Gets the version for the current build. */
+function getBuildVersion() {
+  if (isSnapshotStamp) {
+    // Note that we cannot store the commit SHA as prerelease segment as it will not comply
+    // with the semver specification in some situations. For example: `1.0.0-00abcdef` will
+    // break since the SHA starts with zeros. To fix this, we create a prerelease segment with
+    // label where the SHA is considered part of the label and not the prerelease number.
+    // Here is an example of the valid format: "1.0.0-sha-00abcdef".
+    // See issue: https://jubianchi.github.io/semver-check/#/^8.0.0/8.2.2-0462599
+    return `${packageJson.version}-sha-${getAbbreviatedCommitSha()}`;
+  }
+  return packageJson.version;
+}

tools/release/check-release-output.ts

@@ -2,17 +2,16 @@ import chalk from 'chalk';
 import {join} from 'path';
 import {checkReleasePackage} from './release-output/check-package';
 import {releasePackages} from './release-output/release-packages';
-import {parseVersionName, Version} from './version-name/parse-version';
 
 /**
  * Checks the release output by running the release-output validations for each
  * release package.
  */
-export function checkReleaseOutput(releaseOutputDir: string, currentVersion: Version) {
+export function checkReleaseOutput(releaseOutputDir: string, expectedVersion: string) {
   let hasFailed = false;
 
   releasePackages.forEach(packageName => {
-    if (!checkReleasePackage(releaseOutputDir, packageName, currentVersion)) {
+    if (!checkReleasePackage(releaseOutputDir, packageName, expectedVersion)) {
       hasFailed = true;
     }
   });
@@ -30,9 +29,10 @@ export function checkReleaseOutput(releaseOutputDir: string, currentVersion: Ver
 
 
 if (require.main === module) {
-  const currentVersion = parseVersionName(require('../../package.json').version);
-  if (currentVersion === null) {
-    throw Error('Version in project "package.json" is invalid.');
+  const [expectedVersion] = process.argv.slice(2);
+  if (expectedVersion === undefined) {
+    console.error('No expected version specified. Please pass one as argument.');
+    process.exit(1);
   }
-  checkReleaseOutput(join(__dirname, '../../dist/releases'), currentVersion);
+  checkReleaseOutput(join(__dirname, '../../dist/releases'), expectedVersion);
 }

tools/release/publish-release.ts

@@ -16,7 +16,7 @@ import {parseVersionName, Version} from './version-name/parse-version';
 
 // The package builder script is not written in TypeScript and needs to
 // be imported through a CommonJS import.
-const {defaultBuildReleasePackages} = require('../../scripts/build-packages-dist');
+const {performNpmReleaseBuild} = require('../../scripts/build-packages-dist');
 
 /**
  * Class that can be instantiated in order to create a new release. The tasks requires user
@@ -90,11 +90,11 @@ class PublishReleaseTask extends BaseReleaseTask {
       await this._promptStableVersionForNextTag();
     }
 
-    defaultBuildReleasePackages();
+    performNpmReleaseBuild();
     console.info(chalk.green(`  ✓   Built the release output.`));
 
     // Checks all release packages against release output validations before releasing.
-    checkReleaseOutput(this.releaseOutputPath, this.currentVersion);
+    checkReleaseOutput(this.releaseOutputPath, this.currentVersion.format());
 
     // Extract the release notes for the new version from the changelog file.
     const extractedReleaseNotes = extractReleaseNotes(

tools/release/release-output/check-package.ts

@@ -2,7 +2,6 @@ import chalk from 'chalk';
 import {existsSync} from 'fs';
 import {sync as glob} from 'glob';
 import {join} from 'path';
-import {Version} from '../version-name/parse-version';
 
 import {
   checkCdkPackage,
@@ -35,7 +34,7 @@ type PackageFailures = Map<string, string[]>;
  * @returns Whether the package passed all checks or not.
  */
 export function checkReleasePackage(
-    releasesPath: string, packageName: string, currentVersion: Version): boolean {
+    releasesPath: string, packageName: string, expectedVersion: string): boolean {
   const packagePath = join(releasesPath, packageName);
   const failures = new Map() as PackageFailures;
   const addFailure = (message, filePath?) => {
@@ -82,7 +81,7 @@ export function checkReleasePackage(
     addFailure('No "README.md" file found in package output.');
   }
 
-  checkPrimaryPackageJson(join(packagePath, 'package.json'), currentVersion)
+  checkPrimaryPackageJson(join(packagePath, 'package.json'), expectedVersion)
       .forEach(f => addFailure(f));
 
   // In case there are failures for this package, we want to print those

tools/release/release-output/output-validations.ts

@@ -115,8 +115,7 @@ export function checkTypeDefinitionFile(filePath: string): string[] {
  * that the version and migrations are set up correctly.
  */
 export function checkPrimaryPackageJson(
-    packageJsonPath: string, currentVersion: Version): string[] {
-  const expectedVersion = currentVersion.format();
+    packageJsonPath: string, expectedVersion: string): string[] {
   const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8'));
   const failures: string[] = [];
 
@@ -125,11 +124,12 @@ export function checkPrimaryPackageJson(
   } else if (packageJson.version !== expectedVersion) {
     failures.push(
         `Unexpected package version. Expected: ${expectedVersion} but got: ${packageJson.version}`);
-  }
-
-  if (packageJson['ng-update'] && packageJson['ng-update'].migrations) {
+  } else if (semver.valid(expectedVersion) === null) {
+    failures.push(`Version does not satisfy SemVer specification: ${packageJson.version}`);
+  } else if (packageJson['ng-update'] && packageJson['ng-update'].migrations) {
     failures.push(...checkMigrationCollection(
-        packageJson['ng-update'].migrations, dirname(packageJsonPath), currentVersion));
+        packageJson['ng-update'].migrations, dirname(packageJsonPath),
+        semver.parse(expectedVersion)!));
   }
 
   return failures;
@@ -144,8 +144,11 @@ export function checkPackageJsonMigrations(
   const packageJson = JSON.parse(readFileSync(packageJsonPath, 'utf8'));
 
   if (packageJson['ng-update'] && packageJson['ng-update'].migrations) {
+    // TODO(devversion): switch release publish tooling to use `SemVer` instead
+    // of custom version parsing/serializing.
     return checkMigrationCollection(
-        packageJson['ng-update'].migrations, dirname(packageJsonPath), currentVersion);
+        packageJson['ng-update'].migrations, dirname(packageJsonPath),
+        semver.parse(currentVersion.format())!);
   }
   return [];
 }
@@ -185,7 +188,7 @@ export function checkCdkPackage(packagePath: string): string[] {
  * has a migration set up for the given target version.
  */
 function checkMigrationCollection(
-    collectionPath: string, packagePath: string, targetVersion: Version): string[] {
+    collectionPath: string, packagePath: string, targetVersion: semver.SemVer): string[] {
   const collection = JSON.parse(readFileSync(join(packagePath, collectionPath), 'utf8'));
   if (!collection.schematics) {
     return ['No schematics found in migration collection.'];
@@ -198,7 +201,7 @@ function checkMigrationCollection(
     const schematicVersion = schematics[name].version;
     try {
       return schematicVersion && semver.gte(schematicVersion, lowerBoundaryVersion) &&
-          semver.lte(schematicVersion, targetVersion.format());
+          semver.lte(schematicVersion, targetVersion);
     } catch {
       failures.push(`Could not parse version for migration: ${name}`);
     }