From df7f51281aefa051041de85cd5ac2affc6ee87ff Mon Sep 17 00:00:00 2001 From: Breadway Date: Fri, 17 Jul 2026 03:28:34 +0800 Subject: [PATCH 1/3] Make config writes atomic with backup-before-overwrite Same discipline as the matching bos-settings fix: state.toml writes went straight to the target path with no backup and no atomicity, so a crash, power loss, or disk-full error mid-write could leave the file truncated or corrupted with no way back. config::atomic_write now writes to a temp file in the same directory, renames it over the target (atomic within one filesystem), and backs up whatever was there before to .bak first. save_doc() goes through it. Added tests covering the backup/no-leftover-tmp-file behavior and a save_doc -> load_doc round trip. --- src/config.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 7 deletions(-) diff --git a/src/config.rs b/src/config.rs index acdca4a..d0f6748 100644 --- a/src/config.rs +++ b/src/config.rs @@ -41,17 +41,36 @@ fn load_doc(path: &Path) -> DocumentMut { } fn save_doc(path: &Path, doc: &DocumentMut) { - if let Some(parent) = path.parent() { - if let Err(e) = std::fs::create_dir_all(parent) { - eprintln!("breadhelp: couldn't create {}: {e}", parent.display()); - return; - } - } - if let Err(e) = std::fs::write(path, doc.to_string()) { + if let Err(e) = atomic_write(path, &doc.to_string()) { eprintln!("breadhelp: couldn't write {}: {e}", path.display()); } } +/// Write `contents` to `path` atomically, backing up whatever was there +/// before overwriting it — same discipline as `bos-settings/src/config/ +/// mod.rs::atomic_write`. Writing straight to the target path means a +/// crash, power loss, or disk-full error mid-write can leave `state.toml` +/// truncated or corrupted with no way back. Writing to a temp file in the +/// same directory first, then `rename`-ing it over the target, avoids +/// that — a rename within one filesystem is atomic. Backing up the +/// previous file first (best-effort) means even a successful-but-wrong +/// write is always recoverable from `.bak`. +fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + if path.exists() { + let backup = PathBuf::from(format!("{}.bak", path.display())); + let _ = std::fs::copy(path, &backup); + } + let dir = path.parent().map(Path::to_path_buf).unwrap_or_else(|| PathBuf::from(".")); + let file_name = path.file_name().and_then(|n| n.to_str()).unwrap_or("config"); + let tmp_path = dir.join(format!(".{file_name}.tmp.{}", std::process::id())); + std::fs::write(&tmp_path, contents)?; + std::fs::rename(&tmp_path, path)?; + Ok(()) +} + #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Mode { Normal, @@ -159,3 +178,52 @@ impl State { save_doc(&self.path, &self.doc); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn atomic_write_backs_up_previous_contents_and_no_tmp_file_left_behind() { + let dir = std::env::temp_dir().join(format!("breadhelp-atomic-write-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("state.toml"); + let backup = dir.join("state.toml.bak"); + + atomic_write(&path, "first").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "first"); + assert!(!backup.exists(), "no backup should be made when there's nothing to back up yet"); + + atomic_write(&path, "second").unwrap(); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "second"); + assert_eq!(std::fs::read_to_string(&backup).unwrap(), "first"); + + let leftover_tmp: Vec<_> = std::fs::read_dir(&dir) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n.contains(".tmp.")) + .collect(); + assert!(leftover_tmp.is_empty(), "temp file should be renamed away, not left behind: {leftover_tmp:?}"); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn save_doc_then_load_doc_round_trips_state() { + let dir = std::env::temp_dir().join(format!("breadhelp-state-roundtrip-test-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("state.toml"); + + let mut doc = DocumentMut::default(); + doc["onboarding"]["completed"] = value(true); + doc["general"]["mode"] = value("dad"); + save_doc(&path, &doc); + + let loaded = load_doc(&path); + assert_eq!(loaded.get("onboarding").and_then(|t| t.get("completed")).and_then(|v| v.as_bool()), Some(true)); + assert_eq!(loaded.get("general").and_then(|t| t.get("mode")).and_then(|v| v.as_str()), Some("dad")); + + let _ = std::fs::remove_dir_all(&dir); + } +} From 7b7fe680fe7af7b9a7e29ad78eb3f1630ff9aa73 Mon Sep 17 00:00:00 2001 From: Breadway Date: Fri, 17 Jul 2026 09:32:55 +0800 Subject: [PATCH 2/3] Migrate config load/save to bread_utils::tomlcfg; timeout-guard hyprctl calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit config.rs's config_dir/load_doc/save_doc/atomic_write were a byte-for-byte copy of bos-settings/src/config/mod.rs's versions (own doc comment said as much) introduced in the same fix pass that added them there — now both delegate to bread_utils::tomlcfg/bread_utils::atomic instead of each carrying their own copy (path dependency for now, see the TODO in Cargo.toml). Also: services/hyprland.rs::query_json used a bare Command::output() with no timeout, so an unresponsive hyprctl (Hyprland wedged/reloading) could block clients()/monitors()/layers() indefinitely — switched to bread_utils::proc::run_json with a 3s cap. Builds clean; all 8 existing tests pass. --- Cargo.lock | 11 ++++++ Cargo.toml | 2 ++ src/config.rs | 74 +++++++++------------------------------- src/services/hyprland.rs | 5 +-- 4 files changed, 33 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e0c8f22..d6b8b0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,12 +37,23 @@ dependencies = [ "serde_json", ] +[[package]] +name = "bread-utils" +version = "0.2.3" +dependencies = [ + "dirs", + "serde", + "serde_json", + "toml_edit 0.22.27", +] + [[package]] name = "breadhelp" version = "0.2.1" dependencies = [ "async-channel", "bread-theme", + "bread-utils", "gdk4", "glib", "gtk4", diff --git a/Cargo.toml b/Cargo.toml index 2ad994b..b414e7d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,3 +17,5 @@ toml = "0.8" # Non-destructive state editing (mirrors bos-settings/src/config/mod.rs). toml_edit = "0.22" async-channel = "2" +# TODO(owner): switch to tag-pinned git dependency once bread-utils is merged and tagged, matching the bread-theme pattern +bread-utils = { path = "../bread-ecosystem-fix-worktree/bread-utils", features = ["toml"] } diff --git a/src/config.rs b/src/config.rs index d0f6748..e2f3e02 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,20 +1,16 @@ -//! `~/.config/breadhelp/state.toml` — non-destructive TOML editing, same -//! `load_doc`/`save_doc` discipline as `bos-settings/src/config/mod.rs`: a -//! missing file yields defaults, a file that exists but fails to parse is -//! backed up once before falling back, so a bad edit is always recoverable. +//! `~/.config/breadhelp/state.toml` — non-destructive TOML editing via +//! `bread_utils::tomlcfg`, the same `load_doc`/`save_doc` discipline +//! `bos-settings/src/config/mod.rs` uses: a missing file yields defaults, a +//! file that exists but fails to parse is backed up once before falling +//! back, so a bad edit is always recoverable. This module used to carry its +//! own byte-for-byte copy of that logic (introduced in the same fix pass +//! that added it to bos-settings) — now both share one implementation. use std::path::{Path, PathBuf}; use toml_edit::{value, DocumentMut}; pub fn config_dir() -> PathBuf { - if let Ok(xdg) = std::env::var("XDG_CONFIG_HOME") { - let p = PathBuf::from(xdg); - if p.is_absolute() { - return p; - } - } - let home = std::env::var("HOME").unwrap_or_else(|_| "/root".to_string()); - PathBuf::from(home).join(".config") + bread_utils::xdg::config_home() } fn state_path() -> PathBuf { @@ -22,55 +18,15 @@ fn state_path() -> PathBuf { } fn load_doc(path: &Path) -> DocumentMut { - let Ok(text) = std::fs::read_to_string(path) else { - return DocumentMut::default(); - }; - match text.parse::() { - Ok(doc) => doc, - Err(e) => { - let backup = PathBuf::from(format!("{}.bak", path.display())); - eprintln!( - "breadhelp: {} failed to parse ({e}); backed up to {} before falling back to defaults", - path.display(), - backup.display() - ); - let _ = std::fs::write(&backup, &text); - DocumentMut::default() - } - } + bread_utils::tomlcfg::load_doc("breadhelp", path) } fn save_doc(path: &Path, doc: &DocumentMut) { - if let Err(e) = atomic_write(path, &doc.to_string()) { + if let Err(e) = bread_utils::tomlcfg::save_doc(path, doc) { eprintln!("breadhelp: couldn't write {}: {e}", path.display()); } } -/// Write `contents` to `path` atomically, backing up whatever was there -/// before overwriting it — same discipline as `bos-settings/src/config/ -/// mod.rs::atomic_write`. Writing straight to the target path means a -/// crash, power loss, or disk-full error mid-write can leave `state.toml` -/// truncated or corrupted with no way back. Writing to a temp file in the -/// same directory first, then `rename`-ing it over the target, avoids -/// that — a rename within one filesystem is atomic. Backing up the -/// previous file first (best-effort) means even a successful-but-wrong -/// write is always recoverable from `.bak`. -fn atomic_write(path: &Path, contents: &str) -> std::io::Result<()> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent)?; - } - if path.exists() { - let backup = PathBuf::from(format!("{}.bak", path.display())); - let _ = std::fs::copy(path, &backup); - } - let dir = path.parent().map(Path::to_path_buf).unwrap_or_else(|| PathBuf::from(".")); - let file_name = path.file_name().and_then(|n| n.to_str()).unwrap_or("config"); - let tmp_path = dir.join(format!(".{file_name}.tmp.{}", std::process::id())); - std::fs::write(&tmp_path, contents)?; - std::fs::rename(&tmp_path, path)?; - Ok(()) -} - #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Mode { Normal, @@ -184,17 +140,21 @@ mod tests { use super::*; #[test] - fn atomic_write_backs_up_previous_contents_and_no_tmp_file_left_behind() { + fn state_toml_write_backs_up_previous_contents_and_no_tmp_file_left_behind() { + // Exercises the same bread_utils::atomic::write_atomic_backed_up + // path save_doc uses (via tomlcfg::save_doc), at the file-layout + // breadhelp actually writes to — the deeper backup/no-leftover-tmp + // behavior itself is covered by bread-utils' own test suite. let dir = std::env::temp_dir().join(format!("breadhelp-atomic-write-test-{}", std::process::id())); std::fs::create_dir_all(&dir).unwrap(); let path = dir.join("state.toml"); let backup = dir.join("state.toml.bak"); - atomic_write(&path, "first").unwrap(); + bread_utils::atomic::write_atomic_backed_up(&path, "first").unwrap(); assert_eq!(std::fs::read_to_string(&path).unwrap(), "first"); assert!(!backup.exists(), "no backup should be made when there's nothing to back up yet"); - atomic_write(&path, "second").unwrap(); + bread_utils::atomic::write_atomic_backed_up(&path, "second").unwrap(); assert_eq!(std::fs::read_to_string(&path).unwrap(), "second"); assert_eq!(std::fs::read_to_string(&backup).unwrap(), "first"); diff --git a/src/services/hyprland.rs b/src/services/hyprland.rs index 1060481..4e570fc 100644 --- a/src/services/hyprland.rs +++ b/src/services/hyprland.rs @@ -46,8 +46,9 @@ pub struct LayerClient { } fn query_json(cmd: &str) -> Option { - let output = Command::new("hyprctl").args(["-j", cmd]).output().ok()?; - output.status.success().then(|| serde_json::from_slice(&output.stdout).ok()).flatten() + // Timeout-guarded: an unresponsive hyprctl (Hyprland wedged/reloading) + // used to be able to block clients()/monitors()/layers() indefinitely. + bread_utils::proc::run_json("hyprctl", &["-j", cmd], std::time::Duration::from_secs(3)) } pub fn clients() -> Vec { From 767e7f743411ea890b9177cfc8e1b52b4c46f0e4 Mon Sep 17 00:00:00 2001 From: Breadway Date: Sun, 19 Jul 2026 03:35:32 +0800 Subject: [PATCH 3/3] Switch to tag-pinned bread-ecosystem deps; bump version to v0.2.2 --- Cargo.lock | 35 ++++++++++++++++++----------------- Cargo.toml | 3 +-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d6b8b0c..01eb182 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -22,9 +22,9 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "bitflags" -version = "2.13.0" +version = "2.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4388bee8683e3d04af747c73422af53102d2bd24d9eadb6cbc100baef4b43f8" +checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" [[package]] name = "bread-theme" @@ -39,7 +39,8 @@ dependencies = [ [[package]] name = "bread-utils" -version = "0.2.3" +version = "0.3.0" +source = "git+https://git.breadway.dev/Breadway/bread-ecosystem?tag=v0.3.0#8e82d2d833e992ce939a5b836f910ee109f2e939" dependencies = [ "dirs", "serde", @@ -178,24 +179,24 @@ dependencies = [ [[package]] name = "futures-channel" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d" +checksum = "262590f4fe6afeb0bc83be1daa64e52657fe185690a958af7f3ad0e92085c5ae" dependencies = [ "futures-core", ] [[package]] name = "futures-core" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" +checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" [[package]] name = "futures-executor" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "baf29c38818342a3b26b5b923639e7b1f4a61fc5e76102d4b1981c6dc7a7579d" +checksum = "6754879cc9f2c66f88c6e5c35344bb0bdb0708b0352b1201815667c7eabc7458" dependencies = [ "futures-core", "futures-task", @@ -204,15 +205,15 @@ dependencies = [ [[package]] name = "futures-io" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cecba35d7ad927e23624b22ad55235f2239cfa44fd10428eecbeba6d6a717718" +checksum = "4577ecaa3c4f96589d473f679a71b596316f6641bc350038b962a5daf0085d7a" [[package]] name = "futures-macro" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" +checksum = "2d6d3cde68c518367be28956066ddfef33813991b77a55005a69dae04bf3b10b" dependencies = [ "proc-macro2", "quote", @@ -221,15 +222,15 @@ dependencies = [ [[package]] name = "futures-task" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" +checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" [[package]] name = "futures-util" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" +checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" dependencies = [ "futures-core", "futures-macro", diff --git a/Cargo.toml b/Cargo.toml index b414e7d..2203c2b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,5 +17,4 @@ toml = "0.8" # Non-destructive state editing (mirrors bos-settings/src/config/mod.rs). toml_edit = "0.22" async-channel = "2" -# TODO(owner): switch to tag-pinned git dependency once bread-utils is merged and tagged, matching the bread-theme pattern -bread-utils = { path = "../bread-ecosystem-fix-worktree/bread-utils", features = ["toml"] } +bread-utils = { git = "https://git.breadway.dev/Breadway/bread-ecosystem", tag = "v0.3.0", features = ["toml"] }