From ca81e73ebe3c6a9590a8d7790aca4e2280341343 Mon Sep 17 00:00:00 2001 From: Maxime Bargiel Date: Fri, 12 Oct 2018 15:30:13 -0400 Subject: [PATCH] Update dest atime after copy/copy-sync --- .../__tests__/copy-sync-preserve-time.test.js | 24 ++++++++++--------- lib/copy-sync/copy-sync.js | 10 ++++++-- lib/copy/__tests__/copy-preserve-time.test.js | 22 +++++++++-------- lib/copy/copy.js | 13 ++++++---- 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js index c190da70..a70c8ea7 100644 --- a/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js +++ b/lib/copy-sync/__tests__/copy-sync-preserve-time.test.js @@ -3,7 +3,7 @@ const fs = require(process.cwd()) const os = require('os') const path = require('path') -const utimes = require('../../util/utimes') +const utimesSync = require('../../util/utimes').utimesMillisSync const assert = require('assert') const nodeVersion = process.versions.node const nodeVersionMajor = parseInt(nodeVersion.split('.')[0], 10) @@ -19,11 +19,18 @@ describeIfPractical('copySync() - preserveTimestamps option', () => { let TEST_DIR, SRC, DEST, FILES beforeEach(done => { - TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-preserve-time') + TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-preserve-timestamp') SRC = path.join(TEST_DIR, 'src') DEST = path.join(TEST_DIR, 'dest') FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')] - FILES.forEach(f => fs.ensureFileSync(path.join(SRC, f))) + const timestamp = Date.now() / 1000 - 5 + FILES.forEach(f => { + const filePath = path.join(SRC, f) + fs.ensureFileSync(filePath) + // rewind timestamps to make sure that coarser OS timestamp resolution + // does not alter results + utimesSync(filePath, timestamp, timestamp) + }) done() }) @@ -43,14 +50,9 @@ describeIfPractical('copySync() - preserveTimestamps option', () => { const fromStat = fs.statSync(a) const toStat = fs.statSync(b) if (options.preserveTimestamps) { - // https://github.com/nodejs/io.js/issues/2069 - if (process.platform !== 'win32') { - assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) - assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime()) - } else { - assert.strictEqual(toStat.mtime.getTime(), utimes.timeRemoveMillis(fromStat.mtime.getTime())) - assert.strictEqual(toStat.atime.getTime(), utimes.timeRemoveMillis(fromStat.atime.getTime())) - } + // Windows sub-second precision fixed: https://github.com/nodejs/io.js/issues/2069 + assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) + assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime()) } else { assert.notStrictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) // the access time might actually be the same, so check only modification time diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 14ad9939..b849d31f 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -66,7 +66,10 @@ function copyFile (srcStat, src, dest, opts) { fs.copyFileSync(src, dest) fs.chmodSync(dest, srcStat.mode) if (opts.preserveTimestamps) { - return utimesSync(dest, srcStat.atime, srcStat.mtime) + // 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) + utimesSync(dest, updatedSrcStat.atime, updatedSrcStat.mtime) } return } @@ -87,7 +90,10 @@ function copyFileFallback (srcStat, src, dest, opts) { pos += bytesRead } - if (opts.preserveTimestamps) fs.futimesSync(fdw, srcStat.atime, srcStat.mtime) + if (opts.preserveTimestamps) { + const updatedSrcStat = fs.statSync(src) + fs.futimesSync(dest, updatedSrcStat.atime, updatedSrcStat.mtime) + } fs.closeSync(fdr) fs.closeSync(fdw) diff --git a/lib/copy/__tests__/copy-preserve-time.test.js b/lib/copy/__tests__/copy-preserve-time.test.js index ae83ab4c..a5b63150 100644 --- a/lib/copy/__tests__/copy-preserve-time.test.js +++ b/lib/copy/__tests__/copy-preserve-time.test.js @@ -4,7 +4,7 @@ const fs = require(process.cwd()) const os = require('os') const path = require('path') const copy = require('../copy') -const utimes = require('../../util/utimes') +const utimesSync = require('../../util/utimes').utimesMillisSync const assert = require('assert') const nodeVersion = process.versions.node const nodeVersionMajor = parseInt(nodeVersion.split('.')[0], 10) @@ -24,7 +24,14 @@ describeIfPractical('copy() - preserve timestamp', () => { SRC = path.join(TEST_DIR, 'src') DEST = path.join(TEST_DIR, 'dest') FILES = ['a-file', path.join('a-folder', 'another-file'), path.join('a-folder', 'another-folder', 'file3')] - FILES.forEach(f => fs.ensureFileSync(path.join(SRC, f))) + const timestamp = Date.now() / 1000 - 5 + FILES.forEach(f => { + const filePath = path.join(SRC, f) + fs.ensureFileSync(filePath) + // rewind timestamps to make sure that coarser OS timestamp resolution + // does not alter results + utimesSync(filePath, timestamp, timestamp) + }) done() }) @@ -46,14 +53,9 @@ describeIfPractical('copy() - preserve timestamp', () => { const fromStat = fs.statSync(a) const toStat = fs.statSync(b) if (options.preserveTimestamps) { - // https://github.com/nodejs/io.js/issues/2069 - if (process.platform !== 'win32') { - assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) - assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime()) - } else { - assert.strictEqual(toStat.mtime.getTime(), utimes.timeRemoveMillis(fromStat.mtime.getTime())) - assert.strictEqual(toStat.atime.getTime(), utimes.timeRemoveMillis(fromStat.atime.getTime())) - } + // Windows sub-second precision fixed: https://github.com/nodejs/io.js/issues/2069 + assert.strictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) + assert.strictEqual(toStat.atime.getTime(), fromStat.atime.getTime()) } else { assert.notStrictEqual(toStat.mtime.getTime(), fromStat.mtime.getTime()) // the access time might actually be the same, so check only modification time diff --git a/lib/copy/copy.js b/lib/copy/copy.js index 3dfbc540..4d842ad0 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -95,7 +95,7 @@ function copyFile (srcStat, src, dest, opts, cb) { if (typeof fs.copyFile === 'function') { return fs.copyFile(src, dest, err => { if (err) return cb(err) - return setDestModeAndTimestamps(srcStat, dest, opts, cb) + return setDestModeAndTimestamps(srcStat, src, dest, opts, cb) }) } return copyFileFallback(srcStat, src, dest, opts, cb) @@ -107,15 +107,20 @@ function copyFileFallback (srcStat, src, dest, opts, cb) { const ws = fs.createWriteStream(dest, { mode: srcStat.mode }) ws.on('error', err => cb(err)) .on('open', () => rs.pipe(ws)) - .once('close', () => setDestModeAndTimestamps(srcStat, dest, opts, cb)) + .once('close', () => setDestModeAndTimestamps(srcStat, src, dest, opts, cb)) }) } -function setDestModeAndTimestamps (srcStat, dest, opts, cb) { +function setDestModeAndTimestamps (srcStat, src, dest, opts, cb) { fs.chmod(dest, srcStat.mode, err => { if (err) return cb(err) if (opts.preserveTimestamps) { - return utimes(dest, srcStat.atime, srcStat.mtime, 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) + return utimes(dest, srcStat.atime, srcStat.mtime, cb) + }) } return cb() })