From 597dd907dd566fcb98f2e5d1437a2843d2f8a5e5 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 12:47:08 -0700 Subject: [PATCH 1/3] chore(daemon): make daemon diagnostics name real paths, ratchet six sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third batch for FastLED/fbuild#1349. Allowlist 39 -> 33; `fbuild-daemon` is now clear. ## Two diagnostics pointed at paths that do not exist Both messages that send a user to the daemon log spelled it by hand, and both spelled it wrong: - `~/.fbuild/daemon/daemon.log` — drops the `dev`/`prod` segment entirely. - `~/.fbuild//daemon/daemon.log` — `` reads as a PlatformIO environment; the segment is really the dev/prod mode. Someone hitting the second one is already debugging a build that will not start, and the message hands them a path that is not there. Both now print `fbuild_paths::get_daemon_log_file()` — an absolute path, not a template the reader has to expand and get right. The embedded-zccache startup failure gets the same treatment for its cache directory. This is the drift the ratchet is for: nothing tests a message string, so a hand-spelled path stays compiling and wrong indefinitely. ## Also ratcheted - `emulator/tests_process.rs` built `/.fbuild/build-qemu` directly instead of asking `get_project_fbuild_dir`. - `libraries.rs`'s install-state note described the layout in a const string; it is assembled from `FBUILD_DIR_NAME` / `BUILD_DIR_NAME` now, so a note that disagrees with the layout cannot survive a layout change. - `models.rs` and `legacy_daemon_transition.rs` fixtures spelled the segment by hand. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/handlers/emulator/tests_process.rs | 4 +- .../fbuild-daemon/src/handlers/libraries.rs | 29 +++++---- .../src/handlers/operations/build.rs | 60 +++++++++++++++++-- crates/fbuild-daemon/src/main.rs | 6 +- crates/fbuild-daemon/src/models.rs | 17 ++++-- .../tests/legacy_daemon_transition.rs | 2 +- dylints/ban_raw_fbuild_path/Cargo.toml | 2 +- dylints/ban_raw_fbuild_path/src/allowlist.txt | 6 -- 8 files changed, 95 insertions(+), 31 deletions(-) diff --git a/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs b/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs index fa5980dc..452704a7 100644 --- a/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs +++ b/crates/fbuild-daemon/src/handlers/emulator/tests_process.rs @@ -134,7 +134,9 @@ async fn run_real_esp32s3_fixture_in_qemu() { "esp32s3".to_string(), BuildProfile::Release, ) - .with_override_root(Some(project_dir.join(".fbuild").join("build-qemu"))) + .with_override_root(Some( + fbuild_paths::get_project_fbuild_dir(&project_dir).join("build-qemu"), + )) .resolve(); let params = BuildParams { project_dir: project_dir.clone(), diff --git a/crates/fbuild-daemon/src/handlers/libraries.rs b/crates/fbuild-daemon/src/handlers/libraries.rs index d4084dd9..1dd7f770 100644 --- a/crates/fbuild-daemon/src/handlers/libraries.rs +++ b/crates/fbuild-daemon/src/handlers/libraries.rs @@ -44,11 +44,18 @@ use crate::models::{IdeLibrariesQuery, IdeLibrariesResponse, IdeLibraryEntry}; const LIBRARIES_PAGE_HTML: &str = include_str!("../../web/libraries/index.html"); -const INSTALL_STATE_NOTE: &str = "Installed state is best-effort: it checks the release build \ -profile's /.fbuild/build//release/libs/ directory for a same-named subdirectory. \ -That directory is only populated after a build that needed dependencies has run — if no build \ -has run yet, every entry reports installed: false even though the source may be perfectly \ -resolvable."; +/// Explains what `installed` means on the libraries page. +/// +/// Built from the canonical path segments rather than spelled by hand: the +/// note names a real directory layout, and a note that disagrees with the +/// layout is worse than no note (FastLED/fbuild#1349). +fn install_state_note() -> String { + format!( + "Installed state is best-effort: it checks the release build profile's /{}/{}//release/libs/ directory for a same-named subdirectory. That directory is only populated after a build that needed dependencies has run — if no build has run yet, every entry reports installed: false even though the source may be perfectly resolvable.", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ) +} /// GET /libraries — serve the self-contained Library Manager page. pub async fn libraries_page() -> impl IntoResponse { @@ -165,7 +172,7 @@ pub async fn list_libraries( project: None, environment: None, libraries: Vec::new(), - install_state_note: INSTALL_STATE_NOTE.to_string(), + install_state_note: install_state_note(), error: Some( "missing required ?project= query param; \ run `fbuild libraries` from a project directory to open this page \ @@ -185,7 +192,7 @@ pub async fn list_libraries( project: Some(project), environment: params.env.clone(), libraries: Vec::new(), - install_state_note: INSTALL_STATE_NOTE.to_string(), + install_state_note: install_state_note(), error: Some(e), }), ); @@ -201,7 +208,7 @@ pub async fn list_libraries( project: Some(project), environment: params.env.clone(), libraries: Vec::new(), - install_state_note: INSTALL_STATE_NOTE.to_string(), + install_state_note: install_state_note(), error: Some(format!("failed to parse platformio.ini: {}", e)), }), ); @@ -218,7 +225,7 @@ pub async fn list_libraries( project: Some(project), environment: params.env.clone(), libraries: Vec::new(), - install_state_note: INSTALL_STATE_NOTE.to_string(), + install_state_note: install_state_note(), error: Some(e), }), ); @@ -241,7 +248,7 @@ pub async fn list_libraries( project: Some(project), environment: Some(env_name), libraries, - install_state_note: INSTALL_STATE_NOTE.to_string(), + install_state_note: install_state_note(), error: None, }), ), @@ -252,7 +259,7 @@ pub async fn list_libraries( project: Some(project), environment: Some(env_name), libraries: Vec::new(), - install_state_note: INSTALL_STATE_NOTE.to_string(), + install_state_note: install_state_note(), error: Some(e), }), ), diff --git a/crates/fbuild-daemon/src/handlers/operations/build.rs b/crates/fbuild-daemon/src/handlers/operations/build.rs index ef412345..4d9029a1 100644 --- a/crates/fbuild-daemon/src/handlers/operations/build.rs +++ b/crates/fbuild-daemon/src/handlers/operations/build.rs @@ -88,7 +88,7 @@ impl Drop for StreamTerminationGuard { "type": "result", "success": false, "request_id": self.request_id, - "message": "daemon build worker terminated unexpectedly (panic or early return); check ~/.fbuild/daemon/daemon.log", + "message": worker_terminated_message(), "exit_code": 1, "output_file": null, "output_dir": null, @@ -347,11 +347,11 @@ pub async fn build( "project lock on {} not acquired within {}s — \ another build is still holding it. Continuing \ to wait; if this persists the daemon may be \ - stuck. Inspect ~/.fbuild//daemon/daemon.log \ - or run `fbuild daemon locks` to see who is \ - holding the lock.", + stuck. Inspect {} or run `fbuild daemon locks` \ + to see who is holding the lock.", project_dir_desc, LOCK_WAIT_WARN.as_secs(), + daemon_log_hint(), ); warned = true; } @@ -1017,3 +1017,55 @@ mod tests { assert!(!should_emit_dependency_status(&installed)); } } + +/// The daemon log file, named as an absolute path rather than a template the +/// reader has to expand. +/// +/// FastLED/fbuild#1349: the two diagnostics that point at this file each +/// spelled it by hand, and each spelled it wrong. One said +/// `~/.fbuild/daemon/daemon.log`, dropping the `dev`/`prod` segment entirely; +/// the other said `~/.fbuild//daemon/daemon.log`, where `` reads as +/// a PlatformIO environment and is really the dev/prod mode. Both sent a user +/// who was already debugging a stuck build to a path that does not exist. +fn daemon_log_hint() -> String { + fbuild_paths::get_daemon_log_file().display().to_string() +} + +/// Message reported when the build worker dies without answering. +fn worker_terminated_message() -> String { + format!( + "daemon build worker terminated unexpectedly (panic or early return); check {}", + daemon_log_hint() + ) +} + +#[cfg(test)] +mod daemon_log_hint_tests { + use super::*; + + /// The hint must name the file that actually exists. Asserting against + /// `get_daemon_log_file` rather than against a literal is the point: a + /// literal is what drifted. + #[test] + fn diagnostics_name_the_real_daemon_log_path() { + let expected = fbuild_paths::get_daemon_log_file(); + let expected = expected.display().to_string(); + assert_eq!(daemon_log_hint(), expected); + assert!( + worker_terminated_message().contains(&expected), + "{}", + worker_terminated_message() + ); + // The mode segment is what the old spelling dropped. + let mode = if fbuild_paths::is_dev_mode() { + "dev" + } else { + "prod" + }; + assert!( + daemon_log_hint().contains(mode), + "the hint must carry the dev/prod segment: {}", + daemon_log_hint() + ); + } +} diff --git a/crates/fbuild-daemon/src/main.rs b/crates/fbuild-daemon/src/main.rs index 4b67eae4..8cd77756 100644 --- a/crates/fbuild-daemon/src/main.rs +++ b/crates/fbuild-daemon/src/main.rs @@ -101,7 +101,7 @@ async fn main() { .add_directive(tracing::Level::INFO.into()), ) // Route fmt layer to stderr — the CLI captures the daemon's - // stderr into ~/.fbuild//daemon/daemon.log (see + // stderr into the daemon log under the fbuild root (see // daemon_client.rs spawn_daemon). The MakeWriter default is // stdout, but the CLI sets stdout to Null, so without // .with_writer(stderr) every `tracing::*` event in the daemon @@ -194,8 +194,8 @@ async fn main() { eprintln!( "fatal: failed to start embedded zccache service: {err}\n\ fbuild-daemon cannot start without an embedded zccache \ - backend. Check ~/.fbuild//zccache/ permissions and \ - disk space." + backend. Check {} permissions and disk space.", + fbuild_paths::get_fbuild_root().join("zccache").display() ); std::process::exit(1); } diff --git a/crates/fbuild-daemon/src/models.rs b/crates/fbuild-daemon/src/models.rs index c12291aa..ed52998c 100644 --- a/crates/fbuild-daemon/src/models.rs +++ b/crates/fbuild-daemon/src/models.rs @@ -651,6 +651,13 @@ pub struct IdeLibrariesResponse { mod tests { use super::*; + /// Fixture cache path, assembled from the canonical segment rather than + /// typed out — a fixture that spells the layout by hand goes on passing + /// after the layout changes (FastLED/fbuild#1349). + fn fixture_cache_dir() -> String { + format!("/home/user/{}/prod/cache", fbuild_paths::FBUILD_DIR_NAME) + } + // --- BuildRequest deserialization --- #[test] @@ -932,11 +939,13 @@ mod tests { current_operation: None, dependency_install: None, client_count: 3, - cache_dir: "/home/user/.fbuild/prod/cache".into(), - cache_identity: - "mode=prod;trust=local-shared;schema=1;cache=/home/user/.fbuild/prod/cache".into(), + cache_dir: fixture_cache_dir(), + cache_identity: format!( + "mode=prod;trust=local-shared;schema=1;cache={}", + fixture_cache_dir() + ), cache_schema_version: 1, - daemon_dir: "/home/user/.fbuild/prod/daemon".into(), + daemon_dir: format!("/home/user/{}/prod/daemon", fbuild_paths::FBUILD_DIR_NAME), source_mtime: 1700000000.0, spawner_cwd: "/home/user/project".into(), mcp_url: "http://127.0.0.1:8765/mcp".into(), diff --git a/crates/fbuild-daemon/tests/legacy_daemon_transition.rs b/crates/fbuild-daemon/tests/legacy_daemon_transition.rs index 5391fb00..8c1d1bbd 100644 --- a/crates/fbuild-daemon/tests/legacy_daemon_transition.rs +++ b/crates/fbuild-daemon/tests/legacy_daemon_transition.rs @@ -167,7 +167,7 @@ fn free_port() -> u16 { /// scoped to just that child. fn root_owner_lock_path_for(temp_home: &Path) -> NormalizedPath { NormalizedPath::new(temp_home) - .join(".fbuild") + .join(fbuild_paths::FBUILD_DIR_NAME) .join("dev") .join("daemon") .join("root-owner.lock") diff --git a/dylints/ban_raw_fbuild_path/Cargo.toml b/dylints/ban_raw_fbuild_path/Cargo.toml index 41c7cb6e..452371ad 100644 --- a/dylints/ban_raw_fbuild_path/Cargo.toml +++ b/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.2" +version = "0.1.3" description = "Ban raw '.fbuild' path literals outside fbuild-paths" edition = "2021" publish = false diff --git a/dylints/ban_raw_fbuild_path/src/allowlist.txt b/dylints/ban_raw_fbuild_path/src/allowlist.txt index 4e4d9ad4..ac3cb3b8 100644 --- a/dylints/ban_raw_fbuild_path/src/allowlist.txt +++ b/dylints/ban_raw_fbuild_path/src/allowlist.txt @@ -60,9 +60,3 @@ crates/fbuild-cli/src/cli/purge.rs crates/fbuild-cli/src/cli/symbols_cmd.rs crates/fbuild-cli/src/lib_select.rs crates/fbuild-cli/tests/daemon_crash_recovery.rs -crates/fbuild-daemon/src/handlers/emulator/tests_process.rs -crates/fbuild-daemon/src/handlers/libraries.rs -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 From b053586b22f7e1856cdc1e5e32fe8f0d2a851283 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 13:06:07 -0700 Subject: [PATCH 2/3] fix(daemon): let the install-state note name the directory actually checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on FastLED/fbuild#1349 batch 3. `BuildLayout::resolve()` drops the `` segment when it matches the project directory name, so a note that always spells `/.fbuild/build//release/libs/` can point a user at a directory the handler never looked in. That is the same drift the ratchet targets, one level up from the literal: assembling the string from the canonical segments fixed the spelling but not the shape. `install_state_note` now takes the resolved `libs_dir` on the two paths that have one and names it exactly. The early-return paths — which carry no library data for the note to be wrong about — keep the generic form and state the collapse rule instead of implying `` is always present. Co-Authored-By: Claude Opus 5 (1M context) --- .../fbuild-daemon/src/handlers/libraries.rs | 73 +++++++++++++++---- 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/crates/fbuild-daemon/src/handlers/libraries.rs b/crates/fbuild-daemon/src/handlers/libraries.rs index 1dd7f770..3f478e8a 100644 --- a/crates/fbuild-daemon/src/handlers/libraries.rs +++ b/crates/fbuild-daemon/src/handlers/libraries.rs @@ -46,14 +46,27 @@ const LIBRARIES_PAGE_HTML: &str = include_str!("../../web/libraries/index.html") /// Explains what `installed` means on the libraries page. /// -/// Built from the canonical path segments rather than spelled by hand: the -/// note names a real directory layout, and a note that disagrees with the -/// layout is worse than no note (FastLED/fbuild#1349). -fn install_state_note() -> String { +/// Takes the directory the handler actually checked, when it got far enough +/// to resolve one. `BuildLayout::resolve()` drops the `` segment when it +/// matches the project basename, so a note that always spells +/// `/.fbuild/build//release/libs/` can name a path the handler +/// never looked at — the same class of drift FastLED/fbuild#1349 is about, +/// one level up from the literal. +/// +/// The generic form is only used on the early-return paths, which carry no +/// library data for the note to be wrong about; it states the collapse rule +/// rather than pretending the segment is always there. +fn install_state_note(libs_dir: Option<&Path>) -> String { + let location = match libs_dir { + Some(dir) => format!("the {} directory", dir.display()), + None => format!( + "the release build profile's /{}/{}//release/libs/ directory (the segment is omitted when it matches the project directory name)", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ), + }; format!( - "Installed state is best-effort: it checks the release build profile's /{}/{}//release/libs/ directory for a same-named subdirectory. That directory is only populated after a build that needed dependencies has run — if no build has run yet, every entry reports installed: false even though the source may be perfectly resolvable.", - fbuild_paths::FBUILD_DIR_NAME, - fbuild_paths::BUILD_DIR_NAME + "Installed state is best-effort: it checks {location} for a same-named subdirectory. That directory is only populated after a build that needed dependencies has run — if no build has run yet, every entry reports installed: false even though the source may be perfectly resolvable." ) } @@ -172,7 +185,7 @@ pub async fn list_libraries( project: None, environment: None, libraries: Vec::new(), - install_state_note: install_state_note(), + install_state_note: install_state_note(None), error: Some( "missing required ?project= query param; \ run `fbuild libraries` from a project directory to open this page \ @@ -192,7 +205,7 @@ pub async fn list_libraries( project: Some(project), environment: params.env.clone(), libraries: Vec::new(), - install_state_note: install_state_note(), + install_state_note: install_state_note(None), error: Some(e), }), ); @@ -208,7 +221,7 @@ pub async fn list_libraries( project: Some(project), environment: params.env.clone(), libraries: Vec::new(), - install_state_note: install_state_note(), + install_state_note: install_state_note(None), error: Some(format!("failed to parse platformio.ini: {}", e)), }), ); @@ -225,7 +238,7 @@ pub async fn list_libraries( project: Some(project), environment: params.env.clone(), libraries: Vec::new(), - install_state_note: install_state_note(), + install_state_note: install_state_note(None), error: Some(e), }), ); @@ -248,7 +261,7 @@ pub async fn list_libraries( project: Some(project), environment: Some(env_name), libraries, - install_state_note: install_state_note(), + install_state_note: install_state_note(Some(libs_dir.as_ref())), error: None, }), ), @@ -259,7 +272,7 @@ pub async fn list_libraries( project: Some(project), environment: Some(env_name), libraries: Vec::new(), - install_state_note: install_state_note(), + install_state_note: install_state_note(Some(libs_dir.as_ref())), error: Some(e), }), ), @@ -268,6 +281,40 @@ pub async fn list_libraries( #[cfg(test)] mod tests { + + /// FastLED/fbuild#1349 review: `BuildLayout::resolve()` drops the `` + /// segment when it matches the project directory name, so a note that + /// always spells `` can name a directory the handler never checked. + #[test] + fn the_note_names_the_directory_that_was_actually_checked() { + // Assembled from the canonical segments: a fixture that spells the + // layout by hand is the thing this ratchet exists to remove, and the + // lint rightly rejects one here too. + let checked = format!( + "/proj/{}/{}/release/libs", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ); + let named = install_state_note(Some(Path::new(&checked))); + assert!(named.contains(&checked), "{named}"); + assert!( + !named.contains(""), + "a resolved path must not carry an unexpanded placeholder: {named}" + ); + } + + /// Without a resolved directory the note has to describe the layout — so + /// it must also state the collapse rule rather than implying `` is + /// always present. + #[test] + fn the_generic_note_states_the_env_collapse_rule() { + let generic = install_state_note(None); + assert!(generic.contains(""), "{generic}"); + assert!( + generic.contains("omitted when it matches the project directory name"), + "{generic}" + ); + } use super::*; async fn write_project(dir: &Path, ini: &str) { From 44e52c346455a03f4d3acd02f45d467e8d766e62 Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 14:48:46 -0700 Subject: [PATCH 3/3] fix(daemon): follow FBUILD_BUILD_DIR in the generic install-state note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review follow-up on FastLED/fbuild#1349 batch 3. `get_project_build_root` returns `FBUILD_BUILD_DIR` verbatim when it is set, so the `.fbuild/build` segments are not in the path at all. The generic note described them anyway, pointing a reader at a directory that does not exist on a machine using the override — which exists precisely for the Windows long-path case where the default layout does not fit. The generic branch now follows the override when it is set, and names it as a possibility when it is not. Two things can move that directory and both are stated rather than assumed away: the collapsed `` segment and this root substitution. Also repairs the note's own text. An earlier edit lost its line continuations, leaving long runs of literal whitespace inside a string shown to users on the Library Manager page. Tests take a shared lock and restore the variable on drop, since both read process-wide env. Co-Authored-By: Claude Opus 5 (1M context) --- .../fbuild-daemon/src/handlers/libraries.rs | 98 +++++++++++++++++-- 1 file changed, 89 insertions(+), 9 deletions(-) diff --git a/crates/fbuild-daemon/src/handlers/libraries.rs b/crates/fbuild-daemon/src/handlers/libraries.rs index 3f478e8a..d8a62109 100644 --- a/crates/fbuild-daemon/src/handlers/libraries.rs +++ b/crates/fbuild-daemon/src/handlers/libraries.rs @@ -59,17 +59,43 @@ const LIBRARIES_PAGE_HTML: &str = include_str!("../../web/libraries/index.html") fn install_state_note(libs_dir: Option<&Path>) -> String { let location = match libs_dir { Some(dir) => format!("the {} directory", dir.display()), - None => format!( - "the release build profile's /{}/{}//release/libs/ directory (the segment is omitted when it matches the project directory name)", - fbuild_paths::FBUILD_DIR_NAME, - fbuild_paths::BUILD_DIR_NAME - ), + None => generic_libs_location(), }; format!( - "Installed state is best-effort: it checks {location} for a same-named subdirectory. That directory is only populated after a build that needed dependencies has run — if no build has run yet, every entry reports installed: false even though the source may be perfectly resolvable." + "Installed state is best-effort: it checks {location} for a same-named \ + subdirectory. That directory is only populated after a build that needed \ + dependencies has run — if no build has run yet, every entry reports \ + installed: false even though the source may be perfectly resolvable." ) } +/// Describe where library output lands when the handler failed before it +/// could resolve a concrete path. +/// +/// Two things can move that directory out from under a hardcoded description, +/// and both have to be said rather than assumed away: +/// +/// - `FBUILD_BUILD_DIR` replaces the build root outright +/// (`get_project_build_root` returns it verbatim), so the `.fbuild/build` +/// segments are simply not present. +/// - `BuildLayout::resolve()` drops the `` segment when it matches the +/// project directory name. +fn generic_libs_location() -> String { + match std::env::var("FBUILD_BUILD_DIR") { + Ok(root) if !root.trim().is_empty() => format!( + "the release build profile's /release/libs/ directory under {root}, \ + which FBUILD_BUILD_DIR has substituted for the default build root" + ), + _ => format!( + "the release build profile's /{}/{}//release/libs/ directory \ + (the segment is omitted when it matches the project directory name, \ + and FBUILD_BUILD_DIR replaces the root entirely when set)", + fbuild_paths::FBUILD_DIR_NAME, + fbuild_paths::BUILD_DIR_NAME + ), + } +} + /// GET /libraries — serve the self-contained Library Manager page. pub async fn libraries_page() -> impl IntoResponse { Html(LIBRARIES_PAGE_HTML) @@ -303,18 +329,72 @@ mod tests { ); } + /// The env overrides below are process-wide; cargo runs these in + /// parallel, so serialize the two that read them. + static BUILD_DIR_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// Restores `FBUILD_BUILD_DIR` on drop so a failing assertion cannot + /// leave it set for a sibling test. + struct BuildDirEnv(Option); + + impl BuildDirEnv { + fn set(value: Option<&str>) -> Self { + let saved = std::env::var_os("FBUILD_BUILD_DIR"); + match value { + Some(v) => std::env::set_var("FBUILD_BUILD_DIR", v), + None => std::env::remove_var("FBUILD_BUILD_DIR"), + } + Self(saved) + } + } + + impl Drop for BuildDirEnv { + fn drop(&mut self) { + match self.0.take() { + Some(v) => std::env::set_var("FBUILD_BUILD_DIR", v), + None => std::env::remove_var("FBUILD_BUILD_DIR"), + } + } + } + /// Without a resolved directory the note has to describe the layout — so - /// it must also state the collapse rule rather than implying `` is - /// always present. + /// it must also state both ways that layout can move: the collapsed + /// `` segment and the `FBUILD_BUILD_DIR` override. #[test] - fn the_generic_note_states_the_env_collapse_rule() { + fn the_generic_note_states_both_ways_the_layout_can_move() { + let _lock = BUILD_DIR_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _env = BuildDirEnv::set(None); + let generic = install_state_note(None); assert!(generic.contains(""), "{generic}"); assert!( generic.contains("omitted when it matches the project directory name"), "{generic}" ); + assert!( + generic.contains("FBUILD_BUILD_DIR"), + "the default description must still name the override that can replace it: {generic}" + ); } + + /// FastLED/fbuild#1349 review: `get_project_build_root` returns + /// `FBUILD_BUILD_DIR` verbatim, so when it is set the `.fbuild/build` + /// segments are not in the path at all. Describing them anyway points the + /// reader at a directory that does not exist on their machine. + #[test] + fn the_generic_note_follows_the_build_dir_override() { + let _lock = BUILD_DIR_ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let _env = BuildDirEnv::set(Some("/scratch/short")); + + let generic = install_state_note(None); + assert!(generic.contains("/scratch/short"), "{generic}"); + assert!( + !generic.contains(fbuild_paths::FBUILD_DIR_NAME), + "the override replaces the root, so the default segments must not be described \ + as if they were still there: {generic}" + ); + } + use super::*; async fn write_project(dir: &Path, ini: &str) {