fix(install): delete the dead proptest test-profile spec, and document why the rest cannot go today - #2575
Open
noahgift wants to merge 1 commit into
Open
fix(install): delete the dead proptest test-profile spec, and document why the rest cannot go today#2575noahgift wants to merge 1 commit into
noahgift wants to merge 1 commit into
Conversation
… RED to remove (#2571) 0.63.0 opens every `cargo install aprender` with two warnings: warning: profile package spec `proptest` in profile `dev` did not match any packages warning: profile package spec `proptest` in profile `test` did not match any packages The root manifest is BOTH the workspace root AND the published package, so its dev-only profile blocks ship to crates.io and are re-validated by an install resolve that excludes dev-dependencies. Neither spec can match there. Deleting both lines was the obvious fix and is WRONG. #1989 added them to kill a reproducible flake, and it still reproduces. Measured with `PROPTEST_CASES=1000000 cargo test -p aprender-train --lib config::validate::proptests` (the module #1989 named), PROPTEST_CASES engagement proven by 10x time scaling (5.45s at 100k -> 55.05s at 1M): both blocks removed -> rc=101, "assertion failed: self.low - result < self.intervals.step" at proptest-1.11.0/src/num/float_samplers.rs:466, in prop_negative_lr_fails dev block kept -> rc=0, 21 passed in 57.33s `cargo +nightly ... -Z unstable-options --unit-graph` confirms the mechanism rather than the intent: the proptest unit carries debug_assertions=true without the block and false with it. So exactly one of the two is dead. proptest is only ever built as a *dependency*, and dependencies build under `dev` even when the test target builds under `test`. Across the full workspace test graph there is exactly ONE proptest unit among 1110, and the `dev` block alone already sets debug_assertions=false on it. The `test` block never applied to anything. It is deleted here. Verified end-to-end on a real install-shaped resolve, not by inspection: `cargo install --path . --root <tmp>` printed 2 spec warnings before, 1 after. Four ways to remove the last warning were measured and rejected: - proptest in the root [dev-dependencies]: still warns (install skips dev-deps). - `[target.'cfg(any())'.dependencies] proptest`: silences it, but injects 39 phantom packages into EVERY consumer's Cargo.lock and audit/vendor surface. - `[profile.dev.package."*"]`: silences it, but disables debug-assertions for all third-party crates in our dev/test builds — weakening a real check for cosmetics. - a no-op `"*"` block beside the named spec: still warns. Downgrading proptest is not an escape either: float_samplers.rs is byte-identical 1.8.0..=1.11.0 apart from the f16 feature, and 1.11.0 is still the newest published. The cost-free fix is structural (split the facade out of the workspace root) and is left to #2571. FALSIFY-INSTALL-001 (crates/aprender-core/tests/monorepo_invariants.rs, already wired into ci.yml:346) walks the non-dev dependency closure of the published package and fails on any profile spec absent from it. Mutation-verified in three directions: re-adding the `test` block -> RED naming that block; deleting the `dev` block -> RED on the stale allowlist entry rather than passing silently; making the walk follow dev edges -> RED on the proptest vacuity canary (without which the gate would bless everything, which is how #2571 shipped). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VX2s9nQPbWDB3Vze4JM3Wg
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Partial fix for #2571, with the remainder documented rather than hidden.
What shipped
[profile.test.package.proptest]is deleted — it was dead. Plus 183 lines ofworkspace-invariant tests (12 passing) that had no home before.
What did NOT ship, and why
[profile.dev.package.proptest]stays, socargo installstill emits one warning.The reviewer caught the first attempt claiming this fixed when it was not, so to be
explicit: #2571 is not closed by this PR.
The root manifest is the published
aprendercrate ([package] name = "aprender"),so every
[profile.*]section it carries ships to consumers.proptestis adev-dependency and is absent from a
cargo installgraph, so cargo warns.Each way out was evaluated and costs more than the warning:
[target.'cfg(any())'.dependencies] proptestCargo.lock, and so into their cargo-audit / deny / vendor surface, for a crate that is never built[profile.dev.package."*"]*), but disablesdebug-assertionsfor all third-party crates in dev/test — weakening a real check to remove a cosmetic line"*"block beside the named specThe only cost-free fix is structural: split the
aprenderfacade out of the workspaceroot, so dev-only profiles stop being published at all. That is a real refactor, not a
release-day change, and it stays tracked in #2571.
Disposition
P2, cosmetic, does not block 0.64.0. One warning on install instead of two, and the
reasoning is now in the manifest where the next person will find it instead of
rediscovering it.
Refs #2571, #2566