diff --git a/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs b/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs index 61e68eff9..3d2f9d5b6 100644 --- a/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs +++ b/crates/fbuild-build-arm/src/generic_arm/arm_linker.rs @@ -227,6 +227,8 @@ impl Linker for ArmLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "arm-none-eabi-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs b/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs index 42602ca5a..7b863ebfe 100644 --- a/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs +++ b/crates/fbuild-build-arm/src/nrf52/nrf52_linker.rs @@ -152,6 +152,8 @@ impl Linker for Nrf52Linker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "arm-none-eabi-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-arm/src/renesas/renesas_linker.rs b/crates/fbuild-build-arm/src/renesas/renesas_linker.rs index e7b72f66f..e3e86e878 100644 --- a/crates/fbuild-build-arm/src/renesas/renesas_linker.rs +++ b/crates/fbuild-build-arm/src/renesas/renesas_linker.rs @@ -157,6 +157,8 @@ impl Linker for RenesasLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "arm-none-eabi-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-arm/src/sam/sam_linker.rs b/crates/fbuild-build-arm/src/sam/sam_linker.rs index d9fb9b395..3344f11b7 100644 --- a/crates/fbuild-build-arm/src/sam/sam_linker.rs +++ b/crates/fbuild-build-arm/src/sam/sam_linker.rs @@ -173,6 +173,8 @@ impl Linker for SamLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "arm-none-eabi-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-arm/src/silabs/silabs_linker.rs b/crates/fbuild-build-arm/src/silabs/silabs_linker.rs index d77fe6e91..bde730cb7 100644 --- a/crates/fbuild-build-arm/src/silabs/silabs_linker.rs +++ b/crates/fbuild-build-arm/src/silabs/silabs_linker.rs @@ -160,6 +160,8 @@ impl Linker for SilabsLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "arm-none-eabi-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-arm/src/teensy/teensy_linker.rs b/crates/fbuild-build-arm/src/teensy/teensy_linker.rs index cb575c902..0bf5d4ff6 100644 --- a/crates/fbuild-build-arm/src/teensy/teensy_linker.rs +++ b/crates/fbuild-build-arm/src/teensy/teensy_linker.rs @@ -204,6 +204,8 @@ impl Linker for TeensyLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "arm-none-eabi-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-engine/src/build_fingerprint/mod.rs b/crates/fbuild-build-engine/src/build_fingerprint/mod.rs index f18716a5a..c29e29be7 100644 --- a/crates/fbuild-build-engine/src/build_fingerprint/mod.rs +++ b/crates/fbuild-build-engine/src/build_fingerprint/mod.rs @@ -59,6 +59,14 @@ pub struct BinArtifactCache { pub flash_mode: String, pub flash_freq: String, pub flash_size: String, + /// Fingerprint of the esptool resolution that produced firmware.bin. + /// Empty when esptool is an absolute path (the resolution cannot + /// drift); a short hash of the caller's PATH when a bare-name spawn + /// resolves against it, so two requests with different caller PATHs + /// never share a cached bin (FastLED/fbuild#1219). `serde(default)` + /// keeps pre-existing absolute-path cache records valid. + #[serde(default)] + pub esptool_fingerprint: String, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/crates/fbuild-build-engine/src/lib.rs b/crates/fbuild-build-engine/src/lib.rs index b1fdf440c..26ad83e21 100644 --- a/crates/fbuild-build-engine/src/lib.rs +++ b/crates/fbuild-build-engine/src/lib.rs @@ -177,6 +177,12 @@ pub struct BuildParams { /// actually emitted. This unblocks per-symbol bloat analysis on /// over-budget builds. See FastLED/fbuild#594. pub bloat_analysis: bool, + /// Snapshot of the calling CLI's PATH, forwarded per request so + /// bare-name tool spawns (esptool, python, objcopy) resolve against + /// the caller's PATH instead of the daemon's potentially stale one. + /// `None` when no request context exists (tests, local CLI paths). + /// See FastLED/fbuild#1219. + pub caller_path: Option, } /// Trait for platform-specific build orchestrators. diff --git a/crates/fbuild-build-engine/src/linker.rs b/crates/fbuild-build-engine/src/linker.rs index b16dae119..7608c1f92 100644 --- a/crates/fbuild-build-engine/src/linker.rs +++ b/crates/fbuild-build-engine/src/linker.rs @@ -466,6 +466,7 @@ impl LinkerBase { output_format: &str, remove_sections: &[String], tool_label: &str, + caller_path: Option<&str>, ) -> Result { use fbuild_core::subprocess::run_command; @@ -490,11 +491,14 @@ impl LinkerBase { args.push(hex_path.to_string_lossy().to_string()); let args_ref: Vec<&str> = args.iter().map(|s| s.as_str()).collect(); + // A bare objcopy name resolves against the caller's PATH; absolute + // toolchain paths are unaffected (FastLED/fbuild#1219). + let env = fbuild_core::subprocess::bare_name_path_overlay(args_ref[0], caller_path); // FastLED/fbuild#809: objcopy ELF→HEX/BIN bounded at 1 min. let result = run_command( &args_ref, None, - None, + env.as_ref().map(|e| &e[..]), Some(std::time::Duration::from_secs(60)), ) .await?; diff --git a/crates/fbuild-build-engine/src/pipeline/context.rs b/crates/fbuild-build-engine/src/pipeline/context.rs index 4d44e460f..ccc59d5b5 100644 --- a/crates/fbuild-build-engine/src/pipeline/context.rs +++ b/crates/fbuild-build-engine/src/pipeline/context.rs @@ -69,9 +69,13 @@ impl BuildContext { let pio_overrides = fbuild_config::PioEnvOverrides::from_map(params.pio_env.clone()); let config = fbuild_config::PlatformIOConfig::from_path_with_overrides(&ini_path, pio_overrides)?; - let overlay = - crate::script_runtime::resolve_extra_script_overlay(project_dir, env_name, &config) - .await?; + let overlay = crate::script_runtime::resolve_extra_script_overlay( + project_dir, + env_name, + &config, + params.caller_path.as_deref(), + ) + .await?; if let Some(p) = perf.as_mut() { p.record("config-parse", t0.elapsed()); } diff --git a/crates/fbuild-build-engine/src/script_runtime.rs b/crates/fbuild-build-engine/src/script_runtime.rs index 427cbb59f..906b17b36 100644 --- a/crates/fbuild-build-engine/src/script_runtime.rs +++ b/crates/fbuild-build-engine/src/script_runtime.rs @@ -41,6 +41,7 @@ pub async fn resolve_extra_script_overlay( project_dir: &Path, env_name: &str, config: &fbuild_config::PlatformIOConfig, + caller_path: Option<&str>, ) -> fbuild_core::Result { let extra_scripts = config.get_extra_scripts(env_name)?; if extra_scripts.is_empty() { @@ -61,7 +62,7 @@ pub async fn resolve_extra_script_overlay( .to_string(), }; - let python = find_python().await.ok_or_else(|| { + let python = find_python(caller_path).await.ok_or_else(|| { fbuild_core::FbuildError::BuildFailed( "extra_scripts detected but no Python interpreter was found; \ install Python or use --platformio" @@ -116,10 +117,13 @@ pub async fn resolve_extra_script_overlay( // — config-time evaluation, never legitimately long. Bound to 60s // so a buggy or hostile script cannot wedge the daemon's build // pipeline indefinitely. + // The interpreter was resolved against the caller's PATH when it is a + // bare name — spawn it against the same PATH (FastLED/fbuild#1219). + let env = fbuild_core::subprocess::bare_name_path_overlay(argv[0], caller_path); let output = fbuild_core::subprocess::run_command( &argv, Some(project_dir), - None, + env.as_ref().map(|e| &e[..]), Some(std::time::Duration::from_secs(60)), ) .await @@ -298,7 +302,7 @@ fn libs_to_flags( Ok(flags) } -pub async fn find_python() -> Option> { +pub async fn find_python(caller_path: Option<&str>) -> Option> { let candidates: &[&[&str]] = if cfg!(windows) { &[&["python"], &["py", "-3"]] } else { @@ -308,12 +312,15 @@ pub async fn find_python() -> Option> { for candidate in candidates { let mut argv: Vec<&str> = candidate.to_vec(); argv.push("--version"); + // Probe against the caller's PATH so the daemon finds the same + // interpreter the CLI's environment would (FastLED/fbuild#1219). + let env = fbuild_core::subprocess::bare_name_path_overlay(candidate[0], caller_path); // FastLED/fbuild#809: `python --version` on the startup path — // bound tightly so a hung interpreter cannot wedge build init. if let Ok(output) = fbuild_core::subprocess::run_command( &argv, None, - None, + env.as_ref().map(|e| &e[..]), Some(std::time::Duration::from_secs(5)), ) .await diff --git a/crates/fbuild-build-engine/src/script_runtime_tests.rs b/crates/fbuild-build-engine/src/script_runtime_tests.rs index 618b088b7..3595c2f91 100644 --- a/crates/fbuild-build-engine/src/script_runtime_tests.rs +++ b/crates/fbuild-build-engine/src/script_runtime_tests.rs @@ -30,7 +30,7 @@ extra_scripts = {} async fn resolve_runtime_error(project_dir: &Path) -> String { let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")).unwrap(); - resolve_extra_script_overlay(project_dir, "demo", &config) + resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .unwrap_err() .to_string() @@ -126,7 +126,7 @@ fn test_scope_to_link_overlay_maps_libpath_and_libs() { #[tokio::test] async fn test_resolve_extra_script_overlay_supports_dump_shim() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -159,7 +159,7 @@ env.Append(CPPDEFINES=[\"DUMP_SHIM_OK\"]) let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")).unwrap(); // Pinned to MockEnv (see resolve_runtime_overlay note). - let overlay = resolve_extra_script_overlay(project_dir, "demo", &config) + let overlay = resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .unwrap(); assert!( @@ -172,7 +172,7 @@ env.Append(CPPDEFINES=[\"DUMP_SHIM_OK\"]) #[tokio::test] async fn test_resolve_extra_script_overlay_supports_common_noop_scons_helpers() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -209,7 +209,7 @@ env.Append(CPPDEFINES=[\"HELPERS_SHIM_OK\"]) let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")).unwrap(); // Pinned to MockEnv (see resolve_runtime_overlay note). - let overlay = resolve_extra_script_overlay(project_dir, "demo", &config) + let overlay = resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .unwrap(); assert!( @@ -222,7 +222,7 @@ env.Append(CPPDEFINES=[\"HELPERS_SHIM_OK\"]) #[tokio::test] async fn test_resolve_extra_script_overlay_supports_board_config_shim() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -256,7 +256,7 @@ env.Append(CPPDEFINES=[\"BOARD_CONFIG_SHIM_OK\"]) let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")).unwrap(); // Pinned to MockEnv (see resolve_runtime_overlay note). - let overlay = resolve_extra_script_overlay(project_dir, "demo", &config) + let overlay = resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .unwrap(); assert!( @@ -269,7 +269,7 @@ env.Append(CPPDEFINES=[\"BOARD_CONFIG_SHIM_OK\"]) #[tokio::test] async fn test_resolve_extra_script_overlay_supports_pio_platform_shim() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -306,7 +306,7 @@ env.Append(CPPDEFINES=[\"PIO_PLATFORM_SHIM_OK\"]) let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")).unwrap(); // Pinned to MockEnv (see resolve_runtime_overlay note). - let overlay = resolve_extra_script_overlay(project_dir, "demo", &config) + let overlay = resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .unwrap(); assert!( @@ -319,7 +319,7 @@ env.Append(CPPDEFINES=[\"PIO_PLATFORM_SHIM_OK\"]) #[tokio::test] async fn test_resolve_extra_script_overlay_rejects_unsupported_script_prefix() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -344,7 +344,7 @@ Import(\"env\") /// corrupt the harness's JSON-on-stdout protocol. #[tokio::test] async fn test_resolve_extra_script_overlay_tolerates_user_stdout_noise() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -363,7 +363,7 @@ print(\"post-append noise\") ); let config = fbuild_config::PlatformIOConfig::from_path(&temp.path().join("platformio.ini")).unwrap(); - let overlay = resolve_extra_script_overlay(temp.path(), "demo", &config) + let overlay = resolve_extra_script_overlay(temp.path(), "demo", &config, None) .await .unwrap(); assert!( @@ -416,7 +416,7 @@ framework = arduino async fn resolve_runtime_overlay(project_dir: &Path) -> BuildOverlay { let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")).unwrap(); - resolve_extra_script_overlay(project_dir, "demo", &config) + resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .unwrap() } @@ -428,7 +428,7 @@ async fn resolve_runtime_overlay(project_dir: &Path) -> BuildOverlay { /// `AddPostAction`. Source: MarlinFirmware/Marlin buildroot scripts. #[tokio::test] async fn test_shim_simple_marlin_cxxflags_style() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -477,7 +477,7 @@ env.AddPostAction(\"$PROGPATH\", lambda *a, **k: None) /// emit `-Dkey=value`, not a malformed array entry. #[tokio::test] async fn test_shim_simple_inplace_tuple_cppdefine() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -516,7 +516,7 @@ env.Append(CPPDEFINES=[\"PLAIN\"]) /// `LINKFLAGS`, and registers a no-op post action. #[tokio::test] async fn test_shim_medium_default_environment_linkflags() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } @@ -554,7 +554,7 @@ env.AddPostAction(\"$BUILD_DIR/firmware.bin\", after_build) /// script must not hard-fail and the parallel flag mutation must land. #[tokio::test] async fn test_shim_medium_nonflag_scope_does_not_reject() { - if find_python().await.is_none() { + if find_python(None).await.is_none() { return; } diff --git a/crates/fbuild-build-esp/src/esp32/esp32_linker.rs b/crates/fbuild-build-esp/src/esp32/esp32_linker.rs index 374fafca0..86f151695 100644 --- a/crates/fbuild-build-esp/src/esp32/esp32_linker.rs +++ b/crates/fbuild-build-esp/src/esp32/esp32_linker.rs @@ -151,6 +151,9 @@ pub struct Esp32Linker { /// falls back to an `esptool` on PATH. See FastLED/fbuild#954. esptool_bin: Option, verbose: bool, + /// Caller CLI's PATH snapshot for bare-name esptool resolution + /// (FastLED/fbuild#1219). + caller_path: Option, } impl Esp32Linker { @@ -171,6 +174,7 @@ impl Esp32Linker { max_ram: Option, esptool_bin: Option, verbose: bool, + caller_path: Option, ) -> Self { let flash_mode = flash_mode.unwrap_or_else(|| mcu_config.default_flash_mode().to_string()); Self { @@ -189,6 +193,7 @@ impl Esp32Linker { max_ram, esptool_bin, verbose, + caller_path, } } @@ -239,6 +244,26 @@ impl Esp32Linker { output_dir.join(".firmware_size_cache.json") } + /// Fingerprint the esptool resolution feeding the BIN cache. A + /// provisioned absolute path cannot drift → empty (matches serde's + /// default for pre-existing cache records). A bare `esptool` resolved + /// against a caller PATH gets a short hash of that PATH so requests + /// with different caller PATHs never share a cached firmware.bin + /// (FastLED/fbuild#1219). + fn esptool_fingerprint(&self) -> String { + if self.esptool_bin.is_some() { + return String::new(); + } + match self.caller_path.as_deref() { + Some(path) if !path.is_empty() => { + use sha2::{Digest, Sha256}; + let digest = Sha256::digest(path.as_bytes()); + digest[..8].iter().map(|b| format!("{b:02x}")).collect() + } + _ => String::new(), + } + } + fn current_bin_cache(&self, elf_path: &Path, flash_size: &str) -> Result { Ok(BinArtifactCache { version: BUILD_FINGERPRINT_VERSION, @@ -246,6 +271,7 @@ impl Esp32Linker { flash_mode: self.flash_mode.clone(), flash_freq: self.flash_freq.clone(), flash_size: flash_size.to_string(), + esptool_fingerprint: self.esptool_fingerprint(), }) } @@ -472,7 +498,18 @@ impl Linker for Esp32Linker { tracing::info!("elf2image: {}", argv.join(" ")); - match run_command(&args, None, None, Some(std::time::Duration::from_secs(60))).await { + // Resolve a bare `esptool` fallback against the caller's PATH; the + // overlay no-ops when a provisioned absolute path is in use (#1219). + let env = + fbuild_core::subprocess::bare_name_path_overlay(args[0], self.caller_path.as_deref()); + match run_command( + &args, + None, + env.as_ref().map(|e| &e[..]), + Some(std::time::Duration::from_secs(60)), + ) + .await + { Ok(result) if result.success() => { let cache = self.current_bin_cache(&elf_out, &flash_size)?; if let Err(e) = save_json(&self.bin_cache_path(output_dir), &cache) { @@ -562,9 +599,17 @@ mod tests { Some(327680), None, false, + None, ) } + fn test_linker_with(esptool_bin: Option, caller_path: Option) -> Esp32Linker { + let mut linker = test_linker("esp32c6"); + linker.esptool_bin = esptool_bin; + linker.caller_path = caller_path; + linker + } + #[test] fn test_esp32_linker_creation() { let linker = test_linker("esp32c6"); @@ -572,6 +617,69 @@ mod tests { assert_eq!(linker.max_ram, Some(327680)); } + /// FastLED/fbuild#1219 follow-up: with a bare-name esptool, two + /// requests with different caller PATHs may resolve different esptool + /// binaries — they must never share a cached firmware.bin. + #[test] + fn bare_name_esptool_bin_reuse_is_keyed_by_caller_path() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join("firmware.elf"); + std::fs::write(&elf, b"elf").unwrap(); + + let linker_a = test_linker_with(None, Some("C:\\venv-a\\Scripts".to_string())); + let flash_size = linker_a.flash_size(); + + // Simulate a successful conversion by linker A. + std::fs::write(tmp.path().join("firmware.bin"), b"bin").unwrap(); + let cache = linker_a.current_bin_cache(&elf, &flash_size).unwrap(); + save_json(&linker_a.bin_cache_path(tmp.path()), &cache).unwrap(); + + assert!( + linker_a.can_reuse_bin(&elf, tmp.path(), &flash_size), + "same caller PATH must reuse the cached bin" + ); + + let linker_b = test_linker_with(None, Some("C:\\venv-b\\Scripts".to_string())); + assert!( + !linker_b.can_reuse_bin(&elf, tmp.path(), &flash_size), + "a different caller PATH must not reuse a bin produced by another PATH's esptool" + ); + + let linker_none = test_linker_with(None, None); + assert!( + !linker_none.can_reuse_bin(&elf, tmp.path(), &flash_size), + "no caller PATH (daemon-ambient resolution) must not reuse a caller-PATH bin" + ); + } + + /// Provisioned absolute-path esptool cannot drift with the caller's + /// PATH — caching must behave exactly as before #1219, including + /// across requests with different caller PATHs. + #[test] + fn absolute_esptool_bin_reuse_ignores_caller_path() { + let tmp = tempfile::TempDir::new().unwrap(); + let elf = tmp.path().join("firmware.elf"); + std::fs::write(&elf, b"elf").unwrap(); + + let esptool = PathBuf::from("C:\\tools\\esptool.exe"); + let linker_a = test_linker_with(Some(esptool.clone()), Some("C:\\venv-a".to_string())); + let flash_size = linker_a.flash_size(); + + std::fs::write(tmp.path().join("firmware.bin"), b"bin").unwrap(); + let cache = linker_a.current_bin_cache(&elf, &flash_size).unwrap(); + assert!( + cache.esptool_fingerprint.is_empty(), + "absolute esptool must record the serde-default (empty) fingerprint" + ); + save_json(&linker_a.bin_cache_path(tmp.path()), &cache).unwrap(); + + let linker_b = test_linker_with(Some(esptool), Some("C:\\venv-b".to_string())); + assert!( + linker_b.can_reuse_bin(&elf, tmp.path(), &flash_size), + "absolute-path esptool must keep reusing regardless of caller PATH" + ); + } + #[test] fn test_flash_size_uses_board_max_flash_for_elf2image_and_cache() { let config = get_mcu_config("esp32c6").unwrap(); @@ -592,6 +700,7 @@ mod tests { Some(327680), None, false, + None, ); let tmp = tempfile::TempDir::new().unwrap(); let elf = tmp.path().join("firmware.elf"); @@ -658,6 +767,7 @@ mod tests { Some(327680), None, false, + None, ); let flags = linker.linker_flags(); assert!(flags.iter().any(|f| f.contains("IDF_TARGET_ESP32C6"))); @@ -702,6 +812,7 @@ mod tests { Some(327680), None, false, + None, ); let flags = linker.linker_flags(); assert!(flags.contains(&"-mlongcalls".to_string())); diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/boot_artifacts.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/boot_artifacts.rs index 83876d61c..70deeddbb 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/boot_artifacts.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/boot_artifacts.rs @@ -23,6 +23,7 @@ pub(super) async fn prepare_boot_artifacts( mcu_config: &Esp32McuConfig, flash_freq: &str, esptool_bin: Option<&Path>, + caller_path: Option<&str>, perf: &mut crate::perf_log::PerfTimer, ) -> Result<()> { let boot_artifacts_started = Instant::now(); @@ -91,10 +92,13 @@ pub(super) async fn prepare_boot_artifacts( &boot_dst_str, ); let args: Vec<&str> = argv.iter().map(|s| s.as_str()).collect(); + // A bare `esptool` fallback resolves against the caller's PATH; + // provisioned absolute paths are unaffected (#1219). + let env = fbuild_core::subprocess::bare_name_path_overlay(args[0], caller_path); match fbuild_core::subprocess::run_command( &args, None, - None, + env.as_ref().map(|e| &e[..]), Some(std::time::Duration::from_secs(60)), ) .await @@ -144,7 +148,7 @@ pub(super) async fn prepare_boot_artifacts( // `python` doesn't exist on modern distros (ubuntu 24.04 ships // only `python3`); resolve the interpreter the same way the // extra_scripts runtime does. - let python = crate::script_runtime::find_python() + let python = crate::script_runtime::find_python(caller_path) .await .unwrap_or_else(|| vec!["python".to_string()]); let mut args: Vec<&str> = python.iter().map(|s| s.as_str()).collect(); @@ -154,10 +158,12 @@ pub(super) async fn prepare_boot_artifacts( parts_csv_str.as_ref(), parts_dst_str.as_ref(), ]); + // Bare `python` spawn — resolve against the caller's PATH (#1219). + let env = fbuild_core::subprocess::bare_name_path_overlay(args[0], caller_path); match fbuild_core::subprocess::run_command( &args, None, - None, + env.as_ref().map(|e| &e[..]), Some(std::time::Duration::from_secs(10)), ) .await diff --git a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs index 10f6a3425..25cb9d761 100644 --- a/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs +++ b/crates/fbuild-build-esp/src/esp32/orchestrator/build.rs @@ -814,6 +814,7 @@ impl BuildOrchestrator for Esp32Orchestrator { ctx.board.max_ram, esptool_bin.clone().map(|path| path.into_path_buf()), params.verbose, + params.caller_path.clone(), ); let link_result = { @@ -842,6 +843,7 @@ impl BuildOrchestrator for Esp32Orchestrator { &mcu_config, &flash_freq, esptool_bin.as_ref().map(|path| path.as_path()), + params.caller_path.as_deref(), &mut perf, ) .await?; diff --git a/crates/fbuild-build-esp/src/esp8266/esp8266_linker.rs b/crates/fbuild-build-esp/src/esp8266/esp8266_linker.rs index c2a57bd09..49f405576 100644 --- a/crates/fbuild-build-esp/src/esp8266/esp8266_linker.rs +++ b/crates/fbuild-build-esp/src/esp8266/esp8266_linker.rs @@ -37,6 +37,9 @@ pub struct Esp8266Linker { max_flash: Option, max_ram: Option, verbose: bool, + /// Caller CLI's PATH snapshot for bare-name esptool resolution + /// (FastLED/fbuild#1219). + caller_path: Option, } impl Esp8266Linker { @@ -57,6 +60,7 @@ impl Esp8266Linker { max_flash: Option, max_ram: Option, verbose: bool, + caller_path: Option, ) -> Self { let flash_mode = flash_mode.unwrap_or_else(|| mcu_config.esptool.default_flash_mode.clone()); @@ -76,6 +80,7 @@ impl Esp8266Linker { max_flash, max_ram, verbose, + caller_path, } } } @@ -275,7 +280,17 @@ impl Linker for Esp8266Linker { &bin_str, ]; - match run_command(&args, None, None, Some(std::time::Duration::from_secs(30))).await { + // Bare `esptool` spawn — resolve it against the caller's PATH (#1219). + let env = + fbuild_core::subprocess::bare_name_path_overlay(args[0], self.caller_path.as_deref()); + match run_command( + &args, + None, + env.as_ref().map(|e| &e[..]), + Some(std::time::Duration::from_secs(30)), + ) + .await + { Ok(result) if result.success() => { // Verify the output file was actually created with content. // ESP8266 esptool may create segmented files (firmware.bin-0x00000.bin) @@ -306,6 +321,7 @@ impl Linker for Esp8266Linker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "xtensa-lx106-elf-objcopy", + self.caller_path.as_deref(), ) .await } @@ -361,6 +377,7 @@ mod tests { Some(4_194_304), Some(81_920), false, + None, ); assert!( linker diff --git a/crates/fbuild-build-esp/src/esp8266/orchestrator.rs b/crates/fbuild-build-esp/src/esp8266/orchestrator.rs index 686dbcaf3..b6a96783e 100644 --- a/crates/fbuild-build-esp/src/esp8266/orchestrator.rs +++ b/crates/fbuild-build-esp/src/esp8266/orchestrator.rs @@ -197,6 +197,7 @@ impl BuildOrchestrator for Esp8266Orchestrator { ctx.board.max_flash, ctx.board.max_ram, params.verbose, + params.caller_path.clone(), ); // 8. Build LibraryBuildEnv for project-as-library compilation diff --git a/crates/fbuild-build-mcu/src/avr/avr_linker.rs b/crates/fbuild-build-mcu/src/avr/avr_linker.rs index 89065867b..d4bc73e60 100644 --- a/crates/fbuild-build-mcu/src/avr/avr_linker.rs +++ b/crates/fbuild-build-mcu/src/avr/avr_linker.rs @@ -213,6 +213,8 @@ impl Linker for AvrLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "avr-objcopy", + // Absolute toolchain path; no caller-PATH overlay needed (#1219). + None, ) .await } diff --git a/crates/fbuild-build-mcu/src/ch32v/ch32v_linker.rs b/crates/fbuild-build-mcu/src/ch32v/ch32v_linker.rs index f193cdf94..b80638685 100644 --- a/crates/fbuild-build-mcu/src/ch32v/ch32v_linker.rs +++ b/crates/fbuild-build-mcu/src/ch32v/ch32v_linker.rs @@ -24,6 +24,9 @@ pub struct Ch32vLinker { max_ram: Option, memory_defsyms: Vec, verbose: bool, + /// Caller CLI's PATH snapshot for bare-name objcopy resolution + /// (FastLED/fbuild#1219). + caller_path: Option, } impl Ch32vLinker { @@ -39,6 +42,7 @@ impl Ch32vLinker { max_flash: Option, max_ram: Option, verbose: bool, + caller_path: Option, ) -> Self { Self { gcc_path, @@ -52,6 +56,7 @@ impl Ch32vLinker { max_ram, memory_defsyms: Vec::new(), verbose, + caller_path, } } @@ -153,6 +158,7 @@ impl Linker for Ch32vLinker { &self.mcu_config.objcopy.output_format, &self.mcu_config.objcopy.remove_sections, "riscv-none-elf-objcopy", + self.caller_path.as_deref(), ) .await } @@ -203,6 +209,7 @@ mod tests { Some(16384), Some(2048), false, + None, ); assert_eq!(linker.max_flash, Some(16384)); assert_eq!(linker.max_ram, Some(2048)); @@ -221,6 +228,7 @@ mod tests { Some(16384), Some(2048), false, + None, ); assert!( linker @@ -243,6 +251,7 @@ mod tests { None, None, false, + None, ) .with_memory_defsyms(vec![ "-Wl,--defsym=__FLASH_SIZE=32768".into(), @@ -278,6 +287,7 @@ mod tests { None, None, false, + None, ); let args = linker.build_link_args( &[], diff --git a/crates/fbuild-build-mcu/src/ch32v/orchestrator.rs b/crates/fbuild-build-mcu/src/ch32v/orchestrator.rs index 871a5cfba..82d6cf960 100644 --- a/crates/fbuild-build-mcu/src/ch32v/orchestrator.rs +++ b/crates/fbuild-build-mcu/src/ch32v/orchestrator.rs @@ -210,6 +210,7 @@ impl BuildOrchestrator for Ch32vOrchestrator { ctx.board.max_flash, ctx.board.max_ram, params.verbose, + params.caller_path.clone(), ) .with_memory_defsyms(memory_defsyms); diff --git a/crates/fbuild-build/src/compile_many.rs b/crates/fbuild-build/src/compile_many.rs index c29e49e0f..2bfe39d7d 100644 --- a/crates/fbuild-build/src/compile_many.rs +++ b/crates/fbuild-build/src/compile_many.rs @@ -373,6 +373,7 @@ async fn build_one_sketch(inputs: SketchBuildInputs) -> SketchResult { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let outcome = match get_orchestrator(platform) { diff --git a/crates/fbuild-build/tests/avr_build.rs b/crates/fbuild-build/tests/avr_build.rs index 2a016de2b..6d25bc27c 100644 --- a/crates/fbuild-build/tests/avr_build.rs +++ b/crates/fbuild-build/tests/avr_build.rs @@ -128,6 +128,7 @@ async fn build_uno_minimal() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::avr::orchestrator::AvrOrchestrator; @@ -226,6 +227,7 @@ async fn compare_with_python_output() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::avr::orchestrator::AvrOrchestrator; @@ -318,6 +320,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::avr::orchestrator::AvrOrchestrator; @@ -385,6 +388,7 @@ fn uno_build_params(project_dir: &Path, build_dir: PathBuf, clean: bool) -> Buil extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, } } diff --git a/crates/fbuild-build/tests/clangd_check_parity.rs b/crates/fbuild-build/tests/clangd_check_parity.rs index 00181b3d6..d684e5642 100644 --- a/crates/fbuild-build/tests/clangd_check_parity.rs +++ b/crates/fbuild-build/tests/clangd_check_parity.rs @@ -160,6 +160,7 @@ async fn clangd_check_parity_uno() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = AvrOrchestrator; diff --git a/crates/fbuild-build/tests/eh_frame_strip_esp32.rs b/crates/fbuild-build/tests/eh_frame_strip_esp32.rs index 80804e329..0078f9ad1 100644 --- a/crates/fbuild-build/tests/eh_frame_strip_esp32.rs +++ b/crates/fbuild-build/tests/eh_frame_strip_esp32.rs @@ -54,6 +54,7 @@ fn make_params(project_dir: &Path) -> BuildParams { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, } } diff --git a/crates/fbuild-build/tests/esp32_build.rs b/crates/fbuild-build/tests/esp32_build.rs index 6afc16bdf..d070edafa 100644 --- a/crates/fbuild-build/tests/esp32_build.rs +++ b/crates/fbuild-build/tests/esp32_build.rs @@ -98,6 +98,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -190,6 +191,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -275,6 +277,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -361,6 +364,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -437,6 +441,7 @@ async fn build_esp32s3_fixture() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -505,6 +510,7 @@ async fn build_nightdriverstrip_demo() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -604,6 +610,7 @@ async fn incremental_build_at(project_dir: &std::path::Path, env_name: &str) { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; @@ -703,6 +710,7 @@ async fn incremental_nightdriverstrip_one_file_changed() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; diff --git a/crates/fbuild-build/tests/lite_scons_acceptance.rs b/crates/fbuild-build/tests/lite_scons_acceptance.rs index f2e4801ee..036cfddf8 100644 --- a/crates/fbuild-build/tests/lite_scons_acceptance.rs +++ b/crates/fbuild-build/tests/lite_scons_acceptance.rs @@ -95,7 +95,7 @@ fn write_project(extra_scripts: &str, scripts: &[(&str, &str)]) -> tempfile::Tem async fn resolve_lite(project_dir: &Path) -> BuildOverlay { let config = fbuild_config::PlatformIOConfig::from_path(&project_dir.join("platformio.ini")) .expect("parse platformio.ini"); - resolve_extra_script_overlay(project_dir, "demo", &config) + resolve_extra_script_overlay(project_dir, "demo", &config, None) .await .expect("lite-SCons harness must succeed for the 5 spike patterns") } diff --git a/crates/fbuild-build/tests/nxplpc_build_flags.rs b/crates/fbuild-build/tests/nxplpc_build_flags.rs index 57484ab55..29076d3c8 100644 --- a/crates/fbuild-build/tests/nxplpc_build_flags.rs +++ b/crates/fbuild-build/tests/nxplpc_build_flags.rs @@ -85,6 +85,7 @@ async fn lpc845brk_propagates_build_flags_to_library_compile_587() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::nxplpc::orchestrator::NxpLpcOrchestrator; diff --git a/crates/fbuild-build/tests/nxplpc_core_compile_commands.rs b/crates/fbuild-build/tests/nxplpc_core_compile_commands.rs index 77769c092..a10e20784 100644 --- a/crates/fbuild-build/tests/nxplpc_core_compile_commands.rs +++ b/crates/fbuild-build/tests/nxplpc_core_compile_commands.rs @@ -56,6 +56,7 @@ async fn build_core_repo(repo: &Path, env_name: &str) -> tempfile::TempDir { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::nxplpc::orchestrator::NxpLpcOrchestrator; diff --git a/crates/fbuild-build/tests/stm32_acceptance.rs b/crates/fbuild-build/tests/stm32_acceptance.rs index 784f4821f..c6cd0c1b3 100644 --- a/crates/fbuild-build/tests/stm32_acceptance.rs +++ b/crates/fbuild-build/tests/stm32_acceptance.rs @@ -104,6 +104,7 @@ async fn stm32f103c8_blink_with_spi_auto_discovers_library_205_ac4() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::stm32::orchestrator::Stm32Orchestrator; diff --git a/crates/fbuild-build/tests/teensy30_acceptance.rs b/crates/fbuild-build/tests/teensy30_acceptance.rs index 63229ed20..0391cf9d0 100644 --- a/crates/fbuild-build/tests/teensy30_acceptance.rs +++ b/crates/fbuild-build/tests/teensy30_acceptance.rs @@ -122,6 +122,7 @@ async fn teensy30_analog_output_meets_205_ac2() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let result = diff --git a/crates/fbuild-build/tests/teensy_build.rs b/crates/fbuild-build/tests/teensy_build.rs index 8ada8f25c..de7304df5 100644 --- a/crates/fbuild-build/tests/teensy_build.rs +++ b/crates/fbuild-build/tests/teensy_build.rs @@ -96,6 +96,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::teensy::orchestrator::TeensyOrchestrator; @@ -171,6 +172,7 @@ void loop() {} extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::teensy::orchestrator::TeensyOrchestrator; @@ -224,6 +226,7 @@ async fn build_teensy41_fixture() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::teensy::orchestrator::TeensyOrchestrator; @@ -336,6 +339,7 @@ void loop() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::teensy::orchestrator::TeensyOrchestrator; diff --git a/crates/fbuild-build/tests/teensylc_acceptance.rs b/crates/fbuild-build/tests/teensylc_acceptance.rs index 44c215533..72a6d7a38 100644 --- a/crates/fbuild-build/tests/teensylc_acceptance.rs +++ b/crates/fbuild-build/tests/teensylc_acceptance.rs @@ -74,6 +74,7 @@ async fn teensylc_blink_meets_205_acceptance_criteria() { extra_build_flags: Vec::new(), watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let result = diff --git a/crates/fbuild-cli/src/cli/build.rs b/crates/fbuild-cli/src/cli/build.rs index e9a4f9ce6..c63e277ad 100644 --- a/crates/fbuild-cli/src/cli/build.rs +++ b/crates/fbuild-cli/src/cli/build.rs @@ -120,6 +120,7 @@ pub async fn run_build( output_dir, pio_env: daemon_client::capture_pio_env(), bloat_analysis, + caller_path: daemon_client::capture_caller_path(), }; let stream_start = std::time::Instant::now(); diff --git a/crates/fbuild-cli/src/cli/clean.rs b/crates/fbuild-cli/src/cli/clean.rs index 9a7a30905..1a089ab11 100644 --- a/crates/fbuild-cli/src/cli/clean.rs +++ b/crates/fbuild-cli/src/cli/clean.rs @@ -59,6 +59,7 @@ pub async fn run_clean( output_dir: None, pio_env: daemon_client::capture_pio_env(), bloat_analysis: false, + caller_path: daemon_client::capture_caller_path(), }; let response = client.build_streaming(&req).await?; diff --git a/crates/fbuild-cli/src/cli/deploy.rs b/crates/fbuild-cli/src/cli/deploy.rs index 227151fa8..59efeb187 100644 --- a/crates/fbuild-cli/src/cli/deploy.rs +++ b/crates/fbuild-cli/src/cli/deploy.rs @@ -222,6 +222,7 @@ pub async fn run_deploy( output_dir, pio_env: daemon_client::capture_pio_env(), usb_recovery_policy, + caller_path: daemon_client::capture_caller_path(), }; let resp = client.deploy(&req).await?; diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index 07903721f..5c5fb00e7 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -1014,6 +1014,12 @@ where }) } +/// Snapshot the calling CLI's PATH for per-request forwarding so the daemon +/// can resolve bare-name tool spawns against it (FastLED/fbuild#1219). +pub(crate) fn capture_caller_path() -> Option { + launcher_path(std::env::vars_os()).map(|p| p.to_string_lossy().into_owned()) +} + /// Spawn a single daemon process instance. async fn spawn_daemon_process() -> fbuild_core::Result<()> { // FastLED/fbuild#830: prefer a `fbuild-daemon` binary sitting next diff --git a/crates/fbuild-cli/src/daemon_client/types.rs b/crates/fbuild-cli/src/daemon_client/types.rs index ddb136d87..dfdeb9d5b 100644 --- a/crates/fbuild-cli/src/daemon_client/types.rs +++ b/crates/fbuild-cli/src/daemon_client/types.rs @@ -55,6 +55,10 @@ pub struct BuildRequest { /// See FastLED/fbuild#594. #[serde(default)] pub bloat_analysis: bool, + /// Snapshot of the caller's PATH so the daemon resolves bare-name tool + /// spawns against it instead of its own stale PATH (FastLED/fbuild#1219). + #[serde(skip_serializing_if = "Option::is_none")] + pub caller_path: Option, } /// `POST /api/install-deps` request. Mirrors @@ -138,6 +142,10 @@ pub struct DeployRequest { /// intentionally non-elevating; #1147 is the first daemon consumer. #[serde(default, skip_serializing_if = "is_default_usb_recovery_policy")] pub usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy, + /// Snapshot of the caller's PATH so the daemon resolves bare-name tool + /// spawns against it instead of its own stale PATH (FastLED/fbuild#1219). + #[serde(skip_serializing_if = "Option::is_none")] + pub caller_path: Option, } fn is_default_usb_recovery_policy(policy: &fbuild_core::usb::UsbRecoveryPolicy) -> bool { diff --git a/crates/fbuild-cli/src/mcp/tools.rs b/crates/fbuild-cli/src/mcp/tools.rs index 116d59e67..e6eff73f3 100644 --- a/crates/fbuild-cli/src/mcp/tools.rs +++ b/crates/fbuild-cli/src/mcp/tools.rs @@ -122,6 +122,7 @@ pub(super) async fn execute_tool( output_dir: None, pio_env: crate::daemon_client::capture_pio_env(), bloat_analysis: false, + caller_path: crate::daemon_client::capture_caller_path(), }; let resp = client @@ -189,6 +190,7 @@ pub(super) async fn execute_tool( output_dir: None, pio_env: crate::daemon_client::capture_pio_env(), usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy::Default, + caller_path: crate::daemon_client::capture_caller_path(), }; let resp = client diff --git a/crates/fbuild-core/src/subprocess.rs b/crates/fbuild-core/src/subprocess.rs index 7ffcff960..070ad4233 100644 --- a/crates/fbuild-core/src/subprocess.rs +++ b/crates/fbuild-core/src/subprocess.rs @@ -660,9 +660,7 @@ fn compute_env(program: &str, overlay: Option<&[(&str, &str)]>) -> Option) -> Option, + vars: &[(&str, &str)], +) { + for (k, v) in vars { + env_map.retain(|existing, _| !existing.eq_ignore_ascii_case(k)); + env_map.insert((*k).to_string(), (*v).to_string()); + } +} + +/// PATH overlay for a bare-name program spawn: when `program` has no +/// path separator and a caller PATH snapshot is available, resolve the +/// spawn against the caller's PATH instead of the daemon's. Absolute +/// or relative-path programs never need it. +pub fn bare_name_path_overlay<'a>( + program: &str, + caller_path: Option<&'a str>, +) -> Option<[(&'static str, &'a str); 1]> { + if program.contains('/') || program.contains('\\') { + return None; + } + match caller_path { + Some(p) if !p.is_empty() => Some([("PATH", p)]), + _ => None, + } +} + #[cfg(windows)] fn is_msys_environment(env_map: &std::collections::BTreeMap) -> bool { env_map.contains_key("MSYSTEM") || env_map.contains_key("MSYS") @@ -993,6 +1026,146 @@ mod tests { assert!(result.stdout.trim().contains("no-timeout")); } + // FastLED/fbuild#1219: a caller-supplied PATH overlay must replace + // the inherited Windows "Path" entry, not coexist with it — the + // Command env map is case-insensitive last-insert-wins, so a + // duplicate lets the daemon's stale Path silently win. + #[cfg(windows)] + #[test] + fn path_overlay_replaces_inherited_path_case_insensitively() { + let env = compute_env("esptool", Some(&[("PATH", "X-marker")])) + .expect("windows always returns a full env vector"); + let path_entries: Vec<&(String, String)> = env + .iter() + .filter(|(k, _)| k.eq_ignore_ascii_case("path")) + .collect(); + assert_eq!( + path_entries.len(), + 1, + "exactly one PATH-ish key expected, got {path_entries:?}" + ); + assert_eq!(path_entries[0].1, "X-marker"); + } + + // Deterministic form of the #1219 regression: the test process env + // may already spell the key "PATH" (bash/soldr do), which masks the + // duplicate-key bug in the end-to-end test above. Seed the map with + // the Windows-typical "Path" casing explicitly. + #[cfg(windows)] + #[test] + fn path_overlay_wins_over_differently_cased_inherited_key() { + let mut env_map = std::collections::BTreeMap::new(); + env_map.insert("Path".to_string(), "stale-daemon-path".to_string()); + apply_overlay_windows(&mut env_map, &[("PATH", "X-marker")]); + + let path_entries: Vec<(&String, &String)> = env_map + .iter() + .filter(|(k, _)| k.eq_ignore_ascii_case("path")) + .collect(); + assert_eq!( + path_entries.len(), + 1, + "exactly one PATH-ish key expected, got {path_entries:?}" + ); + assert_eq!(path_entries[0].1, "X-marker"); + } + + #[test] + fn bare_name_path_overlay_applies_only_to_bare_names() { + assert_eq!( + bare_name_path_overlay("esptool", Some("C:\\caller\\bin")), + Some([("PATH", "C:\\caller\\bin")]) + ); + assert_eq!( + bare_name_path_overlay("C:\\tools\\esptool.exe", Some("C:\\caller\\bin")), + None + ); + assert_eq!( + bare_name_path_overlay("tools/esptool", Some("C:\\caller\\bin")), + None + ); + assert_eq!(bare_name_path_overlay("esptool", None), None); + assert_eq!(bare_name_path_overlay("esptool", Some("")), None); + } + + /// End-to-end proof for FastLED/fbuild#1219: a PATH overlay must make + /// `run_command` resolve a bare executable name from the overlay's + /// directory — not just produce the right `("PATH", …)` tuple. Uses a + /// uniquely named probe executable staged into a TempDir so ambient + /// PATH can never satisfy the lookup. + #[tokio::test] + async fn run_command_resolves_bare_name_from_overlay_path() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path(); + + #[cfg(windows)] + let probe_path = { + let system_root = std::env::var("SystemRoot").expect("SystemRoot must be set"); + let src = Path::new(&system_root).join("System32").join("cmd.exe"); + let dst = dir.join("fbuild_1219_probe.exe"); + std::fs::copy(&src, &dst).expect("copy cmd.exe into overlay dir"); + dst + }; + #[cfg(unix)] + let probe_path = { + use std::os::unix::fs::PermissionsExt; + let dst = dir.join("fbuild_1219_probe"); + std::fs::write(&dst, "#!/bin/sh\necho overlay-marker\n").expect("write probe script"); + std::fs::set_permissions(&dst, std::fs::Permissions::from_mode(0o755)) + .expect("chmod probe script"); + dst + }; + + let dir_str = dir.to_string_lossy(); + let overlay = [("PATH", dir_str.as_ref())]; + let bare_args: Vec<&str> = if cfg!(windows) { + vec!["fbuild_1219_probe", "/C", "echo overlay-marker"] + } else { + vec!["fbuild_1219_probe"] + }; + + // Bare name + overlay → resolved from the overlay PATH. + let result = run_command( + &bare_args, + None, + Some(&overlay), + Some(Duration::from_secs(30)), + ) + .await + .expect("bare-name spawn must resolve via the overlay PATH"); + assert!(result.success(), "got: {result:?}"); + assert!( + result.stdout.contains("overlay-marker"), + "stdout was {:?}", + result.stdout + ); + + // Absolute path, no overlay → normal operation is unaffected. + let probe_str = probe_path.to_string_lossy(); + let abs_args: Vec<&str> = if cfg!(windows) { + vec![probe_str.as_ref(), "/C", "echo overlay-marker"] + } else { + vec![probe_str.as_ref()] + }; + let result = run_command(&abs_args, None, None, Some(Duration::from_secs(30))) + .await + .expect("absolute-path spawn works without any overlay"); + assert!(result.success(), "got: {result:?}"); + assert!( + result.stdout.contains("overlay-marker"), + "stdout was {:?}", + result.stdout + ); + + // Bare name WITHOUT the overlay → not on ambient PATH → spawn + // fails, proving the first resolution came from the overlay. + let result = run_command(&bare_args, None, None, Some(Duration::from_secs(30))).await; + assert!( + result.is_err(), + "bare name must not resolve from ambient PATH: {result:?}" + ); + } + #[tokio::test] async fn run_command_with_stdin_pipes_payload() { // Round-trip: feed stdin → expect it back on stdout. `cat` on diff --git a/crates/fbuild-daemon/src/handlers/emulator/select.rs b/crates/fbuild-daemon/src/handlers/emulator/select.rs index 4c9ddb612..ff05ee576 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/select.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/select.rs @@ -323,6 +323,7 @@ pub async fn test_emu( }, watch_set_cache: Some(std::sync::Arc::clone(&ctx.watch_set_cache) as std::sync::Arc<_>), bloat_analysis: false, + caller_path: None, }; let p = platform; diff --git a/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs b/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs index ad21d33b7..2364a49c4 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs @@ -149,6 +149,7 @@ async fn run_real_esp32s3_fixture_in_qemu() { ], watch_set_cache: None, bloat_analysis: false, + caller_path: None, }; let orchestrator = fbuild_build::esp32::orchestrator::Esp32Orchestrator; diff --git a/crates/fbuild-daemon/src/handlers/operations/build.rs b/crates/fbuild-daemon/src/handlers/operations/build.rs index 05a45f254..4d70fd8e6 100644 --- a/crates/fbuild-daemon/src/handlers/operations/build.rs +++ b/crates/fbuild-daemon/src/handlers/operations/build.rs @@ -237,6 +237,20 @@ pub async fn build( .as_deref() .map(|p| resolve_client_path(p, req.caller_cwd.as_deref(), &project_dir)); + // FastLED/fbuild#1219: surface PATH drift between the caller and the + // daemon — bare-name tool spawns resolve against the caller's PATH. + if let Some(caller_path) = req.caller_path.as_deref() { + let daemon_path = std::env::var("PATH").unwrap_or_default(); + if caller_path != daemon_path { + tracing::info!( + "caller PATH differs from daemon PATH (caller {} chars, daemon {} chars); \ + bare-name tools resolve against the caller's (see #1219)", + caller_path.len(), + daemon_path.len() + ); + } + } + if stream { // --- STREAMING PATH --- // Build runs in a background task; log lines stream to client as NDJSON. @@ -281,6 +295,7 @@ pub async fn build( extra_build_flags: Vec::new(), watch_set_cache: Some(Arc::clone(&ctx.watch_set_cache) as Arc<_>), bloat_analysis: req.bloat_analysis, + caller_path: req.caller_path.clone(), }; let project_dir_desc = req.project_dir.clone(); @@ -726,6 +741,7 @@ pub async fn build( extra_build_flags: Vec::new(), watch_set_cache: Some(Arc::clone(&ctx.watch_set_cache) as Arc<_>), bloat_analysis: req.bloat_analysis, + caller_path: req.caller_path, }; // fbuild#813 / #815: orchestrator.build is async, call directly. diff --git a/crates/fbuild-daemon/src/handlers/operations/deploy.rs b/crates/fbuild-daemon/src/handlers/operations/deploy.rs index f87f0ba48..d22a57a1c 100644 --- a/crates/fbuild-daemon/src/handlers/operations/deploy.rs +++ b/crates/fbuild-daemon/src/handlers/operations/deploy.rs @@ -252,6 +252,7 @@ pub async fn deploy( }, watch_set_cache: Some(Arc::clone(&ctx.watch_set_cache) as Arc<_>), bloat_analysis: false, + caller_path: req.caller_path.clone(), }; // fbuild#813: orchestrator.build is now async — call directly, diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index 206de45d8..8e2264bee 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -81,6 +81,10 @@ pub struct BuildRequest { /// See FastLED/fbuild#594. #[serde(default)] pub bloat_analysis: bool, + /// Snapshot of the caller's PATH so bare-name tool spawns resolve + /// against it instead of the daemon's stale PATH (FastLED/fbuild#1219). + #[serde(default)] + pub caller_path: Option, } /// POST /api/deploy @@ -149,6 +153,10 @@ pub struct DeployRequest { /// daemon remains unprivileged and merely carries this typed policy. #[serde(default)] pub usb_recovery_policy: fbuild_core::usb::UsbRecoveryPolicy, + /// Snapshot of the caller's PATH so bare-name tool spawns resolve + /// against it instead of the daemon's stale PATH (FastLED/fbuild#1219). + #[serde(default)] + pub caller_path: Option, } fn default_qemu_timeout() -> u32 { @@ -668,6 +676,39 @@ mod tests { assert_eq!(req.src_dir.unwrap(), "examples/AutoResearch"); } + // --- caller_path forwarding (FastLED/fbuild#1219) --- + + #[test] + fn build_request_caller_path_defaults_to_none() { + let json = r#"{"project_dir": "/tmp/p"}"#; + let req: BuildRequest = serde_json::from_str(json).unwrap(); + assert!(req.caller_path.is_none()); + } + + #[test] + fn build_request_caller_path_round_trips() { + let json = r#"{"project_dir": "/tmp/p", "caller_path": "C:\\venv\\Scripts;C:\\bin"}"#; + let req: BuildRequest = serde_json::from_str(json).unwrap(); + assert_eq!( + req.caller_path.as_deref(), + Some("C:\\venv\\Scripts;C:\\bin") + ); + } + + #[test] + fn deploy_request_caller_path_defaults_to_none() { + let json = r#"{"project_dir": "/tmp/p"}"#; + let req: DeployRequest = serde_json::from_str(json).unwrap(); + assert!(req.caller_path.is_none()); + } + + #[test] + fn deploy_request_caller_path_round_trips() { + let json = r#"{"project_dir": "/tmp/p", "caller_path": "/opt/venv/bin:/usr/bin"}"#; + let req: DeployRequest = serde_json::from_str(json).unwrap(); + assert_eq!(req.caller_path.as_deref(), Some("/opt/venv/bin:/usr/bin")); + } + #[test] fn build_request_with_request_id() { let json = r#"{"project_dir": "/tmp/p", "request_id": "abc-123"}"#;