-
-
Notifications
You must be signed in to change notification settings - Fork 34k
Description
In the fs.cp... API, we have options such as errorOnExist and force that are intended to control what happens when the destination already exists. There is, however, a discrepancy in how the options are implemented depending on whether the src/dest are files or directories.
For instance, let's suppose we have the following directory layout:
~/tmp
| -- ~/tmp/a (file)
| -- ~/tmp/b (file)
| -- ~/tmp/c (directory)
| | -- /tmp/c/1 (file)
| -- ~/tmp/d (directory)
| -- /tmp/d/2 (file)
Then, in the fs.cp operation, let's first try copying a to b, with options errorOnExist: true and force: false we get the appropriate error indicating that the destination exists and we're not going to overwrite it:
> fs.cp('a', 'b', { errorOnExist: true, force: false }, console.log)
undefined
> SystemError [ERR_FS_CP_EEXIST]: Target already exists: cp returned EEXIST (b already exists) b
at mayCopyFile (node:internal/fs/cp/cp:250:11)
at onFile (node:internal/fs/cp/cp:242:10)
at getStatsForCopy (node:internal/fs/cp/cp:211:12) {
code: 'ERR_FS_CP_EEXIST',
info: {
message: 'b already exists',
path: 'b',
syscall: 'cp',
errno: 17,
code: 'EEXIST'
},
errno: [Getter/Setter: 17],
syscall: [Getter/Setter: 'cp'],
path: [Getter/Setter: 'b']
}However, if we try copying c to d, with options errorOnExist: true and force: false, we happily go ahead and copy contents of 'c' over to 'd', erroring only if one of the individual files in d overlaps with a file in c
> fs.cp('c', 'd', { recursive: true, errorOnExist: true, force: false }, console.log)
undefined
> null undefinedThis just feels wrong. d already exists and we're being told not to force. I would expect the behavior to be the same as copying the files. If d exists, this operation should fail.
I don't think we can change this behavior now without it being semver-major and potentially breaking a lot of things but we really ought to have an option of enforcing that no copy is performed if d exists.
@nodejs/fs @anonrig @dario-piotrowicz