ok/jj
1
0
Fork 0
forked from mirrors/jj

working_copy: implement symlinks on windows with a helper function

enables symlink tests on windows, ignoring failures due to disabled developer mode,
and updates windows.md
This commit is contained in:
Thomas Castiglione 2024-02-04 16:03:03 +08:00 committed by gulbanana
parent fed682a430
commit d661f59f9d
8 changed files with 119 additions and 48 deletions

View file

@ -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.

11
Cargo.lock generated
View file

@ -1745,6 +1745,7 @@ dependencies = [
"version_check",
"watchman_client",
"whoami",
"winreg",
"zstd",
]
@ -3545,6 +3546,16 @@ dependencies = [
"memchr",
]
[[package]]
name = "winreg"
version = "0.52.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a277a57398d4bfa075df44f501a17cfdf8542d224f0d36095a2adc7aee4ef0a5"
dependencies = [
"cfg-if",
"windows-sys 0.48.0",
]
[[package]]
name = "yaml-rust"
version = "0.4.5"

View file

@ -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

View file

@ -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`.

View file

@ -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 }

View file

@ -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;

View file

@ -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:?}");

View file

@ -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());