diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index 136ba51ce..7f5f33e34 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/crates/engine/src/cmd/recovery.rs @@ -316,6 +316,20 @@ impl FolderStore { } } + /// The outcome `r` of removing file `f`. A file that is not there counts as removed, unless a + /// file is on the way to it: Windows reports that as a missing file, and it is named as for a + /// listing. + pub(crate) fn removed(f: &std::path::Path, r: std::io::Result<()>) -> std::result::Result<(), String> { + match r { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => match f.parent().and_then(Self::in_the_way) { + Some((p, true)) => Err(format!("{} is not a folder: {e}", p.display())), + _ => Ok(()), + }, + Err(e) => Err(Self::file_error(f, &e)), + Ok(()) => Ok(()), + } + } + /// Refuse area folder `dir` unless it is a folder itself, not a link to one, and (on Unix) /// belongs to this account. Listing areas leaves links out, so a name another account had /// linked to a folder of its own in a shared recovery folder looked free, and claiming it @@ -453,10 +467,7 @@ impl RecoveryStore for FolderStore { } fn remove(&self, name: &str) -> std::result::Result<(), String> { let f = self.file(name)?; - match std::fs::remove_file(&f) { - Err(e) if e.kind() != std::io::ErrorKind::NotFound => Err(Self::file_error(&f, &e)), - _ => Ok(()), - } + Self::removed(&f, std::fs::remove_file(&f)) } fn location(&self) -> String { self.0.to_string_lossy().into_owned() diff --git a/crates/engine/src/tests/recovery.rs b/crates/engine/src/tests/recovery.rs index 2b991e332..0ba51f20a 100644 --- a/crates/engine/src/tests/recovery.rs +++ b/crates/engine/src/tests/recovery.rs @@ -514,6 +514,24 @@ fn folder_errors_name_a_file_in_the_way() { assert!(wrong.is_empty(), "{wrong:?}"); } +/// A removal that finds the copy missing because a file is on the way to it (how Windows reports a +/// file in the way) names that file. A copy that is just not there still counts as removed. +#[test] +fn a_removal_reported_missing_names_a_file_in_the_way() { + let base = std::env::temp_dir().join(format!("vc-recovery-remove-in-way-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + std::fs::create_dir_all(&base).unwrap(); + let file = base.join("file"); + std::fs::write(&file, b"").unwrap(); + let missing = || std::io::Error::from(std::io::ErrorKind::NotFound); + let behind = FolderStore::removed(&file.join("recovery").join("1-1").join("Untitled-1-1.vectorcraft"), Err(missing())); + let gone = FolderStore::removed(&base.join("recovery").join("1-1").join("Untitled-1-1.vectorcraft"), Err(missing())); + let _ = std::fs::remove_dir_all(&base); + let named = format!("{} is not a folder: ", file.display()); + assert!(behind.as_ref().is_err_and(|e| e.starts_with(&named)), "{behind:?}"); + assert_eq!(gone, Ok(())); +} + /// A folder of the store's path that is missing and can't be made: a write or a lock names it as /// the folder it couldn't make, and a read names it as missing. An area folder that can't be made /// in a store folder that is there is named the same way.