Skip to content

Commit

Permalink
fs: add support for mode flag to specify the copy behavior
Browse files Browse the repository at this point in the history
`fs.copyFile()` supports copy-on-write operation
if the underlying platform supports it by passing a mode flag.
This behavior was added in
a16d88d.

This patch adds `mode` flag to `fs.cp()`, `fs.cpSync()`,
and `fsPromises.cp()` to allow to change their behaviors
to copy files.

This test case is based on the test case that was introduced
when we add `fs.constants.COPYFILE_FICLONE`.
a16d88d.

This test strategy is:

- If the platform supports copy-on-write operation,
  check whether the destination is expected
- Otherwise, the operation will fail
  and check whether the failure error information is expected.

Fixes: #47080
PR-URL: #47084
Reviewed-By: Antoine du Hamel <[email protected]>
  • Loading branch information
tetsuharuohzeki authored and danielleadams committed Jul 6, 2023
1 parent dee3245 commit 383a3f8
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 3 deletions.
22 changes: 21 additions & 1 deletion doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,10 @@ try {
<!-- YAML
added: v16.7.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/47084
description: Accept an additional `mode` option to specify
the copy behavior as the `mode` argument of `fs.copyFile()`.
- version: v17.6.0
pr-url: https://github.com/nodejs/node/pull/41819
description: Accepts an additional `verbatimSymlinks` option to specify
Expand All @@ -986,6 +990,8 @@ changes:
operation will ignore errors if you set this to false and the destination
exists. Use the `errorOnExist` option to change this behavior.
**Default:** `true`.
* `mode` {integer} modifiers for copy operation. **Default:** `0`.
See `mode` flag of [`fsPromises.copyFile()`][].
* `preserveTimestamps` {boolean} When `true` timestamps from `src` will
be preserved. **Default:** `false`.
* `recursive` {boolean} copy directories recursively **Default:** `false`
Expand Down Expand Up @@ -2269,6 +2275,10 @@ copyFile('source.txt', 'destination.txt', constants.COPYFILE_EXCL, callback);
<!-- YAML
added: v16.7.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/47084
description: Accept an additional `mode` option to specify
the copy behavior as the `mode` argument of `fs.copyFile()`.
- version: v18.0.0
pr-url: https://github.com/nodejs/node/pull/41678
description: Passing an invalid callback to the `callback` argument
Expand Down Expand Up @@ -2298,6 +2308,8 @@ changes:
operation will ignore errors if you set this to false and the destination
exists. Use the `errorOnExist` option to change this behavior.
**Default:** `true`.
* `mode` {integer} modifiers for copy operation. **Default:** `0`.
See `mode` flag of [`fs.copyFile()`][].
* `preserveTimestamps` {boolean} When `true` timestamps from `src` will
be preserved. **Default:** `false`.
* `recursive` {boolean} copy directories recursively **Default:** `false`
Expand Down Expand Up @@ -5120,7 +5132,11 @@ copyFileSync('source.txt', 'destination.txt', constants.COPYFILE_EXCL);
<!-- YAML
added: v16.7.0
changes:
- version: v17.6.0
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/47084
description: Accept an additional `mode` option to specify
the copy behavior as the `mode` argument of `fs.copyFile()`.
- version: v17.6.0
pr-url: https://github.com/nodejs/node/pull/41819
description: Accepts an additional `verbatimSymlinks` option to specify
whether to perform path resolution for symlinks.
Expand All @@ -5143,6 +5159,8 @@ changes:
operation will ignore errors if you set this to false and the destination
exists. Use the `errorOnExist` option to change this behavior.
**Default:** `true`.
* `mode` {integer} modifiers for copy operation. **Default:** `0`.
See `mode` flag of [`fs.copyFileSync()`][].
* `preserveTimestamps` {boolean} When `true` timestamps from `src` will
be preserved. **Default:** `false`.
* `recursive` {boolean} copy directories recursively **Default:** `false`
Expand Down Expand Up @@ -7907,6 +7925,7 @@ the file contents.
[`fs.chmod()`]: #fschmodpath-mode-callback
[`fs.chown()`]: #fschownpath-uid-gid-callback
[`fs.copyFile()`]: #fscopyfilesrc-dest-mode-callback
[`fs.copyFileSync()`]: #fscopyfilesyncsrc-dest-mode
[`fs.createReadStream()`]: #fscreatereadstreampath-options
[`fs.createWriteStream()`]: #fscreatewritestreampath-options
[`fs.exists()`]: #fsexistspath-callback
Expand Down Expand Up @@ -7940,6 +7959,7 @@ the file contents.
[`fs.writeFile()`]: #fswritefilefile-data-options-callback
[`fs.writev()`]: #fswritevfd-buffers-position-callback
[`fsPromises.access()`]: #fspromisesaccesspath-mode
[`fsPromises.copyFile()`]: #fspromisescopyfilesrc-dest-mode
[`fsPromises.open()`]: #fspromisesopenpath-flags-mode
[`fsPromises.opendir()`]: #fspromisesopendirpath-options
[`fsPromises.rm()`]: #fspromisesrmpath-options
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/cp/cp-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ function mayCopyFile(srcStat, src, dest, opts) {
}

function copyFile(srcStat, src, dest, opts) {
copyFileSync(src, dest);
copyFileSync(src, dest, opts.mode);
if (opts.preserveTimestamps) handleTimestamps(srcStat.mode, src, dest);
return setDestMode(dest, srcStat.mode);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/cp/cp.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ async function mayCopyFile(srcStat, src, dest, opts) {
}

async function _copyFile(srcStat, src, dest, opts) {
await copyFile(src, dest);
await copyFile(src, dest, opts.mode);
if (opts.preserveTimestamps) {
return handleTimestampsAndMode(srcStat.mode, src, dest);
}
Expand Down
1 change: 1 addition & 0 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,7 @@ const validateCpOptions = hideStackFrames((options) => {
validateBoolean(options.preserveTimestamps, 'options.preserveTimestamps');
validateBoolean(options.recursive, 'options.recursive');
validateBoolean(options.verbatimSymlinks, 'options.verbatimSymlinks');
options.mode = getValidMode(options.mode, 'copyFile');
if (options.dereference === true && options.verbatimSymlinks === true) {
throw new ERR_INCOMPATIBLE_OPTION_PAIR('dereference', 'verbatimSymlinks');
}
Expand Down
104 changes: 104 additions & 0 deletions test/parallel/test-fs-cp.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,30 @@ function nextdir() {
assertDirEquivalent(src, dest);
}

// It copies a nested folder structure with mode flags.
// This test is based on fs.promises.copyFile() with `COPYFILE_FICLONE_FORCE`.
(() => {
const src = './test/fixtures/copy/kitchen-sink';
const dest = nextdir();
try {
cpSync(src, dest, mustNotMutateObjectDeep({
recursive: true,
mode: fs.constants.COPYFILE_FICLONE_FORCE,
}));
} catch (err) {
// If the platform does not support `COPYFILE_FICLONE_FORCE` operation,
// it should enter this path.
assert.strictEqual(err.syscall, 'copyfile');
assert(err.code === 'ENOTSUP' || err.code === 'ENOTTY' ||
err.code === 'ENOSYS' || err.code === 'EXDEV');
return;
}

// If the platform support `COPYFILE_FICLONE_FORCE` operation,
// it should reach to here.
assertDirEquivalent(src, dest);
})();

// It does not throw errors when directory is copied over and force is false.
{
const src = nextdir();
Expand Down Expand Up @@ -107,6 +131,14 @@ function nextdir() {
});
}

// It rejects if options.mode is invalid.
{
assert.throws(
() => cpSync('a', 'b', { mode: -1 }),
{ code: 'ERR_OUT_OF_RANGE' }
);
}


// It throws an error when both dereference and verbatimSymlinks are enabled.
{
Expand Down Expand Up @@ -425,6 +457,31 @@ if (!isWindows) {
}));
}

// It copies a nested folder structure with mode flags.
// This test is based on fs.promises.copyFile() with `COPYFILE_FICLONE_FORCE`.
{
const src = './test/fixtures/copy/kitchen-sink';
const dest = nextdir();
cp(src, dest, mustNotMutateObjectDeep({
recursive: true,
mode: fs.constants.COPYFILE_FICLONE_FORCE,
}), mustCall((err) => {
if (!err) {
// If the platform support `COPYFILE_FICLONE_FORCE` operation,
// it should reach to here.
assert.strictEqual(err, null);
assertDirEquivalent(src, dest);
return;
}

// If the platform does not support `COPYFILE_FICLONE_FORCE` operation,
// it should enter this path.
assert.strictEqual(err.syscall, 'copyfile');
assert(err.code === 'ENOTSUP' || err.code === 'ENOTTY' ||
err.code === 'ENOSYS' || err.code === 'EXDEV');
}));
}

// It does not throw errors when directory is copied over and force is false.
{
const src = nextdir();
Expand Down Expand Up @@ -799,6 +856,14 @@ if (!isWindows) {
);
}

// It throws if options is not object.
{
assert.throws(
() => cp('a', 'b', { mode: -1 }, () => {}),
{ code: 'ERR_OUT_OF_RANGE' }
);
}

// Promises implementation of copy.

// It copies a nested folder structure with files and folders.
Expand All @@ -810,6 +875,35 @@ if (!isWindows) {
assertDirEquivalent(src, dest);
}

// It copies a nested folder structure with mode flags.
// This test is based on fs.promises.copyFile() with `COPYFILE_FICLONE_FORCE`.
{
const src = './test/fixtures/copy/kitchen-sink';
const dest = nextdir();
let p = null;
let successFiClone = false;
try {
p = await fs.promises.cp(src, dest, mustNotMutateObjectDeep({
recursive: true,
mode: fs.constants.COPYFILE_FICLONE_FORCE,
}));
successFiClone = true;
} catch (err) {
// If the platform does not support `COPYFILE_FICLONE_FORCE` operation,
// it should enter this path.
assert.strictEqual(err.syscall, 'copyfile');
assert(err.code === 'ENOTSUP' || err.code === 'ENOTTY' ||
err.code === 'ENOSYS' || err.code === 'EXDEV');
}

if (successFiClone) {
// If the platform support `COPYFILE_FICLONE_FORCE` operation,
// it should reach to here.
assert.strictEqual(p, undefined);
assertDirEquivalent(src, dest);
}
}

// It accepts file URL as src and dest.
{
const src = './test/fixtures/copy/kitchen-sink';
Expand Down Expand Up @@ -847,6 +941,16 @@ if (!isWindows) {
);
}

// It rejects if options.mode is invalid.
{
await assert.rejects(
fs.promises.cp('a', 'b', {
mode: -1,
}),
{ code: 'ERR_OUT_OF_RANGE' }
);
}

function assertDirEquivalent(dir1, dir2) {
const dir1Entries = [];
collectEntries(dir1, dir1Entries);
Expand Down

0 comments on commit 383a3f8

Please sign in to comment.