From e1842133bb65f0fff8caea8230200377893165ad Mon Sep 17 00:00:00 2001 From: caomengxuan666 <2507560089@qq.com> Date: Tue, 9 Jun 2026 18:46:40 +0800 Subject: [PATCH 1/2] Preserve Windows directory symlinks in cp --- src/uu/cp/src/cp.rs | 30 +++++++++++++++++++++++++++--- tests/by-util/test_cp.rs | 1 + 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 6fd2abe9ef9..d2c848a5331 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -13,6 +13,8 @@ use std::fs::{self, Metadata, OpenOptions, Permissions}; use std::os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt}; #[cfg(unix)] use std::os::unix::net::UnixListener; +#[cfg(windows)] +use std::os::windows::fs::MetadataExt; use std::path::{Path, PathBuf, StripPrefixError}; use std::{fmt, io}; #[cfg(all(unix, not(target_os = "android")))] @@ -1934,6 +1936,8 @@ pub(crate) fn copy_attributes( fn symlink_file( source: &Path, dest: &Path, + #[cfg(windows)] source_type_hint: Option<&Path>, + #[cfg(not(windows))] _source_type_hint: Option<&Path>, #[cfg(not(target_os = "wasi"))] symlinked_files: &mut HashSet, #[cfg(target_os = "wasi")] _symlinked_files: &mut HashSet, ) -> CopyResult<()> { @@ -1959,7 +1963,13 @@ fn symlink_file( } #[cfg(windows)] { - std::os::windows::fs::symlink_file(source, dest).map_err(|e| { + let create_symlink = if source_type_hint.is_some_and(source_is_directory_for_symlink) { + std::os::windows::fs::symlink_dir + } else { + std::os::windows::fs::symlink_file + }; + + create_symlink(source, dest).map_err(|e| { CpError::IoErrContext( e, translate!("cp-error-cannot-create-symlink", @@ -1977,6 +1987,20 @@ fn symlink_file( } } +#[cfg(windows)] +fn source_is_directory_for_symlink(source: &Path) -> bool { + const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10; + + source.symlink_metadata().is_ok_and(|metadata| { + let file_type = metadata.file_type(); + if file_type.is_symlink() { + metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY != 0 + } else { + file_type.is_dir() + } + }) +} + fn context_for(src: &Path, dest: &Path) -> String { format!("{} -> {}", src.quote(), dest.quote()) } @@ -2322,7 +2346,7 @@ fn handle_copy_mode( if dest.exists() && options.overwrite == OverwriteMode::Clobber(ClobberMode::Force) { fs::remove_file(dest)?; } - symlink_file(source, dest, symlinked_files)?; + symlink_file(source, dest, Some(source), symlinked_files)?; } CopyMode::Update => { if dest.exists() { @@ -2886,7 +2910,7 @@ fn copy_link( if dest.is_symlink() || dest.is_file() { delete_path(dest, options)?; } - symlink_file(&link, dest, symlinked_files)?; + symlink_file(&link, dest, Some(source), symlinked_files)?; copy_attributes( source, dest, diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 1b3fde67814..bebbf85a16c 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2874,6 +2874,7 @@ fn test_copy_dir_symlink() { at.symlink_dir("dir", "dir-link"); ucmd.args(&["-r", "dir-link", "copy"]).succeeds(); assert_eq!(at.resolve_link("copy"), "dir"); + assert!(at.plus("copy").is_dir()); } #[test] From d33d0cd43b7459622418161a0a572c8710d90cbe Mon Sep 17 00:00:00 2001 From: caomengxuan666 <2507560089@qq.com> Date: Wed, 10 Jun 2026 02:57:56 +0800 Subject: [PATCH 2/2] Address Windows symlink cp review feedback --- src/uu/cp/src/cp.rs | 77 ++++++++++++++++++++++++++++++++-------- tests/by-util/test_cp.rs | 10 ++++++ 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index d2c848a5331..124eb15465e 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -1936,8 +1936,7 @@ pub(crate) fn copy_attributes( fn symlink_file( source: &Path, dest: &Path, - #[cfg(windows)] source_type_hint: Option<&Path>, - #[cfg(not(windows))] _source_type_hint: Option<&Path>, + #[cfg(windows)] source_metadata: &Metadata, #[cfg(not(target_os = "wasi"))] symlinked_files: &mut HashSet, #[cfg(target_os = "wasi")] _symlinked_files: &mut HashSet, ) -> CopyResult<()> { @@ -1963,7 +1962,7 @@ fn symlink_file( } #[cfg(windows)] { - let create_symlink = if source_type_hint.is_some_and(source_is_directory_for_symlink) { + let create_symlink = if source_is_directory_for_symlink(source_metadata) { std::os::windows::fs::symlink_dir } else { std::os::windows::fs::symlink_file @@ -1987,18 +1986,12 @@ fn symlink_file( } } +/// Returns true when `source_metadata` should be recreated as a directory symlink. #[cfg(windows)] -fn source_is_directory_for_symlink(source: &Path) -> bool { +fn source_is_directory_for_symlink(source_metadata: &Metadata) -> bool { const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10; - source.symlink_metadata().is_ok_and(|metadata| { - let file_type = metadata.file_type(); - if file_type.is_symlink() { - metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY != 0 - } else { - file_type.is_dir() - } - }) + source_metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY != 0 } fn context_for(src: &Path, dest: &Path) -> String { @@ -2346,7 +2339,13 @@ fn handle_copy_mode( if dest.exists() && options.overwrite == OverwriteMode::Clobber(ClobberMode::Force) { fs::remove_file(dest)?; } - symlink_file(source, dest, Some(source), symlinked_files)?; + symlink_file( + source, + dest, + #[cfg(windows)] + source_metadata, + symlinked_files, + )?; } CopyMode::Update => { if dest.exists() { @@ -2821,7 +2820,14 @@ fn copy_helper( } if source_metadata.is_symlink() { - copy_link(source, dest, symlinked_files, options)?; + copy_link( + source, + dest, + #[cfg(windows)] + source_metadata, + symlinked_files, + options, + )?; } else { // Use O_NOFOLLOW on the source open iff cp is in no-dereference mode. // In that case source_metadata was obtained via lstat, so a path swap @@ -2900,6 +2906,7 @@ fn copy_node( fn copy_link( source: &Path, dest: &Path, + #[cfg(windows)] source_metadata: &Metadata, symlinked_files: &mut HashSet, options: &Options, ) -> CopyResult<()> { @@ -2910,7 +2917,13 @@ fn copy_link( if dest.is_symlink() || dest.is_file() { delete_path(dest, options)?; } - symlink_file(&link, dest, Some(source), symlinked_files)?; + symlink_file( + &link, + dest, + #[cfg(windows)] + source_metadata, + symlinked_files, + )?; copy_attributes( source, dest, @@ -2986,6 +2999,8 @@ fn disk_usage_directory(p: &Path) -> io::Result { #[cfg(test)] mod tests { + #[cfg(windows)] + use crate::source_is_directory_for_symlink; use crate::{Attributes, Preserve, aligned_ancestors, localize_to_target}; use std::path::Path; @@ -3056,4 +3071,36 @@ mod tests { } ); } + + #[test] + #[cfg(windows)] + fn test_source_is_directory_for_symlink_detects_directory_symlinks() { + let temp_dir = tempfile::tempdir().unwrap(); + let target = temp_dir.path().join("target"); + let symlink = temp_dir.path().join("target-link"); + + std::fs::create_dir(&target).unwrap(); + if std::os::windows::fs::symlink_dir(&target, &symlink).is_err() { + return; + } + + let metadata = symlink.symlink_metadata().unwrap(); + assert!(source_is_directory_for_symlink(&metadata)); + } + + #[test] + #[cfg(windows)] + fn test_source_is_directory_for_symlink_rejects_file_symlinks() { + let temp_dir = tempfile::tempdir().unwrap(); + let target = temp_dir.path().join("target"); + let symlink = temp_dir.path().join("target-link"); + + std::fs::write(&target, b"data").unwrap(); + if std::os::windows::fs::symlink_file(&target, &symlink).is_err() { + return; + } + + let metadata = symlink.symlink_metadata().unwrap(); + assert!(!source_is_directory_for_symlink(&metadata)); + } } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index bebbf85a16c..ff528f7d11c 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -2874,6 +2874,16 @@ fn test_copy_dir_symlink() { at.symlink_dir("dir", "dir-link"); ucmd.args(&["-r", "dir-link", "copy"]).succeeds(); assert_eq!(at.resolve_link("copy"), "dir"); +} + +#[test] +#[cfg(windows)] +fn test_copy_windows_dir_symlink_preserves_directory_type() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("dir"); + at.symlink_dir("dir", "dir-link"); + ucmd.args(&["-r", "dir-link", "copy"]).succeeds(); + assert_eq!(at.resolve_link("copy"), "dir"); assert!(at.plus("copy").is_dir()); }