From a558cf18d81e0936795ca26340bb40cf482e7e86 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 22 Jul 2020 10:33:27 +0200 Subject: [PATCH] build: snapshot builds incorrectly modify semver versions 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: https://github.com/angular/material2-docs-content/commit/e624c7130d90026574b46d12d488e470f0a18c55 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. --- .bazelrc | 12 ++++-- .circleci/config.yml | 9 ++++- scripts/build-packages-dist.js | 37 +++++++++++++------ scripts/deploy/publish-build-artifacts.sh | 11 ------ scripts/deploy/publish-docs-content.sh | 11 ------ tools/bazel-stamp-vars.js | 28 +++++++++++--- tools/release/check-release-output.ts | 14 +++---- tools/release/publish-release.ts | 6 +-- tools/release/release-output/check-package.ts | 5 +-- .../release-output/output-validations.ts | 21 ++++++----- 10 files changed, 88 insertions(+), 66 deletions(-) diff --git a/.bazelrc b/.bazelrc index 9511066af1db..574b73e4b1fd 100644 --- a/.bazelrc +++ b/.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 # ################################ diff --git a/.circleci/config.yml b/.circleci/config.yml index e3d5a3507dc1..5b3005385db1 100644 --- a/.circleci/config.yml +++ b/.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 diff --git a/scripts/build-packages-dist.js b/scripts/build-packages-dist.js index 993c674b2ab0..47feade58d6a 100755 --- a/scripts/build-packages-dist.js +++ b/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. diff --git a/scripts/deploy/publish-build-artifacts.sh b/scripts/deploy/publish-build-artifacts.sh index 9f101f6ddee0..e055f7367601 100755 --- a/scripts/deploy/publish-build-artifacts.sh +++ b/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. diff --git a/scripts/deploy/publish-docs-content.sh b/scripts/deploy/publish-docs-content.sh index 502a7a56265e..a9995b0a32c4 100755 --- a/scripts/deploy/publish-docs-content.sh +++ b/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" diff --git a/tools/bazel-stamp-vars.js b/tools/bazel-stamp-vars.js index 488df05d7d98..ed275e3f7590 100644 --- a/tools/bazel-stamp-vars.js +++ b/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; +} diff --git a/tools/release/check-release-output.ts b/tools/release/check-release-output.ts index babed12be7a4..7fb8686f5ced 100644 --- a/tools/release/check-release-output.ts +++ b/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); } diff --git a/tools/release/publish-release.ts b/tools/release/publish-release.ts index ac093d5d7a75..b36d46d56d15 100644 --- a/tools/release/publish-release.ts +++ b/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( diff --git a/tools/release/release-output/check-package.ts b/tools/release/release-output/check-package.ts index 1aa5631cf240..aaef47408910 100644 --- a/tools/release/release-output/check-package.ts +++ b/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; * @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 diff --git a/tools/release/release-output/output-validations.ts b/tools/release/release-output/output-validations.ts index 99ae03da37e4..752a296c00cb 100644 --- a/tools/release/release-output/output-validations.ts +++ b/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}`); }