Fix writing hardlinks (#11340)

* don't use backup files with hardlinks

* check if the inodes remain the same in the test

* move funcs to faccess and use AsRawHandle

* use a copy as a backup for hardlinks

* delete backup after copy

Cargo.lock

@@ -1455,6 +1455,7 @@ dependencies = [
  "once_cell",
  "open",
  "pulldown-cmark",
+ "same-file",
  "serde",
  "serde_json",
  "signal-hook",

helix-stdx/src/faccess.rs

@@ -74,6 +74,11 @@ mod imp {
 
         Ok(())
     }
+
+    pub fn hardlink_count(p: &Path) -> std::io::Result<u64> {
+        let metadata = p.metadata()?;
+        Ok(metadata.nlink())
+    }
 }
 
 // Licensed under MIT from faccess except for `chown`, `copy_metadata` and `is_acl_inherited`
@@ -94,8 +99,8 @@ mod imp {
         SID_IDENTIFIER_AUTHORITY, TOKEN_DUPLICATE, TOKEN_QUERY,
     };
     use windows_sys::Win32::Storage::FileSystem::{
-        FILE_ACCESS_RIGHTS, FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ,
+        GetFileInformationByHandle, BY_HANDLE_FILE_INFORMATION, FILE_ACCESS_RIGHTS,
-        FILE_GENERIC_WRITE,
+        FILE_ALL_ACCESS, FILE_GENERIC_EXECUTE, FILE_GENERIC_READ, FILE_GENERIC_WRITE,
     };
     use windows_sys::Win32::System::Threading::{GetCurrentThread, OpenThreadToken};
 
@@ -103,7 +108,7 @@ mod imp {
 
     use std::ffi::c_void;
 
-    use std::os::windows::{ffi::OsStrExt, fs::OpenOptionsExt};
+    use std::os::windows::{ffi::OsStrExt, fs::OpenOptionsExt, io::AsRawHandle};
 
     struct SecurityDescriptor {
         sd: PSECURITY_DESCRIPTOR,
@@ -411,6 +416,18 @@ mod imp {
 
         Ok(())
     }
+
+    pub fn hardlink_count(p: &Path) -> std::io::Result<u64> {
+        let file = std::fs::File::open(p)?;
+        let handle = file.as_raw_handle() as isize;
+        let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { std::mem::zeroed() };
+
+        if unsafe { GetFileInformationByHandle(handle, &mut info) } == 0 {
+            Err(std::io::Error::last_os_error())
+        } else {
+            Ok(info.nNumberOfLinks as u64)
+        }
+    }
 }
 
 // Licensed under MIT from faccess except for `copy_metadata`
@@ -457,3 +474,7 @@ pub fn readonly(p: &Path) -> bool {
 pub fn copy_metadata(from: &Path, to: &Path) -> io::Result<()> {
     imp::copy_metadata(from, to)
 }
+
+pub fn hardlink_count(p: &Path) -> io::Result<u64> {
+    imp::hardlink_count(p)
+}

helix-term/Cargo.toml

@@ -86,3 +86,4 @@ helix-loader = { path = "../helix-loader" }
 smallvec = "1.13"
 indoc = "2.0.5"
 tempfile = "3.10.1"
+same-file = "1.0.1"

helix-term/tests/test/commands/write.rs

@@ -649,6 +649,40 @@ async fn test_symlink_write_relative() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_hardlink_write() -> anyhow::Result<()> {
+    let dir = tempfile::tempdir()?;
+
+    let mut file = tempfile::NamedTempFile::new_in(&dir)?;
+    let hardlink_path = dir.path().join("linked");
+    std::fs::hard_link(file.path(), &hardlink_path)?;
+
+    let mut app = helpers::AppBuilder::new()
+        .with_file(&hardlink_path, None)
+        .build()?;
+
+    test_key_sequence(
+        &mut app,
+        Some("ithe gostak distims the doshes<ret><esc>:w<ret>"),
+        None,
+        false,
+    )
+    .await?;
+
+    reload_file(&mut file).unwrap();
+    let mut file_content = String::new();
+    file.as_file_mut().read_to_string(&mut file_content)?;
+
+    assert_eq!(
+        LineFeedHandling::Native.apply("the gostak distims the doshes"),
+        file_content
+    );
+    assert!(helix_stdx::faccess::hardlink_count(&hardlink_path)? > 1);
+    assert!(same_file::is_same_file(file.path(), &hardlink_path)?);
+
+    Ok(())
+}
+
 async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> {
     let mut file = tempfile::NamedTempFile::new()?;
 

helix-view/src/document.rs

@@ -934,6 +934,9 @@ impl Document {
                     "Path is read only"
                 ));
             }
+
+            // Assume it is a hardlink to prevent data loss if the metadata cant be read (e.g. on certain Windows configurations)
+            let is_hardlink = helix_stdx::faccess::hardlink_count(&write_path).unwrap_or(2) > 1;
             let backup = if path.exists() {
                 let path_ = write_path.clone();
                 // hacks: we use tempfile to handle the complex task of creating
@@ -942,14 +945,22 @@ impl Document {
                 // since the path doesn't exist yet, we just want
                 // the path
                 tokio::task::spawn_blocking(move || -> Option<PathBuf> {
-                    tempfile::Builder::new()
+                    let mut builder = tempfile::Builder::new();
-                        .prefix(path_.file_name()?)
+                    builder.prefix(path_.file_name()?).suffix(".bck");
-                        .suffix(".bck")
+
+                    let backup_path = if is_hardlink {
+                        builder
+                            .make_in(path_.parent()?, |backup| std::fs::copy(&path_, backup))
+                            .ok()?
+                            .into_temp_path()
+                    } else {
+                        builder
                             .make_in(path_.parent()?, |backup| std::fs::rename(&path_, backup))
                             .ok()?
                             .into_temp_path()
-                        .keep()
+                    };
-                        .ok()
+
+                    backup_path.keep().ok()
                 })
                 .await
                 .ok()
@@ -972,7 +983,23 @@ impl Document {
             };
 
             if let Some(backup) = backup {
+                if is_hardlink {
+                    let mut delete = true;
                     if write_result.is_err() {
+                        // Restore backup
+                        let _ = tokio::fs::copy(&backup, &write_path).await.map_err(|e| {
+                            delete = false;
+                            log::error!("Failed to restore backup on write failure: {e}")
+                        });
+                    }
+
+                    if delete {
+                        // Delete backup
+                        let _ = tokio::fs::remove_file(backup)
+                            .await
+                            .map_err(|e| log::error!("Failed to remove backup file on write: {e}"));
+                    }
+                } else if write_result.is_err() {
                     // restore backup
                     let _ = tokio::fs::rename(&backup, &write_path)
                         .await