Fix self-update on windows (#712)

Fixed #702


* Add new dep windows v0.44.0 for windows only
* Add regression test for #702
* Impl `win::replace_file`
* Use `ReplaceFileW` on windows if `fs::rename` in `fs::atomic_install` fails
* Improve logging and err messages
* Add more regression tests
* Make `BinFile::install_link` atomic: Do not remove file if dst exists
* Fallback to `ReplaceFileW` in `fs::atomic_symlink_file`
* Refactor: Extract new fn `fs::persist`
* Use `fs::persist` instead of `TempFile::persist` in `fs::atomic_install`, which fallbacks to `ReplaceFileW` on windows

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Co-authored-by: Félix Saparelli <felix@passcod.name>

crates/binstalk/Cargo.toml

@@ -39,6 +39,9 @@ tracing = "0.1.37"
 url = { version = "2.3.1", features = ["serde"] }
 xz2 = "0.1.7"
 
+[target.'cfg(target_os = "windows")'.dependencies]
+windows = { version = "0.44.0", features = ["Win32_Storage_FileSystem", "Win32_Foundation"] }
+
 [features]
 default = ["static", "rustls"]
 

crates/binstalk/src/bins.rs

@@ -186,13 +186,6 @@ impl BinFile {
 
     pub fn install_link(&self) -> Result<(), BinstallError> {
         if let Some(link) = &self.link {
-            // Remove existing symlink
-            // TODO: check if existing symlink is correct
-            if link.exists() {
-                debug!("Remove link '{}'", link.display());
-                std::fs::remove_file(link)?;
-            }
-
             let dest = self.link_dest();
             debug!(
                 "Create link '{}' pointing to '{}'",

crates/binstalk/src/fs.rs

@@ -1,7 +1,7 @@
 use std::{fs, io, path::Path};
 
-use tempfile::NamedTempFile;
-use tracing::debug;
+use tempfile::{NamedTempFile, TempPath};
+use tracing::{debug, warn};
 
 /// Atomically install a file.
 ///
@@ -14,7 +14,21 @@ pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> {
     );
 
     if let Err(err) = fs::rename(src, dst) {
-        debug!("Attempting at atomic rename failed: {err:#?}, fallback to creating tempfile.");
+        warn!("Attempting at atomic rename failed: {err:#?}, fallback to other methods.");
+
+        #[cfg(target_os = "windows")]
+        {
+            match win::replace_file(src, dst) {
+                Ok(()) => {
+                    debug!("ReplaceFileW succeeded.",);
+                    return Ok(());
+                }
+                Err(err) => {
+                    warn!("ReplaceFileW failed: {err}, fallback to using tempfile plus rename",);
+                }
+            }
+        }
+
         // src and dst is not on the same filesystem/mountpoint.
         // Fallback to creating NamedTempFile on the parent dir of
         // dst.
@@ -41,12 +55,7 @@ pub fn atomic_install(src: &Path, dst: &Path) -> io::Result<()> {
         );
         tempfile.as_file().set_permissions(permissions)?;
 
-        debug!(
-            "Persisting '{}' to '{}'",
-            tempfile.path().display(),
-            dst.display()
-        );
-        tempfile.persist(dst).map_err(io::Error::from)?;
+        persist(tempfile.into_temp_path(), dst)?;
     } else {
         debug!("Attempting at atomically succeeded.");
     }
@@ -80,10 +89,62 @@ pub fn atomic_symlink_file(dest: &Path, link: &Path) -> io::Result<()> {
     );
     symlink_file(dest, &temp_path)?;
 
-    debug!(
-        "Persisting '{}' to '{}'",
-        temp_path.display(),
-        link.display()
-    );
-    temp_path.persist(link).map_err(io::Error::from)
+    persist(temp_path, link)
+}
+
+fn persist(temp_path: TempPath, to: &Path) -> io::Result<()> {
+    debug!("Persisting '{}' to '{}'", temp_path.display(), to.display());
+    match temp_path.persist(to) {
+        Ok(()) => Ok(()),
+        #[cfg(target_os = "windows")]
+        Err(tempfile::PathPersistError {
+            error,
+            path: temp_path,
+        }) => {
+            warn!(
+                "Failed to persist symlink '{}' to '{}': {error}, fallback to ReplaceFileW",
+                temp_path.display(),
+                to.display(),
+            );
+            win::replace_file(&temp_path, to).map_err(io::Error::from)
+        }
+        #[cfg(not(target_os = "windows"))]
+        Err(err) => Err(err.into()),
+    }
+}
+
+#[cfg(target_os = "windows")]
+mod win {
+    use std::{os::windows::ffi::OsStrExt, path::Path};
+
+    use windows::{
+        core::{Error, PCWSTR},
+        Win32::Storage::FileSystem::{ReplaceFileW, REPLACE_FILE_FLAGS},
+    };
+
+    pub(super) fn replace_file(src: &Path, dst: &Path) -> Result<(), Error> {
+        let mut src: Vec<_> = src.as_os_str().encode_wide().collect();
+        let mut dst: Vec<_> = dst.as_os_str().encode_wide().collect();
+
+        // Ensure it is terminated with 0
+        src.push(0);
+        dst.push(0);
+
+        // SAFETY: We use it according its doc
+        // https://learn.microsoft.com/en-nz/windows/win32/api/winbase/nf-winbase-replacefilew
+        //
+        // NOTE that this function is available since windows XP, so we don't need to
+        // lazily load this function.
+        unsafe {
+            ReplaceFileW(
+                PCWSTR::from_raw(dst.as_ptr()), // lpreplacedfilename
+                PCWSTR::from_raw(src.as_ptr()), // lpreplacementfilename
+                PCWSTR::null(),                 // lpbackupfilename, null for no backup file
+                REPLACE_FILE_FLAGS(0),          // dwreplaceflags
+                None,                           // lpexclude, unused
+                None,                           // lpreserved, unused
+            )
+        }
+        .ok()
+    }
 }