diff --git a/AGENTS.md b/AGENTS.md index b9aaae27..35271e0c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -151,8 +151,9 @@ ripwire . --recall="" # the docs/design and .ripwire_notes rows that answ 1. **hwp-model depends on no other internal crate** (hub and spoke). `hwp5` and `hwpx` do not depend on each other either; they go through the IR. `hwp-convert` and `hwp-render` do not depend on - each other. These are normal-dependency edges; dev-dependencies are allowed (hwp5's tests use - hwpx, hwp-render's use hwp-convert). `scripts/check-crate-edges.sh` checks them. + each other. These are normal-dependency edges, direct or transitive; dev-dependencies are + allowed (hwp5's tests use hwpx, hwp-render's use hwp-convert). `scripts/check-crate-edges.sh` + checks them. 2. **Lossless round-trip gate**: hwp5 → hwp5 identity re-serialization must be byte-identical (`crates/hwp5/tests/identity.rs`). Do not drop unknown records; preserve them as `OpaqueRecord`. 3. **Ground-truth methodology - no guessing**: format behavior is established only by comparing diff --git a/REVIEW.md b/REVIEW.md index 902b4da2..9463a890 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -17,7 +17,7 @@ Run these passes and tag every finding with its pass: - **Crate direction**: `hwp-model` depends on no other internal crate; `hwp5` and `hwpx` never depend on each other and go through the IR; `hwp-convert` and `hwp-render` never depend on each other. - These are normal-dependency edges; dev-dependencies are allowed. + These are normal-dependency edges, direct or transitive; dev-dependencies are allowed. - **Lossless round-trip**: hwp5 -> hwp5 identity re-serialization stays byte-identical; unknown records are preserved as `OpaqueRecord`, never dropped. - **Ground truth, no guessing**: format behavior is established against bytes written by Hangul, and diff --git a/scripts/check-crate-edges.sh b/scripts/check-crate-edges.sh index 33d0ede0..e8a92c0e 100755 --- a/scripts/check-crate-edges.sh +++ b/scripts/check-crate-edges.sh @@ -8,6 +8,9 @@ # added between them would quietly make the duplication pointless. Until now nothing in the # repository stopped one; the invariant lived only in prose. # +# The whole normal-dependency tree is checked, not only direct edges (#407): hwp-render taking a +# normal dependency on hwpx would pull in hwp-convert through hwpx -> hwp-convert. +# # Dev-dependencies are deliberately out of scope (`-e normal`): hwp-render's tests already use # hwp-convert to build documents, which does not put either crate in the other's build graph. # @@ -18,30 +21,83 @@ set -uo pipefail cd "$(dirname "$0")/.." || exit fail=0 +manifest=Cargo.toml -# Direct normal dependencies of a crate, one name per line. +# Normal dependencies of a crate, direct and transitive, one name per line. A cargo failure is a +# nonzero status with cargo's error left on stderr, never an empty list that reads as "ok". deps() { - cargo tree -p "$1" -e normal --depth 1 --prefix none 2>/dev/null | - tail -n +2 | awk '{print $1}' + local tree + tree="$(cargo tree -q --manifest-path "$manifest" -p "$1" -e normal --prefix none)" || return + printf '%s\n' "$tree" | tail -n +2 | awk '{print $1}' | sort -u +} + +# Status 0 if the crate depends on a name matching the pattern, 1 if not, 2 if cargo tree failed. +# The match reads a here-string, not a pipe: under pipefail `... | grep -q` can turn a hit into a +# failure (SIGPIPE upstream), and `grep -c` exits 1 on a zero count. +has_dep() { + local names + names="$(deps "$1")" || return 2 + grep -Eqx "$2" <<<"$names" } forbid() { - local crate="$1" forbidden="$2" count - count="$(deps "$crate" | grep -Ecx "$forbidden")" - if [ "$count" -ne 0 ]; then - echo "FAIL: $crate has a normal dependency on $forbidden (AGENTS.md invariant 1)" >&2 + local crate="$1" forbidden="$2" + has_dep "$crate" "$forbidden" + case $? in + 0) + echo "FAIL: $crate depends on $forbidden, directly or transitively" \ + "(AGENTS.md invariant 1)" >&2 fail=1 - else - echo "ok: $crate does not depend on $forbidden" - fi + ;; + 1) echo "ok: $crate does not depend on $forbidden" ;; + *) + echo "FAIL: cargo tree failed for $crate, so its edges were not checked" >&2 + fail=1 + ;; + esac } if [ "${1:-}" = "--self-test" ]; then # The gate must be able to fail: a dependency that demonstrably exists has to be caught. - if deps hwp-cli | grep -Ecx 'hwp-render' | grep -qx 0; then + if ! has_dep hwp-cli hwp-render; then echo "FAIL: self-test could not see hwp-cli -> hwp-render, so the matcher is broken" >&2 exit 1 fi + # No internal crate reaches another only transitively today, so a throwaway workspace + # supplies the case: edge-a -> edge-b -> edge-c, with no direct edge-a -> edge-c. + fixture="$(mktemp -d)" + trap 'rm -rf "$fixture"' EXIT + printf '[workspace]\nmembers = ["edge-a", "edge-b", "edge-c"]\nresolver = "2"\n' \ + >"$fixture/Cargo.toml" + for c in a b c; do + mkdir -p "$fixture/edge-$c/src" + : >"$fixture/edge-$c/src/lib.rs" + printf '[package]\nname = "edge-%s"\nversion = "0.0.0"\nedition = "2021"\n' "$c" \ + >"$fixture/edge-$c/Cargo.toml" + done + printf '[dependencies]\nedge-b = { path = "../edge-b" }\n' >>"$fixture/edge-a/Cargo.toml" + printf '[dependencies]\nedge-c = { path = "../edge-c" }\n' >>"$fixture/edge-b/Cargo.toml" + manifest="$fixture/Cargo.toml" + if ! has_dep edge-a edge-c; then + echo "FAIL: self-test missed the transitive edge edge-a -> edge-b -> edge-c" >&2 + exit 1 + fi + echo "ok: self-test caught the transitive edge edge-a -> edge-b -> edge-c" + # forbid() must turn both a hit and a cargo failure into a gate failure. The hit case is only + # meaningful after has_dep above has shown cargo works on the fixture. + fail=0 + forbid edge-a edge-c 2>/dev/null + if [ "$fail" -ne 1 ]; then + echo "FAIL: self-test: forbid() did not fail on a forbidden edge" >&2 + exit 1 + fi + fail=0 + forbid no-such-crate edge-c 2>/dev/null + if [ "$fail" -ne 1 ]; then + echo "FAIL: self-test: forbid() did not fail when cargo tree failed" >&2 + exit 1 + fi + echo "ok: self-test saw forbid() fail on a forbidden edge and on a cargo tree failure" echo "== crate-edges self-test: OK ==" exit 0 fi