From 0899c2ee36a714573b223ae85114fb7284fc661e Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Jul 2024 21:57:34 +0200 Subject: [PATCH 1/4] feat!: on Windows, also instruct msys to create real symlinks (#1443) This will only reliably work on with developer setups, but that seems fair to assume. If this causes problems, it's fine to make it opt-in as well. --- tests/tools/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index 95b61596106..4365ab3cb8d 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -590,6 +590,8 @@ fn configure_command<'a>( script_result_directory: &Path, ) -> &'a mut std::process::Command { let never_path = if cfg!(windows) { "-" } else { ":" }; + let mut msys_for_git_bash_on_windows = std::env::var("MSYS").unwrap_or_default(); + msys_for_git_bash_on_windows.push_str(" winsymlinks:nativestrict"); cmd.args(args) .stdout(std::process::Stdio::piped()) .stderr(std::process::Stdio::piped()) @@ -597,6 +599,7 @@ fn configure_command<'a>( .env_remove("GIT_DIR") .env_remove("GIT_ASKPASS") .env_remove("SSH_ASKPASS") + .env("MSYS", msys_for_git_bash_on_windows) .env("GIT_CONFIG_SYSTEM", never_path) .env("GIT_CONFIG_GLOBAL", never_path) .env("GIT_TERMINAL_PROMPT", "false") From 93e088a619db0d4b81e444922f375de65c94a317 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Jul 2024 10:04:11 +0200 Subject: [PATCH 2/4] fix gix-archive tests for when symlinks are allowed --- gix-archive/tests/archive.rs | 50 ++++++++---------------------------- tests/tools/src/lib.rs | 1 - 2 files changed, 10 insertions(+), 41 deletions(-) diff --git a/gix-archive/tests/archive.rs b/gix-archive/tests/archive.rs index 2fb6f364dcc..1a0b1cf895a 100644 --- a/gix-archive/tests/archive.rs +++ b/gix-archive/tests/archive.rs @@ -17,7 +17,7 @@ mod from_tree { #[test] fn basic_usage_internal() -> gix_testtools::Result { basic_usage(gix_archive::Format::InternalTransientNonPersistable, |buf| { - assert_eq!(buf.len(), if cfg!(windows) { 565 } else { 551 }); + assert_eq!(buf.len(), 551); let mut stream = gix_worktree_stream::Stream::from_read(std::io::Cursor::new(buf)); let mut paths_and_modes = Vec::new(); @@ -27,11 +27,7 @@ mod from_tree { entry.read_to_end(&mut buf).expect("stream can always be read"); } - let expected_link_mode = if cfg!(windows) { - EntryKind::Blob - } else { - EntryKind::Link - }; + let expected_link_mode = EntryKind::Link; let expected_exe_mode = if cfg!(windows) { EntryKind::Blob } else { @@ -53,11 +49,7 @@ mod from_tree { ( "symlink-to-a".into(), expected_link_mode, - hex_to_id(if cfg!(windows) { - "45b983be36b73c0788dc9cbcb76cbb80fc7bb057" - } else { - "2e65efe2a145dda7ee51d1741299f848e5bf752e" - }) + hex_to_id("2e65efe2a145dda7ee51d1741299f848e5bf752e") ), ( "dir/b".into(), @@ -119,34 +111,20 @@ mod from_tree { header.mode()?, )); } - let expected_symlink_type = if cfg!(windows) { - EntryType::Regular - } else { - EntryType::Symlink - }; + let expected_symlink_type = EntryType::Symlink; let expected_exe_mode = if cfg!(windows) { 420 } else { 493 }; assert_eq!( out, [ ("prefix/.gitattributes", EntryType::Regular, 56, 420), ("prefix/a", EntryType::Regular, 3, 420), - ( - "prefix/symlink-to-a", - expected_symlink_type, - if cfg!(windows) { 3 } else { 0 }, - 420 - ), + ("prefix/symlink-to-a", expected_symlink_type, 0, 420), ("prefix/dir/b", EntryType::Regular, 3, 420), ("prefix/dir/subdir/exe", EntryType::Regular, 0, expected_exe_mode), ("prefix/extra-file", EntryType::Regular, 21, 420), ("prefix/extra-exe", EntryType::Regular, 0, expected_exe_mode), ("prefix/extra-dir-empty", EntryType::Directory, 0, 420), - ( - "prefix/extra-dir/symlink-to-extra", - expected_symlink_type, - if cfg!(windows) { 21 } else { 0 }, - 420 - ) + ("prefix/extra-dir/symlink-to-extra", expected_symlink_type, 0, 420) ] .into_iter() .map(|(path, b, c, d)| (bstr::BStr::new(path).to_owned(), b, c, d)) @@ -183,7 +161,7 @@ mod from_tree { }, |buf| { assert!( - buf.len() < 1270, + buf.len() < 1280, "much bigger than uncompressed for some reason (565): {} < 1270", buf.len() ); @@ -208,19 +186,11 @@ mod from_tree { ); let mut link = ar.by_name("prefix/symlink-to-a")?; assert!(!link.is_dir()); - assert_eq!( - link.is_symlink(), - cfg!(not(windows)), - "symlinks are supported as well, but only on Unix" - ); - assert_eq!( - link.unix_mode(), - Some(if cfg!(windows) { 0o100644 } else { 0o120644 }), - "the mode specifies what it should be" - ); + assert!(link.is_symlink(), "symlinks are supported as well, but only on Unix"); + assert_eq!(link.unix_mode(), Some(0o120644), "the mode specifies what it should be"); let mut buf = Vec::new(); link.read_to_end(&mut buf)?; - assert_eq!(buf.as_bstr(), if cfg!(windows) { "hi\n" } else { "a" }); + assert_eq!(buf.as_bstr(), "a"); Ok(()) }, ) diff --git a/tests/tools/src/lib.rs b/tests/tools/src/lib.rs index 4365ab3cb8d..411d7c0b127 100644 --- a/tests/tools/src/lib.rs +++ b/tests/tools/src/lib.rs @@ -754,7 +754,6 @@ fn extract_archive( } #[cfg(not(feature = "xz"))] { - use std::io::Read; input_archive.read_to_end(&mut buf)?; } buf From b3973e4bd5e0ea1bf40a5b12ef44647021392d55 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Jul 2024 20:16:45 +0200 Subject: [PATCH 3/4] add more standard traits to `Change` type. Namely: Debug, PartialEq, Eq --- gix-index/src/entry/mode.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/gix-index/src/entry/mode.rs b/gix-index/src/entry/mode.rs index dc3a9a6de94..001b81f9505 100644 --- a/gix-index/src/entry/mode.rs +++ b/gix-index/src/entry/mode.rs @@ -74,6 +74,7 @@ impl From for Mode { } /// A change of a [`Mode`]. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum Change { /// The type of mode changed, like symlink => file. Type { From f48ed0c123db7ac6e1b03d012a6eec963873393b Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Fri, 5 Jul 2024 11:09:25 +0200 Subject: [PATCH 4/4] Allow tests to work with symlinks on Windows --- gix-status/src/index_as_worktree/function.rs | 14 ++++++++++--- gix-status/tests/stack/mod.rs | 14 ++----------- gix-status/tests/status/index_as_worktree.rs | 22 ++------------------ gix-worktree-state/src/checkout/entry.rs | 16 ++++++++++---- gix-worktree-stream/tests/stream.rs | 12 ++--------- 5 files changed, 29 insertions(+), 49 deletions(-) diff --git a/gix-status/src/index_as_worktree/function.rs b/gix-status/src/index_as_worktree/function.rs index cf2e8ea9e51..ac617eb82d1 100644 --- a/gix-status/src/index_as_worktree/function.rs +++ b/gix-status/src/index_as_worktree/function.rs @@ -424,12 +424,19 @@ impl<'index> State<'_, 'index> { self.buf.clear(); self.buf2.clear(); + let file_size_bytes = if cfg!(windows) && metadata.is_symlink() { + // symlinks on Windows seem to have a length of zero, so just pretend + // they have the correct length to avoid short-cutting, and enforce a full buffer check. + entry.stat.size as u64 + } else { + metadata.len() + }; let fetch_data = ReadDataImpl { buf: &mut self.buf, path: worktree_path, rela_path, entry, - file_len: metadata.len(), + file_len: file_size_bytes, filter: &mut self.filter, attr_stack: &mut self.attr_stack, options: self.options, @@ -440,7 +447,7 @@ impl<'index> State<'_, 'index> { odb_reads: self.odb_reads, odb_bytes: self.odb_bytes, }; - let content_change = diff.compare_blobs(entry, metadata.len(), fetch_data, &mut self.buf2)?; + let content_change = diff.compare_blobs(entry, file_size_bytes, fetch_data, &mut self.buf2)?; // This file is racy clean! Set the size to 0 so we keep detecting this as the file is updated. if content_change.is_some() || executable_bit_changed { let set_entry_stat_size_zero = content_change.is_some() && racy_clean; @@ -531,7 +538,8 @@ where let out = if is_symlink && self.options.fs.symlink { // conversion to bstr can never fail because symlinks are only used // on unix (by git) so no reason to use the try version here - let symlink_path = gix_path::into_bstr(std::fs::read_link(self.path)?); + let symlink_path = + gix_path::to_unix_separators_on_windows(gix_path::into_bstr(std::fs::read_link(self.path)?)); self.buf.extend_from_slice(&symlink_path); self.worktree_bytes.fetch_add(self.buf.len() as u64, Ordering::Relaxed); Stream { diff --git a/gix-status/tests/stack/mod.rs b/gix-status/tests/stack/mod.rs index edc4d52028a..362f21261a3 100644 --- a/gix-status/tests/stack/mod.rs +++ b/gix-status/tests/stack/mod.rs @@ -83,21 +83,11 @@ fn paths_leading_through_symlinks_are_rejected() { } fn is_symlink(m: &std::fs::Metadata) -> bool { - if cfg!(windows) { - // On windows, symlinks can't be seen, at least not through std - m.is_file() - } else { - m.is_symlink() - } + m.is_symlink() } fn is_symlinked_dir(m: &std::fs::Metadata) -> bool { - if cfg!(windows) { - // On windows, symlinks can't be seen, at least not through std - m.is_dir() - } else { - m.is_symlink() - } + m.is_symlink() } fn is_file(m: &std::fs::Metadata) -> bool { m.is_file() diff --git a/gix-status/tests/status/index_as_worktree.rs b/gix-status/tests/status/index_as_worktree.rs index e485f0759c7..b9ad8e30b3d 100644 --- a/gix-status/tests/status/index_as_worktree.rs +++ b/gix-status/tests/status/index_as_worktree.rs @@ -469,16 +469,7 @@ fn refresh() { } .into(), ), - ( - BStr::new("empty"), - 3, - Change::Modification { - executable_bit_changed: false, - content_change: Some(()), - set_entry_stat_size_zero: false - } - .into(), - ), + (BStr::new("empty"), 3, Change::Type.into()), ( BStr::new("executable"), 4, @@ -551,16 +542,7 @@ fn modified() { } .into(), ), - ( - BStr::new("empty"), - 3, - Change::Modification { - executable_bit_changed: false, - content_change: Some(()), - set_entry_stat_size_zero: false, - } - .into(), - ), + (BStr::new("empty"), 3, Change::Type.into()), ( BStr::new("executable"), 4, diff --git a/gix-worktree-state/src/checkout/entry.rs b/gix-worktree-state/src/checkout/entry.rs index b08563c60c1..f2dc35b19ab 100644 --- a/gix-worktree-state/src/checkout/entry.rs +++ b/gix-worktree-state/src/checkout/entry.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::{ fs::OpenOptions, io::Write, @@ -145,12 +146,19 @@ where err, path: dest.to_path_buf(), })?; - let symlink_destination = gix_path::try_from_byte_slice(obj.data) - .map_err(|_| crate::checkout::Error::IllformedUtf8 { path: obj.data.into() })?; - if symlink { + #[cfg_attr(not(windows), allow(unused_mut))] + let mut symlink_destination = Cow::Borrowed( + gix_path::try_from_byte_slice(obj.data) + .map_err(|_| crate::checkout::Error::IllformedUtf8 { path: obj.data.into() })?, + ); + #[cfg(windows)] + { + symlink_destination = gix_path::to_native_path_on_windows(gix_path::into_bstr(symlink_destination)) + } + try_op_or_unlink(dest, overwrite_existing, |p| { - gix_fs::symlink::create(symlink_destination, p) + gix_fs::symlink::create(symlink_destination.as_ref(), p) })?; } else { let mut file = try_op_or_unlink(dest, overwrite_existing, |p| { diff --git a/gix-worktree-stream/tests/stream.rs b/gix-worktree-stream/tests/stream.rs index 56a211d5600..a34e640a1a2 100644 --- a/gix-worktree-stream/tests/stream.rs +++ b/gix-worktree-stream/tests/stream.rs @@ -115,11 +115,7 @@ mod from_tree { } else { EntryKind::BlobExecutable }; - let expected_link_mode = if cfg!(windows) { - EntryKind::Blob - } else { - EntryKind::Link - }; + let expected_link_mode = EntryKind::Link; assert_eq!( paths_and_modes, &[ @@ -141,11 +137,7 @@ mod from_tree { ( "symlink-to-a".into(), expected_link_mode, - hex_to_id(if cfg!(windows) { - "45b983be36b73c0788dc9cbcb76cbb80fc7bb057" - } else { - "2e65efe2a145dda7ee51d1741299f848e5bf752e" - }) + hex_to_id("2e65efe2a145dda7ee51d1741299f848e5bf752e") ), ( "dir/.gitattributes".into(),