Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit 1824743

Browse files
ryoqunt-nelson
ryoqun
and
t-nelson
authored
Add special feature for inter-crate safe dev-related code reuse (#32169)
* Add dev-utils feature for inter-crate test code safe reuse * Sanitize mode * Fix typo... * Use case/esac intead of if * port to `cargo tree` + `jq` * Fix typo... * Properly abort on errors * Add trailing commas * Select only normal dependencies * Use more concise comma-separated code * Skip if taint packages are empty * Fold long lines and format code a bit * Fix shellcheck * Improve jq query and remove uneeded marker feature * Use folding heredoc giving up proper indenting.. * Use jq's alternative operator (//) * Rename to dev-context-only-utils * Fix typo... --------- Co-authored-by: Trent Nelson <[email protected]>
1 parent 98ca5a9 commit 1824743

File tree

4 files changed

+164
-1
lines changed

4 files changed

+164
-1
lines changed

‎ci/buildkite-pipeline.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,9 @@ wait_step() {
140140
}
141141

142142
all_test_steps() {
143-
command_step checks ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 20 check
143+
command_step checks1 ". ci/rust-version.sh; ci/docker-run.sh \$\$rust_nightly_docker_image ci/test-checks.sh" 20 check
144+
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
145+
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
144146
wait_step
145147

146148
# Full test suite

‎ci/test-checks.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ echo --- build environment
4242
cargo clippy --version --verbose
4343
$cargoNightly clippy --version --verbose
4444

45+
$cargoNightly hack --version --verbose
46+
4547
# audit is done only with "$cargo stable"
4648
cargo audit --version
4749

@@ -110,6 +112,8 @@ else
110112
_ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" sort --workspace --check
111113
fi
112114

115+
_ scripts/check-dev-context-only-utils.sh tree
116+
113117
_ scripts/cargo-for-all-lock-files.sh -- "+${rust_nightly}" fmt --all -- --check
114118

115119
_ ci/do-audit.sh

‎ci/test-dev-context-only-utils.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#!/usr/bin/env bash
2+
3+
set -eo pipefail
4+
5+
scripts/check-dev-context-only-utils.sh "$@"

‎scripts/check-dev-context-only-utils.sh

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
#!/usr/bin/env bash
2+
3+
set -eo pipefail
4+
cd "$(dirname "$0")/.."
5+
source ci/_
6+
# only nightly is used uniformly as we contain good amount of nightly-only code
7+
# (benches, frozen abi...)
8+
source ci/rust-version.sh nightly
9+
10+
# There's a special common feature called `dev-context-only-utils` to
11+
# overcome cargo's issue: https://github.com/rust-lang/cargo/issues/8379
12+
# This feature is like `cfg(test)`, which works between crates.
13+
#
14+
# Unfortunately, this in turn needs some special checks to avoid common
15+
# pitfalls of `dev-context-only-utils` itself.
16+
#
17+
# Firstly, detect any misuse of dev-context-only-utils as normal/build
18+
# dependencies. Also, allow some exceptions for special purpose crates. This
19+
# white-listing mechanism can be used for core-development-oriented crates like
20+
# bench bins.
21+
#
22+
# Put differently, use of dev-context-only-utils is forbidden for non-dev
23+
# dependencies in general. However, allow its use for non-dev dependencies only
24+
# if its use is confined under a dep. subgraph with all nodes being marked as
25+
# dev-context-only-utils.
26+
27+
# Add your troubled package which seems to want to use `dev-context-only-utils`
28+
# as normal (not dev) dependencies, only if you're sure that there's good
29+
# reason to bend dev-context-only-utils's original intention and that listed
30+
# package isn't part of released binaries.
31+
declare tainted_packages=(
32+
)
33+
34+
# convert to comma separeted (ref: https://stackoverflow.com/a/53839433)
35+
printf -v allowed '"%s",' "${tainted_packages[@]}"
36+
allowed="${allowed%,}"
37+
38+
mode=${1:-full}
39+
case "$mode" in
40+
tree | check-bins | check-all-targets | full)
41+
;;
42+
*)
43+
echo "$0: unrecognized mode: $mode";
44+
exit 1
45+
;;
46+
esac
47+
48+
if [[ $mode = "tree" || $mode = "full" ]]; then
49+
query=$(cat <<EOF
50+
.packages
51+
| map(.name as \$crate
52+
| (.dependencies
53+
| map(select((.kind // "normal") == "normal"))
54+
| map({
55+
"crate" : \$crate,
56+
"dependency" : .name,
57+
"dependencyFeatures" : .features,
58+
})
59+
)
60+
)
61+
| flatten
62+
| map(select(
63+
(.dependencyFeatures
64+
| index("dev-context-only-utils")
65+
) and (.crate as \$needle
66+
| ([$allowed] | index(\$needle))
67+
| not
68+
)
69+
))
70+
| map([.crate, .dependency] | join(": "))
71+
| join("\n ")
72+
EOF
73+
)
74+
75+
abusers="$(_ cargo "+${rust_nightly}" metadata --format-version=1 |
76+
jq -r "$query")"
77+
if [[ -n "$abusers" ]]; then
78+
cat <<EOF 1>&2
79+
\`dev-context-only-utils\` must not be used as normal dependencies, but is by \
80+
"([crate]: [dependency])":
81+
$abusers
82+
EOF
83+
exit 1
84+
fi
85+
86+
# Sanity-check that tainted packages has undergone the proper tedious rituals
87+
# to be justified as such.
88+
query=$(cat <<EOF
89+
.packages
90+
| map([.name, (.features | keys)] as [\$this_crate, \$this_feature]
91+
| if .name as \$needle | ([$allowed] | index(\$needle))
92+
then
93+
{
94+
"crate": \$this_crate,
95+
"crateFeatures": \$this_feature,
96+
}
97+
elif .dependencies | any(
98+
.name as \$needle | ([$allowed] | index(\$needle))
99+
)
100+
then
101+
.dependencies
102+
| map({
103+
"crate": \$this_crate,
104+
"crateFeatures": \$this_feature,
105+
})
106+
else
107+
[]
108+
end)
109+
| flatten
110+
| map(select(
111+
(.crateFeatures | index("dev-context-only-utils")) | not
112+
))
113+
| map(.crate)
114+
| join("\n ")
115+
EOF
116+
)
117+
118+
misconfigured_crates=$(
119+
_ cargo "+${rust_nightly}" metadata \
120+
--format-version=1 \
121+
| jq -r "$query"
122+
)
123+
if [[ -n "$misconfigured_crates" ]]; then
124+
cat <<EOF 1>&2
125+
All crates marked \`tainted\`, as well as their dependents, MUST declare the \
126+
\`dev-context-only-utils\`. The following crates are in violation:
127+
$misconfigured_crates
128+
EOF
129+
exit 1
130+
fi
131+
fi
132+
133+
# Detect possible compilation errors of problematic usage of
134+
# `dev-context-only-utils`-gated code without being explicitly declared as such
135+
# in respective workspace member `Cargo.toml`s. This cannot be detected with
136+
# `--workspace --all-targets`, due to unintentional `dev-context-only-utils`
137+
# feature activation by cargo's feature unification mechanism. So, we use
138+
# `cargo hack` to exhaustively build each individual workspace members in
139+
# isolation to work around.
140+
#
141+
# 1. Check implicit usage of `dev-context-only-utils`-gated code in non-dev (=
142+
# production) code by building without dev dependencies (= tests/benches) for
143+
# each crate
144+
# 2. Check implicit usage of `dev-context-only-utils`-gated code in dev (=
145+
# test/benches) code by building in isolation from other crates, which might
146+
# happen to enable `dev-context-only-utils`
147+
if [[ $mode = "check-bins" || $mode = "full" ]]; then
148+
_ cargo "+${rust_nightly}" hack check --bins
149+
fi
150+
if [[ $mode = "check-all-targets" || $mode = "full" ]]; then
151+
_ cargo "+${rust_nightly}" hack check --all-targets
152+
fi

0 commit comments

Comments
 (0)