Skip to content

The gate's --all-targets enables a dev-dep feature, so the shipped (feature-off) configuration is never linted #1418

Description

@dc0sk

Found while pushing #1416, reviewed by Fable. Related to #1380 and distinct from it: #1380 will be
closed by a step that adds a feature; this one needs a step with fewer targets. Closing #1380 leaves
this open.

The defect

Both lint sites pass --all-targets:

  • scripts/gate.sh:191 → cargo clippy --workspace --no-default-features --all-targets -- -D warnings
  • .cargo-husky/hooks/pre-push:75 → the same command

--all-targets puts dev units in scope, and resolver 2 unifies dev-dep features into the normal build
while it is building them. Three dev-dependencies enable openpulse-modem's instruments feature
(#1277) — crates/openpulse-modem/Cargo.toml:41 (on itself), crates/openpulse-daemon/Cargo.toml:92,
crates/openpulse-kiss/Cargo.toml:59 — and under --workspace any one of them suffices. So the lib
is always linted with instruments on, and the feature-off configuration, which is what the
shipped binaries link (release.yml:60, cargo build --release … --no-default-features), is never
linted at all.

What the hole admits — planted probe, restored in the same command

An ungated production caller of an instruments-only accessor, appended to engine.rs:

pub fn zz_probe_1380(e: &ModemEngine) -> u64 { e.notch_blocks_processed() }
cargo clippy --workspace --no-default-features --all-targets -- -D warnings   -> rc=0    (the gate: invisible)
cargo test  -p openpulse-modem --no-default-features --no-run                 -> rc=0    (the hook: invisible)
cargo clippy -p openpulse-daemon --no-default-features -- -D warnings         -> rc=101
      error[E0599]: no method named `notch_blocks_processed` found for reference `&engine::ModemEngine`

Both automated checks pass a break that the release build rejects. The hook row is the sharp one: an
engine.rs edit makes pkgs = openpulse-modem, so the hook's own cargo test -p openpulse-modem puts
the self dev-dep in scope and compiles the ON lib.

Corollary: #1277's "a production call cannot compile without a visible Cargo.toml diff"
(crates/openpulse-modem/Cargo.toml:39, and server.rs:3606's "not reachable from this crate") is
enforced by release builds only — not by the gate, and not by the hook.

Measured at 8b5896fa, clean tree

Per-unit features read from cargo's --message-format=json compiler-artifact records (cargo tree -e features cannot discriminate — it prints the dev edge either way):

cargo clippy --workspace --no-default-features … modem lib (non-test) features warnings
--all-targets (the gate) ['instruments'] 0
--tests ['instruments'] 0
--bins [] 2
--lib [] 2
(no target flag) [] 2

The trigger is dev units in scope; --all-targets is a proxy for it, so swapping to --tests would
not help. The feature set is inert — cargo clippy --workspace with default features gives the same 2.
GATE: PASS a7ab15cc was recorded while both warnings were live.

Scope — "never linted", not "never compiled"

The OFF configuration is type-checked in ci.yml's macOS build, cross check, and the CLI benchmark
run — all release/-scoped — and in release.yml. It is never linted anywhere. None of the three
required PR checks compiles anything (benchmark.yml's header: "These need no compile"; trace.sh and
reachability.sh make no cargo invocation), so no per-PR automation touches OFF at all.

The two live instances on main

Both are items reachable only from #[cfg(feature = "instruments")] code — one mechanism, not two:

  1. crates/openpulse-modem/src/engine.rs:17 — SoftCombiner, WindowArqFeedback,
    apply_window_retransmit, combine_llrs_map_in_ranges, encode_window_retransmit unused. Reached
    only from receive_with_soft_combining (:5742), receive_with_window_arq (:5941),
    transmit_window_retransmit_packet (:6031), receive_with_window_arq_selective (:6098).
  2. crates/openpulse-modem/src/engine.rs:6930 — private stage_modulate_payload_iq never used; its
    only caller is transmit_iq (:3641), same cfg.

Proposed fix

Add cargo clippy --workspace --no-default-features -- -D warnings as a second pass in gate.sh. Not
a changed flag
— --all-targets must stay, since CLAUDE.md justifies it explicitly ("without it,
test code is never linted and rots — that is how an unused binding sat in session_key.rs").

  • It must be --workspace. Narrowing to -p openpulse-modem --lib would miss a downstream crate's
    production code calling an instruments item, since that crate's lib builds against the ON modem.
  • Runtime: 9.1 s from a cold modem lib (fingerprints differ, so 41 crates recheck), ~1 s warm. The
    existing all-targets pass from the same cold state was 8.1 s.
  • Cost to get green: exactly the 2 warnings above, workspace-wide, nothing else. The edit is a
    #[cfg(feature = "instruments")] on the five imports and on stage_modulate_payload_iq.

Open question for the maintainer: whether the pre-push hook gets the same pass (it is ~1 s warm) or
whether the gate alone is enough, given #1144 already makes the hook a partial check by design.

🤖 Generated with Claude Code

https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions