Merge pull request #3619 from Byron/no-tempfiles-on-windows

sync-all on Windows

crates/gitbutler-core/src/fs.rs

@@ -1,8 +1,11 @@
+use std::io::Write;
 use std::path::{Path, PathBuf};
 
 use anyhow::Result;
 use bstr::BString;
 use gix::dir::walk::EmissionMode;
+use gix::tempfile::create_dir::Retries;
+use gix::tempfile::{AutoRemove, ContainingDirectory};
 use walkdir::WalkDir;
 
 // Returns an ordered list of relative paths for files inside a directory recursively.
@@ -48,3 +51,51 @@ pub fn iter_worktree_files(
         .filter_map(Result::ok)
         .map(|e| e.entry.rela_path))
 }
+
+/// Write a single file so that the write either fully succeeds, or fully fails,
+/// assuming the containing directory already exists.
+pub(crate) fn write<P: AsRef<Path>>(
+    file_path: P,
+    contents: impl AsRef<[u8]>,
+) -> anyhow::Result<()> {
+    let mut temp_file = gix::tempfile::new(
+        file_path.as_ref().parent().unwrap(),
+        ContainingDirectory::Exists,
+        AutoRemove::Tempfile,
+    )?;
+    temp_file.write_all(contents.as_ref())?;
+    Ok(persist_tempfile(temp_file, file_path)?)
+}
+
+/// Write a single file so that the write either fully succeeds, or fully fails,
+/// and create all leading directories.
+pub(crate) fn create_dirs_then_write<P: AsRef<Path>>(
+    file_path: P,
+    contents: impl AsRef<[u8]>,
+) -> std::io::Result<()> {
+    let mut temp_file = gix::tempfile::new(
+        file_path.as_ref().parent().unwrap(),
+        ContainingDirectory::CreateAllRaceProof(Retries::default()),
+        AutoRemove::Tempfile,
+    )?;
+    temp_file.write_all(contents.as_ref())?;
+    persist_tempfile(temp_file, file_path)
+}
+
+fn persist_tempfile(
+    tempfile: gix::tempfile::Handle<gix::tempfile::handle::Writable>,
+    to_path: impl AsRef<Path>,
+) -> std::io::Result<()> {
+    match tempfile.persist(to_path) {
+        Ok(Some(_opened_file)) => {
+            // EXPERIMENT: Does this fix #3601?
+            #[cfg(windows)]
+            _opened_file.sync_all()?;
+            Ok(())
+        }
+        Ok(None) => unreachable!(
+            "BUG: a signal has caused the tempfile to be removed, but we didn't install a handler"
+        ),
+        Err(err) => Err(err.error),
+    }
+}

crates/gitbutler-core/src/snapshots/reflog.rs

@@ -1,8 +1,7 @@
-use crate::storage;
+use crate::fs::write;
 use anyhow::Result;
-use gix::tempfile::{AutoRemove, ContainingDirectory};
 use itertools::Itertools;
-use std::{io::Write, path::PathBuf};
+use std::path::PathBuf;
 
 use crate::projects::Project;
 
@@ -64,7 +63,7 @@ fn set_target_ref(file_path: &PathBuf, sha: &str) -> Result<()> {
     let binding = first_line.join(" ");
     lines[0] = &binding;
     let content = format!("{}\n", lines.join("\n"));
-    write(file_path, &content)
+    write(file_path, content)
 }
 
 fn set_oplog_ref(file_path: &PathBuf, sha: &str) -> Result<()> {
@@ -83,17 +82,5 @@ fn set_oplog_ref(file_path: &PathBuf, sha: &str) -> Result<()> {
     let second_line = [target_ref, sha, &the_rest].join(" ");
 
     let content = format!("{}\n", [first_line, &second_line].join("\n"));
-    write(file_path, &content)
-}
-
-fn write(file_path: &PathBuf, content: &str) -> Result<()> {
-    let mut temp_file = gix::tempfile::new(
-        file_path.parent().unwrap(),
-        ContainingDirectory::Exists,
-        AutoRemove::Tempfile,
-    )?;
-    temp_file.write_all(content.as_bytes())?;
-    storage::persist_tempfile(temp_file, file_path)?;
-
-    Ok(())
+    write(file_path, content)
 }

crates/gitbutler-core/src/snapshots/state.rs

@@ -1,9 +1,7 @@
-use crate::storage;
 use anyhow::Result;
-use gix::tempfile::{AutoRemove, ContainingDirectory};
 use std::{
     fs::File,
-    io::{Read, Write},
+    io::Read,
     path::{Path, PathBuf},
 };
 
@@ -71,11 +69,5 @@ impl OplogHandle {
 
 fn write<P: AsRef<Path>>(file_path: P, oplog: &Oplog) -> anyhow::Result<()> {
     let contents = toml::to_string(&oplog)?;
-    let mut temp_file = gix::tempfile::new(
-        file_path.as_ref().parent().unwrap(),
-        ContainingDirectory::Exists,
-        AutoRemove::Tempfile,
-    )?;
-    temp_file.write_all(contents.as_bytes())?;
-    Ok(storage::persist_tempfile(temp_file, file_path)?)
+    crate::fs::write(file_path, contents)
 }

crates/gitbutler-core/src/storage.rs

@@ -1,6 +1,3 @@
-use gix::tempfile::create_dir::Retries;
-use gix::tempfile::{AutoRemove, ContainingDirectory};
-use std::io::Write;
 use std::{
     fs,
     path::{Path, PathBuf},
@@ -46,15 +43,7 @@ impl Storage {
     /// Generally, the filesystem is used for synchronization, not in-memory primitives.
     pub fn write(&self, rela_path: impl AsRef<Path>, content: &str) -> std::io::Result<()> {
         let file_path = self.local_data_dir.join(rela_path);
-        let dir = file_path.parent().unwrap();
-        // NOTE: This creates a 0o600 files on Unix by default.
-        let mut tempfile = gix::tempfile::new(
-            dir,
-            ContainingDirectory::CreateAllRaceProof(Retries::default()),
-            AutoRemove::Tempfile,
-        )?;
-        tempfile.write_all(content.as_bytes())?;
-        persist_tempfile(tempfile, file_path)
+        crate::fs::create_dirs_then_write(file_path, content)
     }
 
     /// Delete the file or directory at `rela_path`.
@@ -80,16 +69,3 @@ impl Storage {
         Ok(())
     }
 }
-
-pub(crate) fn persist_tempfile(
-    tempfile: gix::tempfile::Handle<gix::tempfile::handle::Writable>,
-    to_path: impl AsRef<Path>,
-) -> std::io::Result<()> {
-    match tempfile.persist(to_path) {
-        Ok(Some(_opened_file)) => Ok(()),
-        Ok(None) => unreachable!(
-            "BUG: a signal has caused the tempfile to be removed, but we didn't install a handler"
-        ),
-        Err(err) => Err(err.error),
-    }
-}

crates/gitbutler-core/src/virtual_branches/state.rs

@@ -1,12 +1,10 @@
-use gix::tempfile::{AutoRemove, ContainingDirectory};
 use std::{
     collections::HashMap,
     fs::File,
-    io::{Read, Write},
+    io::Read,
     path::{Path, PathBuf},
 };
 
-use crate::storage;
 use serde::{Deserialize, Serialize};
 
 use super::{target::Target, Branch};
@@ -152,12 +150,5 @@ impl VirtualBranchesHandle {
 }
 
 fn write<P: AsRef<Path>>(file_path: P, virtual_branches: &VirtualBranches) -> anyhow::Result<()> {
-    let contents = toml::to_string(&virtual_branches)?;
-    let mut temp_file = gix::tempfile::new(
-        file_path.as_ref().parent().unwrap(),
-        ContainingDirectory::Exists,
-        AutoRemove::Tempfile,
-    )?;
-    temp_file.write_all(contents.as_bytes())?;
-    Ok(storage::persist_tempfile(temp_file, file_path)?)
+    crate::fs::write(file_path, toml::to_string(&virtual_branches)?)
 }