Skip to content

Commit

Permalink
Update dest atime after copy/copy-sync
Browse files Browse the repository at this point in the history
  • Loading branch information
@mbargiel
mbargiel committed May 14, 2019
1 parent f3a2eed commit b9edbbe
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 47 deletions.
47 changes: 30 additions & 17 deletions lib/copy-sync/__tests__/copy-sync-preserve-timestamp.test.js
View file
Expand Up @@ -3,7 +3,7 @@
const fs = require('../../')
const os = require('os')
const path = require('path')
const utimes = require('../../util/utimes')
const utimesSync = require('../../util/utimes').utimesMillisSync
const assert = require('assert')
const semver = require('semver')
const nodeVersion = process.version
Expand All @@ -19,21 +19,39 @@ const describeIfPractical = (process.arch === 'ia32' || nodeVersionMajor < 8) ?
describeIfPractical('copySync() - preserveTimestamps option', () => {
let TEST_DIR, SRC, DEST, FILES

beforeEach(done => {
TEST_DIR = path.join(os.tmpdir(), 'fs-extra', 'copy-sync-preserve-time')
function setupFixture (readonly) {
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)))
done()
})
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)
if (readonly) {
fs.chmodSync(filePath, 0o444)
}
})
}

afterEach(done => fs.remove(TEST_DIR, done))

describe('> when preserveTimestamps option is true', () => {
it('should have the same timestamps on copy', () => {
fs.copySync(SRC, DEST, { preserveTimestamps: true })
FILES.forEach(testFile({ preserveTimestamps: true }))
;[
{ subcase: 'writable', readonly: false },
{ subcase: 'readonly', readonly: true }
].forEach(params => {
describe(`>> with ${params.subcase} source files`, () => {
beforeEach(() => setupFixture(params.readonly))

it('should have the same timestamps on copy', () => {
fs.copySync(SRC, DEST, { preserveTimestamps: true })
FILES.forEach(testFile({ preserveTimestamps: true }))
})
})
})
})

Expand All @@ -44,14 +62,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
Expand Down
16 changes: 13 additions & 3 deletions lib/copy-sync/copy-sync.js
View file
Expand Up @@ -68,10 +68,17 @@ function mayCopyFile (srcStat, src, dest, opts) {
function copyFile (srcStat, src, dest, opts) {
if (typeof fs.copyFileSync === 'function') {
fs.copyFileSync(src, dest)
fs.chmodSync(dest, srcStat.mode)
if (opts.preserveTimestamps) {
return utimesSync(dest, srcStat.atime, srcStat.mtime)
// Make sure the file is writable first
if ((srcStat.mode & 0o200) === 0) {
fs.chmodSync(dest, srcStat.mode | 0o200)
}
// 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)
}
fs.chmodSync(dest, srcStat.mode)
return
}
return copyFileFallback(srcStat, src, dest, opts)
Expand All @@ -91,7 +98,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)
Expand Down
50 changes: 32 additions & 18 deletions lib/copy/__tests__/copy-preserve-timestamp.test.js
View file
Expand Up @@ -4,7 +4,7 @@ const fs = require('../../')
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 semver = require('semver')
const nodeVersion = process.version
Expand All @@ -20,22 +20,41 @@ const describeIfPractical = (process.arch === 'ia32' || nodeVersionMajor < 8) ?
describeIfPractical('copy() - preserve timestamp', () => {
let TEST_DIR, SRC, DEST, FILES

beforeEach(done => {
function setupFixture (readonly) {
TEST_DIR = path.join(os.tmpdir(), 'fs-extra', '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)))
done()
})
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)
if (readonly) {
fs.chmodSync(filePath, 0o444)
}
})
}

afterEach(done => fs.remove(TEST_DIR, done))

describe('> when timestamp option is true', () => {
it('should have the same timestamps on copy', done => {
copy(SRC, DEST, { preserveTimestamps: true }, () => {
FILES.forEach(testFile({ preserveTimestamps: true }))
done()
describe('> when preserveTimestamps option is true', () => {
;[
{ subcase: 'writable', readonly: false },
{ subcase: 'readonly', readonly: true }
].forEach(params => {
describe(`>> with ${params.subcase} source files`, () => {
beforeEach(() => setupFixture(params.readonly))

it('should have the same timestamps on copy', done => {
copy(SRC, DEST, { preserveTimestamps: true }, (err) => {
if (err) return done(err)
FILES.forEach(testFile({ preserveTimestamps: true }))
done()
})
})
})
})
})
Expand All @@ -47,14 +66,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
Expand Down
33 changes: 24 additions & 9 deletions lib/copy/copy.js
View file
Expand Up @@ -97,7 +97,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)
Expand All @@ -109,18 +109,33 @@ 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) {
fs.chmod(dest, srcStat.mode, err => {
function setDestModeAndTimestamps (srcStat, src, dest, opts, cb) {
if (!opts.preserveTimestamps) {
return fs.chmod(dest, srcStat.mode, cb)
}

const next = (err) => {
if (err) return cb(err)
if (opts.preserveTimestamps) {
return utimes(dest, srcStat.atime, srcStat.mtime, cb)
}
return 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, (err) => {
if (err) return cb(err)
return fs.chmod(dest, srcStat.mode, cb)
})
})
}

if ((srcStat.mode & 0o200) === 0) {
// Make sure the file is writable first
return fs.chmod(dest, srcStat.mode | 0o200, next)
}
return next()
}

function onDir (srcStat, destStat, src, dest, opts, cb) {
Expand Down

0 comments on commit b9edbbe

Please sign in to comment.