From 384aae04e8ea720ef9c05d21e0de1af3cdda3600 Mon Sep 17 00:00:00 2001 From: Maxime Bargiel Date: Mon, 16 Jul 2018 20:57:29 -0400 Subject: [PATCH] copy*: Set new directory mode only after copying files (MacOS fix) (#600) Fixes #599 --- .../__tests__/copy-sync-readonly-dir.test.js | 52 +++++++++++++++++ lib/copy-sync/copy-sync.js | 6 +- lib/copy/__tests__/copy-readonly-dir.test.js | 56 +++++++++++++++++++ lib/copy/copy.js | 6 +- 4 files changed, 114 insertions(+), 6 deletions(-) create mode 100644 lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js create mode 100644 lib/copy/__tests__/copy-readonly-dir.test.js diff --git a/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js b/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js new file mode 100644 index 00000000..b5648043 --- /dev/null +++ b/lib/copy-sync/__tests__/copy-sync-readonly-dir.test.js @@ -0,0 +1,52 @@ +'use strict' + +// relevant: https://github.com/jprichardson/node-fs-extra/issues/599 + +const fs = require(process.cwd()) +const os = require('os') +const fse = require('../../') +const path = require('path') +const assert = require('assert') +const klawSync = require('klaw-sync') + +/* global afterEach, beforeEach, describe, it */ + +let TEST_DIR = '' + +const FILES = [ + path.join('dir1', 'file1.txt'), + path.join('dir1', 'dir2', 'file2.txt'), + path.join('dir1', 'dir2', 'dir3', 'file3.txt') +] + +describe('+ copySync() - copy a readonly directory with content', () => { + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'test', 'fs-extra', 'copy-readonly-dir') + fse.emptyDir(TEST_DIR, done) + }) + + afterEach(done => { + klawSync(TEST_DIR).forEach(data => fs.chmodSync(data.path, 0o777)) + fse.remove(TEST_DIR, done) + }) + + describe('> when src is readonly directory with content', () => { + it('should copy successfully', () => { + FILES.forEach(file => { + fs.outputFileSync(path.join(TEST_DIR, file), file) + }) + const sourceDir = path.join(TEST_DIR, 'dir1') + const sourceHierarchy = klawSync(sourceDir) + sourceHierarchy.forEach(source => fs.chmodSync(source.path, source.stats.isDirectory() ? 0o555 : 0o444)) + + const targetDir = path.join(TEST_DIR, 'target') + fse.copySync(sourceDir, targetDir) + + // Make sure copy was made and mode was preserved + assert(fs.existsSync(targetDir)) + const targetHierarchy = klawSync(targetDir) + assert(targetHierarchy.length === sourceHierarchy.length) + targetHierarchy.forEach(target => assert(target.stats.mode === target.stats.isDirectory() ? 0o555 : 0o444)) + }) + }) +}) diff --git a/lib/copy-sync/copy-sync.js b/lib/copy-sync/copy-sync.js index 775647db..14ad9939 100644 --- a/lib/copy-sync/copy-sync.js +++ b/lib/copy-sync/copy-sync.js @@ -102,9 +102,9 @@ function onDir (srcStat, destStat, src, dest, opts) { } function mkDirAndCopy (srcStat, src, dest, opts) { - fs.mkdirSync(dest, srcStat.mode) - fs.chmodSync(dest, srcStat.mode) - return copyDir(src, dest, opts) + fs.mkdirSync(dest) + copyDir(src, dest, opts) + return fs.chmodSync(dest, srcStat.mode) } function copyDir (src, dest, opts) { diff --git a/lib/copy/__tests__/copy-readonly-dir.test.js b/lib/copy/__tests__/copy-readonly-dir.test.js new file mode 100644 index 00000000..99a3d9af --- /dev/null +++ b/lib/copy/__tests__/copy-readonly-dir.test.js @@ -0,0 +1,56 @@ +'use strict' + +// relevant: https://github.com/jprichardson/node-fs-extra/issues/599 + +const fs = require(process.cwd()) +const os = require('os') +const fse = require('../../') +const path = require('path') +const assert = require('assert') +const klawSync = require('klaw-sync') + +/* global afterEach, beforeEach, describe, it */ + +let TEST_DIR = '' + +const FILES = [ + path.join('dir1', 'file1.txt'), + path.join('dir1', 'dir2', 'file2.txt'), + path.join('dir1', 'dir2', 'dir3', 'file3.txt') +] + +describe('+ copy() - copy a readonly directory with content', () => { + beforeEach(done => { + TEST_DIR = path.join(os.tmpdir(), 'test', 'fs-extra', 'copy-readonly-dir') + fse.emptyDir(TEST_DIR, done) + }) + + afterEach(done => { + klawSync(TEST_DIR).forEach(data => fs.chmodSync(data.path, 0o777)) + fse.remove(TEST_DIR, done) + }) + + describe('> when src is readonly directory with content', () => { + it('should copy successfully', done => { + FILES.forEach(file => { + fs.outputFileSync(path.join(TEST_DIR, file), file) + }) + + const sourceDir = path.join(TEST_DIR, 'dir1') + const sourceHierarchy = klawSync(sourceDir) + sourceHierarchy.forEach(source => fs.chmodSync(source.path, source.stats.isDirectory() ? 0o555 : 0o444)) + + const targetDir = path.join(TEST_DIR, 'target') + fse.copy(sourceDir, targetDir, err => { + assert.ifError(err) + + // Make sure copy was made and mode was preserved + assert(fs.existsSync(targetDir)) + const targetHierarchy = klawSync(targetDir) + assert(targetHierarchy.length === sourceHierarchy.length) + targetHierarchy.forEach(target => assert(target.stats.mode === target.stats.isDirectory() ? 0o555 : 0o444)) + done() + }) + }) + }) +}) diff --git a/lib/copy/copy.js b/lib/copy/copy.js index af91a53e..3dfbc540 100644 --- a/lib/copy/copy.js +++ b/lib/copy/copy.js @@ -130,11 +130,11 @@ function onDir (srcStat, destStat, src, dest, opts, cb) { } function mkDirAndCopy (srcStat, src, dest, opts, cb) { - fs.mkdir(dest, srcStat.mode, err => { + fs.mkdir(dest, err => { if (err) return cb(err) - fs.chmod(dest, srcStat.mode, err => { + copyDir(src, dest, opts, err => { if (err) return cb(err) - return copyDir(src, dest, opts, cb) + return fs.chmod(dest, srcStat.mode, cb) }) }) }