chore(paths): route fbuild-deploy tool paths through fbuild-paths

crates/fbuild-deploy/src/lpc.rs

@@ -20,13 +20,6 @@ use crate::{DeployOutcome, Deployer, DeploymentResult};
 /// lpc21isp itself.
 pub const LPC21ISP_PATH_ENV_VAR: &str = "FBUILD_LPC21ISP_PATH";
 
-/// Resolve the user's home directory through the neutral host facade
-/// (`%USERPROFILE%` with a `%HOME%` fallback on Windows, `$HOME`
-/// elsewhere) so this module carries no per-OS env logic.
-fn home_dir() -> Option<PathBuf> {
-    fbuild_core::platform::host::home_dir().map(|home| home.into_path_buf())
-}
-
 /// The one canonical location fbuild manages lpc21isp at. FastLED/fbuild#921
 /// treats lpc21isp as a fbuild-owned dependency — auto-install will drop
 /// the binary here in a follow-up PR, and the deployer reads it back from
@@ -38,13 +31,7 @@ fn home_dir() -> Option<PathBuf> {
 /// isolation the rest of `fbuild-paths` applies.
 pub fn managed_lpc21isp_path() -> Option<PathBuf> {
     let exe = fbuild_core::platform::executable::native_name("lpc21isp");
-    let home = home_dir()?;
-    let mode = if std::env::var_os("FBUILD_DEV_MODE").is_some() {
-        "dev"
-    } else {
-        "prod"
-    };
-    Some(home.join(".fbuild").join(mode).join("tools").join(exe))
+    Some(fbuild_paths::try_get_tools_dir()?.join(exe))
 }
 
 /// Resolve where `lpc21isp` lives on this system.
@@ -89,7 +76,14 @@ pub fn find_lpc21isp() -> Option<PathBuf> {
 /// deploy path. Kept as a standalone function so the test module can
 /// assert the exact URLs / paths without shelling out.
 pub(crate) fn lpc21isp_install_hint() -> String {
-    let tools_dir = "~/.fbuild/prod/tools/";
+    // Name the directory fbuild will actually look in — under
+    // `FBUILD_DEV_MODE=1` that is the dev tree, and telling a dev-mode user
+    // to install into `prod` is how a "correctly installed" tool goes on
+    // being not-found (FastLED/fbuild#1349).
+    let tools_dir = fbuild_paths::try_get_tools_dir()
+        .map(|p| p.display().to_string())
+        .unwrap_or_else(fbuild_paths::tools_dir_label);
+    let tools_dir = format!("{tools_dir}{}", std::path::MAIN_SEPARATOR);
     let exe = fbuild_core::platform::executable::native_name("lpc21isp");
     format!(
         "lpc21isp not found on PATH or in any fbuild-managed tools dir.\n\
@@ -714,12 +708,39 @@ fn port_names_match(sys: &str, user: &str) -> bool {
 mod tests {
     use super::*;
 
-    // The resolver tests below temporarily mutate the same process-wide
-    // override. Cargo runs unit tests in parallel, so serialize that narrow
-    // shared state rather than letting one test restore the other test's
-    // value (which was visible as a Windows-only intermittent miss).
+    // The resolver tests below temporarily mutate process-wide env
+    // overrides (`FBUILD_LPC21ISP_PATH`, `FBUILD_DEV_MODE`). Cargo runs unit
+    // tests in parallel, so serialize that narrow shared state rather than
+    // letting one test restore the other test's value (which was visible as a
+    // Windows-only intermittent miss). Tests that only *read* a path derived
+    // from those vars take the lock too — otherwise they sample the value
+    // mid-flip.
     static LPC21ISP_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
 
+    /// Restores an env var when dropped, so a failing assertion cannot leave
+    /// the flipped value behind for whichever test runs next.
+    struct EnvVarGuard {
+        key: &'static str,
+        saved: Option<std::ffi::OsString>,
+    }
+
+    impl EnvVarGuard {
+        fn set(key: &'static str, value: &str) -> Self {
+            let saved = std::env::var_os(key);
+            std::env::set_var(key, value);
+            Self { key, saved }
+        }
+    }
+
+    impl Drop for EnvVarGuard {
+        fn drop(&mut self) {
+            match self.saved.take() {
+                Some(v) => std::env::set_var(self.key, v),
+                None => std::env::remove_var(self.key),
+            }
+        }
+    }
+
     #[test]
     fn test_lpc_deployer_creation() {
         let deployer = LpcDeployer::new("115200", 12_000, 60, None, false);
@@ -905,6 +926,9 @@ mod tests {
 
     #[test]
     fn install_hint_mentions_env_var_and_download_source() {
+        // Reads a `FBUILD_DEV_MODE`-derived path twice; without the lock it
+        // can sample prod on one read and dev on the other.
+        let _lock = LPC21ISP_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
         let hint = lpc21isp_install_hint();
         assert!(
             hint.contains(LPC21ISP_PATH_ENV_VAR),
@@ -914,14 +938,48 @@ mod tests {
             hint.contains("sourceforge.net"),
             "hint must point at a download"
         );
+        let tools_dir = fbuild_paths::try_get_tools_dir()
+            .map(|p| p.display().to_string())
+            .unwrap_or_else(fbuild_paths::tools_dir_label);
         assert!(
-            hint.contains("~/.fbuild/prod/tools/"),
+            hint.contains(&tools_dir),
             "hint must direct the user at the fbuild-managed tools dir \
-             (never an out-of-tree C:\\tools\\ path or PATH-walk)"
+             (never an out-of-tree C:\\tools\\ path or PATH-walk): {hint}"
         );
         assert!(hint.contains("#921"), "hint must cite the tracking issue");
     }
 
+    /// FastLED/fbuild#1349: the managed-tools path must be the one
+    /// `fbuild-paths` defines, not a private re-derivation of it.
+    ///
+    /// The re-derivations tested `FBUILD_DEV_MODE` with `var_os(..).is_some()`,
+    /// so `FBUILD_DEV_MODE=0` — an explicit *opt out* — selected the dev tree
+    /// here while `fbuild_paths::is_dev_mode()` (which requires the literal
+    /// `1`) kept every other path in prod. A deploy would then look for its
+    /// tools in a directory nothing installs into.
+    #[test]
+    fn managed_tools_path_agrees_with_fbuild_paths_on_dev_mode() {
+        let _lock = LPC21ISP_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
+
+        for (value, expected_dev) in [("0", false), ("1", true), ("true", false)] {
+            let _dev_mode = EnvVarGuard::set("FBUILD_DEV_MODE", value);
+            let Some(expected) = fbuild_paths::try_get_tools_dir() else {
+                continue; // no home dir on this host; nothing to compare against
+            };
+            let got = managed_lpc21isp_path().expect("home dir was resolvable just above");
+            assert_eq!(
+                got.parent(),
+                Some(expected.as_path()),
+                "FBUILD_DEV_MODE={value:?} must resolve the same tools dir as fbuild-paths"
+            );
+            assert_eq!(
+                fbuild_paths::is_dev_mode(),
+                expected_dev,
+                "only the literal `1` enables dev mode"
+            );
+        }
+    }
+
     // ---------- FastLED/fbuild#927 baud + hex-flag fixes ----------
 
     #[test]

crates/fbuild-deploy/src/lpc_debugger_reflash.rs

@@ -75,18 +75,7 @@ pub const LPC_LINK2_FIRMWARE_ENV_VAR: &str = "FBUILD_LPC_LINK2_FIRMWARE";
 /// tools under. Honors `FBUILD_DEV_MODE=1` for `~/.fbuild/dev/…`
 /// isolation, same as `find_lpc21isp` and the rest of `fbuild-paths`.
 pub fn managed_tools_dir() -> Option<NormalizedPath> {
-    let home = fbuild_core::platform::host::home_dir()?;
-    let mode = if std::env::var_os("FBUILD_DEV_MODE").is_some() {
-        "dev"
-    } else {
-        "prod"
-    };
-    Some(
-        home.join(".fbuild")
-            .join(mode)
-            .join("tools")
-            .join("lpc-link2-debugger"),
-    )
+    Some(NormalizedPath::from(fbuild_paths::try_get_tools_dir()?).join("lpc-link2-debugger"))
 }
 
 /// Absolute URL for one of the vendored assets under the framework
@@ -189,7 +178,9 @@ pub fn install_hint() -> String {
          Tracked under FastLED/fbuild#921.",
         tools = managed_tools_dir()
             .map(|p| p.display().to_string())
-            .unwrap_or_else(|| "~/.fbuild/prod/tools/lpc-link2-debugger/".to_string()),
+            .unwrap_or_else(|| {
+                format!("{}/lpc-link2-debugger", fbuild_paths::tools_dir_label())
+            }),
         v2_name = CMSIS_DAP_V2_HEX_NAME,
         dfu_env = DFU_UTIL_PATH_ENV_VAR,
         fw_env = LPC_LINK2_FIRMWARE_ENV_VAR,

crates/fbuild-deploy/src/probe_rs.rs

@@ -117,18 +117,7 @@ pub fn probe_rs_release_asset_for_host() -> Result<ProbeRsReleaseAsset> {
 /// Honors `FBUILD_DEV_MODE=1` → `~/.fbuild/dev/tools/probe-rs/` to
 /// match the isolation the rest of `fbuild-paths` applies.
 pub fn managed_probe_rs_dir() -> Option<NormalizedPath> {
-    let home = fbuild_core::platform::host::home_dir()?;
-    let mode = if std::env::var_os("FBUILD_DEV_MODE").is_some() {
-        "dev"
-    } else {
-        "prod"
-    };
-    Some(
-        home.join(".fbuild")
-            .join(mode)
-            .join("tools")
-            .join("probe-rs"),
-    )
+    Some(NormalizedPath::from(fbuild_paths::try_get_tools_dir()?).join("probe-rs"))
 }
 
 pub fn managed_probe_rs_path() -> Option<NormalizedPath> {

crates/fbuild-deploy/src/wchisp.rs

@@ -48,22 +48,10 @@ fn release_asset() -> Result<WchispAsset> {
 }
 
 fn managed_wchisp_path() -> Result<PathBuf> {
-    let home = std::env::var_os(if fbuild_core::platform::host::is_windows() {
-        "USERPROFILE"
-    } else {
-        "HOME"
-    })
-    .map(PathBuf::from)
-    .ok_or_else(|| FbuildError::PackageError("could not determine home directory".to_string()))?;
-    let mode = if std::env::var_os("FBUILD_DEV_MODE").is_some() {
-        "dev"
-    } else {
-        "prod"
-    };
-    Ok(home
-        .join(".fbuild")
-        .join(mode)
-        .join("tools")
+    let tools = fbuild_paths::try_get_tools_dir().ok_or_else(|| {
+        FbuildError::PackageError("could not determine home directory".to_string())
+    })?;
+    Ok(tools
         .join("wchisp")
         .join(fbuild_core::platform::executable::native_name("wchisp")))
 }

crates/fbuild-deploy/src/wlink.rs

@@ -45,22 +45,10 @@ fn release_asset() -> Result<WlinkAsset> {
 }
 
 fn managed_wlink_path() -> Result<PathBuf> {
-    let home = std::env::var_os(if fbuild_core::platform::host::is_windows() {
-        "USERPROFILE"
-    } else {
-        "HOME"
-    })
-    .map(PathBuf::from)
-    .ok_or_else(|| FbuildError::PackageError("could not determine home directory".to_string()))?;
-    let mode = if std::env::var_os("FBUILD_DEV_MODE").is_some() {
-        "dev"
-    } else {
-        "prod"
-    };
-    Ok(home
-        .join(".fbuild")
-        .join(mode)
-        .join("tools")
+    let tools = fbuild_paths::try_get_tools_dir().ok_or_else(|| {
+        FbuildError::PackageError("could not determine home directory".to_string())
+    })?;
+    Ok(tools
         .join("wlink")
         .join(fbuild_core::platform::executable::native_name("wlink")))
 }

crates/fbuild-paths/src/lib.rs

@@ -34,10 +34,53 @@ pub fn is_dev_mode() -> bool {
 }
 
 /// Root fbuild directory: `~/.fbuild/{dev|prod}`
+///
+/// Panics when the home directory cannot be determined. Callers that must
+/// degrade instead of panicking want [`try_get_fbuild_root`].
 pub fn get_fbuild_root() -> PathBuf {
-    let home = dirs_next().expect("could not determine home directory");
+    try_get_fbuild_root().expect("could not determine home directory")
+}
+
+/// [`get_fbuild_root`] for callers that report a missing home directory
+/// rather than panicking on it.
+///
+/// Exists because several tool resolvers return `Option`/`Result` precisely
+/// so a home-less environment is a diagnosable failure and not a crash; they
+/// had each hand-rolled this path to keep that property (FastLED/fbuild#1349).
+pub fn try_get_fbuild_root() -> Option<PathBuf> {
     let mode = if is_dev_mode() { "dev" } else { "prod" };
-    home.join(FBUILD_DIR_NAME).join(mode)
+    Some(dirs_next()?.join(FBUILD_DIR_NAME).join(mode))
+}
+
+/// The segment holding fbuild-managed external tools, under
+/// [`get_fbuild_root`]: `tools`.
+pub const TOOLS_DIR_NAME: &str = "tools";
+
+/// Where fbuild installs and looks for managed external tools:
+/// `~/.fbuild/{dev|prod}/tools`.
+///
+/// Panics when the home directory cannot be determined; see
+/// [`try_get_tools_dir`].
+pub fn get_tools_dir() -> PathBuf {
+    get_fbuild_root().join(TOOLS_DIR_NAME)
+}
+
+/// [`get_tools_dir`] for callers that report a missing home directory rather
+/// than panicking on it.
+pub fn try_get_tools_dir() -> Option<PathBuf> {
+    Some(try_get_fbuild_root()?.join(TOOLS_DIR_NAME))
+}
+
+/// Human-facing label for [`get_tools_dir`] when the real path cannot be
+/// resolved — `~/.fbuild/{dev|prod}/tools`.
+///
+/// Diagnostics that tell a user where to install a managed tool need
+/// *something* to print even on a host with no discoverable home directory.
+/// Producing it here keeps those messages honest about the current mode, and
+/// keeps the `.fbuild` spelling from being re-typed at each call site.
+pub fn tools_dir_label() -> String {
+    let mode = if is_dev_mode() { "dev" } else { "prod" };
+    format!("~/{FBUILD_DIR_NAME}/{mode}/{TOOLS_DIR_NAME}")
 }
 
 /// Root fbuild directory for the OTHER mode (cross-mode fallback).

dylints/ban_raw_fbuild_path/Cargo.toml

@@ -3,7 +3,7 @@ name = "ban_raw_fbuild_path"
 # Bump the version to bust the dylint .so cache when allowlist.txt
 # changes (setup-soldr's dylint-cache key hashes the manifest but not
 # src/allowlist.txt). Same convention ban_manual_slash_normalize follows.
-version = "0.1.0"
+version = "0.1.1"
 description = "Ban raw '.fbuild' path literals outside fbuild-paths"
 edition = "2021"
 publish = false

dylints/ban_raw_fbuild_path/src/allowlist.txt

@@ -62,10 +62,5 @@ crates/fbuild-daemon/src/handlers/operations/build.rs
 crates/fbuild-daemon/src/main.rs
 crates/fbuild-daemon/src/models.rs
 crates/fbuild-daemon/tests/legacy_daemon_transition.rs
-crates/fbuild-deploy/src/lpc.rs
-crates/fbuild-deploy/src/lpc_debugger_reflash.rs
-crates/fbuild-deploy/src/probe_rs.rs
-crates/fbuild-deploy/src/wchisp.rs
-crates/fbuild-deploy/src/wlink.rs
 crates/fbuild-library-select/tests/teensy41_ldf_diag.rs
 crates/fbuild-library/src/library/library_compiler.rs