test(proto): gate the rule that per-OS code lives in platform (R14) - #49
Merged
Merged
Conversation
CLAUDE.md says no #[cfg(windows)] in core/daemon code and the tree obeys it, but nothing checked. It does now, beside the layering test. The suggested rule — fail on an OS cfg outside a #[cfg(test)] region — could not be written as stated. #[cfg(test)] here is not one mod tests at the foot of a file: it sits on an impl, on a match arm, on a pub(crate) mod fake, on single functions, and seven files carry more than one. There is no region a line scan can find, and brace-counting files full of braces inside string literals fails open, which is the wrong direction for a gate. So it takes deny.toml's shape instead: two files permitted by name with a written reason, everything else refused, and a permitted file must hold exactly one #[cfg(test)] with every OS cfg after it. That last clause keeps the permission from widening — a file that grows a second #[cfg(test)] fails rather than silently meaning nothing. Watched failing three ways first: an unpermitted file, a permitted file with the attribute above its test module, and a permitted file with two #[cfg(test)]. cfg! is deliberately not caught. Forty-odd cfg!(windows) sit in production code and they are a different thing: both arms compile, type-check and are reachable from a test on every OS. #[cfg] deletes code from the build. standards/rust.md now says so.
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
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.
CLAUDE.mdsays no#[cfg(windows)]in core/daemon code and the tree obeys it, butnothing checked. It does now, beside the layering test.
The suggested rule could not be written as stated. "Fail on an OS cfg outside a
#[cfg(test)]region" assumes#[cfg(test)]is onemod testsat the foot of a file.Here it sits on an
impl, on a single match arm, on apub(crate) mod fake, onindividual functions — and seven files carry more than one. There is no region a line
scan can find, and brace-counting files that are full of braces inside string literals
fails open, which is the wrong direction for a gate.
So it takes
deny.toml's shape instead: two files permitted by name with a writtenreason, everything else refused, and a permitted file must hold exactly one
#[cfg(test)]with every OS cfg after it. That last clause is what keeps the permissionfrom widening — a file that grows a second
#[cfg(test)]fails rather than silentlymeaning nothing.
Watched failing three ways before it was trusted: an unpermitted file, a permitted file
with the attribute above its test module, and a permitted file with two
#[cfg(test)].cfg!is deliberately not caught. Forty-oddcfg!(windows)sit in production codeand they are a different thing: both arms compile, type-check and are reachable from a
test on every OS.
#[cfg]deletes code from the build, so the other two platformscompile a different program.
standards/rust.mdnow says so, and points atrecipes/mysql.rs, which goes further and passeswindowsas a function argument.CI: green. The first run had one unrelated red —
test (windows-latest)failed in thereal-Caddy suite with
.caddy.staging\Caddyfile: The system cannot find the file specified, which is thedocument::installstaging race already written up in thatfunction's own doc, now reproduced a second time. It is not this change's (a test in
protoand two markdown files cannot reach that suite) and it is not fixed here.