From 2aa0bb3809f4972dcac9c9efe36192a1498f89fc Mon Sep 17 00:00:00 2001 From: James Wainwright Date: Mon, 15 Jan 2024 10:57:44 +0000 Subject: [PATCH 1/4] [ci] Use ci/bazelisk.sh script in CI workflow Signed-off-by: James Wainwright --- azure-pipelines.yml | 8 ++++---- ci/scripts/check-bazel-banned-rules.sh | 2 +- ci/scripts/check-bazel-tags.sh | 2 +- ci/scripts/check-licence-headers.sh | 2 +- ci/scripts/check-module-ids.sh | 2 +- ci/scripts/clang-format.sh | 2 +- ci/scripts/get-bitstream-strategy.sh | 4 ++-- ci/scripts/lib/bazel_query.py | 2 +- ci/scripts/rust-format.sh | 2 +- 9 files changed, 13 insertions(+), 13 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4ae605f82e599..c48b9b0de6d15 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -221,7 +221,7 @@ jobs: echo -//sw/otbn/crypto/... >> "${TARGET_PATTERN_FILE}" echo -//third_party/riscv-compliance/... >> "${TARGET_PATTERN_FILE}" echo -//hw:all >> "${TARGET_PATTERN_FILE}" - ./bazelisk.sh cquery \ + ci/bazelisk.sh cquery \ --noinclude_aspects \ --output=starlark \ --starlark:expr='"-{}".format(target.label)' \ @@ -247,8 +247,8 @@ jobs: ROM_REAL_TARGETS="//sw/device/silicon_creator/rom:package_real" ROM_FAKE_TARGETS="//sw/device/silicon_creator/rom:package_fake" QUERY_CMD_ARGS=(outquery-all --noinclude_aspects --noimplicit_deps) - ROM_REAL_FILES=($(./bazelisk.sh "${QUERY_CMD_ARGS[@]}" "${ROM_REAL_TARGETS}" | sort | uniq)) - ROM_FAKE_FILES=($(./bazelisk.sh "${QUERY_CMD_ARGS[@]}" "${ROM_FAKE_TARGETS}" | sort | uniq)) + ROM_REAL_FILES=($(ci/bazelisk.sh "${QUERY_CMD_ARGS[@]}" "${ROM_REAL_TARGETS}" | sort | uniq)) + ROM_FAKE_FILES=($(ci/bazelisk.sh "${QUERY_CMD_ARGS[@]}" "${ROM_FAKE_TARGETS}" | sort | uniq)) cp -Lvt "${ROM_TARGET}" "${ROM_FAKE_FILES[@]}" "${ROM_REAL_FILES[@]}" - template: ci/upload-artifacts-template.yml parameters: @@ -347,7 +347,7 @@ jobs: - bash: | . util/build_consts.sh mkdir -p "$BIN_DIR/hw/top_earlgrey/" - cp $(./bazelisk.sh outquery-build.verilator //sw/device/tests:uart_smoketest_sim_verilator) \ + cp $(ci/bazelisk.sh outquery-build.verilator //sw/device/tests:uart_smoketest_sim_verilator) \ "$BIN_DIR/hw/top_earlgrey/Vchip_earlgrey_verilator" displayName: Copy verilated binary to $BIN_DIR - template: ci/upload-artifacts-template.yml diff --git a/ci/scripts/check-bazel-banned-rules.sh b/ci/scripts/check-bazel-banned-rules.sh index ef605c7b213be..7ec3da8733910 100755 --- a/ci/scripts/check-bazel-banned-rules.sh +++ b/ci/scripts/check-bazel-banned-rules.sh @@ -6,7 +6,7 @@ set -e -GIT_REPOS=$(./bazelisk.sh query "kind('(new_)?git_repository', //external:*)") +GIT_REPOS=$(ci/bazelisk.sh query "kind('(new_)?git_repository', //external:*)") if [[ ${GIT_REPOS} ]]; then echo "Bazel's 'git_repository' rule is insecure and incompatible with OpenTitan's airgapping strategy." echo "Please replace $GIT_REPOS with our 'http_archive_or_local' rule and set a sha256 so it can be canonically reproducible." diff --git a/ci/scripts/check-bazel-tags.sh b/ci/scripts/check-bazel-tags.sh index 915432cdca635..9a4df57fdd780 100755 --- a/ci/scripts/check-bazel-tags.sh +++ b/ci/scripts/check-bazel-tags.sh @@ -25,7 +25,7 @@ check_empty () { # This check ensures OpenTitan software can be built with a wildcard without # waiting for Verilator using --build_tag_filters=-verilator -untagged=$(./bazelisk.sh query \ +untagged=$(ci/bazelisk.sh query \ "rdeps( //..., //hw:verilator diff --git a/ci/scripts/check-licence-headers.sh b/ci/scripts/check-licence-headers.sh index 220f683974d28..533120ed89e1d 100755 --- a/ci/scripts/check-licence-headers.sh +++ b/ci/scripts/check-licence-headers.sh @@ -30,7 +30,7 @@ set -o pipefail (for F in $(git diff --name-only --diff-filter=ACMRTUXB "$merge_base"); do echo "--test_arg=\"$F\"" done)| \ - xargs -r ./bazelisk.sh test //quality:license_check --test_output=streamed || { + xargs -r ci/bazelisk.sh test //quality:license_check --test_output=streamed || { echo >&2 -n "##vso[task.logissue type=error]" echo >&2 "Licence header check failed." diff --git a/ci/scripts/check-module-ids.sh b/ci/scripts/check-module-ids.sh index 246822491b00a..370d04e8bff32 100755 --- a/ci/scripts/check-module-ids.sh +++ b/ci/scripts/check-module-ids.sh @@ -24,7 +24,7 @@ query_elfs=' )' target_file=$(mktemp) trap 'rm -f "$target_file"' EXIT -./bazelisk.sh query "$query_elfs" >"$target_file" +ci/bazelisk.sh query "$query_elfs" >"$target_file" # We now ask bazel to build all targets but we also add the module ID checker aspect # and we query the corresponding output group to force bazel to run the checks. ./ci/bazelisk.sh build \ diff --git a/ci/scripts/clang-format.sh b/ci/scripts/clang-format.sh index 288c3f81b3b16..f23f3fb7d7858 100755 --- a/ci/scripts/clang-format.sh +++ b/ci/scripts/clang-format.sh @@ -28,4 +28,4 @@ set -o pipefail (for F in $(git diff --name-only "$merge_base" -- "*.cpp" "*.cc" "*.c" "*.h" ':!*/vendor/*'); do echo "--test_arg=\"$F\"" done) | \ - xargs -r ./bazelisk.sh test //quality:clang_format_check --test_output=streamed + xargs -r ci/bazelisk.sh test //quality:clang_format_check --test_output=streamed diff --git a/ci/scripts/get-bitstream-strategy.sh b/ci/scripts/get-bitstream-strategy.sh index 45e67dfe6824e..f71e480c67d76 100755 --- a/ci/scripts/get-bitstream-strategy.sh +++ b/ci/scripts/get-bitstream-strategy.sh @@ -33,12 +33,12 @@ else fi # Retrieve the most recent bitstream at or before HEAD. -BITSTREAM="--refresh HEAD" ./bazelisk.sh build @bitstreams//:manifest +BITSTREAM="--refresh HEAD" ci/bazelisk.sh build @bitstreams//:manifest # Find the bitstream commit from the manifest file. manifest_file=$(ci/scripts/target-location.sh @bitstreams//:manifest) -bitstream_commit=$(./bazelisk.sh run //util/py/scripts:get_bitstream_build_id \ +bitstream_commit=$(ci/bazelisk.sh run //util/py/scripts:get_bitstream_build_id \ -- --manifest ${manifest_file} \ --schema ${REPO_TOP}/rules/scripts/bitstreams_manifest.schema.json \ --design ${bitstream_design}) diff --git a/ci/scripts/lib/bazel_query.py b/ci/scripts/lib/bazel_query.py index e6fbbf2d54571..ca9f944ca0a8d 100644 --- a/ci/scripts/lib/bazel_query.py +++ b/ci/scripts/lib/bazel_query.py @@ -108,7 +108,7 @@ def query(self, query: str) -> List[str]: return self._backend(query) bazel = subprocess.run( - ["./bazelisk.sh", "query", "--output=label", query], + ["ci/bazelisk.sh", "query", "--output=label", query], stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, encoding='utf-8', diff --git a/ci/scripts/rust-format.sh b/ci/scripts/rust-format.sh index 557bb15fdf9d0..bffd0ff7f97b3 100755 --- a/ci/scripts/rust-format.sh +++ b/ci/scripts/rust-format.sh @@ -27,7 +27,7 @@ trap 'echo "code failed rustfmt_check fix with ./bazelisk.sh run //quality:rustf set -o pipefail if ! git diff --quiet $merge_base -- "*.rs" ':!*/vendor/*'; then echo "Rust files changed, running Rust lint checks" - ./bazelisk.sh test //quality:rustfmt_check --test_output=streamed + ci/bazelisk.sh test //quality:rustfmt_check --test_output=streamed else echo "Rust files unchanged, skipping Rust lint checks" fi From 6c251f3f856acd067217cde00d2bfae1ef2b27f1 Mon Sep 17 00:00:00 2001 From: James Wainwright Date: Mon, 15 Jan 2024 14:01:03 +0000 Subject: [PATCH 2/4] [ci] Add Bazel cache GCP key to more jobs Signed-off-by: James Wainwright --- azure-pipelines.yml | 60 +++++++---------------------- ci/bazelisk.sh | 5 ++- ci/load-bazel-cache-write-creds.yml | 17 ++++++++ 3 files changed, 33 insertions(+), 49 deletions(-) create mode 100644 ci/load-bazel-cache-write-creds.yml diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c48b9b0de6d15..aa2699d8a8c90 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -46,6 +46,7 @@ jobs: - publish: $(Pipeline.Workspace)/opentitan-repo.tar.gz artifact: opentitan-repo displayName: Upload repository + - job: lint displayName: Quality (quick lint) # Run code quality checks (quick lint) @@ -179,23 +180,13 @@ jobs: dependsOn: lint condition: and(succeeded(), eq(dependencies.lint.outputs['DetermineBuildType.onlyDocChanges'], '0'), eq(dependencies.lint.outputs['DetermineBuildType.onlyCdcChanges'], '0')) pool: ci-public - variables: - - name: bazelCacheGcpKeyPath - value: '' steps: - template: ci/checkout-template.yml - template: ci/install-package-dependencies.yml - - task: DownloadSecureFile@1 - condition: eq(variables['Build.SourceBranchName'], 'master') - name: bazelCacheGcpKey - inputs: - secureFile: "bazel_cache_gcp_key.json" - # Set the remote cache GCP key path - - bash: echo "##vso[task.setvariable variable=bazelCacheGcpKeyPath]$(bazelCacheGcpKey.secureFilePath)" - condition: eq(variables['Build.SourceBranchName'], 'master') - displayName: GCP key path + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -x -e + # Check the entire build graph for conflicts in loading or analysis # phases. For context, see issue #18726. # First, test with an empty bitstream cache entry. @@ -214,7 +205,6 @@ jobs: # shallow exclusion; tests deeper under //hw will still be found. # * It excludes targets that depend on bitstream_splice rules, since the # environment does not have access to Vivado. - export GCP_BAZEL_CACHE_KEY=$(bazelCacheGcpKeyPath) TARGET_PATTERN_FILE=target_pattern.txt echo //... > "${TARGET_PATTERN_FILE}" echo -//quality/... >> "${TARGET_PATTERN_FILE}" @@ -260,21 +250,10 @@ jobs: timeoutInMinutes: 120 dependsOn: sw_build pool: ci-public - variables: - - name: bazelCacheGcpKeyPath - value: '' steps: - template: ci/checkout-template.yml - template: ci/install-package-dependencies.yml - - task: DownloadSecureFile@1 - condition: eq(variables['Build.SourceBranchName'], 'master') - name: bazelCacheGcpKey - inputs: - secureFile: "bazel_cache_gcp_key.json" - # Set the remote cache GCP key path - - bash: echo "##vso[task.setvariable variable=bazelCacheGcpKeyPath]$(bazelCacheGcpKey.secureFilePath)" - condition: eq(variables['Build.SourceBranchName'], 'master') - displayName: GCP key path + - template: ci/load-bazel-cache-write-creds.yml - download: current artifact: target_pattern_file - bash: | @@ -317,24 +296,12 @@ jobs: pool: ci-public timeoutInMinutes: 240 dependsOn: lint - variables: - - name: bazelCacheGcpKeyPath - value: '' steps: - template: ci/checkout-template.yml - template: ci/install-package-dependencies.yml - - task: DownloadSecureFile@1 - condition: eq(variables['Build.SourceBranchName'], 'master') - name: bazelCacheGcpKey - inputs: - secureFile: "bazel_cache_gcp_key.json" - - bash: echo "##vso[task.setvariable variable=bazelCacheGcpKeyPath]$(bazelCacheGcpKey.secureFilePath)" - condition: eq(variables['Build.SourceBranchName'], 'master') - displayName: GCP key path - # Set the remote cache GCP key path + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -x -e - export GCP_BAZEL_CACHE_KEY=$(bazelCacheGcpKeyPath) ci/scripts/run-verilator-tests.sh displayName: Build & execute tests - template: ci/publish-bazel-test-results.yml @@ -374,6 +341,7 @@ jobs: parameters: downloadPartialBuildBinFrom: - chip_englishbreakfast_verilator + - template: ci/load-bazel-cache-write-creds.yml - bash: | . util/build_consts.sh ci/scripts/run-english-breakfast-verilator-tests.sh @@ -435,15 +403,7 @@ jobs: steps: - template: ci/checkout-template.yml - template: ci/install-package-dependencies.yml - - task: DownloadSecureFile@1 - condition: eq(variables['Build.SourceBranchName'], 'master') - name: bazelCacheGcpKey - inputs: - secureFile: "bazel_cache_gcp_key.json" - - bash: echo "##vso[task.setvariable variable=bazelCacheGcpKeyPath]$(bazelCacheGcpKey.secureFilePath)" - condition: eq(variables['Build.SourceBranchName'], 'master') - displayName: GCP key path - # Set the remote cache GCP key path + - template: ci/load-bazel-cache-write-creds.yml - bash: | ci/bazelisk.sh test --test_tag_filters=-nightly //sw/otbn/crypto/... displayName: Execute tests @@ -560,6 +520,7 @@ jobs: downloadPartialBuildBinFrom: - chip_earlgrey_cw310 - sw_build + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -e . util/build_consts.sh @@ -586,6 +547,7 @@ jobs: downloadPartialBuildBinFrom: - chip_earlgrey_cw310 - sw_build + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -e . util/build_consts.sh @@ -613,6 +575,7 @@ jobs: - chip_earlgrey_cw310 - chip_earlgrey_cw310_hyperdebug - sw_build + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -e . util/build_consts.sh @@ -639,6 +602,7 @@ jobs: downloadPartialBuildBinFrom: - chip_earlgrey_cw310_hyperdebug - sw_build + - template: ci/load-bazel-cache-write-creds.yml # We run the update command twice to workaround an issue with udev on the container. # Where rusb cannot dynamically update its device list in CI (udev is not completely # functional). If the device is in normal mode, the first thing that opentitantool @@ -678,6 +642,7 @@ jobs: downloadPartialBuildBinFrom: - chip_earlgrey_cw340 - sw_build + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -e . util/build_consts.sh @@ -704,6 +669,7 @@ jobs: downloadPartialBuildBinFrom: - chip_earlgrey_cw310 - sw_build + - template: ci/load-bazel-cache-write-creds.yml - bash: | set -e . util/build_consts.sh diff --git a/ci/bazelisk.sh b/ci/bazelisk.sh index 89722b1e91639..e765a457dbc57 100755 --- a/ci/bazelisk.sh +++ b/ci/bazelisk.sh @@ -15,12 +15,13 @@ echo "Running bazelisk in $(pwd)." # An additional bazelrc must be synthesized to specify precisely how to use the # GCP bazel cache. +GCP_CREDS_FILE="$GCP_BAZEL_CACHE_KEY_SECUREFILEPATH" GCP_BAZELRC="$(mktemp /tmp/XXXXXX.bazelrc)" trap 'rm ${GCP_BAZELRC}' EXIT -if [[ -n "${GCP_BAZEL_CACHE_KEY}" && -f "${GCP_BAZEL_CACHE_KEY}" ]]; then +if [[ -n "$GCP_CREDS_FILE" && -f "$GCP_CREDS_FILE" ]]; then echo "Applying GCP cache key; will upload to the cache." - echo "build --google_credentials=${GCP_BAZEL_CACHE_KEY}" >> "${GCP_BAZELRC}" + echo "build --google_credentials=${GCP_CREDS_FILE}" >> "${GCP_BAZELRC}" else echo "No key/invalid path to key. Download from cache only." echo "build --remote_upload_local_results=false" >> "${GCP_BAZELRC}" diff --git a/ci/load-bazel-cache-write-creds.yml b/ci/load-bazel-cache-write-creds.yml new file mode 100644 index 0000000000000..e413b287e285c --- /dev/null +++ b/ci/load-bazel-cache-write-creds.yml @@ -0,0 +1,17 @@ +# Copyright lowRISC contributors. +# Licensed under the Apache License, Version 2.0, see LICENSE for details. +# SPDX-License-Identifier: Apache-2.0 + +# Azure Pipelines template for downloading a "secure file" containing the write +# credentials for a GCP bucket where we store a Bazel cache. +# +# The path to the downloaded file is automatically stored in an environment +# variable `GCP_BAZEL_CACHE_KEY_SECUREFILEPATH`. This file is loaded by the +# `ci/bazelisk.sh` script. + +steps: + - task: DownloadSecureFile@1 + condition: eq(variables['Build.SourceBranchName'], 'master') + name: GCP_BAZEL_CACHE_KEY + inputs: + secureFile: "bazel_cache_gcp_key.json" From 6bc6e820475a048cdd959711e0c30a9cf729fbc0 Mon Sep 17 00:00:00 2001 From: James Wainwright Date: Mon, 15 Jan 2024 17:29:52 +0000 Subject: [PATCH 3/4] [ci] Fix ci/bazelisk.sh printing to stdout Signed-off-by: James Wainwright --- ci/bazelisk.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/bazelisk.sh b/ci/bazelisk.sh index e765a457dbc57..756f2d19a1615 100755 --- a/ci/bazelisk.sh +++ b/ci/bazelisk.sh @@ -11,7 +11,7 @@ if [[ -n "${PWD_OVERRIDE}" ]]; then cd "${PWD_OVERRIDE}" || exit fi -echo "Running bazelisk in $(pwd)." +echo "Running bazelisk in $(pwd)." >&2 # An additional bazelrc must be synthesized to specify precisely how to use the # GCP bazel cache. @@ -20,10 +20,10 @@ GCP_BAZELRC="$(mktemp /tmp/XXXXXX.bazelrc)" trap 'rm ${GCP_BAZELRC}' EXIT if [[ -n "$GCP_CREDS_FILE" && -f "$GCP_CREDS_FILE" ]]; then - echo "Applying GCP cache key; will upload to the cache." + echo "Applying GCP cache key; will upload to the cache." >&2 echo "build --google_credentials=${GCP_CREDS_FILE}" >> "${GCP_BAZELRC}" else - echo "No key/invalid path to key. Download from cache only." + echo "No key/invalid path to key. Download from cache only." >&2 echo "build --remote_upload_local_results=false" >> "${GCP_BAZELRC}" fi From b92c555bc0e02067cbcc8b3cc93b652542450913 Mon Sep 17 00:00:00 2001 From: James Wainwright Date: Wed, 17 Jan 2024 14:36:01 +0000 Subject: [PATCH 4/4] [bazel] Allow ./bazelisk.sh to use --bazelrc flags These flags need to come _before_ the subcommand, i.e.: ``` bazel --bazelrc=... cquery # VALID bazel cquery --bazelrc=... # INVALID ``` which is done in this script by pushing flags that come before the subcommand name into an array and re-applying them in the correct order later. If these Bash arrays are confusing and you've come across this commit in a `git blame` then hopefully this will help: 1. Bash supports arrays defined using parentheses: `array=("foo" "bar")` 2. Arrays can't be passed as arguments to functions, they are expanded and get mixed into other arguments, so we use a global instead. 4. the syntax for expanding an array into its elements with correct quoting on each is: `"${array[@]}"`. Signed-off-by: James Wainwright --- bazelisk.sh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/bazelisk.sh b/bazelisk.sh index 7ba3153f71757..9b4e3e0c94109 100755 --- a/bazelisk.sh +++ b/bazelisk.sh @@ -93,10 +93,15 @@ function outquery_starlark_expr() { esac } +# Arguments: +# $qexpr: starlark expression - see `outquery_starlark_expr` +# $name: name of an array containing Bazel arguments that should come _before_ +# the subcommand (e.g. `--bazelrc=...`). function do_outquery() { local qexpr="$1" shift - "$file" cquery "$@" \ + + "$file" "${pre_cmd_args[@]}" cquery "$@" \ --output=starlark --starlark:expr="$qexpr" \ --ui_event_filters=-info --noshow_progress } @@ -121,6 +126,14 @@ function main() { fi fi + # Shift all flags (starting with `-`) that come before the subcommand + # into an array. + pre_cmd_args=() + while [[ "${1-}" == -* ]]; do + pre_cmd_args+=("$1") + shift + done + case "${1-}" in outquery*) # The custom 'outquery' command can be used to query bazel for the @@ -150,13 +163,13 @@ function main() { local qexpr outfile qexpr="$(outquery_starlark_expr outquery)" outfile=$(do_outquery "$qexpr" "$@") - "$file" build "$@" + "$file" "${pre_cmd_args[@]}" build "$@" # shellcheck disable=SC2059 # We are intentionally using $command_template as a format string. eval "$(printf "$command_template" "$outfile")" ;; *) - exec "$file" "$@" + exec "$file" "${pre_cmd_args[@]}" "$@" ;; esac }