Repository navigation
CI check: fs: respect force: false for symlinks in cp - #1
Closed
kuldeepsinh19 wants to merge 3 commits into
Closed
kuldeepsinh19 wants to merge 3 commits into
kuldeepsinh19 wants to merge 3 commits into
Conversation
When cp() or fsPromises.cp() copied a symlink with force set to false, an existing destination symlink was still replaced, and an existing file failed with EEXIST from symlink(2). errorOnExist was ignored in both cases. This is a breaking change: with force set to false, an existing destination is now left in place, as it already is when copying a file, or the copy fails with ERR_FS_CP_EEXIST when errorOnExist is set. This is the behavior the documentation describes for both options. Copies with force set to true, the default, are unchanged. Refs: nodejs#66774 Signed-off-by: kuldeepsinh19 <kuldeepsinhrajput1919@gmail.com>
cpSync() had the same problem as cp() when it copied a symlink, on its own or inside a directory copied with a filter function: with force set to false, an existing destination symlink was replaced, an existing file failed with EEXIST from symlink(2), and a dangling destination symlink failed with ENOENT. errorOnExist was ignored. This is a breaking change: with force set to false, an existing destination is now left in place, or the copy fails with ERR_FS_CP_EEXIST when errorOnExist is set, as with cp(). onLink() now takes the options to check them. Copies with force set to true, the default, are unchanged. Refs: nodejs#66774 Signed-off-by: kuldeepsinh19 <kuldeepsinhrajput1919@gmail.com>
Without a filter function, a recursive cpSync() copies the directory in C++. With force set to false, that walk still replaced an existing destination symlink, as a comment there pointed out, and failed with EEXIST for an existing file, for a dangling symlink, and for anything already there when verbatimSymlinks was set. This is a breaking change: with force set to false, an existing destination is now left in place, as in the JavaScript walk. It is looked up with symlink_status(), so that a dangling symlink counts as existing, as it does for lstat() there. Copies with force set to true, the default, are unchanged. Fixes: nodejs#66774 Signed-off-by: kuldeepsinh19 <kuldeepsinhrajput1919@gmail.com>
Owner
Author
|
Superseded by the rebased CI check in the next PR. |
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.
CI-only PR inside my fork to run the full Node test workflows before opening the upstream PR for nodejs#66774. Not for merge.