Skip to content

Guarantee path-as-identity sites use NormalizedPath (map keys + Eq) #1274

Description

@zackees

Narrow replacement for #1271 (closed — see the closing comment there for why the blanket PathBufNormalizedPath migration is the wrong shape).

The gap

fbuild's path-identity bug class (#436/#437/#282) is already covered by two operation-targeting lints:

  • ban_raw_path_prefix_comparestarts_with/strip_prefix on raw paths (allowlist: 17)
  • ban_manual_slash_normalize — hand-rolled \/ (allowlist: 5)

The one hazard neither catches: a raw Path/PathBuf used as a HashMap/HashSet key or compared with ==. Raw Path equality is byte-exact, so C:\Fooc:/foo and //?/C:\xC:\x — exactly the Windows cache-key/watcher mismatch from #436/#437. NormalizedPath (case-folded, slash-normalized, UNC-stripped key) is the fix, and it is already used in the ~51 files that hold the primary keys.

Scope (bounded — do NOT expand to a type-wide ban)

  • Audit sites where a path is a map/set key or an ==/match operand:
    rg 'HashMap<PathBuf|HashSet<PathBuf|BTreeMap<PathBuf|HashMap<.*&?Path' plus raw path_a == path_b comparisons in crates/*/src/.
  • For each, confirm the key type is NormalizedPath (or the comparison routes through normalize_for_key). Migrate only these.
  • Consider a focused dylint: "raw Path/PathBuf as HashMap/HashSet key" — the residual the existing two lints miss. Only if the audit finds enough live sites to justify it.

Explicit non-goals

  • Not a blanket PathBufNormalizedPath swap. Plumbing paths (the ~3200 .join() + ~650 -> PathBuf that feed argv/Command) stay PathBuf.
  • Not driving the ban_std_pathbuf allowlist to zero. Leave it as a documented legacy no-op (or delete it — the operation lints carry the real load).
  • Do not swap physical canonicalization for NormalizedPath: NormalizedPath::new() strips .. lexically (path.rs:295) and case-folds key (path.rs:332), which is wrong across symlinks. library_manager::resolve_local_library_dir (test-emu does not resolve symlink library dependencies on Windows #1256/fix(library): resolve relative local dependency roots #1258) must keep std::fs::canonicalize.

Prerequisite if a migration reveals path-algebra needs

Only NormalizedPath::join() returns Self; parent()/with_extension()/components()/strip_prefix() deref to &Path/PathBuf. If the audit needs in-type path manipulation, add those Self-returning methods first — with a written decision on the lexical-.. / case-fold semantics — rather than sprinkling .into().

Related

#1271 (closed), #436, #437, #282, ban_raw_path_prefix_compare, ban_manual_slash_normalize.

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