From e563ce3f30704cd3506a30657a7fa86ddc5808f8 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 23 Jun 2023 14:31:04 +0900 Subject: [PATCH] Split to scripts/check-dev-utils.sh --- ci/buildkite-pipeline.sh | 6 ++- ci/test-checks.sh | 68 +----------------------------- ci/test-dev-utils.sh | 5 +++ scripts/check-dev-utils.sh | 84 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 69 deletions(-) create mode 100644 ci/test-dev-utils.sh create mode 100644 scripts/check-dev-utils.sh diff --git a/ci/buildkite-pipeline.sh b/ci/buildkite-pipeline.sh index aaf3498f04f2ca..0057fe584e5692 100755 --- a/ci/buildkite-pipeline.sh +++ b/ci/buildkite-pipeline.sh @@ -140,7 +140,9 @@ wait_step() { } all_test_steps() { - command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 30 check + command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 20 check + command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-utils.sh check-bins" 15 check + command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-utils.sh check-all-targets" 15 check wait_step # Full test suite @@ -314,7 +316,7 @@ pull_or_push_steps() { if [ -z "$diff_other_than_version_bump" ]; then echo "Diff only contains version bump." - command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 30 + command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 20 exit 0 fi fi diff --git a/ci/test-checks.sh b/ci/test-checks.sh index 18b0129b1bc800..bf92240f856023 100755 --- a/ci/test-checks.sh +++ b/ci/test-checks.sh @@ -112,73 +112,7 @@ else _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" sort --workspace --check fi -# There's a special common feature called `dev-utils` to -# overcome cargo's issue: https://github.com/rust-lang/cargo/issues/8379 -# This feature is like `cfg(test)`, which works between crates. -# -# Unfortunately, this in turn needs some special checks to avoid common -# pitfalls of `dev-utils` itself. -# -# Firstly, detect any misuse of dev-utils as normal/build dependencies. -# Also, allow some exceptions for special purpose crates. This white-listing -# mechanism can be used for core-development-oriented crates like bench bins. -# -# Put differently, use of dev-utils is forbidden for non-dev dependencies in -# general. However, allow its use for non-dev dependencies only if its use -# is confined under a dep. subgraph with all nodes being marked as dev-utils. - -# Add your troubled package which seems to want to use `dev-utils` as normal (not -# dev) dependencies, only if you're sure that there's good reason to bend -# dev-util's original intention and that listed package isn't part of released -# binaries. -# Note also that dev-utils-ci-marker feature must be added and all of its -# dependencies should be edited likewise if any. -declare dev_util_tainted_packages=( - "solana-banking-bench" -) - -# Run against the entire workspace dep graph (sans $dev_util_tainted_packages) -dev_utils_excludes=$(for tainted in "${dev_util_tainted_packages[@]}"; do - echo "--exclude $tainted" -done) -_ cargo tree --workspace -f "{p} {f}" --edges normal,build \ - $dev_utils_excludes | ( - if grep -E -C 3 -m 10 "[, ]dev-utils([, ]|$)"; then - echo "dev-utils must not be used as normal dependencies" > /dev/stderr - exit 1 - fi -) - -# Sanity-check that tainted packages has undergone the proper tedious rituals -# to be justified as such. -for tainted in "${dev_util_tainted_packages[@]}"; do - # dev-utils-ci-marker is special proxy feature needed only when using - # dev-utils code as part of normal dependency. dev-utils will be enabled - # indirectly via this feature only if prepared correctly - _ cargo tree --workspace -f "{p} {f}" --edges normal,build \ - --invert "$tainted" --features dev-utils-ci-marker | ( - if grep -E -C 3 -m 10 -v "[, ]dev-utils([, ]|$)"; then - echo "$tainted: All inverted dependencies must be with dev-utils" \ - > /dev/stderr - exit 1 - fi - ) -done - -# Detect possible compilation errors of problematic usage of `dev-utils`-gated code -# without being explicitly declared as such in respective workspace member -# `Cargo.toml`s. This cannot be detected with `--workspace --all-targets`, due -# to unintentional `dev-utils` feature activation by cargo's feature -# unification mechanism. -# So, we use `cargo hack` to exhaustively build each individual workspace -# members in isolation to work around. -# -# 1. Check implicit usage of `dev-utils`-gated code in non-dev (= production) code by -# building without dev dependencies (= tests/benches) for each crate -# 2. Check implicit usage of `dev-utils`-gated code in dev (= test/benches) code by -# building in isolation from other crates, which might happen to enable `dev-utils` -_ cargo "+${rust_nightly}" hack check --bins -_ cargo "+${rust_nightly}" hack check --all-targets +_ scripts/check-dev-utils.sh tree _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" fmt --all -- --check diff --git a/ci/test-dev-utils.sh b/ci/test-dev-utils.sh new file mode 100644 index 00000000000000..6ec155e743ec4f --- /dev/null +++ b/ci/test-dev-utils.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -eo pipefail + +scripts/check-dev-utils.sh "$@" diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh new file mode 100644 index 00000000000000..5a8a7d695ebeee --- /dev/null +++ b/scripts/check-dev-utils.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash + +set -eo pipefail +cd "$(dirname "$0")/.." +source ci/_ +# only nightly is used uniformly as we contain good amount of nightly-only code +# (benches, frozen abi...) +source ci/rust-version.sh nightly + +# There's a special common feature called `dev-utils` to +# overcome cargo's issue: https://github.com/rust-lang/cargo/issues/8379 +# This feature is like `cfg(test)`, which works between crates. +# +# Unfortunately, this in turn needs some special checks to avoid common +# pitfalls of `dev-utils` itself. +# +# Firstly, detect any misuse of dev-utils as normal/build dependencies. +# Also, allow some exceptions for special purpose crates. This white-listing +# mechanism can be used for core-development-oriented crates like bench bins. +# +# Put differently, use of dev-utils is forbidden for non-dev dependencies in +# general. However, allow its use for non-dev dependencies only if its use +# is confined under a dep. subgraph with all nodes being marked as dev-utils. + +# Add your troubled package which seems to want to use `dev-utils` as normal (not +# dev) dependencies, only if you're sure that there's good reason to bend +# dev-util's original intention and that listed package isn't part of released +# binaries. +# Note also that dev-utils-ci-marker feature must be added and all of its +# dependencies should be edited likewise if any. +declare dev_util_tainted_packages=( + "solana-banking-bench" +) + +mode=$1 + +if [[ $mode = "tree" || $mode = "full" ]]; then + # Run against the entire workspace dep graph (sans $dev_util_tainted_packages) + dev_utils_excludes=$(for tainted in "${dev_util_tainted_packages[@]}"; do + echo "--exclude $tainted" + done) + _ cargo "+${rust_nightly}" tree --workspace -f "{p} {f}" --edges normal,build \ + $dev_utils_excludes | ( + if grep -E -C 3 -m 10 "[, ]dev-utils([, ]|$)"; then + echo "dev-utils must not be used as normal dependencies" > /dev/stderr + exit 1 + fi + ) + + # Sanity-check that tainted packages has undergone the proper tedious rituals + # to be justified as such. + for tainted in "${dev_util_tainted_packages[@]}"; do + # dev-utils-ci-marker is special proxy feature needed only when using + # dev-utils code as part of normal dependency. dev-utils will be enabled + # indirectly via this feature only if prepared correctly + _ cargo "+${rust_nightly}" tree --workspace -f "{p} {f}" --edges normal,build \ + --invert "$tainted" --features dev-utils-ci-marker | ( + if grep -E -C 3 -m 10 -v "[, ]dev-utils([, ]|$)"; then + echo "$tainted: All inverted dependencies must be with dev-utils" \ + > /dev/stderr + exit 1 + fi + ) + done +fi + +# Detect possible compilation errors of problematic usage of `dev-utils`-gated code +# without being explicitly declared as such in respective workspace member +# `Cargo.toml`s. This cannot be detected with `--workspace --all-targets`, due +# to unintentional `dev-utils` feature activation by cargo's feature +# unification mechanism. +# So, we use `cargo hack` to exhaustively build each individual workspace +# members in isolation to work around. +# +# 1. Check implicit usage of `dev-utils`-gated code in non-dev (= production) code by +# building without dev dependencies (= tests/benches) for each crate +# 2. Check implicit usage of `dev-utils`-gated code in dev (= test/benches) code by +# building in isolation from other crates, which might happen to enable `dev-utils` +if [[ $mode = "check-bins" || $mode = "full" ]]; then + _ cargo "+${rust_nightly}" hack check --bins +fi +if [[ $mode = "check-all-targets" || $mode = "full" ]]; then + _ cargo "+${rust_nightly}" hack check --all-targets +fi