From 720d74203a7c026c1583f3a86ac47745d5082923 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Thu, 11 Mar 2021 15:08:28 -0500 Subject: [PATCH 1/2] Clean up CircleCI merge SHA logic --- .circleci/compute_merge_commit.sh | 43 +++++++++++++++++++ .circleci/config.yml | 19 ++++----- .circleci/fetch_merge_commit.sh | 55 +++++-------------------- .circleci/get_pr_number.sh | 55 +++++++++++++++++++++++++ .gitignore | 2 +- build-system/common/ci.js | 10 ++--- build-system/tasks/bundle-size/index.js | 15 ++++--- 7 files changed, 132 insertions(+), 67 deletions(-) create mode 100755 .circleci/compute_merge_commit.sh create mode 100755 .circleci/get_pr_number.sh diff --git a/.circleci/compute_merge_commit.sh b/.circleci/compute_merge_commit.sh new file mode 100755 index 000000000000..0aaad50c2560 --- /dev/null +++ b/.circleci/compute_merge_commit.sh @@ -0,0 +1,43 @@ +#!/bin/bash +# +# Copyright 2021 The AMP HTML Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS-IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the license. + +# This script establishes the merge commit at the start of a CircleCI build so +# all stages use the same commit. + +set -e +err=0 + +GREEN() { echo -e "\n\033[0;32m$1\033[0m"; } + +# Try to determine the PR number. +curl -sS https://raw.githubusercontent.com/rsimha/amphtml/2021-03-11-CircleConfigUpdates/.circleci/get_pr_number.sh | bash +source $BASH_ENV + +# If PR_NUMBER doesn't exist, there is nothing more to do. +if [[ -z "$PR_NUMBER" ]]; then + exit 0 +fi + +# GitHub provides refs/pull//merge, an up-to-date merge branch for +# every PR branch that can be cleanly merged to master. For more details, see: +# https://discuss.circleci.com/t/show-test-results-for-prospective-merge-of-a-github-pr/1662 +MERGE_BRANCH="refs/pull/$PR_NUMBER/merge" +echo $(GREEN "Computing merge SHA of $MERGE_BRANCH...") +CIRCLE_MERGE_SHA="$(git ls-remote https://github.com/ampproject/amphtml.git "$MERGE_BRANCH" | awk '{print $1}')" + +echo "$CIRCLE_MERGE_SHA" > .CIRCLECI_MERGE_COMMIT +echo $(GREEN "Stored merge SHA $CIRCLE_MERGE_SHA in .CIRCLECI_MERGE_COMMIT.") +exit 0 diff --git a/.circleci/config.yml b/.circleci/config.yml index 651a3daba8bd..09a2a07d2c94 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -17,6 +17,10 @@ push_builds_only: &push_builds_only - /^amp-release-.*$/ executors: + amphtml-docker-small-executor: + docker: + - image: cimg/base:2020.12 + resource_class: small amphtml-medium-executor: machine: image: ubuntu-2004:202010-01 @@ -29,11 +33,6 @@ executors: machine: image: ubuntu-2004:202010-01 resource_class: xlarge - amphtml-docker-executor: - docker: - # The official, bare bones Docker Image from CircleCI - # https://circleci.com/docs/2.0/circleci-images/?section=executors-and-images#circleci-base-image - - image: cimg/base:2020.12 commands: save_merge_commit: @@ -42,7 +41,7 @@ commands: name: 'Save Merge Commit' key: git-{{ .Branch }}-{{ .Revision }} paths: - - .CIRCLECI_WORKFLOW_MERGE_COMMIT + - .CIRCLECI_MERGE_COMMIT restore_merge_commit: steps: - restore_cache: @@ -54,7 +53,7 @@ commands: - restore_merge_commit - run: name: 'Fetch Merge Commit' - command: ./.circleci/fetch_merge_commit.sh merge + command: ./.circleci/fetch_merge_commit.sh - run: name: 'Check Config' command: ./.circleci/check_config.sh @@ -81,11 +80,11 @@ commands: jobs: 'Compute Merge Commit': executor: - name: amphtml-docker-executor + name: amphtml-docker-small-executor steps: - run: - name: 'Fetch Merge Commit' - command: curl -o- https://raw.githubusercontent.com/ampproject/amphtml/master/.circleci/fetch_merge_commit.sh | bash /dev/stdin fetch + name: 'Compute Merge Commit' + command: curl -sS https://raw.githubusercontent.com/rsimha/amphtml/2021-03-11-CircleConfigUpdates/.circleci/compute_merge_commit.sh | bash - save_merge_commit 'Checks': executor: diff --git a/.circleci/fetch_merge_commit.sh b/.circleci/fetch_merge_commit.sh index 89535a8b43fc..22f4de959f3e 100755 --- a/.circleci/fetch_merge_commit.sh +++ b/.circleci/fetch_merge_commit.sh @@ -16,64 +16,29 @@ # This script fetches the merge commit of a PR branch with master to make sure # PRs are tested against all the latest changes on CircleCI. -# Reference: https://circleci.com/docs/2.0/env-vars/#built-in-environment-variables set -e err=0 GREEN() { echo -e "\n\033[0;32m$1\033[0m"; } -YELLOW() { echo -e "\n\033[0;33m$1\033[0m"; } RED() { echo -e "\n\033[0;31m$1\033[0m"; } -if [[ -z "$1" ]]; then - echo "Usage: fetch_merge_commit.sh [fetch | merge]" - exit 1 -fi +# Try to determine the PR number. +./.circleci/get_pr_number.sh +source $BASH_ENV -# Push builds are only run against master and amp-release branches. -if [[ "$CIRCLE_BRANCH" == "master" || "$CIRCLE_BRANCH" =~ ^amp-release-* ]]; then - echo $(GREEN "Nothing to do because $CIRCLE_BRANCH is not a PR branch.") - # Warn if the build is linked to a PR on a different repo (known CircleCI bug). - if [[ -n "$CIRCLE_PULL_REQUEST" && ! "$CIRCLE_PULL_REQUEST" =~ ^https://github.com/ampproject/amphtml* ]]; then - echo $(YELLOW "WARNING: Build is incorrectly linked to a PR outside ampproject/amphtml:") - echo $(YELLOW "$CIRCLE_PULL_REQUEST") - fi - exit 0 -fi - -# CIRCLE_PR_NUMBER is present for PRs originating from forks, but absent for PRs -# originating from a branch on the main repo. In such cases, extract the PR -# number from CIRCLE_PULL_REQUEST. -if [[ "$CIRCLE_PR_NUMBER" ]]; then - PR_NUMBER=$CIRCLE_PR_NUMBER -else - PR_NUMBER=${CIRCLE_PULL_REQUEST#"https://github.com/ampproject/amphtml/pull/"} -fi - -# If neither CIRCLE_PR_NUMBER nor CIRCLE_PULL_REQUEST are available, it's -# possible this is a PR branch that is yet to be associated with a PR. Exit -# early becaue there is no merge commit to fetch. +# If PR_NUMBER doesn't exist, there is nothing more to do. if [[ -z "$PR_NUMBER" ]]; then - echo $(GREEN "Nothing to do because $CIRCLE_BRANCH is not yet linked to a PR.") exit 0 fi -if [[ "$1" == "fetch" ]]; then - # GitHub provides refs/pull//merge, an up-to-date merge branch for - # every PR branch that can be cleanly merged to master. For more details, see: - # https://discuss.circleci.com/t/show-test-results-for-prospective-merge-of-a-github-pr/1662 - MERGE_BRANCH="refs/pull/$PR_NUMBER/merge" - echo $(GREEN "Fetching merge SHA from $MERGE_BRANCH...") - - CIRCLE_MERGE_SHA="$(git ls-remote https://github.com/ampproject/amphtml.git "$MERGE_BRANCH" | awk '{print $1}')" - echo $(GREEN "Fetched merge SHA $CIRCLE_MERGE_SHA...") - echo "$CIRCLE_MERGE_SHA" > .CIRCLECI_WORKFLOW_MERGE_COMMIT - exit 0 -fi +# Extract the merge commit for this workflow and make it visible to other steps. +CIRCLECI_MERGE_COMMIT="$(cat .CIRCLECI_MERGE_COMMIT)" +echo "export CIRCLECI_MERGE_COMMIT=$CIRCLECI_MERGE_COMMIT" >> $BASH_ENV -export CIRCLE_MERGE_SHA="$(cat .CIRCLECI_WORKFLOW_MERGE_COMMIT)" -echo $(GREEN "Fetching merge commit $CIRCLE_MERGE_SHA...") -(set -x && git pull --ff-only origin "$CIRCLE_MERGE_SHA") || err=$? +# Fetch the merge commit. This ensures that all CI stages use the same commit. +echo $(GREEN "Fetching merge commit $CIRCLECI_MERGE_COMMIT...") +(set -x && git pull --ff-only origin "$CIRCLECI_MERGE_COMMIT") || err=$? # If a clean merge is not possible, do not proceed with the build. GitHub's UI # will show an error indicating there was a merge conflict. diff --git a/.circleci/get_pr_number.sh b/.circleci/get_pr_number.sh new file mode 100755 index 000000000000..61ab493125e6 --- /dev/null +++ b/.circleci/get_pr_number.sh @@ -0,0 +1,55 @@ +#!/bin/bash +# +# Copyright 2021 The AMP HTML Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS-IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the license. + +# This script extracts the PR number (if there is one) for a CircleCI build. +# Reference: https://circleci.com/docs/2.0/env-vars/#built-in-environment-variables + +set -e +err=0 + +GREEN() { echo -e "\n\033[0;32m$1\033[0m"; } +YELLOW() { echo -e "\n\033[0;33m$1\033[0m"; } + +# Push builds are only run against master and amp-release branches. +if [[ "$CIRCLE_BRANCH" == "master" || "$CIRCLE_BRANCH" =~ ^amp-release-* ]]; then + echo $(GREEN "Nothing to do because $CIRCLE_BRANCH is not a PR branch.") + # Warn if the build is linked to a PR on a different repo (known CircleCI bug). + if [[ -n "$CIRCLE_PULL_REQUEST" && ! "$CIRCLE_PULL_REQUEST" =~ ^https://github.com/ampproject/amphtml* ]]; then + echo $(YELLOW "WARNING: Build is incorrectly linked to a PR outside ampproject/amphtml:") + echo $(YELLOW "$CIRCLE_PULL_REQUEST") + fi + exit 0 +fi + +# CIRCLE_PR_NUMBER is present for PRs originating from forks, but absent for PRs +# originating from a branch on the main repo. In such cases, extract the PR +# number from CIRCLE_PULL_REQUEST. +if [[ "$CIRCLE_PR_NUMBER" ]]; then + PR_NUMBER=$CIRCLE_PR_NUMBER +else + PR_NUMBER=${CIRCLE_PULL_REQUEST#"https://github.com/ampproject/amphtml/pull/"} +fi + +# If neither CIRCLE_PR_NUMBER nor CIRCLE_PULL_REQUEST are available, it's +# possible this is a PR branch that is yet to be associated with a PR. Exit +# early becaue there is no merge commit to fetch. +if [[ -z "$PR_NUMBER" ]]; then + echo $(GREEN "Nothing to do because $CIRCLE_BRANCH is not yet linked to a PR.") + exit 0 +fi + +echo "export PR_NUMBER=$PR_NUMBER" >> $BASH_ENV +echo $(GREEN "This is a PR build for #$PR_NUMBER.") diff --git a/.gitignore b/.gitignore index 22f5f9f6d580..5d088532bc29 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,6 @@ .DS_Store .g4ignore -.CIRCLECI_WORKFLOW_MERGE_COMMIT +.CIRCLECI_MERGE_COMMIT build/ .amp-dep-check c diff --git a/build-system/common/ci.js b/build-system/common/ci.js index 49ce7bf9ae0c..7c2015135fcd 100644 --- a/build-system/common/ci.js +++ b/build-system/common/ci.js @@ -195,12 +195,12 @@ function ciJobUrl() { } /** - * Returns the merge commits SHA when running a CI Pull Request build. + * Returns the merge commit for a CircleCI PR build. CIRCLECI_MERGE_COMMIT is + * populated by .circleci/fetch_merge_commit.sh. * @return {string} */ -function circleciPrMergeSha() { - // CIRCLE_MERGE_SHA is populated by .circleci/fetch_merge_commit.sh. - return isCircleci ? env('CIRCLE_MERGE_SHA') : ''; +function circleciPrMergeCommit() { + return isCircleci ? env('CIRCLECI_MERGE_COMMIT') : ''; } /** @@ -230,10 +230,10 @@ module.exports = { ciCommitSha, ciJobId, ciJobUrl, - circleciPrMergeSha, ciPullRequestBranch, ciPullRequestSha, ciPushBranch, + circleciPrMergeCommit, ciRepoSlug, isCiBuild, isCircleciBuild, diff --git a/build-system/tasks/bundle-size/index.js b/build-system/tasks/bundle-size/index.js index 0231fff18a3c..ae13a501407d 100644 --- a/build-system/tasks/bundle-size/index.js +++ b/build-system/tasks/bundle-size/index.js @@ -29,8 +29,8 @@ const { isPullRequestBuild, isPushBuild, ciPushBranch, + circleciPrMergeCommit, ciRepoSlug, - circleciPrMergeSha, } = require('../../common/ci'); const { VERSION: internalRuntimeVersion, @@ -171,24 +171,27 @@ async function skipBundleSize() { */ async function reportBundleSize() { if (isPullRequestBuild()) { + const headSha = gitCommitHash(); const baseSha = gitCiMasterBaseline(); - const commitHash = gitCommitHash(); + const mergeSha = circleciPrMergeCommit(); log( 'Reporting bundle sizes for commit', - cyan(shortSha(commitHash)), + cyan(shortSha(headSha)), 'using baseline commit', - cyan(shortSha(baseSha)) + '...' + cyan(shortSha(baseSha)), + 'and merge commit', + cyan(shortSha(mergeSha)) + '...' ); try { const response = await requestPost({ uri: url.resolve( bundleSizeAppBaseUrl, - path.join('commit', commitHash, 'report') + path.join('commit', headSha, 'report') ), json: true, body: { baseSha, - mergeSha: circleciPrMergeSha(), + mergeSha, bundleSizes: await getBrotliBundleSizes(), }, }); From 736dd0a1a2554f8ee7e9d115ef768a0a962edb71 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Thu, 11 Mar 2021 21:47:07 -0500 Subject: [PATCH 2/2] Fix `curl` URLs --- .circleci/compute_merge_commit.sh | 2 +- .circleci/config.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.circleci/compute_merge_commit.sh b/.circleci/compute_merge_commit.sh index 0aaad50c2560..a2df9ee0a8bd 100755 --- a/.circleci/compute_merge_commit.sh +++ b/.circleci/compute_merge_commit.sh @@ -23,7 +23,7 @@ err=0 GREEN() { echo -e "\n\033[0;32m$1\033[0m"; } # Try to determine the PR number. -curl -sS https://raw.githubusercontent.com/rsimha/amphtml/2021-03-11-CircleConfigUpdates/.circleci/get_pr_number.sh | bash +curl -sS https://raw.githubusercontent.com/ampproject/amphtml/master/.circleci/get_pr_number.sh | bash source $BASH_ENV # If PR_NUMBER doesn't exist, there is nothing more to do. diff --git a/.circleci/config.yml b/.circleci/config.yml index 09a2a07d2c94..5dc6b18794ee 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -84,7 +84,7 @@ jobs: steps: - run: name: 'Compute Merge Commit' - command: curl -sS https://raw.githubusercontent.com/rsimha/amphtml/2021-03-11-CircleConfigUpdates/.circleci/compute_merge_commit.sh | bash + command: curl -sS https://raw.githubusercontent.com/ampproject/amphtml/master/.circleci/compute_merge_commit.sh | bash - save_merge_commit 'Checks': executor: