diff --git a/apps/craft-cloud-desktop/src/main.rs b/apps/craft-cloud-desktop/src/main.rs index cf791e1..9f24411 100644 --- a/apps/craft-cloud-desktop/src/main.rs +++ b/apps/craft-cloud-desktop/src/main.rs @@ -5,6 +5,9 @@ //! - Linux: `--tray` is the tray process (no window: [`tray::process`]), which the start-at-login entry runs (an //! older entry's `--hidden` means the same). The window is a process of its own that ends when closed (Wayland can't //! hide a window), and starts the tray process if none runs. +//! +//! `--close` ends the window and the tray process that run (an installer's upgrade: also an older version's) and +//! prints which did (`window`, `tray`, a line each); the agent is left alone. #![cfg_attr(windows, windows_subsystem = "windows")] #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -29,6 +32,12 @@ fn main() -> ExitCode { eprintln!("Craft Cloud can't find your user data folder"); return ExitCode::FAILURE; }; + if has("--close") { + for name in single_instance::close_running(&dir) { + println!("{}", if name == Name::Tray { "tray" } else { "window" }); + } + return ExitCode::SUCCESS; + } let (outside_tx, outside_rx) = std::sync::mpsc::channel::(); let window: Arc> = Arc::new(OnceLock::new()); match single_instance::claim(&dir, Name::Window) { diff --git a/apps/craft-cloud-desktop/src/single_instance.rs b/apps/craft-cloud-desktop/src/single_instance.rs index 15d0bc1..4f3f1e9 100644 --- a/apps/craft-cloud-desktop/src/single_instance.rs +++ b/apps/craft-cloud-desktop/src/single_instance.rs @@ -44,6 +44,27 @@ impl Name { pub fn port_file(self) -> String { format!("{}.port", self.stem()) } + + /// Its lock and port files in `dir`. + fn files(self, dir: &Path) -> Files { + Files { lock: dir.join(self.lock_file()), port: dir.join(self.port_file()) } + } + + /// Where versions before the folder of its own kept them: loose in the per-user data folder (`parent`). + fn old_files(self, parent: &Path) -> Files { + let stem = match self { + Name::Window => "craft-cloud-app", + Name::Tray => "craft-cloud-tray", + }; + Files { lock: parent.join(format!("{stem}.lock")), port: parent.join(format!("{stem}.port")) } + } +} + +/// A process kind's lock and port files. +#[derive(Debug, Clone)] +struct Files { + lock: PathBuf, + port: PathBuf, } /// What a later start asks the first. @@ -82,8 +103,7 @@ pub struct First { /// Another one's address. pub struct Running { - dir: PathBuf, - name: Name, + files: Files, } /// The folder of the locks and the channels, next to the agent's data folder (the uninstallers remove it). @@ -102,31 +122,67 @@ fn folder_beside(data: &Path) -> Option { /// Takes the lock of `name` in `dir`, or finds the process that holds it. pub fn claim(dir: &Path, name: Name) -> std::io::Result { std::fs::create_dir_all(dir)?; - let lock = OpenOptions::new().create(true).truncate(false).write(true).open(dir.join(name.lock_file()))?; + claim_files(name.files(dir)) +} + +fn claim_files(files: Files) -> std::io::Result { + let lock = OpenOptions::new().create(true).truncate(false).write(true).open(&files.lock)?; match lock.try_lock() { Ok(()) => {} - Err(std::fs::TryLockError::WouldBlock) => return Ok(Instance::Running(Running { dir: dir.to_owned(), name })), + Err(std::fs::TryLockError::WouldBlock) => return Ok(Instance::Running(Running { files })), Err(std::fs::TryLockError::Error(e)) => return Err(e), } let listener = TcpListener::bind(SocketAddr::from((Ipv4Addr::LOCALHOST, 0)))?; let token = token()?; let port = listener.local_addr()?.port(); // Written whole, then renamed into place, so a later start never reads half of it. - let temp = dir.join(format!("{}.tmp", name.port_file())); + let mut temp = files.port.clone().into_os_string(); + temp.push(".tmp"); std::fs::write(&temp, format!("{port} {token}\n"))?; - std::fs::rename(&temp, dir.join(name.port_file()))?; + std::fs::rename(&temp, &files.port)?; Ok(Instance::First(First { lock, listener, token })) } /// Whether a process of `name` runs (holds its lock), without taking it for long. pub fn is_running(dir: &Path, name: Name) -> bool { - let Ok(lock) = OpenOptions::new().create(true).truncate(false).write(true).open(dir.join(name.lock_file())) else { return false }; + holds(&name.files(dir).lock) +} + +/// Whether a process holds the lock `lock` (one that isn't there isn't made). +fn holds(lock: &Path) -> bool { + let Ok(lock) = OpenOptions::new().write(true).open(lock) else { return false }; matches!(lock.try_lock(), Err(std::fs::TryLockError::WouldBlock)) } /// Sends `command` to the running process of `name`, if one answers (within a few seconds). pub fn send(dir: &Path, name: Name, command: Command) -> std::io::Result<()> { - Running { dir: dir.to_owned(), name }.send(command) + Running { files: name.files(dir) }.send(command) +} + +/// Ends the window and the tray process that run for `dir`, also those of a version that kept its files loose in +/// the folder above (an upgrade replaces the programs under them), and waits (a few seconds) for each to let go of its +/// lock. The kinds that were running. The agent is left alone. +pub fn close_running(dir: &Path) -> Vec { + let parent = dir.parent().unwrap_or(dir); + let mut closed = Vec::new(); + for name in [Name::Window, Name::Tray] { + for files in [name.files(dir), name.old_files(parent)] { + if !holds(&files.lock) { + continue; + } + let lock = files.lock.clone(); + if (Running { files }).send(Command::Quit).is_ok() { + let deadline = Instant::now() + PATIENCE; + while holds(&lock) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(50)); + } + if !closed.contains(&name) { + closed.push(name); + } + } + } + } + closed } fn token() -> std::io::Result { @@ -180,14 +236,14 @@ impl Running { match self.try_send(command) { Ok(()) => return Ok(()), // The first may be starting: its port file comes a moment after its lock. - Err(_) if Instant::now() < deadline && is_running(&self.dir, self.name) => std::thread::sleep(Duration::from_millis(100)), + Err(_) if Instant::now() < deadline && holds(&self.files.lock) => std::thread::sleep(Duration::from_millis(100)), Err(e) => return Err(e), } } } fn try_send(&self, command: Command) -> std::io::Result<()> { - let text = std::fs::read_to_string(self.dir.join(self.name.port_file()))?; + let text = std::fs::read_to_string(&self.files.port)?; let (port, token) = text.trim().split_once(' ').ok_or_else(|| std::io::Error::other("the port file doesn't read"))?; let port: u16 = port.parse().map_err(|_| std::io::Error::other("the port file doesn't read"))?; let mut stream = TcpStream::connect_timeout(&SocketAddr::from((Ipv4Addr::LOCALHOST, port)), Duration::from_secs(1))?; @@ -253,6 +309,29 @@ mod tests { assert!(inside.join("window.lock").is_file() && inside.join("window.port").is_file()); } + /// An upgrade closes what runs: here a tray in the folder and a window of an older version, whose files are loose + /// in the folder above. Each is told to quit; nothing runs where nothing is told. + #[test] + fn close_running_tells_the_window_and_the_tray_new_and_old() { + let parent = tempfile::tempdir().unwrap(); + let dir = parent.path().join(FOLDER_NAME); + assert!(close_running(&dir).is_empty(), "nothing runs"); + std::fs::create_dir_all(&dir).unwrap(); + let (told, heard) = channel(); + let Instance::First(tray) = claim(&dir, Name::Tray).unwrap() else { panic!("not first") }; + let Instance::First(old_window) = claim_files(Name::Window.old_files(parent.path())).unwrap() else { panic!("not first") }; + for (name, first) in [(Name::Tray, tray), (Name::Window, old_window)] { + let told = told.clone(); + first.listen(move |command| { + let _ = told.send((name, command)); + }); + } + assert_eq!(close_running(&dir), [Name::Window, Name::Tray]); + let mut heard: Vec<_> = (0..2).map(|_| heard.recv_timeout(Duration::from_secs(10)).unwrap()).collect(); + heard.sort_by_key(|(name, _)| *name == Name::Tray); + assert_eq!(heard, [(Name::Window, Command::Quit), (Name::Tray, Command::Quit)]); + } + #[test] fn a_wrong_token_shows_nothing() { let dir = tempfile::tempdir().unwrap(); diff --git a/apps/craft-cloud-setup/src/lib.rs b/apps/craft-cloud-setup/src/lib.rs index 942eab7..dd842da 100644 --- a/apps/craft-cloud-setup/src/lib.rs +++ b/apps/craft-cloud-setup/src/lib.rs @@ -6,7 +6,8 @@ //! API (it finishes the pass under way), waited for up to a minute, then ended, and its process checked gone. An //! agent that can't be stopped stops the install before anything is changed. Then the files, the Uninstall key //! (`HKCU\…\Uninstall\CraftCloud`, its `UninstallString` calling this setup, copied into the program folder, with -//! `--uninstall`), and the shortcut. An agent that ran is started again (and waited for), also when the install +//! `--uninstall`), and the shortcut. Start at login (the agent's and the tray's `Run` values, where there are any) is +//! pointed at this install, wherever it pointed before. An agent that ran is started again (and waited for), also when the install //! fails after the stop. An older install in //! `%LOCALAPPDATA%\Programs\craft-cloud` whose agent the `Run` value starts is moved: the new agent's //! `autostart on` points the value at the new folder (read back to check); the old folder's files are left alone. @@ -146,6 +147,13 @@ pub trait Machine { fn shortcut(&self, lnk: &Path, target: &Path) -> Result<(), String>; /// Removes the folder `dir` once this process has ended (it holds this setup's own exe). fn remove_after_exit(&self, dir: &Path) -> Result<(), String>; + /// Whether the process `pid` runs (any program); `true` when that can't be told. + fn process_runs(&self, pid: u32) -> bool; + /// Renames (moves) `from` to `to` in one step, which fails as a whole: on Windows a file a program holds open (or + /// a folder with one in it) can't be moved, so a failure leaves everything where it was. (A running exe can be.) + fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { + std::fs::rename(from, to).map_err(|e| format!("{} can't be moved (is it still running?): {e}", from.display())) + } } /// How to install. @@ -255,6 +263,7 @@ fn install_stopped( let dir = places.dir(); let old = places.programs.join(OLD_DIR_NAME); let mut warnings = Vec::new(); + clear_leftovers(places, machine, true); std::fs::create_dir_all(&dir).map_err(io(&dir))?; for (name, bytes) in payload { write_replacing(&dir.join(name), bytes)?; @@ -265,20 +274,29 @@ fn install_stopped( write_replacing(&setup_copy, &bytes)?; } - // The older install's agent starts at login: the new one takes that over (no agent runs now, so the new agent - // writes its own entry). The old files stay. The value is read back: only a value that starts the new agent is a - // move. + // Start at login runs this install from now on. The agent's value, wherever it points (the older install, or a + // build run from elsewhere): the new agent takes it over (no agent runs now, so it writes its own entry), and the + // value is read back; only one that starts the new agent counts. The app's tray value, which the app writes for + // whichever copy last ran: pointed at this one. A value that isn't there stays away (start at login was turned + // off). The older install's files stay. let mut moved_from = None; - let run_value = registry.get(&places.run_key, AGENT_RUN_VALUE); - if old.join(AGENT_EXE).is_file() && run_value.as_deref().is_some_and(|v| starts_from(v, &old)) { + if let Some(value) = registry.get(&places.run_key, AGENT_RUN_VALUE).filter(|v| !starts_from(v, &dir)) { let moved = machine.run_agent(&dir.join(AGENT_EXE), &["autostart", "on"]).and_then(|()| match registry.get(&places.run_key, AGENT_RUN_VALUE) { Some(v) if starts_from(&v, &dir) => Ok(()), other => Err(format!("the Run value is still {}", other.unwrap_or_else(|| "missing".into()))), }); match moved { - Ok(()) => moved_from = Some(old.clone()), - Err(e) => warnings.push(format!("start at login still runs the old agent in {} ({e})", old.display())), + Ok(()) if starts_from(&value, &old) => moved_from = Some(old.clone()), + Ok(()) => {} + Err(e) if starts_from(&value, &old) => warnings.push(format!("start at login still runs the old agent in {} ({e})", old.display())), + Err(e) => warnings.push(format!("start at login still runs another agent ({value}): {e}")), + } + } + if registry.get(&places.run_key, APP_RUN_VALUE).is_some_and(|v| !starts_from(&v, &dir)) { + let tray = format!("\"{}\" --hidden", dir.join(APP_EXE).display()); + if let Err(e) = registry.set_string(&places.run_key, APP_RUN_VALUE, &tray) { + warnings.push(format!("the tray still starts at login from another copy ({e})")); } } @@ -330,42 +348,145 @@ fn restarted_after(places: &Places, registry: &dyn Registry, machine: &dyn Machi /// Removes the install at `places`; `running` is this setup's own exe (in the program folder when started from /// "Apps & features": then the folder goes once this process has ended). pub fn uninstall(places: &Places, registry: &dyn Registry, machine: &dyn Machine, running: &Path) -> Result<(), String> { - let dir = places.dir(); machine.close_app(); - match machine.stop_agent(&places.agent_data) { - AgentStop::NotRunning | AgentStop::Stopped => {} + let was_running = match machine.stop_agent(&places.agent_data) { + AgentStop::NotRunning => false, + AgentStop::Stopped => true, AgentStop::StillRunning(why) => return Err(still_running(&why)), AgentStop::Unknown(why) => return Err(restarted_after(places, registry, machine, &unknown_stop(&why))), - } + }; + let mut removed_values = Vec::new(); + uninstall_stopped(places, registry, machine, running, &mut removed_values) + .map_err(|e| put_back(places, registry, machine, was_running, &removed_values, &e)) +} + +/// The uninstall's changes once nothing runs; the `Run` values it removed (name, value) go into `removed_values`. +fn uninstall_stopped( + places: &Places, + registry: &dyn Registry, + machine: &dyn Machine, + running: &Path, + removed_values: &mut Vec<(&'static str, String)>, +) -> Result<(), String> { + let dir = places.dir(); + clear_leftovers(places, machine, false); for value in [AGENT_RUN_VALUE, APP_RUN_VALUE] { - if registry.get(&places.run_key, value).as_deref().is_some_and(|v| starts_from(v, &dir)) { + if let Some(v) = registry.get(&places.run_key, value).filter(|v| starts_from(v, &dir)) { registry.remove_value(&places.run_key, value)?; + removed_values.push((value, v)); } } let shortcut = places.shortcut(); if shortcut.exists() { std::fs::remove_file(&shortcut).map_err(io(&shortcut))?; } - // The files before the "Apps & features" entry: if one can't go, the entry stays, to run it again. + // The files before the "Apps & features" entry: if they can't go, the entry stays, to run it again. if dir.exists() { - let running_inside = running.parent().is_some_and(|p| same_path(p, &dir)); - if running_inside { - for entry in std::fs::read_dir(&dir).map_err(io(&dir))? { - let path = entry.map_err(io(&dir))?.path(); - if !same_path(&path, running) { - remove_patiently(&path, places.patience)?; - } - } - machine.remove_after_exit(&dir)?; - } else { - remove_patiently(&dir, places.patience)?; - } + remove_program_folder(places, machine, running)?; } registry.remove_key(&places.uninstall_key)?; remove_app_files(&places.app_files); Ok(()) } +/// Removes the program folder. Its files are first moved aside, into `.removing-` next to it, each move +/// one step that fails as a whole if a file is held open: if any can't be moved, those moved go back, and the folder +/// is as it was (so the agent can be put back). Only then is anything deleted: the moved-aside folder (what can't be +/// deleted yet, such as an exe still running, once this setup has ended), and the program folder itself (once this +/// setup has ended, when this setup runs from it). +fn remove_program_folder(places: &Places, machine: &dyn Machine, running: &Path) -> Result<(), String> { + let dir = places.dir(); + let aside = dir.with_file_name(format!("{DIR_NAME}.removing-{}", std::process::id())); + let running_inside = running.parent().is_some_and(|p| same_path(p, &dir)); + if running_inside { + // This setup's own exe can't move: everything else does, one by one, back again if one can't. + std::fs::create_dir_all(&aside).map_err(io(&aside))?; + let mut moved: Vec<(PathBuf, PathBuf)> = Vec::new(); + for entry in std::fs::read_dir(&dir).map_err(io(&dir))? { + let path = entry.map_err(io(&dir))?.path(); + if same_path(&path, running) { + continue; + } + let to = aside.join(path.file_name().unwrap_or_default()); + if let Err(e) = machine.rename(&path, &to) { + let mut stranded = Vec::new(); + for (from, to) in moved.iter().rev() { + if machine.rename(to, from).is_err() { + stranded.push(from.file_name().unwrap_or_default().to_string_lossy().into_owned()); + } + } + let _ = std::fs::remove_dir(&aside); + if stranded.is_empty() { + return Err(e); + } + return Err(format!( + "{e}; and {} couldn't be moved back from {} (the next run of the setup puts them back)", + stranded.join(", "), + aside.display() + )); + } + moved.push((path, to)); + } + } else { + machine.rename(&dir, &aside)?; + } + // Past the point of no return: what can't be deleted now or after the setup is a leftover the next run removes. + if remove_patiently(&aside, places.patience).is_err() { + let _ = machine.remove_after_exit(&aside); + } + if running_inside { machine.remove_after_exit(&dir) } else { Ok(()) } +} + +/// Folders an earlier uninstall moved aside (`.removing-`) and left behind, its setup no longer running: +/// a roll-back that couldn't finish, or a delete that couldn't. On install (`restore`), every file the program folder +/// lacks is moved back into it first; then, and on uninstall, the leftover is deleted (best effort). +fn clear_leftovers(places: &Places, machine: &dyn Machine, restore: bool) { + let dir = places.dir(); + let prefix = format!("{DIR_NAME}.removing-"); + let Ok(entries) = std::fs::read_dir(&places.programs) else { return }; + for entry in entries.filter_map(Result::ok) { + let name = entry.file_name().to_string_lossy().into_owned(); + let Some(pid) = name.strip_prefix(&prefix).and_then(|pid| pid.parse::().ok()) else { continue }; + if pid == std::process::id() || machine.process_runs(pid) { + continue; + } + let leftover = entry.path(); + if restore && std::fs::create_dir_all(&dir).is_ok() { + for file in std::fs::read_dir(&leftover).into_iter().flatten().filter_map(Result::ok) { + let back = dir.join(file.file_name()); + if !back.exists() { + let _ = machine.rename(&file.path(), &back); + } + } + } + let _ = remove_patiently(&leftover, places.patience); + } +} + +/// The uninstall failed (`failure`) after the agent was stopped and maybe its start at login removed: while the +/// program folder is whole, its `Run` values are put back and, if it ran, it is started again, so this computer keeps +/// syncing until the uninstall is run again. What is said. +fn put_back( + places: &Places, + registry: &dyn Registry, + machine: &dyn Machine, + was_running: bool, + removed_values: &[(&'static str, String)], + failure: &str, +) -> String { + let dir = places.dir(); + if ![APP_EXE, AGENT_EXE, AGENT_BG_EXE].iter().all(|file| dir.join(file).is_file()) { + return format!("{failure}. Run the uninstall again to finish it."); + } + let restored = removed_values.iter().all(|(name, value)| registry.set_string(&places.run_key, name, value).is_ok()); + let told = match (removed_values.is_empty(), restored) { + (true, _) => String::new(), + (false, true) => " Start at login was put back.".to_owned(), + (false, false) => " Start at login couldn't all be put back: run the uninstall again, or the setup.".to_owned(), + }; + if was_running { format!("{}{told}", restarted_after(places, registry, machine, failure)) } else { format!("{failure}.{told}") } +} + /// The app's single-instance files in `app_files`, the folder itself once empty, and the loose ones older versions /// kept beside it. Only these names: anything else there stays. Last, and best effort: a file an app that couldn't be /// closed still holds stays, and is harmless. diff --git a/apps/craft-cloud-setup/src/tests.rs b/apps/craft-cloud-setup/src/tests.rs index 441539f..2da7a85 100644 --- a/apps/craft-cloud-setup/src/tests.rs +++ b/apps/craft-cloud-setup/src/tests.rs @@ -13,6 +13,11 @@ struct Recorder { stuck: bool, /// The agent accepts the quit, then whether it has ended can't be told (tasklist fails). unknowable: bool, + /// Moving a file or folder fails (one is still in use). + rename_fails: bool, + /// Only the move with this number (from 1) fails. + rename_fails_at: Option, + renames: Mutex, /// Where `autostart on` writes the agent's Run value, as the agent does (for the program it was run as)… registry: Option<(Arc, String)>, /// …unless an old agent still running answered it and registered itself. @@ -71,6 +76,20 @@ impl Machine for Recorder { self.note(format!("remove after exit {}", dir.display())); Ok(()) } + fn process_runs(&self, _pid: u32) -> bool { + false + } + fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { + let number = { + let mut renames = self.renames.lock().unwrap(); + *renames += 1; + *renames + }; + if self.rename_fails || self.rename_fails_at == Some(number) { + return Err(format!("{} can't be moved (is it still running?)", from.display())); + } + std::fs::rename(from, to).map_err(|e| e.to_string()) + } } /// A user's folders in a temporary place: programs, the Start menu, the agent's data folder and the synced folder. @@ -196,6 +215,147 @@ fn an_install_that_fails_after_the_stop_starts_the_agent_again() { assert_eq!(machine.asked(), ["close app", "stop agent"]); } +/// An uninstall that fails after the agent was stopped and its start at login removed (here the shortcut can't be +/// removed): the Run values are put back and the agent is started again; without a running agent, only put back. +#[test] +fn an_uninstall_that_fails_after_the_stop_puts_the_agent_back() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let value = format!("\"{}\" run --log", u.places.dir().join(AGENT_BG_EXE).display()); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, &value).unwrap(); + let before = registry.values.lock().unwrap().clone(); + // A "shortcut" that is a folder with something in it: removing it fails. + std::fs::create_dir_all(u.places.shortcut().join("x")).unwrap(); + + let machine = Recorder::default(); + *machine.running.lock().unwrap() = true; + let e = uninstall(&u.places, ®istry, &machine, &u.setup).unwrap_err(); + assert!(e.contains("The agent was started again.") && e.ends_with("Start at login was put back."), "{e}"); + assert_eq!(machine.asked(), ["close app", "stop agent", "start Craft Cloud/craft-cloud-agent-bg.exe"]); + assert_eq!(*registry.values.lock().unwrap(), before, "the Run value is back, the entry stays"); + + let machine = Recorder::default(); + let e = uninstall(&u.places, ®istry, &machine, &u.setup).unwrap_err(); + assert!(e.ends_with("Start at login was put back.") && !e.contains("started again"), "{e}"); + assert_eq!(machine.asked(), ["close app", "stop agent"]); + assert_eq!(*registry.values.lock().unwrap(), before); +} + +/// The commonest failure: a file in the program folder is still held, so it can't be moved aside. Nothing in the +/// folder has been touched: the Run values are put back and the agent started again. From "Apps & features" too. +#[test] +fn an_uninstall_with_a_file_still_held_leaves_the_folder_whole_and_puts_the_agent_back() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let dir = u.places.dir(); + let value = format!("\"{}\" run --log", dir.join(AGENT_BG_EXE).display()); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, &value).unwrap(); + let before = registry.values.lock().unwrap().clone(); + for running in [u.setup.clone(), dir.join(SETUP_EXE)] { + let machine = Recorder { rename_fails: true, ..Recorder::default() }; + *machine.running.lock().unwrap() = true; + let e = uninstall(&u.places, ®istry, &machine, &running).unwrap_err(); + assert!(e.contains("can't be moved") && e.contains("The agent was started again.") && e.ends_with("Start at login was put back."), "{e}"); + assert_eq!(machine.asked(), ["close app", "stop agent", "start Craft Cloud/craft-cloud-agent-bg.exe"]); + assert_eq!(*registry.values.lock().unwrap(), before); + for (file, bytes) in V2 { + assert_eq!(std::fs::read(dir.join(file)).unwrap(), *bytes, "{file} is in place"); + } + let siblings: Vec<_> = std::fs::read_dir(&u.places.programs).unwrap().map(|e| e.unwrap().file_name()).collect(); + assert_eq!(siblings, [DIR_NAME], "nothing left aside"); + } +} + +/// From "Apps & features" (the setup runs from the folder) the files move one by one: the third can't, so the two +/// moved come back, the Run values are put back and the agent is started again. +#[test] +fn a_move_that_fails_partway_puts_the_moved_files_back() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let dir = u.places.dir(); + let value = format!("\"{}\" run --log", dir.join(AGENT_BG_EXE).display()); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, &value).unwrap(); + let before = registry.values.lock().unwrap().clone(); + let machine = Recorder { rename_fails_at: Some(3), ..Recorder::default() }; + *machine.running.lock().unwrap() = true; + let e = uninstall(&u.places, ®istry, &machine, &dir.join(SETUP_EXE)).unwrap_err(); + assert!(e.contains("can't be moved") && !e.contains("couldn't be moved back") && e.ends_with("Start at login was put back."), "{e}"); + assert_eq!(*machine.renames.lock().unwrap(), 5, "two moves, the failed third, two moves back"); + assert_eq!(machine.asked(), ["close app", "stop agent", "start Craft Cloud/craft-cloud-agent-bg.exe"]); + assert_eq!(*registry.values.lock().unwrap(), before); + for (file, bytes) in V2 { + assert_eq!(std::fs::read(dir.join(file)).unwrap(), *bytes, "{file} is back"); + } + let siblings: Vec<_> = std::fs::read_dir(&u.places.programs).unwrap().map(|e| e.unwrap().file_name()).collect(); + assert_eq!(siblings, [DIR_NAME], "nothing left aside"); + + // Moves back that fail too: the two files left aside are named. + let e = uninstall(&u.places, ®istry, &FailingBack(Recorder::default()), &dir.join(SETUP_EXE)).unwrap_err(); + let (_, stranded) = e.split_once("; and ").unwrap(); + assert_eq!(V2.iter().filter(|(file, _)| stranded.contains(file)).count(), 2, "{e}"); + assert!(e.contains("the next run of the setup puts them back"), "{e}"); +} + +/// A machine whose third move fails, and every move after it. +struct FailingBack(Recorder); + +impl Machine for FailingBack { + fn stop_agent(&self, data: &Path) -> AgentStop { + self.0.stop_agent(data) + } + fn close_app(&self) {} + fn run_agent(&self, agent: &Path, args: &[&str]) -> Result<(), String> { + self.0.run_agent(agent, args) + } + fn start_agent(&self, agent_bg: &Path, data: &Path) -> Result<(), String> { + self.0.start_agent(agent_bg, data) + } + fn shortcut(&self, lnk: &Path, target: &Path) -> Result<(), String> { + self.0.shortcut(lnk, target) + } + fn remove_after_exit(&self, dir: &Path) -> Result<(), String> { + self.0.remove_after_exit(dir) + } + fn process_runs(&self, _pid: u32) -> bool { + false + } + fn rename(&self, from: &Path, to: &Path) -> Result<(), String> { + let mut renames = self.0.renames.lock().unwrap(); + *renames += 1; + if *renames >= 3 { + return Err(format!("{} can't be moved", from.display())); + } + std::fs::rename(from, to).map_err(|e| e.to_string()) + } +} + +/// A folder an earlier uninstall left aside (its setup gone) holding the agent's exe the program folder lacks: the +/// next install moves it back first and installs over it; the leftover is gone. An uninstall just deletes one. +#[test] +fn a_leftover_aside_folder_is_restored_by_install_and_removed_by_uninstall() { + let u = user("Test"); + let registry = MemoryRegistry::default(); + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + let dir = u.places.dir(); + let leftover = dir.with_file_name(format!("{DIR_NAME}.removing-4000000")); + std::fs::create_dir_all(&leftover).unwrap(); + std::fs::rename(dir.join(AGENT_BG_EXE), leftover.join(AGENT_BG_EXE)).unwrap(); + std::fs::write(leftover.join("notes.txt"), "kept by an older version").unwrap(); + // The install's payload lacks notes.txt: only a file moved back can be there afterwards. + install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); + assert!(!leftover.exists(), "the leftover is gone"); + assert_eq!(std::fs::read(dir.join(AGENT_BG_EXE)).unwrap(), b"agent-bg v2"); + assert!(dir.join("notes.txt").is_file(), "what the folder lacked was moved back"); + + std::fs::create_dir_all(&leftover).unwrap(); + std::fs::write(leftover.join(AGENT_EXE), "old").unwrap(); + uninstall(&u.places, ®istry, &Recorder::default(), &u.setup).unwrap(); + assert!(!leftover.exists() && !dir.exists()); +} + /// The agent accepts the quit, then tasklist fails: the install and the uninstall stop with nothing else changed, /// and the agent (which may be stopping) is started again from the folder its start at login names. #[test] @@ -223,6 +383,33 @@ fn an_agent_asked_to_quit_whose_end_cant_be_told_is_started_again() { assert_eq!(*registry.values.lock().unwrap(), before); } +/// Start-at-login values that run another copy (here a development build in a cargo target folder, and an agent +/// somewhere else) are pointed at the installed one; a value that isn't there stays away. +#[test] +fn start_at_login_is_pointed_at_the_installed_copy() { + let u = user("Test"); + let registry = Arc::new(MemoryRegistry::default()); + let dir = u.places.dir(); + let dev_tray = r#""C:\Users\u\craft-cloud-target\release\craft-cloud.exe" --hidden"#; + let dev_agent = r#""C:\Users\u\craft-cloud-target\release\craft-cloud-agent-bg.exe" run --log"#; + registry.set_string(&u.places.run_key, APP_RUN_VALUE, dev_tray).unwrap(); + registry.set_string(&u.places.run_key, AGENT_RUN_VALUE, dev_agent).unwrap(); + let machine = Recorder { registry: Some((registry.clone(), u.places.run_key.clone())), ..Recorder::default() }; + let installed = install(&u.places, &*registry, &machine, V2, &u.setup, &options(false)).unwrap(); + assert!(installed.warnings.is_empty(), "{:?}", installed.warnings); + assert_eq!(installed.moved_from, None, "not the older install"); + assert_eq!(registry.get(&u.places.run_key, APP_RUN_VALUE), Some(format!("\"{}\" --hidden", dir.join(APP_EXE).display()))); + assert!(registry.get(&u.places.run_key, AGENT_RUN_VALUE).is_some_and(|v| starts_from(&v, &dir))); + assert!(machine.asked().contains(&"Craft Cloud/craft-cloud-agent.exe autostart on".to_owned())); + + // Already pointing here: left as it is, and the agent isn't asked. Not there: stays away. + let machine = Recorder { registry: Some((registry.clone(), u.places.run_key.clone())), ..Recorder::default() }; + registry.remove_value(&u.places.run_key, APP_RUN_VALUE).unwrap(); + install(&u.places, &*registry, &machine, V2, &u.setup, &options(false)).unwrap(); + assert_eq!(registry.get(&u.places.run_key, APP_RUN_VALUE), None); + assert!(!machine.asked().iter().any(|a| a.contains("autostart")), "{:?}", machine.asked()); +} + /// BEAST's install, made by hand in `Programs\craft-cloud`: its agent starts at login from there. The new agent takes /// start at login over; the old files stay. #[test] @@ -377,24 +564,23 @@ fn a_running_agents_file_is_read_and_it_is_asked_to_quit() { assert!(request.contains(&format!("Host: 127.0.0.1:{port}\r\n")) && request.contains("Authorization: Bearer a1b2c3\r\n"), "{request}"); } -/// Files that can't be removed (still held by a program that is ending): tried again for a while; if they still -/// can't, the uninstall fails with the "Apps & features" entry still there, so it can be run again. +/// Files moved aside that can't be deleted at once (here a read-only folder): the uninstall finishes, and the +/// moved-aside folder goes once the setup has ended. #[cfg(unix)] #[test] -fn files_that_cant_be_removed_leave_the_uninstall_entry() { +fn files_moved_aside_that_cant_be_deleted_yet_go_after_the_setup() { use std::os::unix::fs::PermissionsExt; let u = user("Test"); let registry = MemoryRegistry::default(); install(&u.places, ®istry, &Recorder::default(), V2, &u.setup, &options(false)).unwrap(); let dir = u.places.dir(); std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o500)).unwrap(); - let removed = uninstall(&u.places, ®istry, &Recorder::default(), &u.setup); - std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(0o700)).unwrap(); - assert!(removed.is_err()); - assert_eq!(registry.get(&u.places.uninstall_key, "DisplayName").as_deref(), Some("Craft Cloud"), "removed last"); - // Once they can be removed, running it again finishes. - uninstall(&u.places, ®istry, &Recorder::default(), &u.setup).unwrap(); + let machine = Recorder::default(); + uninstall(&u.places, ®istry, &machine, &u.setup).unwrap(); + let aside = dir.with_file_name(format!("{DIR_NAME}.removing-{}", std::process::id())); + std::fs::set_permissions(&aside, std::fs::Permissions::from_mode(0o700)).unwrap(); assert!(!dir.exists() && registry.get(&u.places.uninstall_key, "DisplayName").is_none()); + assert_eq!(machine.asked().last().cloned(), Some(format!("remove after exit {}", aside.display()))); } /// A file that is held for a moment (a program ending) is removed once it is free. @@ -550,3 +736,25 @@ fn the_program_folder_goes_once_the_setup_has_ended() { assert!(outside.join("keep.txt").exists(), "the link wasn't followed (cmd: {with_cmd})"); } } + +/// The `cmd` remover with a `tasklist` that can't run never takes the setup for ended: it waits, and the folder stays. +#[cfg(windows)] +#[test] +fn the_cmd_remover_waits_while_tasklist_cant_tell() { + let root = tempfile::tempdir().unwrap(); + let dir = root.path().join("Craft Cloud"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join(SETUP_EXE), "setup").unwrap(); + // Three rounds of asking, a second's wait each, then it gives up (an hour in the real one). + let started = std::time::Instant::now(); + let mut remover = crate::windows::remove_after_with_cmd_without_tasklist(3, std::process::id(), "craft-cloud-setup.exe", &dir).spawn().unwrap(); + remover.wait().unwrap(); + assert!(started.elapsed() >= std::time::Duration::from_secs(2), "it waited each round (a broken line ends at once): {:?}", started.elapsed()); + assert!(dir.join(SETUP_EXE).exists(), "nothing was removed while it couldn't tell"); + let mine = format!("craft-cloud-uninstall-{}-", std::process::id()); + for file in std::fs::read_dir(std::env::temp_dir()).unwrap().filter_map(Result::ok) { + if file.file_name().to_string_lossy().starts_with(&mine) { + let _ = std::fs::remove_file(file.path()); + } + } +} diff --git a/apps/craft-cloud-setup/src/windows.rs b/apps/craft-cloud-setup/src/windows.rs index 063b9cd..5360e68 100644 --- a/apps/craft-cloud-setup/src/windows.rs +++ b/apps/craft-cloud-setup/src/windows.rs @@ -72,20 +72,45 @@ pub fn remove_after(pid: u32, dir: &Path) -> Command { } /// [`remove_after`] without PowerShell: `cmd` alone asks `tasklist` every second whether the process `pid` (whose -/// exe is `image`) still runs, for up to an hour, then removes the folder ([`RMDIR`], again a moment later if a file -/// was still held). The `if` is in parentheses of its own: cmd would take a following `& exit` as part of it. +/// exe is `image`) still runs, for up to an hour, then removes the folder ([`RMDIR`], tried again every second for +/// half a minute while a file is still held). pub fn remove_after_with_cmd(pid: u32, image: &str, dir: &Path) -> Command { - let tasklist = system_program("tasklist.exe"); + remove_after_with_cmd_using(&system_program("tasklist.exe"), 3600, pid, image, dir) +} + +/// [`remove_after_with_cmd`] with `tasklist` at `tasklist`, asked `rounds` times at most. +/// +/// `tasklist`'s answer goes to a file first (`CRAFT_CLOUD_TASKS`, in the temporary folder), so a `tasklist` that +/// fails (and writes nothing) is told apart from one that lists nothing: then cmd waits and asks again, rather than +/// take the setup for ended. Only `find` decides: the wait after it ends in `ver`, which always succeeds, so a `ping` +/// that fails isn't read as "ended" either. `find` and `ping` are named by their full paths: cmd looks in its working +/// folder first. +/// Each `if` is in parentheses of its own: cmd would take a following `& …` as part of it. +fn remove_after_with_cmd_using(tasklist: &Path, rounds: u32, pid: u32, image: &str, dir: &Path) -> Command { + let (find, ping) = (system_program("find.exe"), system_program("PING.EXE")); + let wait = format!("\"{}\" -n 2 127.0.0.1 >nul", ping.display()); + let remove = format!("for /l %j in (1,1,30) do @(if exist \"%CRAFT_CLOUD_DIR%\" ({RMDIR} & (if exist \"%CRAFT_CLOUD_DIR%\" ({wait}))))"); let line = format!( - "/d /c for /l %i in (1,1,3600) do @(\"{tasklist}\" /FI \"PID eq {pid}\" /NH | find /i \"{image}\" >nul && ping -n 2 127.0.0.1 >nul \ - || ({RMDIR} & (if exist \"%CRAFT_CLOUD_DIR%\" (ping -n 2 127.0.0.1 >nul & {RMDIR})) & exit))", - tasklist = tasklist.display() + "/d /c for /l %i in (1,1,{rounds}) do @((\"{tasklist}\" /FI \"PID eq {pid}\" /NH >\"%CRAFT_CLOUD_TASKS%\") \ + && (\"{find}\" /i \"{image}\" \"%CRAFT_CLOUD_TASKS%\" >nul && ({wait} & ver >nul) || ({remove} & del \"%CRAFT_CLOUD_TASKS%\" & exit)) || {wait})", + tasklist = tasklist.display(), + find = find.display() ); + // One file per remover: two may run at once (the moved-aside folder's and the program folder's). + static REMOVERS: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let n = REMOVERS.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let tasks = std::env::temp_dir().join(format!("craft-cloud-uninstall-{pid}-{n}.txt")); let mut command = quiet(&system_program("cmd.exe")); - command.raw_arg(line).env("CRAFT_CLOUD_DIR", dir); + command.raw_arg(line).env("CRAFT_CLOUD_DIR", dir).env("CRAFT_CLOUD_TASKS", tasks); detached_from(command, dir) } +/// For tests: [`remove_after_with_cmd_using`] with a `tasklist` that can't run, asked `rounds` times. +#[cfg(test)] +pub fn remove_after_with_cmd_without_tasklist(rounds: u32, pid: u32, image: &str, dir: &Path) -> Command { + remove_after_with_cmd_using(Path::new(r"C:\no-such-folder\tasklist.exe"), rounds, pid, image, dir) +} + /// In a process group of its own, and working in the folder above `dir` (a remover working inside it would hold it). fn detached_from(mut command: Command, dir: &Path) -> Command { command.creation_flags(CREATE_NO_WINDOW | CREATE_NEW_PROCESS_GROUP).current_dir(dir.parent().unwrap_or(dir)); @@ -259,6 +284,17 @@ impl Machine for ThisMachine { if status.success() && lnk.exists() { Ok(()) } else { Err(format!("the shortcut wasn't written ({status})")) } } + fn process_runs(&self, pid: u32) -> bool { + let out = quiet(&system_program("tasklist.exe")).args(["/FI", &format!("PID eq {pid}"), "/NH", "/FO", "CSV"]).stdout(Stdio::piped()).output(); + match out { + Ok(out) if out.status.success() && !out.stdout.is_empty() => { + String::from_utf8_lossy(&out.stdout).lines().any(|line| line.split("\",\"").nth(1) == Some(pid.to_string().as_str())) + } + // Can't tell: taken as running, so nothing of it is touched. + _ => true, + } + } + fn remove_after_exit(&self, dir: &Path) -> Result<(), String> { // Waits for this setup to end, however long its last message stays open: through PowerShell, else `cmd`. let pid = std::process::id(); diff --git a/packaging/linux/install.sh b/packaging/linux/install.sh index 2aba19d..26e83cf 100755 --- a/packaging/linux/install.sh +++ b/packaging/linux/install.sh @@ -45,30 +45,94 @@ has_unit() { [ -f "$unit" ] && command -v systemctl >/dev/null 2>&1 } -# Starts the agent again once the process $1 (or, with no pid known, the one whose local-api.json is in the data -# folder) has ended and so let go of the data folder: an agent started before that finds the folder taken and exits. -# Through systemd when $2 is "systemd", else `$3 run --log`. In the background: the wait may be long. +# Runs "$@" in the background holding none of this script's streams (stdin, stdout and stderr on /dev/null), in a +# session of its own where setsid is there: whatever waits for this script's output (ssh, a pipe) isn't kept waiting +# for the programs it starts. +detach() { + if command -v setsid >/dev/null 2>&1; then + setsid "$@" /dev/null 2>&1 & + else + nohup "$@" /dev/null 2>&1 & + fi +} + +# The variables that pick the agent's folders, those that are set, as `"--setenv=NAME=${NAME}"` words for the +# caller's `eval "set -- \"\$@\" $(env_flags)"`: a systemd unit doesn't get this shell's environment. +env_flags() { + for name in CRAFT_CLOUD_AGENT_DIR XDG_DATA_HOME XDG_CONFIG_HOME XDG_STATE_HOME; do + if eval "[ -n \"\${$name+x}\" ]"; then + printf ' "--setenv=%s=${%s}"' "$name" "$name" + fi + done +} + +# Whether the process $1 still runs and is still the one whose name (/proc//comm) was $2: a pid of an agent that +# has ended may be another program's by now. +alive() { + kill -0 "$1" 2>/dev/null && { [ -z "$2" ] || [ "$(cat "/proc/$1/comm" 2>/dev/null)" = "$2" ]; } +} + +# Starts the agent again once the process $1 (named $4; or, with no pid known, the one whose local-api.json is in the +# data folder) has ended and so let go of the data folder: an agent started before that finds the folder taken and +# exits. Through systemd when $2 is "systemd", else `$3 run --log`. In the background: the wait may be long. +# +# The waiter only waits: the agent it starts runs outside it (its own systemd unit, or detached), so the waiter is +# gone once the agent is up. Where the user has systemd, the waiter is a unit of its own (systemd-run --user, with +# the folder variables passed along): it outlives this login session for as long as the user's systemd runs (another +# session, or lingering), and its fixed name keeps it to one at a time; a second install that gives up while it +# waits finds it and adds none. start_after_exit() { - nohup sh -c ' + waiter=' + pid=$1 how=$2 agent=$3 data=$4 comm=$5 + alive() { + kill -0 "$1" 2>/dev/null && { [ -z "$2" ] || [ "$(cat "/proc/$1/comm" 2>/dev/null)" = "$2" ]; } + } n=0 - if [ -n "$1" ]; then - while kill -0 "$1" 2>/dev/null && [ "$n" -lt 86400 ]; do + if [ -n "$pid" ]; then + while alive "$pid" "$comm" && [ "$n" -lt 86400 ]; do n=$((n + 1)) sleep 1 done else - while [ -e "$4/local-api.json" ] && [ "$n" -lt 86400 ]; do + while [ -e "$data/local-api.json" ] && [ "$n" -lt 86400 ]; do n=$((n + 1)) sleep 1 done sleep 5 fi - if [ "$2" = systemd ]; then + if [ "$how" = systemd ]; then systemctl --user start craft-cloud-agent.service + exit + fi + set -- + for name in CRAFT_CLOUD_AGENT_DIR XDG_DATA_HOME XDG_CONFIG_HOME XDG_STATE_HOME; do + if eval "[ -n \"\${$name+x}\" ]"; then + eval "set -- \"\$@\" \"--setenv=$name=\${$name}\"" + fi + done + if command -v systemd-run >/dev/null 2>&1 && systemd-run --user --quiet --collect --unit="craft-cloud-agent-run-$$" "$@" "$agent" run --log >/dev/null 2>&1; then + : + elif [ -n "${INVOCATION_ID:-}" ]; then + # In a unit of its own (this waiter), whatever it starts ends with it: said in the journal instead. + echo "Craft Cloud: the agent couldn'"'"'t be started again; run '"'"'craft-cloud-agent run --log'"'"' or log in again" >&2 + elif command -v setsid >/dev/null 2>&1; then + setsid "$agent" run --log /dev/null 2>&1 & else - "$3" run --log + nohup "$agent" run --log /dev/null 2>&1 & fi - ' sh "$1" "$2" "$3" "$data" >/dev/null 2>&1 & + ' + w_pid=$1 w_how=$2 w_agent=$3 w_comm=$4 + if command -v systemd-run >/dev/null 2>&1; then + set -- + eval "set -- \"\$@\" $(env_flags)" + if systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart "$@" /bin/sh -c "$waiter" sh "$w_pid" "$w_how" "$w_agent" "$data" "$w_comm" >/dev/null 2>&1; then + return 0 + fi + if command -v systemctl >/dev/null 2>&1 && systemctl --user --quiet is-active craft-cloud-agent-restart.service 2>/dev/null; then + return 0 + fi + fi + detach sh -c "$waiter" sh "$w_pid" "$w_how" "$w_agent" "$data" "$w_comm" } # The agent that ran and was stopped is started again (once): this computer keeps syncing. @@ -80,7 +144,7 @@ start_again() { systemctl --user stop craft-cloud-agent.service || true systemctl --user start craft-cloud-agent.service || true elif [ -x "$lib/craft-cloud-agent" ]; then - nohup "$lib/craft-cloud-agent" run --log >/dev/null 2>&1 & + detach "$lib/craft-cloud-agent" run --log fi } @@ -90,11 +154,11 @@ start_again() { give_up() { again=", and it starts again once done" if has_unit; then - start_after_exit "$pid" systemd "" + start_after_exit "$pid" systemd "" "$comm" elif [ -x "$lib/craft-cloud-agent" ]; then - start_after_exit "$pid" run "$lib/craft-cloud-agent" + start_after_exit "$pid" run "$lib/craft-cloud-agent" "$comm" elif command -v craft-cloud-agent >/dev/null 2>&1; then - start_after_exit "$pid" run "$(command -v craft-cloud-agent)" + start_after_exit "$pid" run "$(command -v craft-cloud-agent)" "$comm" else again=" (it stops once done; start it again with 'craft-cloud-agent run')" fi @@ -111,9 +175,13 @@ stopped=0 tries=0 # Read before the quit: the agent removes the file as it stops. pid=$(agent_pid) +comm=$(cat "/proc/$pid/comm" 2>/dev/null || true) while "$here/craft-cloud-agent" quit >/dev/null 2>&1; do was_running=1 - [ -n "$pid" ] || pid=$(agent_pid) + if [ -z "$pid" ]; then + pid=$(agent_pid) + comm=$(cat "/proc/$pid/comm" 2>/dev/null || true) + fi tries=$((tries + 1)) if [ "$tries" = 3 ]; then echo "Waiting for the Craft Cloud agent to finish its sync…" @@ -125,7 +193,7 @@ while "$here/craft-cloud-agent" quit >/dev/null 2>&1; do done if [ "$was_running" = 1 ]; then if [ -n "$pid" ]; then - while kill -0 "$pid" 2>/dev/null; do + while alive "$pid" "$comm"; do tries=$((tries + 1)) if [ "$tries" -ge "$wait_for" ]; then give_up @@ -139,8 +207,36 @@ if [ "$was_running" = 1 ]; then stopped=1 fi -# From here on the agent is stopped: whatever ends the script (a failed copy too) starts it again. -trap 'if [ "$stopped" = 1 ] && [ "$restarted" = 0 ]; then start_again; fi' EXIT +# From here on the agent is stopped and the app may be closed: whatever ends the script (a failed copy too) starts +# them again, once. +closed="" +finished=0 +finish() { + [ "$finished" = 0 ] || return 0 + finished=1 + if [ "$stopped" = 1 ] && [ "$restarted" = 0 ]; then + start_again + fi + # The tray needs the desktop session's bus: from elsewhere (ssh), it is left to the next login, and said. + case "$closed" in + *tray*) + if [ -n "${DBUS_SESSION_BUS_ADDRESS:-}" ] || [ -S "${XDG_RUNTIME_DIR:-/nonexistent}/bus" ]; then + if [ -x "$lib/craft-cloud" ]; then + detach "$lib/craft-cloud" --tray + fi + else + echo "The Craft Cloud tray was closed for the upgrade; run 'craft-cloud --tray' in your desktop session (or log in again) to get it back." >&2 + fi + ;; + esac +} +trap finish EXIT +# Ctrl-C and the like end the script through `exit`, so the EXIT trap runs (dash doesn't run it on a signal). +trap 'exit 130' INT TERM HUP + +# The app's window and tray process, an older version's too, are closed before the copy: an old tray would keep its +# icon next to the new one's. A tray that ran is started again at the end. +closed=$("$here/craft-cloud" --close 2>/dev/null || true) mkdir -p "$lib" "$bin" "$apps" "$icons" for name in craft-cloud craft-cloud-agent; do @@ -177,9 +273,7 @@ if [ -f "$unit" ] || [ -f "$entry" ]; then "$lib/craft-cloud-agent" autostart on >/dev/null || echo "Start at login still runs the old agent: run 'craft-cloud-agent autostart on'." >&2 fi -if [ "$stopped" = 1 ]; then - start_again -fi +finish if [ "$menu" = 1 ]; then echo "Craft Cloud is installed in $lib. Open it from the app menu, or run craft-cloud." diff --git a/xtask/src/package.rs b/xtask/src/package.rs index 89055ad..7dea318 100644 --- a/xtask/src/package.rs +++ b/xtask/src/package.rs @@ -258,6 +258,17 @@ mod tests { let _ = std::fs::remove_dir_all(&tmp); } + /// A running agent for install.sh: a `sleep` of `lasts` seconds that isn't this test's child (so it never lingers + /// as a zombie), named in `local-api.json` in `data`. Its pid. + #[cfg(target_os = "linux")] + fn agent_process(data: &Path, lasts: u32) -> u32 { + let out = Command::new("sh").args(["-c", &format!("sleep {lasts} >/dev/null 2>&1 & echo $!")]).output().unwrap(); + let pid = String::from_utf8_lossy(&out.stdout).trim().parse::().unwrap(); + std::fs::write(data.join("local-api.json"), format!("{{\n \"schema\": 1,\n \"port\": 1,\n \"token\": \"t\",\n \"pid\": {pid}\n}}")) + .unwrap(); + pid + } + /// An unpacked tarball in a temporary home, with an agent stand-in that answers `quit` `quits` times (it runs, /// finishing a pass) and then no more, a `systemctl` stand-in, and the agent's systemd unit: lptp1's upgrade. The /// running agent's process is a `sleep` of `lasts` seconds (none for 0), named in `local-api.json`; every call @@ -269,7 +280,7 @@ mod tests { src: PathBuf, calls: PathBuf, bin: PathBuf, - pid: Option, + pid: std::cell::Cell>, } #[cfg(target_os = "linux")] @@ -284,17 +295,7 @@ mod tests { std::fs::create_dir_all(dir).unwrap(); } // The agent's process: a `sleep` that isn't this test's child (so it never lingers as a zombie). - let pid = (lasts > 0).then(|| { - let out = Command::new("sh").args(["-c", &format!("sleep {lasts} >/dev/null 2>&1 & echo $!")]).output().unwrap(); - String::from_utf8_lossy(&out.stdout).trim().parse::().unwrap() - }); - if let Some(pid) = pid { - std::fs::write( - data.join("local-api.json"), - format!("{{\n \"schema\": 1,\n \"port\": 1,\n \"token\": \"t\",\n \"pid\": {pid}\n}}"), - ) - .unwrap(); - } + let pid = (lasts > 0).then(|| agent_process(&data, lasts)); let pid_text = pid.map(|p| p.to_string()).unwrap_or_default(); let note = format!("$(if [ -n \"{pid_text}\" ] && kill -0 {pid_text} 2>/dev/null; then echo ' [old agent running]'; fi)"); let calls = tmp.join("calls.txt"); @@ -306,18 +307,45 @@ mod tests { count = count.display() ); std::fs::write(src.join("craft-cloud-agent"), agent).unwrap(); - std::fs::write(src.join("craft-cloud"), "#!/bin/sh\nexit 0\n").unwrap(); + // The app: an (old) tray runs, which `--close` ends and says; a tray started stays (its pid in tray.pid). + let app = format!( + "#!/bin/sh\necho \"$0 $*\" >> \"{calls}\"\n[ \"$1\" = --close ] && echo tray\n\ + if [ \"$1\" = --tray ]; then echo $$ > \"{tray}\"; exec sleep 60; fi\nexit 0\n", + calls = calls.display(), + tray = tmp.join("tray.pid").display() + ); + std::fs::write(src.join("craft-cloud"), app).unwrap(); std::fs::write(src.join("craft-cloud.png"), "png").unwrap(); std::fs::write(src.join(LICENSES), "licences").unwrap(); std::fs::copy(crate::root().join("packaging/linux/install.sh"), src.join("install.sh")).unwrap(); - std::fs::write(bin.join("systemctl"), format!("#!/bin/sh\necho \"systemctl $*{note}\" >> \"{}\"\n", calls.display())).unwrap(); - for file in [src.join("craft-cloud-agent"), src.join("craft-cloud"), src.join("install.sh"), bin.join("systemctl")] { + // systemd-run: one unit of a name at a time (a file in `units/` while it runs; it records its options); + // systemctl's is-active says whether a unit runs. + let units = tmp.join("units"); + std::fs::create_dir_all(&units).unwrap(); + let systemctl = format!( + "#!/bin/sh\nif [ \"$3\" = is-active ]; then [ -e \"{units}/${{4%.service}}\" ]; exit; fi\necho \"systemctl $*{note}\" >> \"{calls}\"\n", + units = units.display(), + calls = calls.display() + ); + std::fs::write(bin.join("systemctl"), systemctl).unwrap(); + let systemd_run = format!( + "#!/bin/sh\nopts=\"\"\nunit=\"\"\nwhile [ \"${{1#--}}\" != \"$1\" ]; do\n opts=\"$opts $1\"\n case \"$1\" in --unit=*) unit=${{1#--unit=}} ;; esac\n shift\ndone\n\ + [ -e \"{units}/$unit\" ] && exit 1\ncase \"$unit\" in craft-cloud-agent-run-*) [ -e \"{units}/refuse-agent\" ] && exit 1 ;; esac\n\ + echo \"systemd-run$opts\" >> \"{calls}\"\ntouch \"{units}/$unit\"\n\ + (INVOCATION_ID=test \"$@\" 2>> \"{units}/journal\"; rm -f \"{units}/$unit\") >/dev/null &\n", + units = units.display(), + calls = calls.display() + ); + std::fs::write(bin.join("systemd-run"), systemd_run).unwrap(); + for file in + [src.join("craft-cloud-agent"), src.join("craft-cloud"), src.join("install.sh"), bin.join("systemctl"), bin.join("systemd-run")] + { std::fs::set_permissions(file, std::fs::Permissions::from_mode(0o755)).unwrap(); } let unit = home.join(".config/systemd/user/craft-cloud-agent.service"); std::fs::create_dir_all(unit.parent().unwrap()).unwrap(); std::fs::write(&unit, "[Service]\nExecStart=/home/fabio/.local/bin/craft-cloud-agent run\n").unwrap(); - Self { tmp, home, src, calls, bin, pid } + Self { tmp, home, src, calls, bin, pid: std::cell::Cell::new(pid) } } fn without_unit(self) -> Self { @@ -326,14 +354,34 @@ mod tests { } fn install(&self, wait: &str) -> std::process::Output { - Command::new("sh") - .arg(self.src.join("install.sh")) + self.install_with(wait, &[]) + } + + /// install.sh, in a desktop session (a session bus), with `vars` set too. + fn install_with(&self, wait: &str, vars: &[(&str, &str)]) -> std::process::Output { + self.install_in("sh", wait, vars) + } + + /// [`Self::install_with`] run by `shell` (`sh` is dash here; Arch's is bash). + fn install_in(&self, shell: &str, wait: &str, vars: &[(&str, &str)]) -> std::process::Output { + let mut sh = Command::new(shell); + sh.arg(self.src.join("install.sh")) .env_clear() .env("HOME", &self.home) .env("PATH", format!("{}:/usr/bin:/bin", self.bin.display())) - .env("CRAFT_CLOUD_INSTALL_WAIT", wait) - .output() - .unwrap() + .env("DBUS_SESSION_BUS_ADDRESS", "unix:path=/test-session-bus") + .env("CRAFT_CLOUD_INSTALL_WAIT", wait); + for (name, value) in vars { + sh.env(name, value); + } + sh.output().unwrap() + } + + /// A new agent runs (`lasts` seconds), busy with a pass: it answers one more `quit`. + fn agent_runs_again(&self, lasts: u32) { + let pid = agent_process(&self.home.join(".local/share/craft-cloud-agent"), lasts); + self.pid.set(Some(pid)); + let _ = std::fs::remove_file(self.tmp.join("quits.txt")); } fn calls(&self) -> Vec { @@ -342,7 +390,7 @@ mod tests { /// The old agent's process ends (its pass is done). fn agent_ends(&self) { - if let Some(pid) = self.pid { + if let Some(pid) = self.pid.get() { let _ = Command::new("kill").arg(pid.to_string()).status(); } } @@ -355,6 +403,18 @@ mod tests { self.home.join(".local/lib/craft-cloud/craft-cloud-agent").display().to_string() } + /// Waits (up to 20 s) for the calls to include `call`. + fn has(&self, call: &str) -> Vec { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + loop { + let calls = self.calls(); + if calls.iter().any(|c| c == call) || std::time::Instant::now() > deadline { + return calls; + } + std::thread::sleep(std::time::Duration::from_millis(100)); + } + } + /// Waits (up to 20 s) for the calls to end with `last`: the agent is started again in the background. fn ends_with(&self, last: &str) -> Vec { let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); @@ -372,6 +432,9 @@ mod tests { impl Drop for Upgrade { fn drop(&mut self) { self.agent_ends(); + if let Ok(pid) = std::fs::read_to_string(self.tmp.join("tray.pid")) { + let _ = Command::new("kill").arg(pid.trim()).status(); + } let _ = std::fs::remove_dir_all(&self.tmp); } } @@ -386,15 +449,22 @@ mod tests { let out = up.install("60"); assert!(out.status.success(), "{out:?}"); let quit = format!("{} quit [old agent running]", up.src_agent()); + let tray = format!("{} --tray", up.home.join(".local/lib/craft-cloud/craft-cloud").display()); let expected = [ quit.clone(), quit.clone(), quit, + format!("{} --close", up.src.join("craft-cloud").display()), format!("{} autostart on", up.lib_agent()), "systemctl --user stop craft-cloud-agent.service".to_owned(), "systemctl --user start craft-cloud-agent.service".to_owned(), + tray.clone(), ]; - assert_eq!(up.calls(), expected, "nothing after the quits until the old process had ended"); + assert_eq!( + up.ends_with(&tray), + expected, + "nothing after the quits until the old process had ended; the old tray closed first, the new one started last" + ); } /// The same without a systemd unit: the new copy is run once the old process has ended. @@ -405,8 +475,9 @@ mod tests { let out = up.install("60"); assert!(out.status.success(), "{out:?}"); let run = format!("{} run --log", up.lib_agent()); - let calls = up.ends_with(&run); - assert_eq!(calls.last(), Some(&run), "{calls:?}"); + // The agent and the tray both start in the background: either may come last. + let calls = up.has(&run); + assert!(calls.contains(&run), "{calls:?}"); assert!(calls.iter().filter(|c| c.ends_with("[old agent running]")).all(|c| c.ends_with("quit [old agent running]")), "{calls:?}"); } @@ -422,6 +493,145 @@ mod tests { assert!(!up.home.join(".local/lib/craft-cloud").exists(), "nothing installed"); std::thread::sleep(std::time::Duration::from_secs(2)); assert!(!up.calls().iter().any(|c| c.starts_with("systemctl")), "not while it runs: {:?}", up.calls()); + assert!(up.calls().iter().any(|c| c == "systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart"), "{:?}", up.calls()); + up.agent_ends(); + let start = "systemctl --user start craft-cloud-agent.service"; + assert_eq!(up.ends_with(start).last().map(String::as_str), Some(start)); + } + + /// Two installs that give up while the agent finishes a long pass: the second finds the first's waiter (a systemd + /// unit of a fixed name) and adds none, so the agent is started once. + #[cfg(target_os = "linux")] + #[test] + fn a_second_install_that_gives_up_adds_no_second_waiter() { + let up = Upgrade::new("timeout-twice", 1, 120); + for _ in 0..2 { + let _ = std::fs::remove_file(up.tmp.join("quits.txt")); + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "{out:?}"); + } + up.agent_ends(); + let start = "systemctl --user start craft-cloud-agent.service"; + up.ends_with(start); + // Time for a second start, if there were a second waiter. + std::thread::sleep(std::time::Duration::from_secs(2)); + let calls = up.calls(); + assert_eq!(calls.iter().filter(|c| c.starts_with("systemd-run")).count(), 1, "{calls:?}"); + assert_eq!(calls.iter().filter(|c| *c == start).count(), 1, "{calls:?}"); + } + + /// install.sh's output is a pipe (ssh, a script): it ends when the script does, though the tray it started still + /// runs. Nothing the script starts in the background holds its streams. Under bash too, which (unlike dash) keeps + /// a background `a && b &` list's subshell, and so its streams, for as long as `b` runs. + #[cfg(target_os = "linux")] + #[test] + fn the_scripts_output_ends_with_it_while_the_tray_runs_on() { + for shell in ["sh", "bash"] { + let up = Upgrade::new(&format!("pipe-{shell}"), 1, 1); + let started = std::time::Instant::now(); + let out = up.install_in(shell, "60", &[]); + assert!(out.status.success(), "{shell}: {out:?}"); + assert!(started.elapsed() < std::time::Duration::from_secs(20), "{shell}: the pipe stayed open: {:?}", started.elapsed()); + let tray = up.tmp.join("tray.pid"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while !tray.exists() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + let pid = std::fs::read_to_string(&tray).unwrap(); + assert!(Command::new("kill").args(["-0", pid.trim()]).status().unwrap().success(), "{shell}: the tray still runs"); + } + } + + /// Without a unit, the waiter starts the agent in a unit of its own and ends: once the agent is up no waiter is + /// left, so a later install that gives up adds one again, and the agent is started again a second time. + #[cfg(target_os = "linux")] + #[test] + fn after_a_restart_without_a_unit_a_later_give_up_waits_again() { + let up = Upgrade::new("rerun", 1, 120).without_unit(); + let lib = up.home.join(".local/lib/craft-cloud"); + std::fs::create_dir_all(&lib).unwrap(); + std::fs::copy(up.src.join("craft-cloud-agent"), lib.join("craft-cloud-agent")).unwrap(); + let run = format!("{} run --log", up.lib_agent()); + let waiter = "systemd-run --user --quiet --collect --unit=craft-cloud-agent-restart"; + for round in 1..=2 { + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "round {round}: {out:?}"); + up.agent_ends(); + up.ends_with(&run); + let calls = up.calls(); + assert_eq!(calls.iter().filter(|c| *c == waiter).count(), round, "a waiter each time: {calls:?}"); + assert_eq!(calls.iter().filter(|c| **c == run).count(), round, "{calls:?}"); + assert!(calls.iter().any(|c| c.starts_with("systemd-run --user --quiet --collect --unit=craft-cloud-agent-run-")), "{calls:?}"); + // The waiter's unit is gone (the agent runs in its own). + let unit = up.tmp.join("units/craft-cloud-agent-restart"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while unit.exists() && std::time::Instant::now() < deadline { + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert!(!unit.exists(), "round {round}: the waiter has ended"); + up.agent_runs_again(120); + } + } + + /// The agent's own unit can't be made: the waiter, in a unit of its own, starts nothing that would die with it, + /// and says so in the journal. + #[cfg(target_os = "linux")] + #[test] + fn a_waiter_that_cant_start_the_agents_unit_says_so() { + let up = Upgrade::new("refused", 1, 120).without_unit(); + let lib = up.home.join(".local/lib/craft-cloud"); + std::fs::create_dir_all(&lib).unwrap(); + std::fs::copy(up.src.join("craft-cloud-agent"), lib.join("craft-cloud-agent")).unwrap(); + let units = up.tmp.join("units"); + std::fs::write(units.join("refuse-agent"), "").unwrap(); + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "{out:?}"); + up.agent_ends(); + let journal = units.join("journal"); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + while !std::fs::read_to_string(&journal).unwrap_or_default().contains("couldn't be started again") { + assert!(std::time::Instant::now() < deadline, "nothing said: {:?}", up.calls()); + std::thread::sleep(std::time::Duration::from_millis(100)); + } + assert!(!up.calls().iter().any(|c| c.ends_with("run --log")), "{:?}", up.calls()); + } + + /// A unit doesn't get the installer's environment: the variables that pick the agent's folders are passed to the + /// waiter's unit and the agent's, those that are set and no others. + #[cfg(target_os = "linux")] + #[test] + fn the_folder_variables_go_to_the_units() { + let up = Upgrade::new("setenv", 1, 120).without_unit(); + let lib = up.home.join(".local/lib/craft-cloud"); + std::fs::create_dir_all(&lib).unwrap(); + std::fs::copy(up.src.join("craft-cloud-agent"), lib.join("craft-cloud-agent")).unwrap(); + let data = up.home.join(".local/share/craft-cloud-agent").display().to_string(); + let config = up.home.join("my config").display().to_string(); + let out = up.install_with("2", &[("CRAFT_CLOUD_AGENT_DIR", &data), ("XDG_CONFIG_HOME", &config)]); + assert_eq!(out.status.code(), Some(1), "{out:?}"); + up.agent_ends(); + let calls = up.ends_with(&format!("{} run --log", up.lib_agent())); + let flags = format!(" --setenv=CRAFT_CLOUD_AGENT_DIR={data} --setenv=XDG_CONFIG_HOME={config}"); + let units: Vec<&String> = calls.iter().filter(|c| c.starts_with("systemd-run")).collect(); + assert_eq!(units.len(), 2, "the waiter's and the agent's: {calls:?}"); + for unit in units { + assert!(unit.ends_with(&flags), "{unit}"); + assert!(!unit.contains("XDG_DATA_HOME") && !unit.contains("XDG_STATE_HOME"), "only those set: {unit}"); + } + } + + /// Without systemd-run the waiter runs in the background of this shell (nohup). + #[cfg(target_os = "linux")] + #[test] + fn without_systemd_run_the_waiter_still_starts_the_agent() { + let up = Upgrade::new("timeout-nosystemdrun", 1, 120); + std::fs::remove_file(up.bin.join("systemd-run")).unwrap(); + // The test's PATH has /usr/bin too, where a real systemd-run may be: a stand-in that isn't there. + std::fs::write(up.bin.join("systemd-run"), "#!/bin/sh\nexit 1\n").unwrap(); + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(up.bin.join("systemd-run"), std::fs::Permissions::from_mode(0o755)).unwrap(); + let out = up.install("2"); + assert_eq!(out.status.code(), Some(1), "{out:?}"); up.agent_ends(); let start = "systemctl --user start craft-cloud-agent.service"; assert_eq!(up.ends_with(start).last().map(String::as_str), Some(start));