From 3120c5cebfe6d7180a55578ab49bc10478449c0f Mon Sep 17 00:00:00 2001 From: Mani Maghsoudlou Date: Thu, 6 Feb 2020 13:00:32 -0800 Subject: [PATCH] copy*(): remove copyFileFallback (#755) * copy: remove copyFileFallback * copy-sync: remove copyFileFallback --- .../copy-sync-preserve-timestamp.test.js | 2 +- lib/copy-sync/copy-sync.js | 70 +++++++--------- .../__tests__/copy-preserve-timestamp.test.js | 2 +- lib/copy/copy.js | 83 ++++++++++--------- 4 files changed, 75 insertions(+), 82 deletions(-) diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js index 51e52411..da9f5516 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js @@ -63,8 +63,8 @@ describeIfPractical('copySync() - preserveTimestamps option', () => { assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime(), 'different mtime values') assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime(), 'different atime values') } else { - assert.notStrictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime(), 'same mtime values') // the access time might actually be the same, so check only modification time + assert.notStrictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime(), 'same mtime values') } } } diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index bb6e743a..31f06e44 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -2,7 +2,7 @@ const fs = require('graceful-fs') const path = require('path') -const mkdirpSync = require('../mkdirs').mkdirsSync +const mkdirsSync = require('../mkdirs').mkdirsSync const utimesMillisSync = require('../util/utimes').utimesMillisSync const stat = require('../util/stat') @@ -29,7 +29,7 @@ function copySync (src, dest, opts) { function handleFilterAndCopy (destStat, src, dest, opts) { if (opts.filter && !opts.filter(src, dest)) return const destParent = path.dirname(dest) - if (!fs.existsSync(destParent)) mkdirpSync(destParent) + if (!fs.existsSync(destParent)) mkdirsSync(destParent) return startCopy(destStat, src, dest, opts) } @@ -64,63 +64,51 @@ function mayCopyFile (srcStat, src, dest, opts) { } function copyFile (srcStat, src, dest, opts) { - if (typeof fs.copyFileSync === 'function') { - fs.copyFileSync(src, dest) - if (opts.preserveTimestamps && (srcStat.mode & 0o200) === 0) { - // Make sure the file is writable before setting the timestamp - // otherwise openSync fails with EPERM when invoked with 'r+' - // (through utimes call) - fs.chmodSync(dest, srcStat.mode | 0o200) - } - return setDestTimestampsAndMode(srcStat, src, dest, opts) - } - return copyFileFallback(srcStat, src, dest, opts) + fs.copyFileSync(src, dest) + if (opts.preserveTimestamps) handleTimestamps(srcStat.mode, src, dest) + return setDestMode(dest, srcStat.mode) } -function copyFileFallback (srcStat, src, dest, opts) { - const BUF_LENGTH = 64 * 1024 - const _buff = require('../util/buffer')(BUF_LENGTH) - - const fdr = fs.openSync(src, 'r') - const fdw = fs.openSync(dest, 'w') - let pos = 0 +function handleTimestamps (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)) makeFileWritable(dest, srcMode) + return setDestTimestamps(src, dest) +} - while (pos < srcStat.size) { - const bytesRead = fs.readSync(fdr, _buff, 0, BUF_LENGTH, pos) - fs.writeSync(fdw, _buff, 0, bytesRead) - pos += bytesRead - } +function fileIsNotWritable (srcMode) { + return (srcMode & 0o200) === 0 +} - setDestTimestampsAndMode(srcStat, src, fdw, opts) +function makeFileWritable (dest, srcMode) { + return setDestMode(dest, srcMode | 0o200) +} - fs.closeSync(fdr) - fs.closeSync(fdw) +function setDestMode (dest, srcMode) { + return fs.chmodSync(dest, srcMode) } -function setDestTimestampsAndMode (srcStat, src, dest, opts) { - const utimesSync = typeof dest === 'string' ? utimesMillisSync : fs.futimesSync - const chmodSync = typeof dest === 'string' ? fs.chmodSync : fs.fchmodSync - if (opts.preserveTimestamps) { - // 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) - const updatedSrcStat = fs.statSync(src) - utimesSync(dest, updatedSrcStat.atime, updatedSrcStat.mtime) - } - chmodSync(dest, srcStat.mode) +function setDestTimestamps (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) + const updatedSrcStat = fs.statSync(src) + return utimesMillisSync(dest, updatedSrcStat.atime, updatedSrcStat.mtime) } function onDir (srcStat, destStat, src, dest, opts) { - if (!destStat) return mkDirAndCopy(srcStat, src, dest, opts) + if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts) if (destStat && !destStat.isDirectory()) { throw new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`) } return copyDir(src, dest, opts) } -function mkDirAndCopy (srcStat, src, dest, opts) { +function mkDirAndCopy (srcMode, src, dest, opts) { fs.mkdirSync(dest) copyDir(src, dest, opts) - return fs.chmodSync(dest, srcStat.mode) + return setDestMode(dest, srcMode) } function copyDir (src, dest, opts) { diff --git a/lib/copy/__tests__/copy-preserve-timestamp.test.js b/lib/copy/__tests__/copy-preserve-timestamp.test.js index 55c391dc..6f1336f4 100644 --- a/lib/copy/__tests__/copy-preserve-timestamp.test.js +++ b/lib/copy/__tests__/copy-preserve-timestamp.test.js @@ -66,8 +66,8 @@ describeIfPractical('copy() - preserve timestamp', () => { assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime(), 'different mtime values') assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime(), 'different atime values') } else { - assert.notStrictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime(), 'same mtime values') // the access time might actually be the same, so check only modification time + assert.notStrictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime(), 'same mtime values') } } } diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 20a524cb..328f1025 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -2,7 +2,7 @@ const fs = require('graceful-fs') const path = require('path') -const mkdirp = require('../mkdirs').mkdirs +const mkdirs = require('../mkdirs').mkdirs const pathExists = require('../path-exists').pathExists const utimesMillis = require('../util/utimes').utimesMillis const stat = require('../util/stat') @@ -43,7 +43,7 @@ function checkParentDir (destStat, src, dest, opts, cb) { pathExists(destParent, (err, dirExists) => { if (err) return cb(err) if (dirExists) return startCopy(destStat, src, dest, opts, cb) - mkdirp(destParent, err => { + mkdirs(destParent, err => { if (err) return cb(err) return startCopy(destStat, src, dest, opts, cb) }) @@ -92,64 +92,69 @@ function mayCopyFile (srcStat, src, dest, opts, cb) { } function copyFile (srcStat, src, dest, opts, cb) { - if (typeof fs.copyFile === 'function') { - return fs.copyFile(src, dest, err => { + fs.copyFile(src, dest, err => { + if (err) return cb(err) + if (opts.preserveTimestamps) return handleTimestampsAndMode(srcStat.mode, src, dest, cb) + return setDestMode(dest, srcStat.mode, cb) + }) +} + +function handleTimestampsAndMode (srcMode, src, dest, cb) { + // 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)) { + return makeFileWritable(dest, srcMode, err => { if (err) return cb(err) - if (opts.preserveTimestamps && (srcStat.mode & 0o200) === 0) { - // Make sure the file is writable before setting the timestamp - // otherwise openSync fails with EPERM when invoked with 'r+' - // (through utimes call) - return fs.chmod(dest, srcStat.mode | 0o200, (err) => { - if (err) return cb(err) - return setDestTimestampsAndMode(srcStat, src, dest, opts, cb) - }) - } - return setDestTimestampsAndMode(srcStat, src, dest, opts, cb) + return setDestTimestampsAndMode(srcMode, src, dest, cb) }) } - return copyFileFallback(srcStat, src, dest, opts, cb) + return setDestTimestampsAndMode(srcMode, src, dest, cb) } -function copyFileFallback (srcStat, src, dest, opts, cb) { - const rs = fs.createReadStream(src) - rs.on('error', err => cb(err)).once('open', () => { - // explicitly not setting srcStat mode - will do that last - const ws = fs.createWriteStream(dest) - ws.on('error', err => cb(err)) - .on('open', () => rs.pipe(ws)) - .once('close', () => setDestTimestampsAndMode(srcStat, src, dest, opts, cb)) +function fileIsNotWritable (srcMode) { + return (srcMode & 0o200) === 0 +} + +function makeFileWritable (dest, srcMode, cb) { + return setDestMode(dest, srcMode | 0o200, cb) +} + +function setDestTimestampsAndMode (srcMode, src, dest, cb) { + setDestTimestamps(src, dest, err => { + if (err) return cb(err) + return setDestMode(dest, srcMode, cb) }) } -function setDestTimestampsAndMode (srcStat, src, dest, opts, cb) { - if (opts.preserveTimestamps) { - // 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) - return fs.stat(src, (err, updatedSrcStat) => { - if (err) return cb(err) - return utimesMillis(dest, updatedSrcStat.atime, updatedSrcStat.mtime, (err2) => { - if (err2) return cb(err2) - return fs.chmod(dest, srcStat.mode, cb) - }) - }) - } - return fs.chmod(dest, srcStat.mode, cb) +function setDestMode (dest, srcMode, cb) { + return fs.chmod(dest, srcMode, cb) +} + +function setDestTimestamps (src, dest, cb) { + // 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) + fs.stat(src, (err, updatedSrcStat) => { + if (err) return cb(err) + return utimesMillis(dest, updatedSrcStat.atime, updatedSrcStat.mtime, cb) + }) } function onDir (srcStat, destStat, src, dest, opts, cb) { - if (!destStat) return mkDirAndCopy(srcStat, src, dest, opts, cb) + if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts, cb) if (destStat && !destStat.isDirectory()) { return cb(new Error(`Cannot overwrite non-directory '${dest}' with directory '${src}'.`)) } return copyDir(src, dest, opts, cb) } -function mkDirAndCopy (srcStat, src, dest, opts, cb) { +function mkDirAndCopy (srcMode, src, dest, opts, cb) { fs.mkdir(dest, err => { if (err) return cb(err) copyDir(src, dest, opts, err => { if (err) return cb(err) - return fs.chmod(dest, srcStat.mode, cb) + return setDestMode(dest, srcMode, cb) }) }) }