Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions ci/enforce_platform_boundary.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
LEDGER = ROOT / "ci/platform_boundary_ledger.tsv"
DYLINT_BASELINE = ROOT / "dylints/enforce_platform_boundary/src/baseline.txt"
PLATFORM_ROOT = "crates/fbuild-core/src/platform/"
CONCRETE_PREFIXES = tuple(PLATFORM_ROOT + host + "/" for host in ("windows", "linux", "macos"))
CONCRETE_PREFIXES = (
*(PLATFORM_ROOT + host + "/" for host in ("windows", "linux", "macos")),
)
AUTHORIZED_BOUNDARY_FINDINGS = {
(PLATFORM_ROOT + "executable.rs", "native_path", "std::env::current_exe"),
}
LEDGER_KINDS = {
"attr_cfg",
"cfg_macro",
Expand Down Expand Up @@ -76,6 +81,12 @@ def rows_from_findings(findings: list[research.Finding]) -> list[LedgerRow]:
ordinals: collections.Counter[tuple[str, str, str]] = collections.Counter()
rows: list[LedgerRow] = []
for finding in findings:
if finding.path.startswith(CONCRETE_PREFIXES) or (
finding.path,
finding.kind,
finding.normalized,
) in AUTHORIZED_BOUNDARY_FINDINGS:
continue
key = (finding.path, finding.kind, finding.normalized)
ordinal = ordinals[key]
ordinals[key] += 1
Expand Down Expand Up @@ -181,7 +192,9 @@ def scanner_dylint_counts(rows: list[LedgerRow]) -> collections.Counter[tuple[st
counts[(row.path, row.kind, identifier)] += 1
elif row.kind == "native_path":
normalized = row.normalized
if normalized.startswith("std::os::"):
if normalized == "std::env::current_exe":
key = normalized
elif normalized.startswith("std::os::"):
parts = normalized.split("::")
key = "::".join(parts[:3])
else:
Expand Down
233 changes: 0 additions & 233 deletions ci/platform_boundary_ledger.tsv

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions ci/platform_boundary_research.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
CFG_MACRO_START = re.compile(r"\bcfg\s*!\s*\(")
IDENTIFIER = re.compile(r"\b[A-Za-z_][A-Za-z0-9_]*\b")
NATIVE_PATHS = (
re.compile(r"\bstd\s*::\s*env\s*::\s*current_exe\b"),
re.compile(
r"\bstd\s*::\s*os\s*::\s*(?:windows|unix|linux|macos)\b"
r"(?:\s*::\s*[A-Za-z_][A-Za-z0-9_]*)*"
Expand Down Expand Up @@ -210,6 +211,8 @@ def enclosing_function(text: str, offset: int) -> str:
def classify(path: str, kind: str, normalized: str = "", context: str = "") -> tuple[str, str]:
"""Assign the phase-1 owner class; phase 2 validates this per occurrence."""
if kind in {"native_import", "native_path", "native_dependency"}:
if normalized == "std::env::current_exe":
return "host_executable", "host_mechanic"
if "::fs" in normalized or "permissions" in normalized.lower():
return "fs", "host_mechanic"
if "/fbuild-serial/" in f"/{path}/" or "/fbuild-deploy/" in f"/{path}/":
Expand Down
309 changes: 40 additions & 269 deletions ci/platform_boundary_research.tsv

Large diffs are not rendered by default.

69 changes: 68 additions & 1 deletion ci/test_enforce_platform_boundary.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import dataclasses
import re
import unittest

from ci import enforce_platform_boundary as boundary
Expand All @@ -13,10 +14,76 @@ def setUpClass(cls) -> None:
cls.observed = boundary.rows_from_findings(boundary.research.inventory())

def test_committed_exact_occurrence_ledger_matches_whole_tree(self) -> None:
self.assertEqual(len(self.expected), 504)
self.assertEqual(len(self.expected), 271)
self.assertFalse(boundary.validate_ledger(self.expected))
self.assertFalse(boundary.compare(self.expected, self.observed))

def test_private_platform_implementation_findings_are_not_baselined(self) -> None:
finding = boundary.research.Finding(
"crates/fbuild-core/src/platform/windows/example.rs",
1,
"compile_host_fact",
"std::env::consts::ARCH",
"host",
"host_mechanic",
)

self.assertEqual(boundary.rows_from_findings([finding]), [])

image_finding = boundary.research.Finding(
"crates/fbuild-core/src/platform/executable.rs",
1,
"native_path",
"std::env::current_exe",
"host_executable",
"host_mechanic",
)
self.assertEqual(boundary.rows_from_findings([image_finding]), [])

unauthorized_facade_finding = dataclasses.replace(
image_finding,
kind="cfg_macro",
normalized='cfg!(windows)',
)
self.assertEqual(
boundary.rows_from_findings([unauthorized_facade_finding]),
[
boundary.LedgerRow(
unauthorized_facade_finding.path,
unauthorized_facade_finding.kind,
unauthorized_facade_finding.normalized,
0,
unauthorized_facade_finding.capability,
unauthorized_facade_finding.classification,
)
],
)

def test_no_raw_host_fact_reads_remain_outside_the_boundary(self) -> None:
self.assertFalse(
[
row
for row in self.expected
if row.kind in {"cfg_macro", "compile_host_fact"}
]
)

def test_executable_spelling_does_not_bypass_the_executable_facade(self) -> None:
host_selected_exe = re.compile(
r"if\s+(?:fbuild_core|crate)::platform::host::is_windows\(\)"
r"\s*\{.{0,240}?\.exe",
re.DOTALL,
)
bypasses = []
for source in boundary.research.source_files():
text = source.read_text(encoding="utf-8")
for match in host_selected_exe.finditer(text):
if "platform::executable" not in match.group(0):
bypasses.append(
f"{source.relative_to(boundary.ROOT).as_posix()}:{text.count(chr(10), 0, match.start()) + 1}"
)
self.assertFalse(bypasses, bypasses)

def test_duplicate_and_non_contiguous_ordinal_are_rejected(self) -> None:
malformed = [*self.expected, self.expected[0]]
failures = boundary.validate_ledger(malformed)
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-arm/src/generic_arm/arm_linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ impl Linker for ArmLinker {
// FastLED/fbuild#809: bound the link step at 3 min — large
// STM32 HAL builds are still well inside this budget.
let link_timeout = Some(std::time::Duration::from_secs(180));
let result = if cfg!(windows) && args.len() > 50 {
let result = if fbuild_core::platform::host::is_windows() && args.len() > 50 {
let temp_dir = output_dir.join("tmp");
std::fs::create_dir_all(&temp_dir)?;
// FastLED/fbuild#911 — path-shape slash normalization goes
Expand Down
4 changes: 2 additions & 2 deletions crates/fbuild-build-arm/src/teensy/teensy_linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ impl Linker for TeensyLinker {
// FastLED/fbuild#809: bound the link step at 3 min — teensy41
// links comfortably under this budget.
let link_timeout = Some(std::time::Duration::from_secs(180));
let result = if cfg!(windows) && args.len() > 50 {
let result = if fbuild_core::platform::host::is_windows() && args.len() > 50 {
let temp_dir = output_dir.join("tmp");
std::fs::create_dir_all(&temp_dir)?;
// FastLED/fbuild#911 — path-shape slash normalization goes
Expand Down Expand Up @@ -275,7 +275,7 @@ mod tests {
/// be absolute.
#[test]
fn link_runs_in_absolute_output_dir_not_inherited_cwd() {
let (out, core_dir) = if cfg!(windows) {
let (out, core_dir) = if fbuild_core::platform::host::is_windows() {
("C:\\proj\\.fbuild\\build\\release", "C:\\pkgs\\teensy4")
} else {
("/proj/.fbuild/build/release", "/pkgs/teensy4")
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-engine/src/compiler_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ async fn compile_path_contract_pairs_cwd_and_output_arg_for_282() {

#[test]
fn absolute_from_cwd_is_identity_on_absolute_paths() {
let p = if cfg!(windows) {
let p = if fbuild_core::platform::host::is_windows() {
std::path::PathBuf::from(r"C:\some\absolute\path")
} else {
std::path::PathBuf::from("/some/absolute/path")
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-engine/src/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ mod tests {
/// Absolute path for the running platform (`/x` is *not* absolute on
/// Windows — it has a root but no drive prefix).
fn abs(tail: &str) -> PathBuf {
if cfg!(windows) {
if fbuild_core::platform::host::is_windows() {
PathBuf::from(format!("C:\\{}", tail.replace('/', "\\")))
} else {
PathBuf::from(format!("/{tail}"))
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-engine/src/script_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ pub async fn find_python() -> Option<Vec<String>> {
/// the daemon's spawn-time PATH (FastLED/fbuild#1219). The caller PATH
/// *replaces* the inherited one for the probe; `None` = legacy behavior.
pub async fn find_python_with_path(caller_path: Option<&str>) -> Option<Vec<String>> {
let candidates: &[&[&str]] = if cfg!(windows) {
let candidates: &[&[&str]] = if fbuild_core::platform::host::is_windows() {
&[&["python"], &["py", "-3"]]
} else {
&[&["python3"], &["python"]]
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-engine/src/source_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use walkdir::WalkDir;
fn normalize_glob_separators(pattern: &str) -> String {
// Glob-pattern normalization is INTENTIONALLY unconditional
// (unlike `NormalizedPath::display_slash()` which gates on
// `cfg!(windows)`) — glob patterns come from `platformio.ini` and
// `fbuild_core::platform::host::is_windows()`) — glob patterns come from `platformio.ini` and
// may contain a mix of `\` and `/` regardless of host OS.
pattern.replace('\\', "/")
}
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-esp/src/esp32/esp32_compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl Esp32Compiler {
verbose,
// On MSYS2/Git Bash, std::env::temp_dir() returns "/tmp/" which
// native Windows GCC treats as "C:\tmp\". Use LOCALAPPDATA\Temp.
if cfg!(windows) {
if fbuild_core::platform::host::is_windows() {
std::env::var("LOCALAPPDATA")
.map(|la| PathBuf::from(la).join("Temp"))
.unwrap_or_else(|_| std::env::temp_dir())
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build-esp/src/esp32/esp32_linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ impl Linker for Esp32Linker {
// link step in the codebase (LTO + large SDK archive). 5 min
// is a generous upper bound — anything past that is a wedge.
let link_timeout = Some(std::time::Duration::from_secs(300));
let result = if cfg!(windows) {
let result = if fbuild_core::platform::host::is_windows() {
let flags_for_rsp: Vec<String> = link_args[1..].to_vec();
let rsp_dir = output_dir.join("tmp");
let rsp_path = fbuild_core::response_file::write_response_file(
Expand Down
8 changes: 4 additions & 4 deletions crates/fbuild-build/tests/cache_survives_tar_extract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,9 @@ fn compiler_signature_survives_toolchain_path_change() {
let toolchain_a = TempDir::new().unwrap();
let toolchain_b = TempDir::new().unwrap();

let compiler_filename = if cfg!(windows) { "gcc.exe" } else { "gcc" };
let path_a: PathBuf = toolchain_a.path().join(compiler_filename);
let path_b: PathBuf = toolchain_b.path().join(compiler_filename);
let compiler_filename = fbuild_core::platform::executable::native_name("gcc");
let path_a: PathBuf = toolchain_a.path().join(&compiler_filename);
let path_b: PathBuf = toolchain_b.path().join(&compiler_filename);
assert_ne!(
path_a, path_b,
"test setup invariant: the two compiler paths must differ as absolute path strings"
Expand All @@ -183,7 +183,7 @@ fn compiler_signature_survives_toolchain_path_change() {
See crates/fbuild-build/src/compiler.rs::compiler_identity."
);

let alt_filename = if cfg!(windows) { "clang.exe" } else { "clang" };
let alt_filename = fbuild_core::platform::executable::native_name("clang");
let path_c = toolchain_a.path().join(alt_filename);
let sig_c = build_rebuild_signature(&path_c, &flags, &pre_flags, &extra_flags, &build_unflags);
assert_ne!(
Expand Down
8 changes: 2 additions & 6 deletions crates/fbuild-build/tests/clangd_check_parity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,9 @@ fn uno_project_dir() -> PathBuf {
/// `ClangComponentKind`'s variants.
fn find_clangd_on_path() -> Option<PathBuf> {
let path_var = std::env::var_os("PATH")?;
let exe_name = if cfg!(windows) {
"clangd.exe"
} else {
"clangd"
};
let exe_name = fbuild_core::platform::executable::native_name("clangd");
std::env::split_paths(&path_var)
.map(|dir| dir.join(exe_name))
.map(|dir| dir.join(&exe_name))
.find(|candidate| candidate.is_file())
}

Expand Down
25 changes: 13 additions & 12 deletions crates/fbuild-build/tests/flag_escaping_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,21 @@ fn collect_rs_files(dir: &Path) -> Vec<PathBuf> {
/// Check that a compiler file that has a non-Windows `run_command` path
/// also calls `prepare_flags_for_exec` in that path.
///
/// Heuristic: if a file contains both `run_command` and `cfg!(windows)` (the
/// response-file branch pattern), it MUST also contain `prepare_flags_for_exec`.
/// Heuristic: if a file contains both `run_command` and an `is_windows`
/// reference (the response-file branch pattern), it MUST also contain
/// `prepare_flags_for_exec`. Matching the symbol reference also covers direct,
/// imported, and aliased calls because the import still names `is_windows`.
#[test]
fn compiler_backends_must_sanitize_flags_for_exec() {
let src = crate_src_dir();
let mut rs_files = collect_rs_files(&src);

// Also scan fbuild-packages which has its own library compiler.
let packages_src = Path::new(env!("CARGO_MANIFEST_DIR"))
.parent()
.unwrap()
.join("fbuild-packages")
.join("src");
rs_files.extend(collect_rs_files(&packages_src));
// Also scan the packages and library crates, which own direct compiler
// backends outside this crate's source tree.
let crates_dir = Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap();
for crate_name in ["fbuild-packages", "fbuild-library"] {
rs_files.extend(collect_rs_files(&crates_dir.join(crate_name).join("src")));
}

let mut violations = Vec::new();

Expand All @@ -63,7 +64,7 @@ fn compiler_backends_must_sanitize_flags_for_exec() {
let has_run_command = content.contains("run_command");
let has_response_file =
content.contains("write_response_file") || content.contains("@response");
let has_cfg_windows = content.contains("cfg!(windows)");
let has_windows_host_branch = content.contains("is_windows");

// Linker files use response files for link flags (not -D defines),
// so they don't need prepare_flags_for_exec.
Expand All @@ -75,7 +76,7 @@ fn compiler_backends_must_sanitize_flags_for_exec() {
// compiler backend that must sanitize flags on the non-Windows path.
if has_run_command
&& has_response_file
&& has_cfg_windows
&& has_windows_host_branch
&& !is_linker
&& !content.contains("prepare_flags_for_exec")
{
Expand All @@ -96,7 +97,7 @@ fn compiler_backends_must_sanitize_flags_for_exec() {
to strip backslash-escaped quotes from -D define flags.\n\n\
Violations:\n{}\n\n\
Fix: add `crate::compiler::prepare_flags_for_exec(all_flags)` in the else \
branch of `cfg!(windows)`.",
branch of `fbuild_core::platform::host::is_windows()`.",
violations.join("\n")
);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/fbuild-build/tests/lite_scons_acceptance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use fbuild_build::script_runtime::resolve_extra_script_overlay;
fn python_available() -> bool {
use std::time::{Duration, Instant};

let probes: &[&[&str]] = if cfg!(windows) {
let probes: &[&[&str]] = if fbuild_core::platform::host::is_windows() {
&[&["python", "--version"], &["py", "-3", "--version"]]
} else {
&[&["python3", "--version"], &["python", "--version"]]
Expand Down
28 changes: 17 additions & 11 deletions crates/fbuild-build/tests/zccache_embedded_smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,25 @@ fn find_c_compiler() -> NormalizedPath {
.find(|candidate| candidate.is_file())
.map(NormalizedPath::from)
};
if cfg!(windows) {
if let Some(candidate) = on_path("clang.exe") {
return candidate;
if fbuild_core::platform::host::is_windows() {
for name in fbuild_core::platform::executable::path_candidate_names("clang") {
if let Some(candidate) = on_path(&name) {
return candidate;
}
}
if let Some(program_files) = std::env::var_os("ProgramFiles") {
let candidate = NormalizedPath::new(std::path::Path::new(&program_files))
.join("LLVM")
.join("bin")
.join("clang.exe");
.join(fbuild_core::platform::executable::native_name("clang"));
if candidate.is_file() {
return candidate;
}
}
if let Some(candidate) = on_path("gcc.exe") {
return candidate;
for name in fbuild_core::platform::executable::path_candidate_names("gcc") {
if let Some(candidate) = on_path(&name) {
return candidate;
}
}
panic!("clang.exe or gcc.exe must be installed for this smoke test");
}
Expand Down Expand Up @@ -89,11 +93,13 @@ async fn embedded_compilation_cold_miss_then_warm_hit() {
let tmp = tempfile::TempDir::new().expect("tempdir");
let cache_root = tmp.path().join("zccache");
let source = tmp.path().join("smoke.c");
let object = tmp.path().join(if cfg!(windows) {
"smoke.obj"
} else {
"smoke.o"
});
let object = tmp
.path()
.join(if fbuild_core::platform::host::is_windows() {
"smoke.obj"
} else {
"smoke.o"
});
std::fs::write(&source, "int smoke(void) { return 42; }\n").expect("write source");

let svc = FbuildZccacheService::start_in(cache_root)
Expand Down
6 changes: 3 additions & 3 deletions crates/fbuild-cli/src/cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use crate::daemon_client::{self, BuildRequest, DaemonClient};
use crate::output;

pub async fn open_in_browser(url: &str) -> fbuild_core::Result<()> {
let args: Vec<&str> = if cfg!(target_os = "windows") {
let args: Vec<&str> = if fbuild_core::platform::host::is_windows() {
vec!["cmd", "/c", "start", "", url]
} else if cfg!(target_os = "macos") {
} else if fbuild_core::platform::host::is_macos() {
vec!["open", url]
} else {
vec!["xdg-open", url]
Expand Down Expand Up @@ -159,7 +159,7 @@ pub async fn run_build(

/// Convert MSYS/Git-Bash paths (/c/Users/...) to native Windows paths and canonicalize.
pub async fn normalize_path(path: &str) -> fbuild_core::Result<String> {
let converted = if cfg!(windows) {
let converted = if fbuild_core::platform::host::is_windows() {
// /c/foo → C:\foo
let bytes = path.as_bytes();
if bytes.len() >= 3
Expand Down
Loading
Loading