Skip to content

Adopt NormalizePath everywhere + ban raw PathBuf/Path via dylint #437

Description

@zackees

Background

PR #436 (closing #428) ran into a Windows-only test failure:

left:  Some(\"/bin\\avr-nm\")
right: Some(\"/bin/avr-nm\")

Root cause: PathBuf::from(\"/bin/avr-size\").parent().join(\"avr-nm\") uses the platform separator (\ on Windows), so the in-memory string form drifts from the literal \"/...\" form fbuild builds path-like JSON strings out of. Same bytes in build_info.json mean different things on Linux vs Windows — and different cache keys when those strings feed cache hashing later.

zccache already solved this with core::path::NormalizedPath: an
Arc<Path> + Arc<str> precomputed comparison key that:

  • canonicalises separators (\/) on Windows,
  • case-folds on case-insensitive filesystems (Windows / default macOS),
  • is Hash/Eq/Ord on the cached key — no per-op allocation,
  • is Clone = 2× refcount bump (zero heap),
  • gives stable cache keys across platforms.

Source: https://github.com/zackees/zccache/blob/main/crates/zccache/src/core/path.rs (≈300 LOC).

Proposal

1. Vendor (or depend on) NormalizedPath

Option A: pub use zccache_core::NormalizedPath (already in our dep tree
via soldr/zccache integration).
Option B: vendor a copy into fbuild-core::path::NormalizedPath so we
own the API. Preferred for stability, since zccache is allowed to evolve
its internal types.

Either way, the public type lives in fbuild-core so every crate can
import it without breaking the dep graph.

2. Migrate fbuild to use NormalizedPath for stored / hashed paths

Targets (in priority order):

  • fbuild_build::build_info::BuildInfoprog_path, cc_path,
    cxx_path, ar_path, objcopy_path, size_path, nm_path,
    cppfilt_path, readelf_path, objdump_path, and every entry of the
    aliases block. Today these are String with whatever separator the
    source PathBuf::join chose; fixing them is what motivated this
    issue.
  • fbuild_build::pipeline cache fingerprints (FileStamp,
    BinArtifactCache, SizeArtifactCache) — anywhere a path is stamped
    into a cache key.
  • fbuild_packages toolchain / library directory keys.
  • fbuild_serial device path identifiers.

Locals on the stack and arguments to subprocess invocations can stay
Path / PathBuf — the lint targets stored / hashed / serialized
paths, not throwaway ones.

3. Enforce via dylint

We already have a custom-dylint pipeline (dylints/ban_raw_subprocess

  • .github/workflows/dylint.yml), so adding a second lint is the
    established pattern.

New lint: fbuild_normalize_path (or ban_raw_pathbuf).

Bans:

  • struct X { p: PathBuf } / struct X { p: String } where the field
    name matches *_path / *_dir / *_file.
  • serde::Serialize-derived structs with PathBuf / String path
    fields.
  • HashMap<PathBuf, _> / HashSet<PathBuf> / DashMap<PathBuf, _>.

Replacement guidance:

  • Stored or hashed: NormalizedPath.
  • Local / argument: &Path, &PathBuf, or &NormalizedPath.

Allow-list for places where the raw type is legitimate (subprocess
argv, FFI boundaries, tempfile internals).

CI gate: cargo dylint --all -- --workspace -- -D warnings.

4. Migration plan

Phase 1: land NormalizedPath in fbuild-core + add the dylint with
#[allow] on every existing call site (so CI still passes).

Phase 2: per-crate migration sprints (one PR per crate) removing the
#[allow]s.

Phase 3: drop the allow-list machinery once everything is migrated.

Acceptance

  • NormalizedPath exists in fbuild-core and Hash/Eq on it
    produce identical bytes for \"C:\\Users\\x\\a\" and
    \"C:/Users/x/a\".
  • BuildInfo's nine path fields are NormalizedPath, and
    build_info.json emits the canonical (slash-form, case-folded)
    string on every platform.
  • cargo dylint --all errors on any new pub p: PathBuf /
    pub p: String in a *_path slot.
  • fbuild symbols regression test from PR feat(symbols): ingest toolchain paths from build_info.json #436 passes on Linux,
    macOS, and Windows with the exact same string assertions (no
    pj() helper needed).

Related

🤖 Filed by Claude Code on behalf of @zackees during PR #436.

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