From 1177b4e2f85d052aafc6e2ff173f07033878ddcd Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 25 Mar 2022 21:45:27 +0100 Subject: [PATCH] ci: make payload size tracking script work with CircleCI and Bazel limitation (#45444) For quite some time now, since we started to use Bazel for integration tests, we relied on some size tracking logic that did not actually fully work under Bazel. It was thought that all the necessary CI push/PR information is available to the Bazel test, but that was not the case. This was now fixed with the recent Rules NodeJS v5 update where I made sure the `env.sh` variables are actually available before we write them to the temporary file for the Bazel-access. This now will unveil an issue because payload size goldens would start being based on their branch name. e.g. the golden key in `13.3.x` should not be `master` but `13.3.x`. This makes more sense than `master` as key, but makes things more cumbersome and ideally we would not store the branch name at all (this is a larger change though -- not worth now since we might refactor this anyway). For now we will update the size tracking logic to always use `master` as golden key (like it worked in the past year(s)) With the environment fix we now (again) start uploading payload size results to Firebase. This did not work by accident either. The uploading logic is reliant on the CircleCI commit range which is not working/reliable in upstream branches. This commit removes this reliance on `COMMIT_RANGE` since it's not strictly necessary and currently breaking renovate PRs. We can re-enable this when we have a solution with CircleCI, or a workaround/resolution logic provided in e.g. `ng-dev ci determine-commit-range`. PR Close #45444 --- .circleci/env.sh | 1 - scripts/ci/payload-size.js | 7 +++++-- scripts/ci/payload-size.sh | 18 ++++++------------ 3 files changed, 11 insertions(+), 15 deletions(-) diff --git a/.circleci/env.sh b/.circleci/env.sh index 6543b62b2de..1e997f97173 100755 --- a/.circleci/env.sh +++ b/.circleci/env.sh @@ -85,7 +85,6 @@ echo "export PROJECT_ROOT=\"${PROJECT_ROOT}\";" >> $bazelVarEnv echo "export CI_BRANCH=\"${CI_BRANCH}\";" >> $bazelVarEnv echo "export CI_BUILD_URL=\"${CI_BUILD_URL}\";" >> $bazelVarEnv echo "export CI_COMMIT=\"${CI_COMMIT}\";" >> $bazelVarEnv -echo "export CI_COMMIT_RANGE=\"${CI_COMMIT_RANGE}\";" >> $bazelVarEnv echo "export CI_PULL_REQUEST=\"${CI_PULL_REQUEST}\";" >> $bazelVarEnv echo "export CI_REPO_NAME=\"${CI_REPO_NAME}\";" >> $bazelVarEnv echo "export CI_REPO_OWNER=\"${CI_REPO_OWNER}\";" >> $bazelVarEnv diff --git a/scripts/ci/payload-size.js b/scripts/ci/payload-size.js index 90f8da78399..0f2eaf2ced4 100644 --- a/scripts/ci/payload-size.js +++ b/scripts/ci/payload-size.js @@ -13,12 +13,15 @@ const fs = require('fs'); const path = require('path'); // Get branch and project name from command line arguments. -const [, , limitFile, project, branch, commit] = process.argv; +const [, , limitFile, project, commit] = process.argv; // Load sizes. const currentSizes = JSON.parse(fs.readFileSync('/tmp/current.log', 'utf8')); const allLimitSizes = JSON.parse(fs.readFileSync(limitFile, 'utf8')); -const limitSizes = allLimitSizes[project][branch] || allLimitSizes[project]['master']; + +// TODO: Change the `master` golden key to something more obvious, or remove it. The branch +// name is unreasonable since the limits are always taken from the currently checked-out revision. +const limitSizes = allLimitSizes[project]['master']; // Check current sizes against limits. let failed = false; diff --git a/scripts/ci/payload-size.sh b/scripts/ci/payload-size.sh index f036834c63b..496d0acdfdb 100644 --- a/scripts/ci/payload-size.sh +++ b/scripts/ci/payload-size.sh @@ -66,9 +66,8 @@ checkSize() { name="$1" limitFile="$2" - # In non-PR builds, `CI_BRANCH` is the branch being built (e.g. `pull/12345`), not the targeted branch. - # Thus, PRs will fall back to using the size limits for `master`. - node ${PROJECT_ROOT}/scripts/ci/payload-size.js $limitFile $name ${CI_BRANCH:-} ${CI_COMMIT:-} + # PRs and non-PR pushes will always test against the size-limits of the current revision. + node ${PROJECT_ROOT}/scripts/ci/payload-size.js $limitFile $name ${CI_COMMIT:-} } # Write timestamp to global variable `$payloadData`. @@ -87,15 +86,10 @@ addBuildUrl() { payloadData="$payloadData\"buildUrl\": \"$buildUrl\", " } -# Write the commit message for the current CI commit range to global variable `$payloadData`. -# $1: string - The commit range for this build (in `...` format). +# Write the commit message for the specified CI commit to global variable `$payloadData`. +# $1: string - The commit SHA for this build (in `` format). addMessage() { - commitRange="$1" - - # Grab the set of SHAs for the message. This can fail when you force push or do initial build - # because $CI_COMMIT_RANGE may contain the previous SHA which will not be in the - # force push or commit, hence we default to last commit. - message=$(git --git-dir ${PROJECT_ROOT}/.git log --oneline $commitRange -- || git --git-dir ${PROJECT_ROOT}/.git log --oneline -n1) + message="${1}" message=$(echo $message | sed 's/\r//g' | sed 's/\\/\\\\/g' | sed 's/"/\\"/g') payloadData="$payloadData\"message\": \"$message\", " } @@ -147,7 +141,7 @@ trackPayloadSize() { echo "Uploading data for '$name'..." addTimestamp addBuildUrl $CI_BUILD_URL - addMessage $CI_COMMIT_RANGE + addMessage $CI_COMMIT uploadData $name else echo "Skipped uploading data for '$name', because this is a pull request."