From bb5fe4ae29e6d820ee8f23715c7b5baccf3573b5 Mon Sep 17 00:00:00 2001 From: bcoe Date: Mon, 12 Jul 2021 17:49:27 -0700 Subject: [PATCH] fs: add recursive copy method Introduces recursive copy method, based on fs-extra implementation Refs: https://github.com/nodejs/tooling/issues/98 --- lib/internal/fs/copy/copy.js | 5 +- .../fixtures/copy/files-and-folders/README.md | 1 - .../files-and-folders/sub-folder/hello.mjs | 3 - test/fixtures/copy/kitchen-sink/README.md | 1 + .../fixtures/copy/kitchen-sink/a/b/README2.md | 1 + .../a/b}/index.js | 0 test/fixtures/copy/kitchen-sink/a/c | 1 + .../sub-folder => kitchen-sink/a}/index.js | 0 .../copy/{symlink => kitchen-sink}/index.js | 0 test/fixtures/copy/symlink/a/b/README.md | 1 - test/parallel/test-copy.js | 105 ++++++++++++++---- 11 files changed, 89 insertions(+), 29 deletions(-) delete mode 100644 test/fixtures/copy/files-and-folders/README.md delete mode 100644 test/fixtures/copy/files-and-folders/sub-folder/hello.mjs create mode 100644 test/fixtures/copy/kitchen-sink/README.md create mode 120000 test/fixtures/copy/kitchen-sink/a/b/README2.md rename test/fixtures/copy/{files-and-folders => kitchen-sink/a/b}/index.js (100%) create mode 120000 test/fixtures/copy/kitchen-sink/a/c rename test/fixtures/copy/{files-and-folders/sub-folder => kitchen-sink/a}/index.js (100%) rename test/fixtures/copy/{symlink => kitchen-sink}/index.js (100%) delete mode 120000 test/fixtures/copy/symlink/a/b/README.md diff --git a/lib/internal/fs/copy/copy.js b/lib/internal/fs/copy/copy.js index e58a1133fac565f..8fa4af69dcca816 100644 --- a/lib/internal/fs/copy/copy.js +++ b/lib/internal/fs/copy/copy.js @@ -394,6 +394,8 @@ function copyDirItem(items, item, src, dest, opts, cb) { function onLink(destStat, src, dest, opts, cb) { readlink(src, (err, resolvedSrc) => { if (err) return cb(err); + // TODO(bcoe): I don't know how this could be called, because + // getStatsForCopy will have used stat. Ask during review. if (opts.dereference) { resolvedSrc = resolve(process.cwd(), resolvedSrc); } @@ -428,6 +430,8 @@ function onLink(destStat, src, dest, opts, cb) { // Do not copy if src is a subdir of dest since unlinking // dest in this case would result in removing src contents // and therefore a broken symlink would be created. + // TODO(bcoe): I'm having trouble exercising this code in test, + // ask about during review. if (destStat.isDirectory() && stat.isSrcSubdir(resolvedDest, resolvedSrc)) { return cb(new ERR_FS_COPY_SYMLINK_TO_SUBDIRECTORY({ @@ -440,7 +444,6 @@ function onLink(destStat, src, dest, opts, cb) { } return copyLink(resolvedSrc, dest, cb); }); - }); } diff --git a/test/fixtures/copy/files-and-folders/README.md b/test/fixtures/copy/files-and-folders/README.md deleted file mode 100644 index 73b2ffa3e22002e..000000000000000 --- a/test/fixtures/copy/files-and-folders/README.md +++ /dev/null @@ -1 +0,0 @@ -## Fixtures for testing copy \ No newline at end of file diff --git a/test/fixtures/copy/files-and-folders/sub-folder/hello.mjs b/test/fixtures/copy/files-and-folders/sub-folder/hello.mjs deleted file mode 100644 index 69e838013768966..000000000000000 --- a/test/fixtures/copy/files-and-folders/sub-folder/hello.mjs +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - goodnight: 'moon' -}; diff --git a/test/fixtures/copy/kitchen-sink/README.md b/test/fixtures/copy/kitchen-sink/README.md new file mode 100644 index 000000000000000..fec56017dc1b1ac --- /dev/null +++ b/test/fixtures/copy/kitchen-sink/README.md @@ -0,0 +1 @@ +# Hello diff --git a/test/fixtures/copy/kitchen-sink/a/b/README2.md b/test/fixtures/copy/kitchen-sink/a/b/README2.md new file mode 120000 index 000000000000000..fe840054137e2cc --- /dev/null +++ b/test/fixtures/copy/kitchen-sink/a/b/README2.md @@ -0,0 +1 @@ +../../README.md \ No newline at end of file diff --git a/test/fixtures/copy/files-and-folders/index.js b/test/fixtures/copy/kitchen-sink/a/b/index.js similarity index 100% rename from test/fixtures/copy/files-and-folders/index.js rename to test/fixtures/copy/kitchen-sink/a/b/index.js diff --git a/test/fixtures/copy/kitchen-sink/a/c b/test/fixtures/copy/kitchen-sink/a/c new file mode 120000 index 000000000000000..63d8dbd40c23542 --- /dev/null +++ b/test/fixtures/copy/kitchen-sink/a/c @@ -0,0 +1 @@ +b \ No newline at end of file diff --git a/test/fixtures/copy/files-and-folders/sub-folder/index.js b/test/fixtures/copy/kitchen-sink/a/index.js similarity index 100% rename from test/fixtures/copy/files-and-folders/sub-folder/index.js rename to test/fixtures/copy/kitchen-sink/a/index.js diff --git a/test/fixtures/copy/symlink/index.js b/test/fixtures/copy/kitchen-sink/index.js similarity index 100% rename from test/fixtures/copy/symlink/index.js rename to test/fixtures/copy/kitchen-sink/index.js diff --git a/test/fixtures/copy/symlink/a/b/README.md b/test/fixtures/copy/symlink/a/b/README.md deleted file mode 120000 index 1dfab2425aa964f..000000000000000 --- a/test/fixtures/copy/symlink/a/b/README.md +++ /dev/null @@ -1 +0,0 @@ -../../../../../README.md \ No newline at end of file diff --git a/test/parallel/test-copy.js b/test/parallel/test-copy.js index 28a82fe0e9cf5e3..b710400079ee468 100644 --- a/test/parallel/test-copy.js +++ b/test/parallel/test-copy.js @@ -1,17 +1,23 @@ 'use strict'; const common = require('../common'); +if (!common.hasCrypto) { common.skip('missing crypto'); } const assert = require('assert'); +const { randomUUID } = require('crypto'); const fs = require('fs'); const { copy, + copySync, lstatSync, + mkdirSync, readdirSync, - rmSync, + symlinkSync, } = fs; +const net = require('net'); const { dirname, join } = require('path'); +const isWindows = process.platform === 'win32'; const tmpdir = require('../common/tmpdir'); tmpdir.refresh(); @@ -22,35 +28,34 @@ function nextdir() { // Async version of copy. -// It copies a nested folder structure with files and folders. +// It copies a nested folder structure with files, folders, symlinks. { - const src = dirname(require.resolve('../fixtures/copy/files-and-folders')); + const src = dirname(require.resolve('../fixtures/copy/kitchen-sink')); const dest = nextdir(); copy(src, dest, common.mustCall((err) => { - try { - assert.strictEqual(err, null); - assertDirEquivalent(src, dest); - } finally { - rmSync(dest, { force: true, recursive: true }); - } + assert.strictEqual(err, null); + assertDirEquivalent(src, dest); })); } -// It copies symlink if dereference is false (default value). +// It throws error if existing symlink in dest is in subdirectory src. +// TODO(bcoe): this behavior seemed strange to me, ask about it in review. { - const src = dirname(require.resolve('../fixtures/copy/symlink')); + const src = dirname(require.resolve('../fixtures/copy/kitchen-sink')); const dest = nextdir(); + copySync(src, dest); copy(src, dest, common.mustCall((err) => { - assert.strictEqual(err, null); + assert.strictEqual(err.code, 'ERR_FS_COPY_TO_SUBDIRECTORY'); assertDirEquivalent(src, dest); })); } -// It copies folder symlink links to, when dereference is true. +// It does not fail if the same directory is copied to dest twice, +// when dereference is true, and overwrite true. { - const src = dirname(require.resolve('../fixtures/copy/symlink')); + const src = dirname(require.resolve('../fixtures/copy/kitchen-sink')); const dest = nextdir(); - const destFile = join(dest, 'a/b/README.md'); + const destFile = join(dest, 'a/b/README2.md'); copy(src, dest, { dereference: true }, common.mustCall((err) => { assert.strictEqual(err, null); const stat = lstatSync(destFile); @@ -58,18 +63,45 @@ function nextdir() { })); } +// It copies file itself, rather than symlink, when dereference is true. +{ + const src = require.resolve('../fixtures/copy/kitchen-sink'); + const dest = nextdir(); + const destFile = join(dest, 'foo.js'); + copy(src, destFile, { dereference: true }, common.mustCall((err) => { + assert.strictEqual(err, null); + const stat = lstatSync(destFile); + assert(stat.isFile()); + })); +} + // It returns error when src and dest are identical. { - const src = dirname(require.resolve('../fixtures/copy/symlink')); + const src = dirname(require.resolve('../fixtures/copy/kitchen-sink')); copy(src, src, common.mustCall((err) => { assert.strictEqual(err.code, 'ERR_FS_COPY_TO_SUBDIRECTORY'); })); } +// It returns error if part of dest path is symlink to src. +{ + const src = nextdir(); + mkdirSync(join(src, 'a'), { recursive: true }); + const dest = nextdir(); + // Create symlink in dest pointing to src. + const destLink = join(dest, 'b'); + mkdirSync(dest, { recursive: true }); + symlinkSync(src, destLink); + copy(src, join(dest, 'b', 'c'), common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_FS_COPY_TO_SUBDIRECTORY'); + })); +} + // It returns error if attempt is made to copy directory to file. { - const src = dirname(require.resolve('../fixtures/copy/symlink')); - const dest = require.resolve('../fixtures/copy/files-and-folders'); + const src = nextdir(); + mkdirSync(src, { recursive: true }); + const dest = require.resolve('../fixtures/copy/kitchen-sink'); copy(src, dest, common.mustCall((err) => { assert.strictEqual(err.code, 'ERR_FS_COPY_DIR_TO_NON_DIR'); })); @@ -77,7 +109,7 @@ function nextdir() { // It allows file to be copied to a file path. { - const srcFile = require.resolve('../fixtures/copy/files-and-folders'); + const srcFile = require.resolve('../fixtures/copy/kitchen-sink'); const destFile = join(nextdir(), 'index.js'); copy(srcFile, destFile, { dereference: true }, common.mustCall((err) => { assert.strictEqual(err, null); @@ -88,8 +120,9 @@ function nextdir() { // It returns error if attempt is made to copy file to directory. { - const src = require.resolve('../fixtures/copy/symlink'); - const dest = dirname(require.resolve('../fixtures/copy/files-and-folders')); + const src = require.resolve('../fixtures/copy/kitchen-sink'); + const dest = nextdir(); + mkdirSync(dest, { recursive: true }); copy(src, dest, common.mustCall((err) => { assert.strictEqual(err.code, 'ERR_FS_COPY_NON_DIR_TO_DIR'); })); @@ -97,15 +130,41 @@ function nextdir() { // It returns error if attempt is made to copy to subdirectory of self. { - const src = dirname(require.resolve('../fixtures/copy/files-and-folders')); + const src = dirname(require.resolve('../fixtures/copy/kitchen-sink')); const dest = dirname( - require.resolve('../fixtures/copy/files-and-folders/sub-folder') + require.resolve('../fixtures/copy/kitchen-sink/a') ); copy(src, dest, common.mustCall((err) => { assert.strictEqual(err.code, 'ERR_FS_COPY_TO_SUBDIRECTORY'); })); } +// It returns an error if attempt is made to copy socket. +{ + const dest = nextdir(); + const sid = randomUUID(); + const sock = isWindows ? `\\\\.\\pipe\\${sid}` : `${sid}.sock`; + const server = net.createServer(); + server.listen(sock); + copy(sock, dest, common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_FS_COPY_SOCKET'); + server.close(); + })); +} + +// It copies timestamps from src to dest if preserveTimestamps is true. +{ + const src = dirname(require.resolve('../fixtures/copy/kitchen-sink')); + const dest = nextdir(); + copy(src, dest, { preserveTimestamps: true }, common.mustCall((err) => { + assert.strictEqual(err, null); + assertDirEquivalent(src, dest); + const srcStat = lstatSync(join(src, 'index.js')); + const destStat = lstatSync(join(dest, 'index.js')); + assert.strictEqual(srcStat.mtime.getTime(), destStat.mtime.getTime()); + })); +} + function assertDirEquivalent(dir1, dir2) { const dir1Entries = []; collectEntries(dir1, dir1Entries);