From 6e03f65bf45181db77ba6229716e6624db9eb25b Mon Sep 17 00:00:00 2001 From: ".CooCoo" Date: Wed, 11 Nov 2026 08:22:26 -0800 Subject: [PATCH] craft-update: a failed check for a running attach is reported on macOS (#803) detach_unmounted leaves a disk image's attachments alone when the check for a running `hdiutil attach` of the image gets no answer, and it returned Ok in that case. A failed install then ended with no note that the image may still be attached. It now returns the check's reason as an error, so the install's error says "the disk image may still be attached" with that reason, and craft-update logs it at warn level. --- crates/update/src/install.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/update/src/install.rs b/crates/update/src/install.rs index 070e035be..6094fdbc6 100644 --- a/crates/update/src/install.rs +++ b/crates/update/src/install.rs @@ -1716,7 +1716,8 @@ pub mod macos { /// Detaches the image file `dmg` where it is attached with no volume mounted, which a failed /// attach can leave. An attachment with a mounted volume is left alone: another install may /// be using it. So is every attachment while a program started with `dmg` in its arguments - /// (another process's `hdiutil attach`) is running, or while the check for one gets no answer. + /// (another process's `hdiutil attach`) is running, or while the check for one gets no answer; + /// in that case the error gives the check's reason. fn detach_unmounted(dmg: &Path) -> Result<(), String> { detach_unmounted_with(dmg, process::running, process::run) } @@ -1728,8 +1729,10 @@ pub mod macos { running: impl FnOnce(&Path) -> Result, run: impl Fn(&str, &[&str], Duration) -> Result, ) -> Result<(), String> { - if running(dmg) != Ok(false) { - return Ok(()); + match running(dmg) { + Ok(false) => {} + Ok(true) => return Ok(()), + Err(why) => return Err(format!("couldn't check for a running attach of the image ({why})")), } let info = run(HDIUTIL, &["info", "-plist"], Duration::from_secs(30))?; let image = [Some(dmg.to_path_buf()), std::fs::canonicalize(dmg).ok()]; @@ -4288,11 +4291,16 @@ esac "imagesimage-path{}system-entitiesdev-entry/dev/disk5", dmg.display() ); - for (answer, expected) in [ - (Ok(true), vec![]), - // pgrep gave no answer: an attach of the image may still be running. - (Err("pgrep didn't finish within 30 seconds".to_string()), vec![]), - (Ok(false), vec!["info -plist".to_string(), "detach /dev/disk5 -force -quiet".to_string()]), + for (answer, expected, outcome) in [ + (Ok(true), vec![], Ok(())), + // pgrep gave no answer: an attach of the image may still be running, and the image + // may still be attached. + ( + Err("pgrep didn't finish within 30 seconds".to_string()), + vec![], + Err("couldn't check for a running attach of the image (pgrep didn't finish within 30 seconds)".to_string()), + ), + (Ok(false), vec!["info -plist".to_string(), "detach /dev/disk5 -force -quiet".to_string()], Ok(())), ] { let ran = std::cell::RefCell::new(Vec::new()); let result = super::macos::detach_unmounted_with( @@ -4307,7 +4315,7 @@ esac Ok(if args[0] == "info" { info.clone() } else { String::new() }) }, ); - assert_eq!(result, Ok(())); + assert_eq!(result, outcome, "with the process check giving {answer:?}"); assert_eq!(ran.into_inner(), expected, "with the process check giving {answer:?}"); } }