diff --git a/crates/engine/src/cmd/plugin.rs b/crates/engine/src/cmd/plugin.rs index a5b71d538..e8e57301e 100644 --- a/crates/engine/src/cmd/plugin.rs +++ b/crates/engine/src/cmd/plugin.rs @@ -1096,7 +1096,7 @@ pub fn specs() -> Vec { "Remove Plug-in", [], None, - "{id} uninstall a plug-in (its live effects then leave the geometry as it is) and delete the settings and cache it kept (when their folder can't be moved aside, or its grant can't be removed from the plug-in store, the command fails and the plug-in stays installed); for an id that isn't installed, deletes the settings and cache the plug-in store still keeps for it → {removed}", + "{id} uninstall a plug-in (its live effects then leave the geometry as it is) and delete the settings and cache it kept (when their folder can't be moved aside, or its grant can't be removed from the plug-in store, the command fails and the plug-in stays installed; when the settings can't be moved back either, the error names the folder they stay in, and the next load, install or removal of the plug-in moves them back); for an id that isn't installed, deletes the settings and cache the plug-in store still keeps for it → {removed}", always, remove ), diff --git a/crates/engine/src/store.rs b/crates/engine/src/store.rs index 01ebc4652..9a96f9621 100644 --- a/crates/engine/src/store.rs +++ b/crates/engine/src/store.rs @@ -190,6 +190,12 @@ fn lock_disabled(_: &Path) -> Option { thread_local! { /// Tests: [`Session::put_back_plugin`] fails on this thread, as on a full volume. pub(crate) static PUT_BACK_FAILS: std::cell::Cell = const { std::cell::Cell::new(false) }; + /// Tests: moving a plug-in's settings and cache back from `.removed` ([`move_back`]) + /// fails on this thread, as when another program holds a file in the folder. + pub(crate) static MOVE_BACK_FAILS: std::cell::Cell = const { std::cell::Cell::new(false) }; + /// Tests: writing the `.delete` file beside a folder about to be deleted ([`delete_aside`]) + /// fails on this thread, as on a full volume. + pub(crate) static MARKER_WRITE_FAILS: std::cell::Cell = const { std::cell::Cell::new(false) }; } /// Remove `dir/file` → whether it is gone; a file that isn't there is what was wanted. @@ -590,26 +596,34 @@ impl Session { } /// Keep the settings and cache of plug-in `id` in the attached plug-in store from now on; without - /// one, nowhere (its storage calls then fail as unavailable). + /// one, nowhere (its storage calls then fail as unavailable). Settings and cache that a removal + /// kept are moved back first ([`plugin_storage`]). pub(crate) fn attach_plugin_storage(&self, id: &str) { vectorcraft_plugins::registry::set_storage(id, plugin_storage(self.plugin_dir.as_deref(), id)); } - /// Does the attached plug-in store have a folder of settings and cache for plug-in `id`? A - /// folder behind a `storage` folder that is a link is not counted: the plug-in's storage calls - /// can't reach it and removing the plug-in leaves it. + /// Does the attached plug-in store have a folder of settings and cache for plug-in `id`, or one + /// that a removal kept ([`aside_paths`], [`is_kept`])? A folder behind a `storage` folder + /// that is a link is not counted: the plug-in's storage calls can't reach it and removing the + /// plug-in leaves it. pub(crate) fn has_plugin_storage(&self, id: &str) -> bool { - self.plugin_dir - .as_deref() - .is_some_and(|dir| !is_link(&dir.join(PLUGIN_DATA_DIR)) && std::fs::symlink_metadata(plugin_data_dir(dir, id)).is_ok()) + self.plugin_dir.as_deref().is_some_and(|dir| { + let path = plugin_data_dir(dir, id); + !is_link(&dir.join(PLUGIN_DATA_DIR)) && (std::fs::symlink_metadata(&path).is_ok() || aside_paths(&path).iter().any(|a| is_kept(a))) + }) } /// Delete the settings and cache of plug-in `id` from the store, and stop keeping any for it. - /// The folder is first renamed out of the way (`.removed`), and that fails, leaving it - /// as it was, when the rename does; what the deletion then leaves stays under that name, with a - /// log record. A later call for the id tries to delete it again. `forget` runs once the folder is - /// out of the way, before anything is deleted: when it returns `false`, the folder is renamed - /// back and this fails. + /// Settings and cache that an earlier removal kept ([`is_kept`]) are first moved back when the + /// folder is missing ([`restore_removed`]); what earlier deletions left is deleted. The folder is + /// then renamed out of the way ([`aside_paths`]: `.removed`, or `.removing` while + /// `.removed` holds kept settings), and that fails, leaving it as it was, when the rename + /// does. `forget` runs once the folder is out of the way, before anything is deleted: when it + /// returns `false`, the folder is renamed back and this fails, and the kept settings stay. When + /// that rename fails too, the error also names both paths and the reason, and the next attach or + /// removal for the id moves the folder back. Once `forget` returns `true`, the folder and the + /// kept settings are deleted ([`delete_aside`]); what that deletion leaves stays, with a log + /// record, and a later call for the id tries to delete it again. pub(crate) fn remove_plugin_storage(&self, id: &str, forget: impl FnOnce() -> bool) -> Result<(), StorageError> { let not_forgotten = || StorageError::Io(GRANT_NOT_REMOVED.to_string()); vectorcraft_plugins::registry::set_storage(id, None); @@ -636,28 +650,45 @@ impl Session { close(); return Ok(()); } - let mut aside = path.clone().into_os_string(); - aside.push(".removed"); - let aside = PathBuf::from(aside); - let _ = std::fs::remove_dir_all(&aside); - match std::fs::rename(&path, &aside) { + let asides = aside_paths(&path); + if std::fs::symlink_metadata(&path).is_err() + && let Err(e) = restore_removed(&path) + { + log::warn!("can't move the settings and cache of {} back: {e}", path.display()); + } + for aside in asides.iter().filter(|a| !is_kept(a)) { + delete_aside(aside); + } + let target = if std::fs::symlink_metadata(&path).is_err() { + asides.iter().rev().find(|a| is_kept(a)).unwrap_or(&asides[0]) + } else if !is_kept(&asides[0]) { + &asides[0] + } else if std::fs::symlink_metadata(&asides[1]).is_err() { + &asides[1] + } else { + return Err(StorageError::Io(format!("{}: {} and {} are in the way", path.display(), asides[0].display(), asides[1].display()))); + }; + match std::fs::rename(&path, target) { Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} Err(e) => return Err(StorageError::Io(format!("{}: {e}", path.display()))), Ok(()) => {} } if !forget() { - if std::fs::symlink_metadata(&aside).is_ok() - && let Err(e) = std::fs::rename(&aside, &path) + if std::fs::symlink_metadata(target).is_ok() + && let Err(e) = move_back(target, &path) { - log::warn!("can't move {} back: {e}", aside.display()); + log::warn!("can't move {} back: {e}", target.display()); + return Err(StorageError::Io(format!( + "{GRANT_NOT_REMOVED}, and its settings and cache can't be moved back from {} to {}: {e}", + target.display(), + path.display() + ))); } return Err(not_forgotten()); } close(); - if std::fs::symlink_metadata(&aside).is_ok() - && let Err(e) = std::fs::remove_dir_all(&aside) - { - log::warn!("can't delete {}: {e}", aside.display()); + for aside in &asides { + delete_aside(aside); } Ok(()) } @@ -688,10 +719,107 @@ pub(crate) fn plugin_data_dir(dir: &Path, id: &str) -> PathBuf { dir.join(PLUGIN_DATA_DIR).join(format!("id-{}", case_safe(id))) } -/// Where plug-in `id` keeps its settings and cache in the plug-in store `dir`. +/// `path` with `suffix` added to its last part. +fn suffixed(path: &Path, suffix: &str) -> PathBuf { + let mut named = path.as_os_str().to_os_string(); + named.push(suffix); + PathBuf::from(named) +} + +/// Where [`Session::remove_plugin_storage`] moves the plug-in folder `path` out of the way: +/// `.removed`, and `.removing` while `.removed` holds settings and cache an +/// earlier removal kept ([`is_kept`]). The second one, when it is there, is the newer. +fn aside_paths(path: &Path) -> [PathBuf; 2] { + [suffixed(path, ".removed"), suffixed(path, ".removing")] +} + +/// `