working_copy: implement symlinks on Windows with a helper function

CHANGELOG.md

@@ -90,9 +90,11 @@ 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
+* On Windows, symlinks in the repo are now supported when Developer Mode is enabled.
+  When symlink support is unavailable, they will be materialized as regular files in the
   working copy (instead of resulting in a crash).
-
+  [#2](https://github.com/martinvonz/jj/issues/2)
+
 * On Windows, the `:builtin` pager is now used by default, rather than being
   disabled entirely.
 

Cargo.toml

@@ -60,7 +60,7 @@ insta = { version = "1.35.1", features = ["filters"] }
 itertools = "0.12.1"
 libc = { version = "0.2.153" }
 maplit = "1.0.2"
-minus = { version = "5.5.0", features = [ "dynamic_output", "search" ] }
+minus = { version = "5.5.0", features = ["dynamic_output", "search"] }
 num_cpus = "1.16.0"
 once_cell = "1.19.0"
 ouroboros = "0.18.0"
@@ -109,6 +109,7 @@ unicode-width = "0.1.11"
 version_check = "0.9.4"
 watchman_client = { version = "0.8.0" }
 whoami = "1.4.1"
+winreg = "0.52"
 zstd = "0.12.4"
 
 # put all inter-workspace libraries, i.e. those that use 'path = ...' here in

cli/tests/common/mod.rs

@@ -114,27 +114,29 @@ impl TestEnvironment {
             cmd.env("SSL_CERT_FILE", "/dev/null");
         }
 
-        if cfg!(all(windows, target_env = "gnu")) {
-            // MinGW executables cannot run without `mingw\bin` in the PATH (which we're
-            // clearing above), so we add it again here.
-            if let Ok(path_var) = std::env::var("PATH").or_else(|_| std::env::var("Path")) {
-                // There can be slight variations of this path (e.g. `mingw64\bin`) so we're
-                // intentionally being lenient here.
-                let mingw_directories = path_var
-                    .split(';')
-                    .filter(|dir| dir.contains("mingw"))
-                    .join(";");
-
-                if !mingw_directories.is_empty() {
-                    cmd.env("PATH", mingw_directories);
-                }
-            }
-
-            // MinGW uses `TEMP` to create temporary directories, which we need for some
+        if cfg!(windows) {
+            // Windows uses `TEMP` to create temporary directories, which we need for some
             // tests.
             if let Ok(tmp_var) = std::env::var("TEMP") {
                 cmd.env("TEMP", tmp_var);
             }
+
+            if cfg!(target_env = "gnu") {
+                // MinGW executables cannot run without `mingw\bin` in the PATH (which we're
+                // clearing above), so we add it again here.
+                if let Ok(path_var) = std::env::var("PATH").or_else(|_| std::env::var("Path")) {
+                    // There can be slight variations of this path (e.g. `mingw64\bin`) so we're
+                    // intentionally being lenient here.
+                    let mingw_directories = path_var
+                        .split(';')
+                        .filter(|dir| dir.contains("mingw"))
+                        .join(";");
+
+                    if !mingw_directories.is_empty() {
+                        cmd.env("PATH", mingw_directories);
+                    }
+                }
+            }
         }
 
         cmd

docs/windows.md

@@ -83,3 +83,13 @@ Then only use the `~/my-repo` workspace from Linux.
 
 [issue-2040]: https://github.com/martinvonz/jj/issues/2040
 [array-op]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_arrays?view=powershell-7.4#the-array-sub-expression-operator
+
+## Symbolic link support
+
+`jj` supports symlinks on Windows only when they are enabled by the operating 
+system. This requires Windows 10 version 14972 or higher, as well as Developer
+Mode. If those conditions are not satisfied, `jj` will materialize symlinks as
+ordinary files.
+
+For colocated repositories, Git support must also be enabled using the
+`git config` option `core.symlinks=true`.

lib/Cargo.toml

@@ -65,6 +65,9 @@ zstd = { workspace = true }
 [target.'cfg(unix)'.dependencies]
 rustix = { workspace = true }
 
+[target.'cfg(windows)'.dependencies]
+winreg = { workspace = true }
+
 [dev-dependencies]
 assert_matches = { workspace = true }
 criterion = { workspace = true }

lib/src/file_util.rs

@@ -21,6 +21,8 @@ use std::{io, iter};
 use tempfile::{NamedTempFile, PersistError};
 use thiserror::Error;
 
+pub use self::platform::*;
+
 #[derive(Debug, Error)]
 #[error("Cannot access {path}")]
 pub struct PathError {
@@ -146,6 +148,52 @@ pub fn persist_content_addressed_temp_file<P: AsRef<Path>>(
     }
 }
 
+#[cfg(unix)]
+mod platform {
+    use std::io;
+    use std::os::unix::fs::symlink;
+    use std::path::Path;
+
+    /// Symlinks are always available on UNIX
+    pub fn check_symlink_support() -> io::Result<bool> {
+        Ok(true)
+    }
+
+    pub fn try_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
+        symlink(original, link)
+    }
+}
+
+#[cfg(windows)]
+mod platform {
+    use std::io;
+    use std::os::windows::fs::symlink_file;
+    use std::path::Path;
+
+    use winreg::enums::HKEY_LOCAL_MACHINE;
+    use winreg::RegKey;
+
+    /// Symlinks may or may not be enabled on Windows. They require the
+    /// Developer Mode setting, which is stored in the registry key below.
+    pub fn check_symlink_support() -> io::Result<bool> {
+        let hklm = RegKey::predef(HKEY_LOCAL_MACHINE);
+        let sideloading =
+            hklm.open_subkey("SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\AppModelUnlock")?;
+        let developer_mode: u32 = sideloading.get_value("AllowDevelopmentWithoutDevLicense")?;
+        Ok(developer_mode == 1)
+    }
+
+    pub fn try_symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) -> io::Result<()> {
+        // this will create a nonfunctional link for directories, but at the moment
+        // we don't have enough information in the tree to determine whether the
+        // symlink target is a file or a directory
+        // note: if developer mode is not enabled the error code will be 1314,
+        // ERROR_PRIVILEGE_NOT_HELD
+
+        symlink_file(original, link)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::io::Write;

lib/src/local_working_copy.rs

@@ -21,8 +21,6 @@ use std::fs::{File, Metadata, OpenOptions};
 use std::io::{Read, Write};
 use std::ops::Range;
 #[cfg(unix)]
-use std::os::unix::fs::symlink;
-#[cfg(unix)]
 use std::os::unix::fs::PermissionsExt;
 use std::path::{Path, PathBuf};
 use std::sync::mpsc::{channel, Sender};
@@ -46,6 +44,7 @@ use crate::backend::{
 };
 use crate::commit::Commit;
 use crate::conflicts::{self, materialize_tree_value, MaterializedTreeValue};
+use crate::file_util::{check_symlink_support, try_symlink};
 #[cfg(feature = "watchman")]
 use crate::fsmonitor::watchman;
 use crate::fsmonitor::FsmonitorKind;
@@ -118,7 +117,6 @@ 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
@@ -294,6 +292,7 @@ pub struct TreeState {
     // Currently only path prefixes
     sparse_patterns: Vec<RepoPathBuf>,
     own_mtime: MillisSinceEpoch,
+    symlink_support: bool,
 
     /// The most recent clock value returned by Watchman. Will only be set if
     /// the repo is configured to use the Watchman filesystem monitor and
@@ -549,6 +548,7 @@ impl TreeState {
             file_states: FileStatesMap::new(),
             sparse_patterns: vec![RepoPathBuf::root()],
             own_mtime: MillisSinceEpoch(0),
+            symlink_support: check_symlink_support().unwrap_or(false),
             watchman_clock: None,
         }
     }
@@ -682,8 +682,7 @@ impl TreeState {
         path: &RepoPath,
         disk_path: &Path,
     ) -> Result<SymlinkId, SnapshotError> {
-        #[cfg(unix)]
-        {
+        if self.symlink_support {
             let target = disk_path.read_link().map_err(|err| SnapshotError::Other {
                 message: format!("Failed to read symlink {}", disk_path.display()),
                 err: err.into(),
@@ -695,9 +694,7 @@ impl TreeState {
                         path: disk_path.to_path_buf(),
                     })?;
             Ok(self.store.write_symlink(path, str_target)?)
-        }
-        #[cfg(windows)]
-        {
+        } else {
             let target = fs::read(disk_path).map_err(|err| SnapshotError::Other {
                 message: format!("Failed to read file {}", disk_path.display()),
                 err: err.into(),
@@ -1082,7 +1079,7 @@ impl TreeState {
             Ok(None)
         } else {
             let current_tree_values = current_tree.path_value(repo_path);
-            let new_file_type = if cfg!(windows) {
+            let new_file_type = if !self.symlink_support {
                 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(_)))
@@ -1204,28 +1201,20 @@ impl TreeState {
         Ok(FileState::for_file(executable, size, &metadata))
     }
 
-    #[cfg_attr(windows, allow(unused_variables))]
     fn write_symlink(&self, disk_path: &Path, target: String) -> Result<FileState, CheckoutError> {
-        #[cfg(windows)]
-        {
-            self.write_file(disk_path, &mut target.as_bytes(), false)
-        }
-        #[cfg(unix)]
-        {
-            let target = PathBuf::from(&target);
-            symlink(&target, disk_path).map_err(|err| CheckoutError::Other {
-                message: format!(
-                    "Failed to create symlink from {} to {}",
-                    disk_path.display(),
-                    target.display()
-                ),
-                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 target = PathBuf::from(&target);
+        try_symlink(&target, disk_path).map_err(|err| CheckoutError::Other {
+            message: format!(
+                "Failed to create symlink from {} to {}",
+                disk_path.display(),
+                target.display()
+            ),
+            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))
     }
 
     fn write_conflict(
@@ -1388,7 +1377,11 @@ impl TreeState {
                     ..
                 } => self.write_file(&disk_path, &mut reader, executable)?,
                 MaterializedTreeValue::Symlink { id: _, target } => {
-                    self.write_symlink(&disk_path, target)?
+                    if self.symlink_support {
+                        self.write_symlink(&disk_path, target)?
+                    } else {
+                        self.write_file(&disk_path, &mut target.as_bytes(), false)?
+                    }
                 }
                 MaterializedTreeValue::GitSubmodule(_) => {
                     println!("ignoring git submodule at {path:?}");

lib/tests/test_local_working_copy.rs

@@ -21,6 +21,7 @@ use std::sync::Arc;
 
 use itertools::Itertools;
 use jj_lib::backend::{MergedTreeId, TreeId, TreeValue};
+use jj_lib::file_util::{check_symlink_support, try_symlink};
 use jj_lib::fsmonitor::FsmonitorKind;
 use jj_lib::local_working_copy::LocalWorkingCopy;
 use jj_lib::merge::Merge;
@@ -80,7 +81,6 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) {
         // Executable, but same content as Normal, to test transition where only the bit changed
         ExecutableNormalContent,
         Conflict,
-        #[cfg_attr(windows, allow(dead_code))]
         Symlink,
         Tree,
         GitSubmodule,
@@ -170,7 +170,6 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) {
         Kind::Conflict,
         Kind::Tree,
     ];
-    #[cfg(unix)]
     kinds.push(Kind::Symlink);
     if backend == TestRepoBackend::Git {
         kinds.push(Kind::GitSubmodule);
@@ -244,10 +243,12 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) {
             Kind::Symlink => {
                 assert!(maybe_metadata.is_ok(), "{path:?} should exist");
                 let metadata = maybe_metadata.unwrap();
-                assert!(
-                    metadata.file_type().is_symlink(),
-                    "{path:?} should be a symlink"
-                );
+                if check_symlink_support().unwrap_or(false) {
+                    assert!(
+                        metadata.file_type().is_symlink(),
+                        "{path:?} should be a symlink"
+                    );
+                }
             }
             Kind::Tree => {
                 assert!(maybe_metadata.is_ok(), "{path:?} should exist");
@@ -803,13 +804,11 @@ fn test_gitignores_ignored_directory_already_tracked() {
     )
     .unwrap();
     std::fs::remove_file(deleted_executable_path.to_fs_path(&workspace_root)).unwrap();
-    if cfg!(unix) {
-        let fs_path = modified_symlink_path.to_fs_path(&workspace_root);
-        std::fs::remove_file(&fs_path).unwrap();
-        #[cfg(unix)]
-        std::os::unix::fs::symlink("modified", &fs_path).unwrap();
+    let fs_path = modified_symlink_path.to_fs_path(&workspace_root);
+    std::fs::remove_file(&fs_path).unwrap();
+    if check_symlink_support().unwrap_or(false) {
+        try_symlink("modified", &fs_path).unwrap();
     } else {
-        let fs_path = modified_symlink_path.to_fs_path(&workspace_root);
         std::fs::write(fs_path, "modified").unwrap();
     }
     std::fs::remove_file(deleted_symlink_path.to_fs_path(&workspace_root)).unwrap();
@@ -923,7 +922,6 @@ fn test_gitsubmodule() {
     );
 }
 
-#[cfg(unix)]
 #[test]
 fn test_existing_directory_symlink() {
     let settings = testutils::user_settings();
@@ -933,7 +931,12 @@ fn test_existing_directory_symlink() {
 
     // Creates a symlink in working directory, and a tree that will add a file under
     // the symlinked directory.
-    std::os::unix::fs::symlink("..", workspace_root.join("parent")).unwrap();
+    if check_symlink_support().unwrap_or(false) {
+        try_symlink("..", workspace_root.join("parent")).unwrap();
+    } else {
+        return;
+    }
+
     let file_path = RepoPath::from_internal_string("parent/escaped");
     let tree = create_tree(repo, &[(file_path, "contents")]);
     let commit = commit_with_tree(repo.store(), tree.id());