working copy: materialize symlinks on Windows as regular files

I was a bit surprised to learn (or be reminded?) that checking out
symlinks on Windows leads to a panic. This patch fixes the crash by
materializing symlinks from the repo as regular files. It also updates
the snapshotting code so we preserve the symlink-ness of a path. The
user can update the symlink in the repo by updating the regular file
in the working copy. This seems to match Git's behavior on Windows
when symlinks are disabled.

CHANGELOG.md

@@ -17,6 +17,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed bugs
 
+* On Windows, symlinks in the repo are now materialized as regular files in the
+  working copy (instead of resulting in a crash).
+
 ## [0.14.0] - 2024-02-07
 
 ### Deprecations

lib/src/local_working_copy.rs

@@ -118,6 +118,7 @@ impl FileState {
         }
     }
 
+    #[cfg(unix)]
     fn for_symlink(metadata: &Metadata) -> Self {
         // When using fscrypt, the reported size is not the content size. So if
         // we were to record the content size here (like we do for regular files), we
@@ -681,18 +682,32 @@ impl TreeState {
         path: &RepoPath,
         disk_path: &Path,
     ) -> Result<SymlinkId, SnapshotError> {
-        let target = disk_path.read_link().map_err(|err| SnapshotError::Other {
-            message: format!("Failed to read symlink {}", disk_path.display()),
-            err: err.into(),
-        })?;
-        let str_target =
-            target
-                .to_str()
-                .ok_or_else(|| SnapshotError::InvalidUtf8SymlinkTarget {
+        #[cfg(unix)]
+        {
+            let target = disk_path.read_link().map_err(|err| SnapshotError::Other {
+                message: format!("Failed to read symlink {}", disk_path.display()),
+                err: err.into(),
+            })?;
+            let str_target =
+                target
+                    .to_str()
+                    .ok_or_else(|| SnapshotError::InvalidUtf8SymlinkTarget {
+                        path: disk_path.to_path_buf(),
+                    })?;
+            Ok(self.store.write_symlink(path, str_target)?)
+        }
+        #[cfg(windows)]
+        {
+            let target = fs::read(disk_path).map_err(|err| SnapshotError::Other {
+                message: format!("Failed to read file {}", disk_path.display()),
+                err: err.into(),
+            })?;
+            let string_target =
+                String::from_utf8(target).map_err(|_| SnapshotError::InvalidUtf8SymlinkTarget {
                     path: disk_path.to_path_buf(),
-                    target: target.clone(),
                 })?;
-        Ok(self.store.write_symlink(path, str_target)?)
+            Ok(self.store.write_symlink(path, &string_target)?)
+        }
     }
 
     fn reset_watchman(&mut self) {
@@ -1066,8 +1081,18 @@ impl TreeState {
         if clean {
             Ok(None)
         } else {
-            let new_file_type = new_file_state.file_type.clone();
             let current_tree_values = current_tree.path_value(repo_path);
+            let new_file_type = if cfg!(windows) {
+                let mut new_file_type = new_file_state.file_type.clone();
+                if matches!(new_file_type, FileType::Normal { .. })
+                    && matches!(current_tree_values.as_normal(), Some(TreeValue::Symlink(_)))
+                {
+                    new_file_type = FileType::Symlink;
+                }
+                new_file_type
+            } else {
+                new_file_state.file_type.clone()
+            };
             let new_tree_values = match new_file_type {
                 FileType::Normal { executable } => self.write_path_to_store(
                     repo_path,
@@ -1081,7 +1106,6 @@ impl TreeState {
                 }
                 FileType::GitSubmodule => panic!("git submodule cannot be written to store"),
             };
-
             if new_tree_values != current_tree_values {
                 Ok(Some(new_tree_values))
             } else {
@@ -1184,7 +1208,7 @@ impl TreeState {
     fn write_symlink(&self, disk_path: &Path, target: String) -> Result<FileState, CheckoutError> {
         #[cfg(windows)]
         {
-            println!("ignoring symlink at {}", disk_path.display());
+            self.write_file(disk_path, &mut target.as_bytes(), false)
         }
         #[cfg(unix)]
         {
@@ -1197,11 +1221,11 @@ impl TreeState {
                 ),
                 err: err.into(),
             })?;
+            let metadata = disk_path
+                .symlink_metadata()
+                .map_err(|err| checkout_error_for_stat_error(err, disk_path))?;
+            Ok(FileState::for_symlink(&metadata))
         }
-        let metadata = disk_path
-            .symlink_metadata()
-            .map_err(|err| checkout_error_for_stat_error(err, disk_path))?;
-        Ok(FileState::for_symlink(&metadata))
     }
 
     fn write_conflict(

lib/src/working_copy.rs

@@ -147,8 +147,6 @@ pub enum SnapshotError {
         /// The path of the symlink that has a target that's not valid UTF-8.
         /// This path itself is valid UTF-8.
         path: PathBuf,
-        /// The symlink target with invalid UTF-8.
-        target: PathBuf,
     },
     /// Reading or writing from the commit backend failed.
     #[error("Internal backend error")]

lib/tests/test_local_working_copy.rs

@@ -771,11 +771,7 @@ fn test_gitignores_ignored_directory_already_tracked() {
                     testutils::write_executable_file(&mut tree_builder, path, contents);
                 }
                 Kind::Symlink => {
-                    if cfg!(unix) {
-                        testutils::write_symlink(&mut tree_builder, path, contents);
-                    } else {
-                        testutils::write_normal_file(&mut tree_builder, path, contents);
-                    }
+                    testutils::write_symlink(&mut tree_builder, path, contents);
                 }
             }
         }