diff --git a/crates/fbuild-daemon/src/handlers/emulator/shared.rs b/crates/fbuild-daemon/src/handlers/emulator/shared.rs index 3628688b..b045bd28 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/shared.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/shared.rs @@ -95,9 +95,16 @@ pub(crate) fn qemu_session_dir(project_dir: &Path, env_name: &str) -> PathBuf { pub(crate) fn build_linux_macos_qemu_hint(err: &str) -> String { if cfg!(any(target_os = "linux", target_os = "macos")) { + let prefix = if err.is_empty() { + String::new() + } else { + format!("{}. ", err) + }; format!( - "{}. On Linux/macOS, ensure QEMU runtime deps are installed: libgcrypt, glib2, pixman, SDL2, and libslirp.", - err + "{}The cached QEMU toolchain may be incomplete or corrupt. \ + On Linux/macOS, also ensure runtime deps are installed: \ + libgcrypt, glib2, pixman, SDL2, and libslirp.", + prefix ) } else { err.to_string() @@ -351,6 +358,16 @@ pub(crate) async fn run_qemu_process( } else { MonitorOutcome::Success(format!("{} exited normally", label)) } + } else if status.code() == Some(127) { + MonitorOutcome::Error(format!( + "{} failed to start (exit code 127): a required shared library is missing.\n\ + {}\n\ + The cached QEMU toolchain may be incomplete or corrupt.\n\ + Delete the cached toolchain and retry:\n rm -rf $(dirname {})", + label, + build_linux_macos_qemu_hint(""), + qemu_path.display() + )) } else { MonitorOutcome::Error(format!( "{} exited with code {}", diff --git a/crates/fbuild-packages-fetch/src/lib.rs b/crates/fbuild-packages-fetch/src/lib.rs index 21eecaaf..ef9c003c 100644 --- a/crates/fbuild-packages-fetch/src/lib.rs +++ b/crates/fbuild-packages-fetch/src/lib.rs @@ -270,14 +270,26 @@ impl PackageBase { } /// Check if already installed in cache. + /// + /// Requires both the install directory AND the `.install_complete` sentinel + /// to be present. A directory without the sentinel is an incomplete or + /// partial install (e.g. a CI cache restored from a crashed job before the + /// atomic rename was committed, or an extract interrupted mid-flight). The + /// caller should treat this as "not installed" so the package is + /// re-extracted rather than invoked with a corrupt tree. + /// /// On a cache hit, bumps the LRU timestamp in the DiskCache index. pub fn is_cached(&self) -> bool { let path = self.install_path(); - let cached = path.exists() && path.is_dir(); - if cached { - self.touch_disk_cache(); + if !path.exists() || !path.is_dir() { + return false; } - cached + let sentinel = disk_cache::paths::install_complete_sentinel(&path); + if !sentinel.exists() { + return false; + } + self.touch_disk_cache(); + true } /// Best-effort LRU touch in the DiskCache index. @@ -619,6 +631,9 @@ mod toolchain_gcc_ar_tests { ); let install_path = base.install_path(); std::fs::create_dir_all(&install_path).unwrap(); + // Write the sentinel so is_cached() passes the completeness check. + let sentinel = disk_cache::paths::install_complete_sentinel(&install_path); + std::fs::write(&sentinel, b"").unwrap(); let disk_cache = DiskCache::open_at(&cache_root).unwrap(); let rel_path = install_path.strip_prefix(disk_cache.cache_root()).unwrap(); @@ -697,6 +712,36 @@ mod toolchain_gcc_ar_tests { ); assert!(disk_cache::paths::install_complete_sentinel(&installed).exists()); } + + #[test] + fn is_cached_returns_false_when_sentinel_is_missing() { + let tmp = tempfile::TempDir::new().unwrap(); + let cache_root = tmp.path().join("cache"); + let cache_key = "missing-sentinel-tool"; + let base = PackageBase::with_cache_root( + "tool", + "1.0", + cache_key, + cache_key, + None, + CacheSubdir::Toolchains, + tmp.path(), + &cache_root, + ); + let install_path = base.install_path(); + std::fs::create_dir_all(&install_path).unwrap(); + + // Directory exists but sentinel is missing → should NOT be considered cached. + assert!( + !base.is_cached(), + "directory without .install_complete sentinel must not be cached" + ); + + // Write the sentinel → now it should be cached. + let sentinel = disk_cache::paths::install_complete_sentinel(&install_path); + std::fs::write(&sentinel, b"").unwrap(); + assert!(base.is_cached(), "directory with sentinel should be cached"); + } } #[cfg(test)] diff --git a/crates/fbuild-toolchain/src/toolchain/esp_qemu.rs b/crates/fbuild-toolchain/src/toolchain/esp_qemu.rs index d4b9f3af..c278847c 100644 --- a/crates/fbuild-toolchain/src/toolchain/esp_qemu.rs +++ b/crates/fbuild-toolchain/src/toolchain/esp_qemu.rs @@ -114,49 +114,157 @@ impl EspQemu { } pub async fn resolve_executable(&self) -> Result { - if let Ok(raw) = std::env::var(self.arch.env_var()) { + let resolved = if let Ok(raw) = std::env::var(self.arch.env_var()) { let path = PathBuf::from(raw); let path = validate_qemu_path(path, self.arch.env_var())?; hydrate_windows_runtime(&path)?; validate_windows_runtime(&path)?; - return Ok(path); - } - - if let Some(path) = find_on_path(self.arch.binary_name()) { + path + } else if let Some(path) = find_on_path(self.arch.binary_name()) { hydrate_windows_runtime(&path)?; validate_windows_runtime(&path)?; - return Ok(path); - } - - if self.is_installed() { + path + } else if self.is_installed() { let path = find_qemu_binary(&self.base.install_path(), self.arch)?; hydrate_windows_runtime(&path)?; validate_windows_runtime(&path)?; - return Ok(path); - } - - if let Some(path) = find_existing_idf_qemu(self.arch) { + path + } else if let Some(path) = find_existing_idf_qemu(self.arch) { hydrate_windows_runtime(&path)?; validate_windows_runtime(&path)?; - return Ok(path); - } + path + } else { + let _ = self.ensure_installed().await?; + let path = find_qemu_binary(&self.base.install_path(), self.arch)?; + hydrate_windows_runtime(&path)?; + validate_windows_runtime(&path)?; + path + }; - let _ = self.ensure_installed().await?; - let path = find_qemu_binary(&self.base.install_path(), self.arch)?; - hydrate_windows_runtime(&path)?; - validate_windows_runtime(&path)?; - Ok(path) + // Preflight: verify the binary can actually start (shared library + // deps resolve) before handing it to the caller. On Linux, a + // missing .so exits with code 127 — much clearer to report here + // with the toolchain path than as a bare "exited with code 127" + // from the emulator runner. + preflight_qemu_binary(&resolved)?; + Ok(resolved) } fn validate_install_xtensa(install_dir: &Path) -> Result<()> { - let _ = find_qemu_binary(install_dir, EspQemuArch::Xtensa)?; + let exe = find_qemu_binary(install_dir, EspQemuArch::Xtensa)?; + qemu_validate_bundled_libs(&exe)?; Ok(()) } fn validate_install_riscv32(install_dir: &Path) -> Result<()> { - let _ = find_qemu_binary(install_dir, EspQemuArch::Riscv32)?; + let exe = find_qemu_binary(install_dir, EspQemuArch::Riscv32)?; + qemu_validate_bundled_libs(&exe)?; + Ok(()) + } +} + +/// Validate that the bundled `lib/` directory shipped by the Espressif QEMU +/// tarball is present alongside the binary. +/// +/// The tarball layout is: +/// ```text +/// qemu/ +/// ├── bin/qemu-system-xtensa +/// └── lib/libslirp.so.0 (with rpath $ORIGIN/../lib) +/// ``` +/// +/// `staged_install` already extracts-to-staging-then-atomic-rename and writes +/// a `.install_complete` sentinel, so a complete tree carries both. This +/// check defends against cache restoration from an older fbuild version that +/// predates those guards, or a CI cache that restored a partial tree. +fn qemu_validate_bundled_libs(qemu_binary: &Path) -> Result<()> { + let exe_dir = qemu_binary.parent(); + // Standard layout: binary is under `bin/`, lib is sibling to `bin/`. + if let Some(root) = exe_dir.and_then(|p| p.parent()) { + if root.join("lib").is_dir() { + return Ok(()); + } + } + // Alternative layout: binary is at the top level with a sibling `lib/`. + if let Some(dir) = exe_dir { + if dir.join("lib").is_dir() { + return Ok(()); + } + } + Err(FbuildError::PackageError(format!( + "Espressif QEMU installation at {} appears incomplete: \ + bundled lib/ directory not found. The cached toolchain may be corrupt. \ + Delete the cache entry and retry:\n rm -rf {}", + qemu_binary.display(), + qemu_binary + .parent() + .and_then(|p| p.parent()) + .unwrap_or(qemu_binary.parent().unwrap_or(qemu_binary)) + .display(), + ))) +} + +/// Probe the QEMU binary with `--version` to verify its shared library +/// dependencies resolve at runtime. +/// +/// Returns `Ok(())` if the probe succeeds, or a diagnostic `Err` if the +/// binary exits with code 127 (dynamic linker failure — missing .so). +/// Other probe failures are treated as non-fatal (the real run will +/// surface the error with full context). +fn preflight_qemu_binary(qemu_binary: &Path) -> Result<()> { + #[cfg(not(target_os = "linux"))] + { + let _ = qemu_binary; Ok(()) } + + #[cfg(target_os = "linux")] + { + // Short synchronous probe: verify the QEMU binary can start before we + // hand it to the async emulator runner. Uses run_command_blocking which + // routes through containment (no console flash on Windows, containment + // group on all platforms) and is ~100 ms. + let probe_result = fbuild_core::subprocess::run_command_blocking( + &[&qemu_binary.to_string_lossy(), "--version"], + None, // cwd + None, // env + Some(std::time::Duration::from_secs(5)), + ); + + match probe_result { + Ok(out) if out.success() => Ok(()), + Ok(out) if out.exit_code == 127 => { + // Try to identify which library is missing from the linker error. + let missing = out + .stderr + .lines() + .find(|l| l.contains("error while loading shared libraries")) + .map(|l| l.trim().to_string()); + + Err(FbuildError::PackageError(format!( + "QEMU at {} cannot start: a required shared library is missing.\n\ + {}\n\ + The cached QEMU toolchain appears incomplete or corrupt.\n\ + To fix, delete the cached toolchain and retry:\n rm -rf {}", + qemu_binary.display(), + missing.as_deref().unwrap_or(&format!( + "The dynamic linker reported: {}", + out.stderr.trim() + )), + qemu_binary + .parent() + .and_then(|p| p.parent()) + .unwrap_or(qemu_binary.parent().unwrap_or(qemu_binary)) + .display(), + ))) + } + Ok(_) | Err(_) => { + // Non-127 exit or spawn failure: non-fatal at this stage. + // The real QEMU run will surface the error with full context. + Ok(()) + } + } + } } #[async_trait::async_trait] @@ -703,4 +811,116 @@ mod tests { std::env::remove_var("PATH"); } } + + // ── qemu_validate_bundled_libs ────────────────────────────────── + + #[test] + fn bundled_libs_ok_standard_layout_bin_and_lib() { + let tmp = tempfile::TempDir::new().unwrap(); + let bin_dir = tmp.path().join("qemu").join("bin"); + let lib_dir = tmp.path().join("qemu").join("lib"); + std::fs::create_dir_all(&bin_dir).unwrap(); + std::fs::create_dir_all(&lib_dir).unwrap(); + let exe = bin_dir.join(EspQemuArch::Xtensa.binary_name()); + std::fs::write(&exe, b"").unwrap(); + qemu_validate_bundled_libs(&exe).expect("standard layout should pass"); + } + + #[test] + fn bundled_libs_ok_alt_layout_top_level_with_lib() { + let tmp = tempfile::TempDir::new().unwrap(); + let lib_dir = tmp.path().join("lib"); + std::fs::create_dir_all(&lib_dir).unwrap(); + let exe = tmp.path().join(EspQemuArch::Xtensa.binary_name()); + std::fs::write(&exe, b"").unwrap(); + qemu_validate_bundled_libs(&exe).expect("top-level layout should pass"); + } + + #[test] + fn bundled_libs_missing_lib_dir_is_error() { + let tmp = tempfile::TempDir::new().unwrap(); + let bin_dir = tmp.path().join("qemu").join("bin"); + std::fs::create_dir_all(&bin_dir).unwrap(); + let exe = bin_dir.join(EspQemuArch::Xtensa.binary_name()); + std::fs::write(&exe, b"").unwrap(); + let err = qemu_validate_bundled_libs(&exe).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("incomplete"), + "expected 'incomplete' in error: {msg}" + ); + assert!( + msg.contains("lib/"), + "expected 'lib/' mention in error: {msg}" + ); + } + + #[test] + fn bundled_libs_binary_at_root_no_lib_dir_is_error() { + let tmp = tempfile::TempDir::new().unwrap(); + let exe = tmp.path().join(EspQemuArch::Xtensa.binary_name()); + std::fs::write(&exe, b"").unwrap(); + let err = qemu_validate_bundled_libs(&exe).unwrap_err(); + assert!( + err.to_string().contains("incomplete"), + "should reject root binary without lib/" + ); + } + + // ── preflight_qemu_binary ─────────────────────────────────────── + + #[test] + fn preflight_ok_when_binary_runs_version_successfully() { + // On Linux, a real QEMU binary would pass. On non-Linux, + // preflight is a no-op. We test with a shell script that exits 0 + // so the probe succeeds cross-platform. + let tmp = tempfile::TempDir::new().unwrap(); + let probe = tmp.path().join("probe_qemu"); + if cfg!(windows) { + std::fs::write(&probe, b"@echo off\r\nexit /b 0\r\n").unwrap(); + } else { + std::fs::write(&probe, b"#!/bin/sh\nexit 0\n").unwrap(); + // make executable + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&probe, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + }; + // preflight is a no-op on non-Linux, and on Linux with a fake + // script that exits 0 it should pass. + let result = preflight_qemu_binary(&probe); + assert!( + result.is_ok(), + "preflight should pass when binary returns 0" + ); + } + + #[cfg(target_os = "linux")] + #[test] + fn preflight_linux_detects_missing_shared_library_exit_127() { + use std::os::unix::fs::PermissionsExt; + let tmp = tempfile::TempDir::new().unwrap(); + // Script that prints the canonical dynamic-linker error to stderr + // and exits 127 — same observable as a missing .so. + let probe = tmp.path().join("fake_qemu_missing_so"); + std::fs::write( + &probe, + b"#!/bin/sh\necho 'error while loading shared libraries: libslirp.so.0: cannot open shared object file' >&2\nexit 127\n", + ) + .unwrap(); + std::fs::set_permissions(&probe, std::fs::Permissions::from_mode(0o755)).unwrap(); + + let err = preflight_qemu_binary(&probe).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("shared library"), + "should report missing shared library: {msg}" + ); + assert!( + msg.contains("libslirp.so.0"), + "should name the missing library: {msg}" + ); + assert!(msg.contains("rm -rf"), "should suggest deletion: {msg}"); + } }