From e4b7aa11df245e334700ae923c531f44e8cfed71 Mon Sep 17 00:00:00 2001 From: SukkaW Date: Sat, 21 Oct 2023 23:48:49 +0800 Subject: [PATCH] Apply code review suggestions from @RyanZim --- lib/copy/copy.js | 82 +++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 47 deletions(-) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 3f096759..c53ee8b1 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -7,12 +7,14 @@ const { pathExists } = require('../path-exists') const { utimesMillisAsync } = require('../util/utimes') const stat = require('../util/stat') -async function copy (src, dest, opts) { +async function copy(src, dest, opts = {}) { if (typeof opts === 'function') { opts = { filter: opts } } - opts = opts || {} + opts = typeof opts === 'function' + ? { filter: opts } + : opts opts.clobber = 'clobber' in opts ? !!opts.clobber : true // default to true for now opts.overwrite = 'overwrite' in opts ? !!opts.overwrite : opts.clobber // overwrite falls back to clobber @@ -37,27 +39,24 @@ async function copy (src, dest, opts) { return checkParentDir(destStat, src, dest, opts) } -async function checkParentDir (destStat, src, dest, opts) { +async function checkParentDir(destStat, src, dest, opts) { const destParent = path.dirname(dest) const dirExists = await pathExists(destParent) - if (dirExists) return getStats(destStat, src, dest, opts) - - const parentDirExists = await pathExists(destParent) - if (parentDirExists) return getStats(destStat, src, dest, opts) - - await mkdirs(destParent) + if (!dirExists) { + await mkdirs(destParent) + } return getStats(destStat, src, dest, opts) } -async function runFilter (src, dest, opts) { +async function runFilter(src, dest, opts) { if (!opts.filter) return true return opts.filter(src, dest) } -async function getStats (destStat, src, dest, opts) { +async function getStats(destStat, src, dest, opts) { const statFn = opts.dereference ? fs.stat : fs.lstat const srcStat = await statFn(src) @@ -75,12 +74,9 @@ async function getStats (destStat, src, dest, opts) { throw new Error(`Unknown file: ${src}`) } -function onFile (srcStat, destStat, src, dest, opts) { +async function onFile(srcStat, destStat, src, dest, opts) { if (!destStat) return copyFile(srcStat, src, dest, opts) - return mayCopyFile(srcStat, src, dest, opts) -} -async function mayCopyFile (srcStat, src, dest, opts) { if (opts.overwrite) { await fs.unlink(dest) return copyFile(srcStat, src, dest, opts) @@ -90,7 +86,7 @@ async function mayCopyFile (srcStat, src, dest, opts) { } } -async function copyFile (srcStat, src, dest, opts) { +async function copyFile(srcStat, src, dest, opts) { await fs.copyFile(src, dest) if (opts.preserveTimestamps) { return handleTimestampsAndMode(srcStat.mode, src, dest) @@ -98,67 +94,60 @@ async function copyFile (srcStat, src, dest, opts) { return setDestMode(dest, srcStat.mode) } -async function handleTimestampsAndMode (srcMode, src, dest) { +async function handleTimestampsAndMode(srcMode, src, dest) { // Make sure the file is writable before setting the timestamp // otherwise open fails with EPERM when invoked with 'r+' // (through utimes call) if (fileIsNotWritable(srcMode)) { await makeFileWritable(dest, srcMode) - return setDestTimestampsAndMode(srcMode, src, dest) } return setDestTimestampsAndMode(srcMode, src, dest) } -function fileIsNotWritable (srcMode) { +function fileIsNotWritable(srcMode) { return (srcMode & 0o200) === 0 } -function makeFileWritable (dest, srcMode) { +function makeFileWritable(dest, srcMode) { return setDestMode(dest, srcMode | 0o200) } -async function setDestTimestampsAndMode (srcMode, src, dest) { - await setDestTimestamps(src, dest) - return setDestMode(dest, srcMode) -} - -function setDestMode (dest, srcMode) { - return fs.chmod(dest, srcMode) -} - -async function setDestTimestamps (src, dest) { +async function setDestTimestampsAndMode(srcMode, src, dest) { // The initial srcStat.atime cannot be trusted // because it is modified by the read(2) system call - // (See https://nodejs.org/api/fs.html#fs$stat_time_values) + // (See https://nodejs.org/api/fs.html#fs_stat_time_values) const updatedSrcStat = await fs.stat(src) + await utimesMillisAsync(dest, updatedSrcStat.atime, updatedSrcStat.mtime) - return utimesMillisAsync(dest, updatedSrcStat.atime, updatedSrcStat.mtime) + return setDestMode(dest, srcMode) } -function onDir (srcStat, destStat, src, dest, opts) { - if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts) - return copyDir(src, dest, opts) +function setDestMode(dest, srcMode) { + return fs.chmod(dest, srcMode) } -async function mkDirAndCopy (srcMode, src, dest, opts) { - await fs.mkdir(dest) +async function onDir(srcStat, destStat, src, dest, opts) { + if (!destStat) { + await fs.mkdir(dest) + } await copyDir(src, dest, opts) - - return setDestMode(dest, srcMode) + if (!destStat) { + await setDestMode(dest, srcMode) + } } -async function copyDir (src, dest, opts) { +async function copyDir(src, dest, opts) { const items = await fs.readdir(src) return copyDirItems(items, src, dest, opts) } -function copyDirItems (items, src, dest, opts) { +function copyDirItems(items, src, dest, opts) { const item = items.pop() if (!item) return return copyDirItem(items, item, src, dest, opts) } -async function copyDirItem (items, item, src, dest, opts) { +async function copyDirItem(items, item, src, dest, opts) { const srcItem = path.join(src, item) const destItem = path.join(dest, item) @@ -172,7 +161,7 @@ async function copyDirItem (items, item, src, dest, opts) { return copyDirItems(items, src, dest, opts) } -async function onLink (destStat, src, dest, opts) { +async function onLink(destStat, src, dest, opts) { let resolvedSrc = await fs.readlink(src) if (opts.dereference) { resolvedSrc = path.resolve(process.cwd(), resolvedSrc) @@ -185,6 +174,9 @@ async function onLink (destStat, src, dest, opts) { try { resolvedDest = await fs.readlink(dest) } catch (e) { + // dest exists and is a regular file or directory, + // Windows may throw UNKNOWN error. If dest already exists, + // fs throws error anyway, so no need to guard against it here. if (e.code === 'EINVAL' || e.code === 'UNKNOWN') return fs.symlink(resolvedSrc, dest) throw e } @@ -202,10 +194,6 @@ async function onLink (destStat, src, dest, opts) { throw new Error(`Cannot overwrite '${resolvedDest}' with '${resolvedSrc}'.`) } - return copyLink(resolvedSrc, dest) -} - -async function copyLink (resolvedSrc, dest) { await fs.unlink(dest) return fs.symlink(resolvedSrc, dest) }