Skip to content

Commit

Permalink
remove unnecessary pathsAreIdentical()
Browse files Browse the repository at this point in the history
  • Loading branch information
manidlou committed May 18, 2018
1 parent a7b9773 commit ec3dea8
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 32 deletions.
Expand Up @@ -23,6 +23,8 @@ describe('+ copySync() - prevent copying identical files and dirs', () => {
it('should return an error if src and dest are the same', () => {
const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy_sync')
fs.ensureFileSync(fileSrc)

try {
fs.copySync(fileSrc, fileDest)
} catch (err) {
Expand Down
Expand Up @@ -314,10 +314,11 @@ describe('+ copySync() - prevent copying into itself', () => {
try {
fs.copySync(srcLink, destLink)
} catch (err) {
assert(err)
assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`)
} finally {
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, resolvedDestPath)
}
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, src)
})

it('should error when resolved src path is a subdir of resolved dest path', () => {
Expand All @@ -335,9 +336,10 @@ describe('+ copySync() - prevent copying into itself', () => {
fs.copySync(srcLink, destLink)
} catch (err) {
assert.strictEqual(err.message, `Cannot overwrite '${dest}' with '${srcInDest}'.`)
} finally {
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, dest)
}
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, dest)
})
})
})
Expand Down
16 changes: 3 additions & 13 deletions lib/copy-sync/copy-sync.js
Expand Up @@ -141,7 +141,9 @@ function onLink (destStat, src, dest, opts) {
if (opts.dereference) {
resolvedDest = path.resolve(process.cwd(), resolvedDest)
}
if (pathsAreIdentical(resolvedSrc, resolvedDest)) return
if (isSrcSubdir(resolvedSrc, resolvedDest)) {
throw new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`)
}

// prevent copy if src is a subdir of dest since unlinking
// dest in this case would result in removing src contents
Expand All @@ -168,17 +170,6 @@ function isSrcSubdir (src, dest) {
}, true)
}

function pathsAreIdentical (src, dest) {
const os = process.platform
const resolvedSrc = path.resolve(src)
const resolvedDest = path.resolve(dest)
// case-insensitive paths
if (os === 'darwin' || os === 'win32') {
return resolvedSrc.toLowerCase() === resolvedDest.toLowerCase()
}
return resolvedSrc === resolvedDest
}

function checkStats (src, dest) {
const srcStat = fs.statSync(src)
let destStat
Expand All @@ -192,7 +183,6 @@ function checkStats (src, dest) {
}

function checkPaths (src, dest) {
if (pathsAreIdentical(src, dest)) throw new Error('Source and destination must not be the same.')
const {srcStat, destStat} = checkStats(src, dest)
if (srcStat && destStat && destStat !== notExist && srcStat.ino === destStat.ino) {
throw new Error('Source and destination must not be the same.')
Expand Down
1 change: 1 addition & 0 deletions lib/copy/__tests__/copy-prevent-copying-identical.test.js
Expand Up @@ -23,6 +23,7 @@ describe('+ copy() - prevent copying identical files and dirs', () => {
it('should return an error if src and dest are the same', done => {
const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy')
const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
fs.ensureFileSync(fileSrc)

fs.copy(fileSrc, fileDest, err => {
assert.strictEqual(err.message, 'Source and destination must not be the same.')
Expand Down
4 changes: 3 additions & 1 deletion lib/copy/__tests__/copy-prevent-copying-into-itself.test.js
Expand Up @@ -316,7 +316,9 @@ describe('+ copy() - prevent copying into itself', () => {
fs.symlinkSync(resolvedDestPath, destLink, 'dir')

fs.copy(srcLink, destLink, err => {
assert(err)
assert.strictEqual(err.message, `Cannot copy '${src}' to a subdirectory of itself, '${resolvedDestPath}'.`)
const destln = fs.readlinkSync(destLink)
assert.strictEqual(destln, resolvedDestPath)
done()
})
})
Expand Down
2 changes: 2 additions & 0 deletions lib/copy/__tests__/copy.test.js
Expand Up @@ -24,6 +24,8 @@ describe('fs-extra', () => {
it('should return an error if src and dest are the same', done => {
const fileSrc = path.join(TEST_DIR, 'TEST_fs-extra_copy')
const fileDest = path.join(TEST_DIR, 'TEST_fs-extra_copy')
fse.ensureFileSync(fileSrc)

fse.copy(fileSrc, fileDest, err => {
assert.strictEqual(err.message, 'Source and destination must not be the same.')
done()
Expand Down
13 changes: 0 additions & 13 deletions lib/copy/copy.js
Expand Up @@ -186,7 +186,6 @@ function onLink (destStat, src, dest, opts, cb) {
if (opts.dereference) {
resolvedDest = path.resolve(process.cwd(), resolvedDest)
}
if (pathsAreIdentical(resolvedSrc, resolvedDest)) return cb()
if (isSrcSubdir(resolvedSrc, resolvedDest)) {
return cb(new Error(`Cannot copy '${resolvedSrc}' to a subdirectory of itself, '${resolvedDest}'.`))
}
Expand Down Expand Up @@ -220,17 +219,6 @@ function isSrcSubdir (src, dest) {
}, true)
}

function pathsAreIdentical (src, dest) {
const os = process.platform
const resolvedSrc = path.resolve(src)
const resolvedDest = path.resolve(dest)
// case-insensitive paths
if (os === 'darwin' || os === 'win32') {
return resolvedSrc.toLowerCase() === resolvedDest.toLowerCase()
}
return resolvedSrc === resolvedDest
}

function checkStats (src, dest, cb) {
fs.stat(src, (err, srcStat) => {
if (err) return cb(err)
Expand All @@ -245,7 +233,6 @@ function checkStats (src, dest, cb) {
}

function checkPaths (src, dest, cb) {
if (pathsAreIdentical(src, dest)) return cb(new Error('Source and destination must not be the same.'))
checkStats(src, dest, (err, stats) => {
if (err) return cb(err)
const {srcStat, destStat} = stats
Expand Down

0 comments on commit ec3dea8

Please sign in to comment.