Update dest atime after copy/copy-sync

jprichardson · Oct 15, 2018 · 518036c · 518036c
1 parent 287f234
commit 518036c
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 25 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
@@ -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
+      fs.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
@@ -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
@@ -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
+      fs.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
@@ -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()
   })