From 41cca662d353618004fa323210da214af83fe6dc Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 23 Jun 2023 15:05:12 +0900 Subject: [PATCH 01/18] Add dev-utils feature for inter-crate test code safe reuse --- ci/buildkite-pipeline.sh | 4 +- ci/test-checks.sh | 4 ++ ci/test-dev-utils.sh | 5 +++ scripts/check-dev-utils.sh | 84 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100755 ci/test-dev-utils.sh create mode 100755 scripts/check-dev-utils.sh diff --git a/ci/buildkite-pipeline.sh b/ci/buildkite-pipeline.sh index 250f0b25321a02..2aa10c4dc16026 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" 20 check + command_step checks1 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 20 check + command_step checks2 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-utils.sh check-bins" 15 check + command_step checks3 ". 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 diff --git a/ci/test-checks.sh b/ci/test-checks.sh index bbd66f0f2e98d9..7c90c8e966f09e 100755 --- a/ci/test-checks.sh +++ b/ci/test-checks.sh @@ -42,6 +42,8 @@ echo --- build environment cargo clippy --version --verbose $cargoNightly clippy --version --verbose + $cargoNightly hack --version --verbose + # audit is done only with "$cargo stable" cargo audit --version @@ -110,6 +112,8 @@ else _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" sort --workspace --check fi +_ scripts/check-dev-utils.sh tree + _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" fmt --all -- --check _ ci/do-audit.sh diff --git a/ci/test-dev-utils.sh b/ci/test-dev-utils.sh new file mode 100755 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 100755 index 00000000000000..e0a1f1c10bd2c0 --- /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=( +) + +mode=${1:-full} + +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) + # shellcheck disable=SC2086 # Don't want to double quote $dev_utils_excludes + _ 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 From 2b9940ea919020663d2e9cd8027ce43fcd70e265 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 24 Jun 2023 11:07:30 +0900 Subject: [PATCH 02/18] Sanitize mode --- scripts/check-dev-utils.sh | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index e0a1f1c10bd2c0..cadd6570055ede 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -32,6 +32,13 @@ declare dev_util_tainted_packages=( ) mode=${1:-full} +if [[ $mode != "tree" && + $mode != "check-bins" && + $mode != "check-all-targets" && + $mode != "full" ]]; then + echo "$0: unrecoginized mode: $mode"; + exit 1 +fi if [[ $mode = "tree" || $mode = "full" ]]; then # Run against the entire workspace dep graph (sans $dev_util_tainted_packages) @@ -42,7 +49,7 @@ if [[ $mode = "tree" || $mode = "full" ]]; then _ 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 + echo "$0: dev-utils must not be used as normal dependencies" > /dev/stderr exit 1 fi ) @@ -56,7 +63,7 @@ if [[ $mode = "tree" || $mode = "full" ]]; then _ 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" \ + echo "$0: $tainted: All inverted dependencies must be with dev-utils" \ > /dev/stderr exit 1 fi From 6be169364a84aa8e0cbc2c8b29a89a09381666b7 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 24 Jun 2023 16:01:07 +0900 Subject: [PATCH 03/18] Fix typo... --- scripts/check-dev-utils.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index cadd6570055ede..66d206dc33713c 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -36,7 +36,7 @@ if [[ $mode != "tree" && $mode != "check-bins" && $mode != "check-all-targets" && $mode != "full" ]]; then - echo "$0: unrecoginized mode: $mode"; + echo "$0: unrecognized mode: $mode"; exit 1 fi From 0f8436cf9cb7c39daed1d317b0c6d66b1fc9cf35 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 10:28:12 +0900 Subject: [PATCH 04/18] Use case/esac intead of if --- scripts/check-dev-utils.sh | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 66d206dc33713c..051bec3997bca6 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -32,13 +32,14 @@ declare dev_util_tainted_packages=( ) mode=${1:-full} -if [[ $mode != "tree" && - $mode != "check-bins" && - $mode != "check-all-targets" && - $mode != "full" ]]; then - echo "$0: unrecognized mode: $mode"; - exit 1 -fi +case "$mode" in + tree | check-bins | check-all-targets | full) + ;; + *) + echo "$0: unrecognized mode: $mode"; + exit 1 + ;; +esac if [[ $mode = "tree" || $mode = "full" ]]; then # Run against the entire workspace dep graph (sans $dev_util_tainted_packages) From f8dd093a3464f263a93cefca835b044f20bc5c10 Mon Sep 17 00:00:00 2001 From: Trent Nelson Date: Tue, 27 Jun 2023 15:30:50 -0600 Subject: [PATCH 05/18] port to `cargo tree` + `jq` --- scripts/check-dev-utils.sh | 106 +++++++++++++++++++++++++++++-------- 1 file changed, 83 insertions(+), 23 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 051bec3997bca6..aa1828335d015e 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -28,9 +28,18 @@ source ci/rust-version.sh nightly # binaries. # Note also that dev-utils-ci-marker feature must be added and all of its # dependencies should be edited likewise if any. +declare -r dev_utils_feature="dev-utils" declare dev_util_tainted_packages=( ) +tainted=("${dev_util_tainted_packages[@]}") +if [[ ${#tainted[@]} -gt 0 ]]; then + allowed="\"${tainted[0]}\"" + for package in "${tainted[@]:1}"; do + allowed="${allowed},\"$package\"" + done +fi + mode=${1:-full} case "$mode" in tree | check-bins | check-all-targets | full) @@ -42,34 +51,85 @@ case "$mode" in esac 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) - # shellcheck disable=SC2086 # Don't want to double quote $dev_utils_excludes - _ 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 "$0: dev-utils must not be used as normal dependencies" > /dev/stderr - exit 1 - fi + query=$(cat <&2 + ${dev_utils_feature} must not be used as normal dependencies, but is by: \`[crate]: [dependency]\` + $abusers +EOF + 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 "$0: $tainted: All inverted dependencies must be with dev-utils" \ - > /dev/stderr - exit 1 - fi + query=$(cat <&2 + All crates marked \`tainted\`, as well as their dependents, MUST declare the + \`$dev_utils_feature\`. The following crates are in violation. \`[crate]: [dependant]\` + $misconfigured_crates +EOF + fi fi # Detect possible compilation errors of problematic usage of `dev-utils`-gated code From 94741cdb9e82dbe7c528769aacc6a56277871e73 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 28 Jun 2023 01:12:28 +0000 Subject: [PATCH 06/18] Fix typo... --- scripts/check-dev-utils.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index aa1828335d015e..27b7a3e690414c 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -76,7 +76,7 @@ if [[ $mode = "tree" || $mode = "full" ]]; then EOF ) - abusers="$(_cargo "+${rust_nightly}" metadata --format-version=1 | jq -r "$query")" + abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 | jq -r "$query")" if [[ -n "$abusers" ]]; then cat <&2 ${dev_utils_feature} must not be used as normal dependencies, but is by: \`[crate]: [dependency]\` From ff796e45fd1ef259662eb318b7392249fa399203 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 01:12:42 +0000 Subject: [PATCH 07/18] Properly abort on errors --- scripts/check-dev-utils.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 27b7a3e690414c..3c4b21c77cfd3c 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -82,6 +82,7 @@ EOF ${dev_utils_feature} must not be used as normal dependencies, but is by: \`[crate]: [dependency]\` $abusers EOF + exit 1 fi # Sanity-check that tainted packages has undergone the proper tedious rituals @@ -129,6 +130,7 @@ EOF \`$dev_utils_feature\`. The following crates are in violation. \`[crate]: [dependant]\` $misconfigured_crates EOF + exit 1 fi fi From 9bef11319780945d67db7a8ccf1e2b97d264f6cd Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 10:28:44 +0900 Subject: [PATCH 08/18] Add trailing commas --- scripts/check-dev-utils.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 3c4b21c77cfd3c..2277a1e4e69f51 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -58,7 +58,7 @@ if [[ $mode = "tree" || $mode = "full" ]]; then | map({ "crate" : \$crate, "dependency" : .name, - "dependencyFeatures" : .features + "dependencyFeatures" : .features, }) ) ) @@ -101,7 +101,7 @@ EOF "crate" : .name, "crateFeatures" : .features, "dependant" : \$dependant, - "dependantFeatures" : \$dependant_features + "dependantFeatures" : \$dependant_features, }) ) ) From b5ed3251084449d3f946d27938d1c14e0fe1a08c Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 10:33:20 +0900 Subject: [PATCH 09/18] Select only normal dependencies --- scripts/check-dev-utils.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 2277a1e4e69f51..f8c023f5b2bd80 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -50,11 +50,16 @@ case "$mode" in ;; esac +# cargo metadata uses null for normal dep. while "dev", "build" are natually +# used by other dep. kinds. +declare -r normal_dependency="null"; + if [[ $mode = "tree" || $mode = "full" ]]; then query=$(cat < Date: Fri, 30 Jun 2023 10:59:09 +0900 Subject: [PATCH 10/18] Use more concise comma-separated code --- scripts/check-dev-utils.sh | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index f8c023f5b2bd80..a0d3566899ded0 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -32,13 +32,9 @@ declare -r dev_utils_feature="dev-utils" declare dev_util_tainted_packages=( ) -tainted=("${dev_util_tainted_packages[@]}") -if [[ ${#tainted[@]} -gt 0 ]]; then - allowed="\"${tainted[0]}\"" - for package in "${tainted[@]:1}"; do - allowed="${allowed},\"$package\"" - done -fi +# convert to comma separeted (ref: https://stackoverflow.com/a/53839433) +printf -v allowed '"%s",' "${dev_util_tainted_packages[@]}" +allowed="${allowed%,}" mode=${1:-full} case "$mode" in From 822114a8f39d521fe83b3dd85415de08aed2c081 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 11:01:37 +0900 Subject: [PATCH 11/18] Skip if taint packages are empty --- scripts/check-dev-utils.sh | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index a0d3566899ded0..4d6be8af7817fb 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -88,7 +88,8 @@ EOF # Sanity-check that tainted packages has undergone the proper tedious rituals # to be justified as such. - query=$(cat <&2 + # 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 + misconfigured_crates=$(_ cargo "+${rust_nightly}" metadata --format-version=1 --features dev-utils-ci-marker | jq -r "$query") + if [[ -n "$misconfigured_crates" ]]; then + cat <&2 All crates marked \`tainted\`, as well as their dependents, MUST declare the \`$dev_utils_feature\`. The following crates are in violation. \`[crate]: [dependant]\` $misconfigured_crates EOF - exit 1 + exit 1 + fi fi fi From e86986e666e2d7520088484e1a941d863e25251f Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 11:05:15 +0900 Subject: [PATCH 12/18] Fold long lines and format code a bit --- scripts/check-dev-utils.sh | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 4d6be8af7817fb..33dbf0ae36fdde 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -75,13 +75,15 @@ if [[ $mode = "tree" || $mode = "full" ]]; then | map([.crate, .dependency] | join(": ")) | join("\n ") EOF -) + ) abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 | jq -r "$query")" if [[ -n "$abusers" ]]; then + error="$(echo "${dev_utils_feature}" must not be used as normal dependencies, \ + but is by "([crate]: [dependency])")" cat <&2 - ${dev_utils_feature} must not be used as normal dependencies, but is by: \`[crate]: [dependency]\` - $abusers +$error: + $abusers EOF exit 1 fi @@ -120,17 +122,24 @@ EOF | map([.crate, .dependant] | join(": ")) | join("\n ") EOF -) + ) # 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 - misconfigured_crates=$(_ cargo "+${rust_nightly}" metadata --format-version=1 --features dev-utils-ci-marker | jq -r "$query") + misconfigured_crates=$( + _ cargo "+${rust_nightly}" metadata \ + --format-version=1 \ + --features dev-utils-ci-marker | jq -r "$query" + ) if [[ -n "$misconfigured_crates" ]]; then + error="$(echo All crates marked '`tainted`', as well as their \ + dependents, MUST declare the \`${dev_utils_feature}\` and \ + '`dev-utils-ci-marker`'. The following crates are in violation \ + "([crate]: [dependant])")" cat <&2 - All crates marked \`tainted\`, as well as their dependents, MUST declare the - \`$dev_utils_feature\`. The following crates are in violation. \`[crate]: [dependant]\` - $misconfigured_crates +$error: + $misconfigured_crates EOF exit 1 fi From 4a97ca1affb1e730786eab28487795acbf8c68ac Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 11:46:33 +0900 Subject: [PATCH 13/18] Fix shellcheck --- scripts/check-dev-utils.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 33dbf0ae36fdde..ecb342a4a436a1 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -79,6 +79,8 @@ EOF abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 | jq -r "$query")" if [[ -n "$abusers" ]]; then + # Fold message for heredoc while stripping white-spaces by echo + # shellcheck disable=SC2116 error="$(echo "${dev_utils_feature}" must not be used as normal dependencies, \ but is by "([crate]: [dependency])")" cat <&2 @@ -133,9 +135,11 @@ EOF --features dev-utils-ci-marker | jq -r "$query" ) if [[ -n "$misconfigured_crates" ]]; then - error="$(echo All crates marked '`tainted`', as well as their \ + # Fold message for heredoc while stripping white-spaces by echo + # shellcheck disable=SC2116 + error="$(echo All crates marked \`tainted\', as well as their \ dependents, MUST declare the \`${dev_utils_feature}\` and \ - '`dev-utils-ci-marker`'. The following crates are in violation \ + \`dev-utils-ci-marker\`. The following crates are in violation \ "([crate]: [dependant])")" cat <&2 $error: From 652df19cbcc71a74b44819efc07e1a55a7d525d3 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 30 Jun 2023 16:32:03 +0900 Subject: [PATCH 14/18] Improve jq query and remove uneeded marker feature --- scripts/check-dev-utils.sh | 65 ++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index ecb342a4a436a1..44e1d0b6bccbd8 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -26,8 +26,6 @@ source ci/rust-version.sh nightly # 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 -r dev_utils_feature="dev-utils" declare dev_util_tainted_packages=( ) @@ -73,7 +71,7 @@ if [[ $mode = "tree" || $mode = "full" ]]; then ) )) | map([.crate, .dependency] | join(": ")) - | join("\n ") + | join("\n ") EOF ) @@ -85,68 +83,59 @@ EOF but is by "([crate]: [dependency])")" cat <&2 $error: - $abusers + $abusers EOF exit 1 fi # Sanity-check that tainted packages has undergone the proper tedious rituals # to be justified as such. - if [[ $allowed != '""' ]]; then - query=$(cat <&2 $error: - $misconfigured_crates + $misconfigured_crates EOF - exit 1 - fi + exit 1 fi fi From 58013c2a12c3fe9f73da64a8496ff262ec95fd00 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 1 Jul 2023 14:02:15 +0900 Subject: [PATCH 15/18] Use folding heredoc giving up proper indenting.. --- scripts/check-dev-utils.sh | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index 44e1d0b6bccbd8..bac41620369b1a 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -77,12 +77,9 @@ EOF abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 | jq -r "$query")" if [[ -n "$abusers" ]]; then - # Fold message for heredoc while stripping white-spaces by echo - # shellcheck disable=SC2116 - error="$(echo "${dev_utils_feature}" must not be used as normal dependencies, \ - but is by "([crate]: [dependency])")" cat <&2 -$error: +${dev_utils_feature} must not be used as normal dependencies, but is by \ +"([crate]: [dependency])": $abusers EOF exit 1 @@ -126,13 +123,9 @@ EOF | jq -r "$query" ) if [[ -n "$misconfigured_crates" ]]; then - # Fold message for heredoc while stripping white-spaces by echo - # shellcheck disable=SC2116 - error="$(echo All crates marked \`tainted\', as well as their \ - dependents, MUST declare the \`${dev_utils_feature}\`. The \ - following crates are in violation)" cat <&2 -$error: +All crates marked \`tainted\`, as well as their dependents, MUST declare the \ +\`$dev_utils_feature\`. The following crates are in violation: $misconfigured_crates EOF exit 1 From fd2d6cf94324d9f511deec55aa7107de718c76e2 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sat, 1 Jul 2023 14:21:25 +0900 Subject: [PATCH 16/18] Use jq's alternative operator (//) --- scripts/check-dev-utils.sh | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-utils.sh index bac41620369b1a..6f1d5755571c6b 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-utils.sh @@ -44,16 +44,12 @@ case "$mode" in ;; esac -# cargo metadata uses null for normal dep. while "dev", "build" are natually -# used by other dep. kinds. -declare -r normal_dependency="null"; - if [[ $mode = "tree" || $mode = "full" ]]; then query=$(cat < Date: Tue, 4 Jul 2023 09:54:52 +0900 Subject: [PATCH 17/18] Rename to dev-context-only-utils --- ci/buildkite-pipeline.sh | 4 +- ci/test-checks.sh | 2 +- ci/test-dev-context-only-utils.sh | 5 ++ ci/test-dev-utils.sh | 5 -- ...ils.sh => check-dev-context-only-utils.sh} | 66 ++++++++++--------- 5 files changed, 43 insertions(+), 39 deletions(-) create mode 100755 ci/test-dev-context-only-utils.sh delete mode 100755 ci/test-dev-utils.sh rename scripts/{check-dev-utils.sh => check-dev-context-only-utils.sh} (55%) diff --git a/ci/buildkite-pipeline.sh b/ci/buildkite-pipeline.sh index 2aa10c4dc16026..0f63ca31802a46 100755 --- a/ci/buildkite-pipeline.sh +++ b/ci/buildkite-pipeline.sh @@ -141,8 +141,8 @@ wait_step() { all_test_steps() { command_step checks1 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 20 check - command_step checks2 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-utils.sh check-bins" 15 check - command_step checks3 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-utils.sh check-all-targets" 15 check + command_step checks2 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-context-only-utils.sh check-bins" 15 check + command_step checks3 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-dev-context-only-utils.sh check-all-targets" 15 check wait_step # Full test suite diff --git a/ci/test-checks.sh b/ci/test-checks.sh index 7c90c8e966f09e..86fc49e0c12387 100755 --- a/ci/test-checks.sh +++ b/ci/test-checks.sh @@ -112,7 +112,7 @@ else _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" sort --workspace --check fi -_ scripts/check-dev-utils.sh tree +_ scripts/check-dev-context-only-utils.sh tree _ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" fmt --all -- --check diff --git a/ci/test-dev-context-only-utils.sh b/ci/test-dev-context-only-utils.sh new file mode 100755 index 00000000000000..bec640cdf209f8 --- /dev/null +++ b/ci/test-dev-context-only-utils.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +set -eo pipefail + +scripts/check-dev-context-only-utils.sh "$@" diff --git a/ci/test-dev-utils.sh b/ci/test-dev-utils.sh deleted file mode 100755 index 6ec155e743ec4f..00000000000000 --- a/ci/test-dev-utils.sh +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env bash - -set -eo pipefail - -scripts/check-dev-utils.sh "$@" diff --git a/scripts/check-dev-utils.sh b/scripts/check-dev-context-only-utils.sh similarity index 55% rename from scripts/check-dev-utils.sh rename to scripts/check-dev-context-only-utils.sh index 6f1d5755571c6b..676b3dbd3f7025 100755 --- a/scripts/check-dev-utils.sh +++ b/scripts/check-dev-context-only-utils.sh @@ -7,31 +7,32 @@ source ci/_ # (benches, frozen abi...) source ci/rust-version.sh nightly -# There's a special common feature called `dev-utils` to +# There's a special common feature called `dev-context-only-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. +# pitfalls of `dev-context-only-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. +# Firstly, detect any misuse of dev-context-only-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. +# Put differently, use of dev-context-only-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-context-only-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. -declare -r dev_utils_feature="dev-utils" -declare dev_util_tainted_packages=( +# Add your troubled package which seems to want to use `dev-context-only-utils` +# as normal (not dev) dependencies, only if you're sure that there's good +# reason to bend dev-context-only-utils's original intention and that listed +# package isn't part of released binaries. +declare tainted_packages=( ) # convert to comma separeted (ref: https://stackoverflow.com/a/53839433) -printf -v allowed '"%s",' "${dev_util_tainted_packages[@]}" +printf -v allowed '"%s",' "${tainted_packages[@]}" allowed="${allowed%,}" mode=${1:-full} @@ -60,7 +61,7 @@ if [[ $mode = "tree" || $mode = "full" ]]; then | flatten | map(select( (.dependencyFeatures - | index("${dev_utils_feature}") + | index("dev-context-only-utils") ) and (.crate as \$needle | ([$allowed] | index(\$needle)) | not @@ -71,10 +72,11 @@ if [[ $mode = "tree" || $mode = "full" ]]; then EOF ) - abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 | jq -r "$query")" + abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 | + jq -r "$query")" if [[ -n "$abusers" ]]; then cat <&2 -${dev_utils_feature} must not be used as normal dependencies, but is by \ +\`dev-context-only-utils\' must not be used as normal dependencies, but is by \ "([crate]: [dependency])": $abusers EOF @@ -106,7 +108,7 @@ EOF end) | flatten | map(select( - (.crateFeatures | index("${dev_utils_feature}")) | not + (.crateFeatures | index("dev-context-only-utils")) | not )) | map(.crate) | join("\n ") @@ -121,25 +123,27 @@ EOF if [[ -n "$misconfigured_crates" ]]; then cat <&2 All crates marked \`tainted\`, as well as their dependents, MUST declare the \ -\`$dev_utils_feature\`. The following crates are in violation: +\`dev-context-only-utils\`. The following crates are in violation: $misconfigured_crates EOF exit 1 fi 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. +# Detect possible compilation errors of problematic usage of +# `dev-context-only-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-context-only-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` +# 1. Check implicit usage of `dev-context-only-utils`-gated code in non-dev (= +# production) code by building without dev dependencies (= tests/benches) for +# each crate +# 2. Check implicit usage of `dev-context-only-utils`-gated code in dev (= +# test/benches) code by building in isolation from other crates, which might +# happen to enable `dev-context-only-utils` if [[ $mode = "check-bins" || $mode = "full" ]]; then _ cargo "+${rust_nightly}" hack check --bins fi From be5a110cd3f9e0a025b1343829a2b5a4b84721e4 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 4 Jul 2023 12:40:42 +0900 Subject: [PATCH 18/18] Fix typo... --- scripts/check-dev-context-only-utils.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/check-dev-context-only-utils.sh b/scripts/check-dev-context-only-utils.sh index 676b3dbd3f7025..debb323db113c4 100755 --- a/scripts/check-dev-context-only-utils.sh +++ b/scripts/check-dev-context-only-utils.sh @@ -76,7 +76,7 @@ EOF jq -r "$query")" if [[ -n "$abusers" ]]; then cat <&2 -\`dev-context-only-utils\' must not be used as normal dependencies, but is by \ +\`dev-context-only-utils\` must not be used as normal dependencies, but is by \ "([crate]: [dependency])": $abusers EOF