From 79cfdc0020eecfbba9508352e4e97dc3e981c65b Mon Sep 17 00:00:00 2001 From: Nick Nyssen <314538108+amoisttowe1@users.noreply.github.com> Date: Tue, 20 Oct 2026 01:59:35 -0700 Subject: [PATCH] M14: apply the dead-path rule to families, not just the last resort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-6 review caught that this branch rejected is_file() for one path and kept using it forty lines above. The family loop filtered candidates with is_file(), which follows symlinks, so an entry that exists but no longer resolves was classified "not installed" and skipped — exactly the defect this branch fixes elsewhere. Now both blocks share one rule: presence via symlink_metadata, then a hard requirement that the entry resolves. Absence is still tolerated and reported; a broken entry fails loudly with the family named, because that is the failure we are trying to catch rather than a machine difference. Checked against this Mac first: no family or last-resort path is present-but-dangling here, so the stricter rule cannot produce a spurious red on current macOS. Also corrected two stale comments — the tripwire test is separate from the layout test *below* it, and the oracle's doc comment now states the exception the PR description already does: a Mac where nothing in the list covers Devanagari fails, since such a machine cannot validate anything. Verified solo on macOS 26.6 (Apple M5 Max), no concurrent writers this time: fmt --all clean; clippy -D warnings all-targets quiet; system_fonts 13 passed with and without PDFCRAFT_SYSTEM_FONTS=0; whole crate 1341 passed / 0 failed / 5 ignored; xtask layers (37 crates) and assets (272/6/22). Mutation evidence for the change, via font folders redirected to /tmp so system paths were untouched: with a dangling DevanagariMT.ttc in the system folder the current code fails at "exists but does not resolve", while reverting to is_file() filtering makes the identical defect pass silently. Re-ran the pinned regressions too: cap raised to 512 MiB, MAX_FACES cut to 1, probe swapped to Arabic alef, last_resort derived from first(), misspelling restored — all red. --- crates/ui-egui/src/system_fonts.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/system_fonts.rs b/crates/ui-egui/src/system_fonts.rs index 944f424a2..a9c71ccec 100644 --- a/crates/ui-egui/src/system_fonts.rs +++ b/crates/ui-egui/src/system_fonts.rs @@ -492,7 +492,7 @@ mod tests { } } - /// Retune tripwires, deliberately separate from the layout test above: these assert that three + /// Retune tripwires, deliberately separate from `the_macos_devanagari_list_is_apples_layout` below: these assert that three /// constants still hold the values this module's contract depends on. They cannot fail on their /// own — they exist so that editing one is a decision someone has to make twice. Each was /// checked by deleting the pin and watching the matching regression go green again: raising @@ -549,8 +549,9 @@ mod tests { /// On a Mac, where Apple's fonts are installed: every family folder that really exists is /// reachable from the candidate list, and something in the list covers Devanagari. A family - /// Apple no longer ships is reported rather than failed — this list exists precisely for the - /// case where one of them is missing. CI has no macOS runner (`.github/workflows/ci.yml` + /// Apple no longer ships is reported rather than failed — this list exists precisely for the case + /// where one of them is missing, with one exception: if *nothing* here covers Devanagari the test + /// fails loudly, because a machine with no face to check cannot validate the list at all. CI has no macOS runner (`.github/workflows/ci.yml` /// keeps the org's Mac slots for release builds), so this runs on contributors' and /// maintainers' machines. #[cfg(target_os = "macos")] @@ -562,12 +563,21 @@ mod tests { } let list = devanagari_candidates(); for family in MACOS_DEVANAGARI_FAMILIES { - let installed: Vec = MACOS_FONT_DIRS.iter().map(|dir| Path::new(dir).join(family)).filter(|p| p.is_file()).collect(); - if installed.is_empty() { + // Same rule as the last resort below: an entry that exists counts, even a link whose + // target has gone — "present but unresolved" is precisely the dead-path defect this change + // is about, and filtering on `is_file()` would quietly reclassify it as "not installed". + // Absence is tolerated; a broken entry is not. + let present: Vec = + MACOS_FONT_DIRS.iter().map(|dir| Path::new(dir).join(family)).filter(|p| std::fs::symlink_metadata(p).is_ok()).collect(); + if present.is_empty() { eprintln!("{family}: not installed on this machine, so the list offers nothing that exists — fine, and reported only"); continue; } - for path in &installed { + for path in &present { + assert!( + path.is_file(), + "{family}: {path:?} exists but does not resolve — its target was moved or removed, so this candidate path has gone dead" + ); assert!(list.contains(path), "{path:?} is installed here but the candidate list skips it: {list:?}"); } }