From fb89a29ecaf87433080559957bfa45c799dac869 Mon Sep 17 00:00:00 2001 From: Maxime Bargiel Date: Tue, 30 Jul 2019 22:31:52 -0400 Subject: [PATCH] Workaround fs.utimes and util/utimeMillis catch-22 --- lib/copy-sync/copy-sync.js | 35 +++++++++++++++-------------------- lib/copy/copy.js | 35 +++++++++++++++-------------------- 2 files changed, 30 insertions(+), 40 deletions(-) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 9786111d..466eeb45 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -5,6 +5,7 @@ const fs = require('fs') const path = require('path') const mkdirpSync = require('../mkdirs').mkdirsSync +const utimesMillisSync = require('../util/utimes').utimesMillisSync const stat = require('../util/stat') function copySync (src, dest, opts) { @@ -67,7 +68,13 @@ function mayCopyFile (srcStat, src, dest, opts) { function copyFile (srcStat, src, dest, opts) { if (typeof fs.copyFileSync === 'function') { fs.copyFileSync(src, dest) - return setDestTimestampsAndMode(srcStat, src, dest, opts) + 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, dest, opts) } return copyFileFallback(srcStat, src, dest, opts) } @@ -77,7 +84,7 @@ function copyFileFallback (srcStat, src, dest, opts) { const _buff = require('../util/buffer')(BUF_LENGTH) const fdr = fs.openSync(src, 'r') - const fdw = fs.openSync(dest, 'w', srcStat.mode) + const fdw = fs.openSync(dest, 'w') let pos = 0 while (pos < srcStat.size) { @@ -86,31 +93,19 @@ function copyFileFallback (srcStat, src, dest, opts) { pos += bytesRead } - setFdDestTimestampsAndMode(srcStat, src, fdw, opts) + setDestTimestampsAndMode(srcStat, fdw, opts) fs.closeSync(fdr) fs.closeSync(fdw) } -function setDestTimestampsAndMode (srcStat, src, dest, opts) { - if ((srcStat.mode & 0o200) === 0) { - // Make sure the file is writable before setting the timestamp - // otherwise openSync below fails with EPERM when invoked with 'r+' - fs.chmodSync(dest, srcStat.mode | 0o200) - } - const fdw = fs.openSync(dest, 'r+') - setFdDestTimestampsAndMode(srcStat, src, fdw, opts) - fs.closeSync(fdw) -} - -function setFdDestTimestampsAndMode (srcStat, src, fdw, opts) { +function setDestTimestampsAndMode (srcStat, dest, opts) { + const utimesSync = typeof dest === 'string' ? utimesMillisSync : fs.futimesSync + const chmodSync = typeof dest === 'string' ? fs.chmodSync : fs.fchmodSync if (opts.preserveTimestamps) { - // Cannot rely on previously fetched srcStat because atime is updated by - // the read operation: https://nodejs.org/docs/latest-v8.x/api/fs.html#fs_stat_time_values - const updatedSrcStat = fs.statSync(src) - fs.futimesSync(fdw, updatedSrcStat.atime, updatedSrcStat.mtime) + utimesSync(dest, srcStat.atime, srcStat.mtime) } - fs.fchmodSync(fdw, srcStat.mode) + chmodSync(dest, srcStat.mode) } function onDir (srcStat, destStat, src, dest, opts) { diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 14ffc44e..38a6037c 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -6,7 +6,7 @@ const fs = require('fs') const path = require('path') const mkdirp = require('../mkdirs').mkdirs const pathExists = require('../path-exists').pathExists -const utimes = require('../util/utimes').utimesMillis +const utimesMillis = require('../util/utimes').utimesMillis const stat = require('../util/stat') function copy (src, dest, opts, cb) { @@ -97,17 +97,16 @@ function copyFile (srcStat, src, dest, opts, cb) { if (typeof fs.copyFile === 'function') { return fs.copyFile(src, dest, err => { if (err) return cb(err) - if ((srcStat.mode & 0o200) === 0) { + 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 fs.chmod(dest, srcStat.mode | 0o200, (err) => { + if (err) return cb(err) + return setDestTimestampsAndMode(srcStat, dest, opts, cb) + }) } - return setDestTimestampsAndMode(srcStat, src, dest, opts, cb) + return setDestTimestampsAndMode(srcStat, dest, opts, cb) }) } return copyFileFallback(srcStat, src, dest, opts, cb) @@ -120,24 +119,20 @@ function copyFileFallback (srcStat, src, dest, opts, cb) { const ws = fs.createWriteStream(dest) ws.on('error', err => cb(err)) .on('open', () => rs.pipe(ws)) - .once('close', () => setDestTimestampsAndMode(srcStat, src, dest, opts, cb)) + .once('close', () => setDestTimestampsAndMode(srcStat, dest, opts, cb)) }) } -function setDestTimestampsAndMode (srcStat, src, dest, opts, cb) { - if (!opts.preserveTimestamps) { - return fs.chmod(dest, srcStat.mode, cb) - } - - // Cannot rely on previously fetched srcStat because atime is updated by - // the read operation: https://nodejs.org/docs/latest-v8.x/api/fs.html#fs_stat_time_values - return fs.stat(src, (err, srcStat) => { - if (err) return cb(err) +function setDestTimestampsAndMode (srcStat, dest, opts, cb) { + const utimes = typeof dest === 'string' ? utimesMillis : fs.futimes + const chmod = typeof dest === 'string' ? fs.chmod : fs.fchmod + if (opts.preserveTimestamps) { return utimes(dest, srcStat.atime, srcStat.mtime, (err) => { if (err) return cb(err) - return fs.chmod(dest, srcStat.mode, cb) + return chmod(dest, srcStat.mode, cb) }) - }) + } + return chmod(dest, srcStat.mode, cb) } function onDir (srcStat, destStat, src, dest, opts, cb) {