Preserve Windows directory symlinks in cp by caomengxuan666 · Pull Request #12741 · uutils/coreutils

caomengxuan666 · 2026-06-09T11:04:03Z

Summary

preserve Windows directory symlinks when cp recreates symlinks
choose symlink_dir on Windows when the source symlink metadata has the directory attribute
strengthen the existing directory symlink copy test to verify the copy is usable as a directory

This also addresses microsoft/coreutils#106 downstream.

Verification

cargo fmt -p uu_cp -- --check
cargo check -p uu_cp
manual elevated Windows runtime repro:
- created hello_world_symlink as <SYMLINKD> [hello_world]
- ran target\debug\cp.exe -a hello_world_symlink test
- verified copied test is <SYMLINKD> [hello_world]
- verified dir test works and test\file.txt is readable

github-actions · 2026-06-09T11:46:58Z

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tty-eof (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/pid-pipe (passes in this run but fails in the 'main' branch)

sylvestre · 2026-06-09T18:45:43Z

@lhecker wdyt ?

caomengxuan666 · 2026-06-09T18:58:22Z

Thanks, updated.

sylvestre · 2026-06-09T19:10:29Z

    #[cfg(target_os = "wasi")] _symlinked_files: &mut HashSet<FileInformation>,
 ) -> CopyResult<()> {
+    #[cfg(not(windows))]
+    let _ = source_type_hint;


this isn't enough...

Ok,I would refactor it.I previously had a misunderstanding about this.

sylvestre · 2026-06-09T19:19:16Z

+    Ok(())
+}
+
+#[cfg(windows)]


it is not duplicating a lot of code ...

Updated. I kept the platform-specific signatures, but moved the shared creation/error handling/recording path into helper functions to avoid duplicating that logic.

Indeed, this PR would be a lot easier to review if it was closer to the original structure.
Edit: Ah, GitHub hadn't updated to show me your comment.

Updated again. I kept the original symlink_file cfg block layout and only added a Windows hint argument plus the symlink_dir / symlink_file selection. Non-Windows no longer needs a dummy parameter.

lhecker

The diff is a lot tighter now which is nice.

lhecker · 2026-06-09T19:43:30Z

 fn symlink_file(
    source: &Path,
    dest: &Path,
+    #[cfg(windows)] source_type_hint: &Path,


@sylvestre Do you know how cp's dereference mode interacts with copying symlinks? Because if it doesn't, then we can just pass source_metadata here. That already comes from walkdir's metadata and when options.dereference == false, then it refers to the symlink's metadata.

lhecker · 2026-06-09T19:48:25Z

+        #[cfg(windows)]
+        source,


Is this correct? My reading of the function was that it intentionally dereferences the symlink and then copies the symlink target (aka link), so whether source was a symlink or not shouldn't be relevant right?

I think you are right here.

link is only the target path from read_link, and what we need on Windows is the symlink type from the source symlink itself.

So passing source_metadata may be a better way than passing source and stat it again. I can update it.

Updated. copy_link now uses the existing source_metadata as the Windows symlink type hint.

codspeed-hq · 2026-06-10T07:39:58Z

Merging this PR will not alter performance

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
❌ 1 regressed benchmark
✅ 319 untouched benchmarks
⏩ 46 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Memory	`cp_recursive_deep_tree[(120, 4)]`	562.5 KB	615.6 KB	-8.62%
⚡	Simulation	`ls_recursive_balanced_tree[(6, 4, 15)]`	52.3 ms	49.6 ms	+5.5%
⚡	Simulation	`ls_recursive_wide_tree[(10000, 1000)]`	34.3 ms	33 ms	+3.99%
⚡	Simulation	`single_date_now`	85.6 µs	82.8 µs	+3.35%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing caomengxuan666:fix-windows-cp-directory-symlink-upstream (d33d0cd) with main (f406ea0)}

46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

lhecker

PR appears correct to me now, with just one nit left.

lhecker · 2026-06-10T10:51:22Z

+    const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10;
+
+    let file_type = source_metadata.file_type();
+    if file_type.is_symlink() {
+        source_metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY != 0
+    } else {
+        file_type.is_dir()
+    }


I have read the implementation of is_dir() and found that it basically checks !is_symlink && is_directory where the latter checks for FILE_ATTRIBUTE_DIRECTORY. So, this can be simplified to:

Suggested change

const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10;

let file_type = source_metadata.file_type();

if file_type.is_symlink() {

source_metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY != 0

} else {

file_type.is_dir()

}

const FILE_ATTRIBUTE_DIRECTORY: u32 = 0x10;

source_metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY != 0

Updated, simplified as suggested.

caomengxuan666 mentioned this pull request Jun 9, 2026

cp --no-dereference --preserve=links copies directory symlinks as file symlinks microsoft/coreutils#106

Open

caomengxuan666 marked this pull request as draft June 9, 2026 11:08

caomengxuan666 marked this pull request as ready for review June 9, 2026 11:09

Preserve Windows directory symlinks in cp

e184213

caomengxuan666 force-pushed the fix-windows-cp-directory-symlink-upstream branch from 4f7d22e to e184213 Compare June 9, 2026 11:20