fs: honor dereference for symlinks nested in cpSync trees - #65731
Open
christianaurichzm wants to merge 1 commit into
Open
fs: honor dereference for symlinks nested in cpSync trees#65731christianaurichzm wants to merge 1 commit into
christianaurichzm wants to merge 1 commit into
Conversation
When no filter is given, cpSync copies directory contents in C++. That loop recreated every symlink it found, consulting dereference only for the subdirectory-of-self guards, so a symlink nested in the tree was copied as a link even with dereference set. Only a symlink passed as src was dereferenced, because that one is resolved by stat() in JavaScript before the C++ copy starts. The directory and regular file branches already follow symlinks, so links that resolve to those types can fall through to them. Dangling links have no target to copy and are reported instead. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>
christianaurichzm
force-pushed
the
fs-cp-dereference
branch
from
September 2, 2026 05:44
86f460c to
346ed1c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #65731 +/- ##
==========================================
- Coverage 90.06% 90.05% -0.02%
==========================================
Files 754 754
Lines 256388 256439 +51
Branches 48494 48517 +23
==========================================
- Hits 230926 230925 -1
- Misses 16574 16611 +37
- Partials 8888 8903 +15
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fs.cpSync(src, dest, { dereference: true, recursive: true })copies symlinks found inside the tree as symlinks. Reported in #59168 as a regression in 22.17, and still present onmain:Cause
When no filter is given,
cp-sync.jshands the whole tree tofsBinding.cpSyncCopyDir(). That C++ loop recreates every symlink it finds and consultsdereferenceonly for the subdirectory-of-self guards, so it never copies the target.A symlink passed as
srcis unaffected becausegetStats()resolves that one withstatSync()before the native copy starts. This is why the existing dereference tests pass and why the option only fails for links inside the tree.Fix
With
dereferenceenabled, links resolving to directories or regular files use the corresponding native copy paths, which already follow symlinks. Dangling links have no target to copy and are reported instead.Existing destinations follow the JavaScript walker's
forceanderrorOnExistbehavior, including file/directory conflicts.Verification
The new test fails on
mainand passes with the patch. I also compared the destination cases against the JavaScript path by passing afilter, which keeps the copy out of the native walker.On Linux x64:
make lintparallelsequentiales-moduleall pass.
#65488 refactors the same native walker. If it lands first, I can rebase this fix onto the new implementation.
Fixes: #59168