Repository navigation
Description
Problem
.github/workflows/dylint.yml runs runs-on: ubuntu-latest, and the sweep it performs only compiles the modules the target platform selects. So every #[cfg(windows)] / platform/windows/** module is invisible to the lint gate. Same for macOS-only code.
This is not hypothetical. A full local sweep on Windows against current main reports:
error: use fbuild_core::path::NormalizedPath::display_slash() instead of
hand-rolled backslash-to-slash rewrite
--> crates\fbuild-core\src\platform\windows\fs.rs:21:17
ban_manual_slash_normalize allowlists crates/fbuild-core/src/path.rs — the primitive's old home — but the platform-facade migration (#1344 / #1345 / #1348) moved the Windows implementation to crates/fbuild-core/src/platform/windows/fs.rs, which is not allowlisted. CI has never seen it because CI never compiles it.
Found while running a full local sweep for #1350; that branch does not touch the file, and the violation reproduces on main.
Why it matters
The lint suite is the enforcement arm for a lot of this repo's invariants (ban_std_pathbuf, ban_raw_path_prefix_compare, ban_manual_slash_normalize, enforce_platform_boundary, …). Windows is where most of the path-shaped bugs those lints exist to prevent actually bite — #875, #885, #890, #912 were all Windows path handling. Leaving Windows-only modules unlinted points the gate away from the platform it was written for.
The platform-facade migration makes this sharper, not softer: it deliberately concentrates per-OS mechanics into platform/{windows,unix,macos}/, so the whole point of those directories is code that only one runner ever compiles.
Proposal
- Immediate: decide the disposition of
platform/windows/fs.rs:21. It is the moved primitive, so an allowlist entry is defensible — butdisplay_slashthere IS the workspace primitive now, so the honest fix is probably to allowlist the new path and drop the stalepath.rsentry if it no longer owns the transform. - Structural: add a
windows-latest(and ideallymacos-latest) leg to the Dylint job. The driver build is the expensive part and it caches; the marginal cost is one more runner, against a gate that currently has a platform-shaped hole in it. - Once a Windows leg exists, sweep the other lints' allowlists for entries stranded by the same migration.
ban_std_pathbufhit exactly this failure mode on fix(build): anchor rebuild signatures to the compile workspace #1350, when a 1000-LOC file split movedCOMPILER_IDENTITY_CACHEfrom an allowlisted file into a new one — caught only because the sweep was run locally on Windows.
Repro
Run the local sweep from dylints/README.md → "Running locally" on a Windows host against main. Two things that README does not mention and which are needed for it to work:
RUSTUP_HOMEmust be exported (soldr manages its own rustup root), or the run dies withenvironment variable not found: RUSTUP_HOME.- A throwaway unique
RUSTFLAGS="--cfg <token>"per run, or a cached clean result is replayed and the sweep silently reports zero violations. This is the same trick the workflow itself uses with${GITHUB_RUN_ID}.
Worth folding both into that README as part of this issue.
Metadata
Metadata
Assignees
Labels
Type
Projects
- StatusShow more project fieldsTriage
Activity
fix(daemon): satisfy the workspace lint gate in the new profiling module
fix(daemon): satisfy the workspace lint gate in the new profiling module
feat(daemon): adopt mimalloc-pprof and prove heap, on-CPU, and off-CP…
ci(dylint): scope the lint crates' own tests to the ubuntu leg
ci(dylint): pin the workflow token to contents: read
ci(dylint): add a Windows leg so Windows-only code is actually linted (…
fix(ci): de-flake my own ETXTBSY retry test; stop guessing at #1373
fix(ci): de-flake the ETXTBSY retry test and harden the Windows PATH …