Skip to content

Commit b8147f4

Browse files
committed
Harden checks that unwanted dependencies are absent
The `check` recipe in the `justfile` contains several commands to check that, when particular packages are built without default features, this avoids having a particular other crate appear anywhere in the dependency tree (i.e., without default features, we do not depend on a particular named package). These are done by using `cargo tree` with the appropriate arguments to compute the inverse dependency tree from the crate whose absence we wish to assert. But because `cargo tree` treats an inverse tree with no nodes (not even its root) as a mere warning, we cannot check the exit code. So we parse the output for a warning. As implemented before this commit, those checks already had a high probability of giving the correct result. But they could have falsely passed. The technique being used was to redirect stderr to stdout, then search for the text `warning` anywhere in the combined output. This would falsely pass in either of two edge cases: 1. If the inverse dependency tree was produced, but contained a crate with `warning` in its name (or in associated details). 2. If a different warning than the anticipated one was produced, and the anticipated warning was not produced. This commit hardens the check against both those edge cases, by: 1. Discarding what `cargo tree` sends to stdout, rather than including it. (This has the minor disadvantage that the syntax may look wrong, or be less obvious in its effects, to readers less familiar with POSIX shell redirection syntax. The right side of a redirection operator, i.e. the filename to open or the file descriptor to duplicate, is dereferenced. So `2>&1` duplicates the original stdout and sends stderr to it, which works even though the immediately subsequent redirection `>/dev/null`, short for `1>/dev/null`, sends stdout to the null device. Although this is a common source of confusion, the actual semantics are exactly what we want here.) 2. Parsing more narrowly for the warning, ensuring it starts at the beginning of the line and that the warning message begins with the expected phrase. This, in particular, makes the commands significantly longer. But by showing the message, it also helps clarify what they are for. So even aside from keeping the checks from wrongly passing, this part of the change seems to improve clarity overall.
1 parent ce071bf commit b8147f4

File tree

1 file changed

+8
-4
lines changed

1 file changed

+8
-4
lines changed

justfile

+8-4
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,14 @@ check:
5151
! cargo check -p gix-transport --all-features 2>/dev/null
5252
! cargo check -p gix-protocol --all-features 2>/dev/null
5353
# warning happens if nothing found, no exit code :/
54-
cargo tree -p gix --no-default-features -e normal -i imara-diff 2>&1 | grep -F warning
55-
cargo tree -p gix --no-default-features -e normal -i gix-submodule 2>&1 | grep -F warning
56-
cargo tree -p gix --no-default-features -e normal -i gix-pathspec 2>&1 | grep -F warning
57-
cargo tree -p gix --no-default-features -e normal -i gix-filter 2>&1 | grep -F warning
54+
cargo tree -p gix --no-default-features -e normal -i imara-diff \
55+
2>&1 >/dev/null | grep '^warning: nothing to print\>'
56+
cargo tree -p gix --no-default-features -e normal -i gix-submodule \
57+
2>&1 >/dev/null | grep '^warning: nothing to print\>'
58+
cargo tree -p gix --no-default-features -e normal -i gix-pathspec \
59+
2>&1 >/dev/null | grep '^warning: nothing to print\>'
60+
cargo tree -p gix --no-default-features -e normal -i gix-filter \
61+
2>&1 >/dev/null | grep '^warning: nothing to print\>'
5862
! cargo tree -p gix --no-default-features -i gix-credentials 2>/dev/null
5963
cargo check --no-default-features --features lean
6064
cargo check --no-default-features --features lean-async

0 commit comments

Comments
 (0)