Bug 506009 - Crash reporter client: support profiles on different volumes than the home directory r=gsvelto

Rather than renaming files (which must be on the same
volume/filesystem), we copy and delete them. This is less efficient,
however the files in question are fairly small, so there's probably no
need to rename _when we can_ as an optimization.

Differential Revision: https://phabricator.services.mozilla.com/D217727

toolkit/crashreporter/client/app/src/config.rs

@@ -237,12 +237,22 @@ impl Config {
 
         let move_file = |from: &Path| -> anyhow::Result<PathBuf> {
             let to = pending_crashes_dir.join(from.file_name().unwrap());
-            std::fs::rename(from, &to).with_context(|| {
-                self.build_string("crashreporter-error-moving-path")
-                    .arg("from", from.display().to_string())
-                    .arg("to", to.display().to_string())
-                    .get()
-            })?;
+            // Try to rename, but copy and remove if it fails. `rename` won't work across
+            // mount points. (bug 506009)
+            if let Err(e) = std::fs::rename(from, &to) {
+                log::warn!("failed to move {} to {}: {e}", from.display(), to.display());
+                log::info!("trying to copy and remove instead");
+
+                std::fs::copy(from, &to).with_context(|| {
+                    self.build_string("crashreporter-error-moving-path")
+                        .arg("from", from.display().to_string())
+                        .arg("to", to.display().to_string())
+                        .get()
+                })?;
+                if let Err(e) = std::fs::remove_file(from) {
+                    log::warn!("failed to remove {}: {e}", from.display());
+                }
+            }
             Ok(to)
         };
 

toolkit/crashreporter/client/app/src/std/fs.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use crate::std::mock::{mock_key, MockKey};
+use crate::std::mock::{mock_key, try_hook, MockKey};
 use std::collections::HashMap;
 use std::ffi::OsString;
 use std::io::{ErrorKind, Read, Result, Seek, SeekFrom, Write};
@@ -26,6 +26,10 @@ impl MockFileContent {
     pub fn new_bytes(data: Vec<u8>) -> Self {
         MockFileContent(Arc::new(Mutex::new(data)))
     }
+
+    pub fn make_copy(&self) -> Self {
+        Self::new_bytes(self.0.lock().unwrap().clone())
+    }
 }
 
 impl From<()> for MockFileContent {
@@ -444,6 +448,11 @@ pub fn create_dir_all<P: AsRef<Path>>(path: P) -> Result<()> {
 }
 
 pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<()> {
+    if try_hook(false, "rename_fail") {
+        log::debug!("intentionally failing std::fs::rename");
+        return Err(ErrorKind::Unsupported.into());
+    }
+
     MockFS.get(move |files| {
         let from_name = from.as_ref().file_name().expect("invalid path");
         let item = files
@@ -467,6 +476,36 @@ pub fn rename<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<()> {
     })
 }
 
+pub fn copy<P: AsRef<Path>, Q: AsRef<Path>>(from: P, to: Q) -> Result<()> {
+    MockFS.get(move |files| {
+        let from_name = from.as_ref().file_name().expect("invalid path");
+        let item = files
+            .parent_dir(from.as_ref(), move |d| {
+                d.get(from_name)
+                    .ok_or(ErrorKind::NotFound.into())
+                    .and_then(|e| match &e.content {
+                        MockFSContent::Dir(_) => Err(ErrorKind::NotFound.into()),
+                        MockFSContent::File(f) => f
+                            .as_ref()
+                            .map(|f| MockFSItem {
+                                content: MockFSContent::File(Ok(f.make_copy())),
+                                modified: e.modified.clone(),
+                            })
+                            .map_err(|e| e.kind().into()),
+                    })
+            })
+            .and_then(|r| r)?;
+
+        let to_name = to.as_ref().file_name().expect("invalid path");
+        files
+            .parent_dir(to.as_ref(), move |d| {
+                d.insert(to_name.to_owned(), item);
+                Ok(())
+            })
+            .and_then(|r| r)
+    })
+}
+
 pub fn remove_file<P: AsRef<Path>>(path: P) -> Result<()> {
     MockFS.get(move |files| {
         let name = path.as_ref().file_name().expect("invalid path");

toolkit/crashreporter/client/app/src/test.rs

@@ -1100,6 +1100,19 @@ fn response_stop_sending_reports() {
         .check_exists("data_dir/EndOfLife100.0");
 }
 
+#[test]
+fn rename_failure_uses_copy() {
+    let mut test = GuiTest::new();
+    test.mock.set(mock::MockHook::new("rename_fail"), true);
+    test.run(|interact| {
+        interact.element("quit", |_style, b: &model::Button| b.click.fire(&()));
+    });
+    test.assert_files()
+        .saved_settings(Settings::default())
+        .submitted()
+        .pending();
+}
+
 /// A real temporary directory in the host filesystem.
 ///
 /// The directory is guaranteed to be unique to the test suite process (in case of crash, it can be