Skip to content

dylint: Windows-only code is never linted — the Dylint job runs on ubuntu only #1359

Description

@zackees

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

  1. Immediate: decide the disposition of platform/windows/fs.rs:21. It is the moved primitive, so an allowlist entry is defensible — but display_slash there IS the workspace primitive now, so the honest fix is probably to allowlist the new path and drop the stale path.rs entry if it no longer owns the transform.
  2. Structural: add a windows-latest (and ideally macos-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.
  3. Once a Windows leg exists, sweep the other lints' allowlists for entries stranded by the same migration. ban_std_pathbuf hit exactly this failure mode on fix(build): anchor rebuild signatures to the compile workspace #1350, when a 1000-LOC file split moved COMPILER_IDENTITY_CACHE from 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_HOME must be exported (soldr manages its own rustup root), or the run dies with environment 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

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Status
    Triage

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions