From fa64be8648718721648f37d320a467aab038d945 Mon Sep 17 00:00:00 2001 From: Ryan Zimmerman Date: Wed, 29 Jul 2020 15:16:06 -0400 Subject: [PATCH] BREAKING: Ensure existing dest correct type/target in ensureLink/Symlink Fixes #786 Fixes #870 --- lib/ensure/__tests__/link.test.js | 38 ++------------- lib/ensure/__tests__/symlink.test.js | 72 ++++------------------------ lib/ensure/link.js | 17 ++++--- lib/ensure/symlink.js | 49 +++++++++++++------ lib/util/stat.js | 3 +- 5 files changed, 58 insertions(+), 121 deletions(-) diff --git a/lib/ensure/__tests__/link.test.js b/lib/ensure/__tests__/link.test.js index b78313e55..83dc869b0 100644 --- a/lib/ensure/__tests__/link.test.js +++ b/lib/ensure/__tests__/link.test.js @@ -26,6 +26,8 @@ describe('fse-ensure-link', () => { [['./foo.txt', './alpha/link.txt'], 'file-error', 'file-success'], [['./foo.txt', './alpha/beta/link.txt'], 'file-error', 'file-success'], [['./foo.txt', './alpha/beta/gamma/link.txt'], 'file-error', 'file-success'], + [['./foo.txt', './link-foo.txt'], 'file-error', 'file-success'], + [['./dir-foo/foo.txt', './link-foo.txt'], 'file-error', 'file-error'], [['./missing.txt', './link.txt'], 'file-error', 'file-error'], [['./missing.txt', './missing-dir/link.txt'], 'file-error', 'file-error'], [['./foo.txt', './link.txt'], 'file-success', 'file-success'], @@ -34,7 +36,7 @@ describe('fse-ensure-link', () => { [['../foo.txt', './link.txt'], 'file-error', 'file-error'], [['../dir-foo/foo.txt', './link.txt'], 'file-error', 'file-error'], // error is thrown if destination path exists - [['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-dest-exists'], + [['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-error'], [[path.resolve(path.join(TEST_DIR, './foo.txt')), './link.txt'], 'file-success', 'file-success'], [[path.resolve(path.join(TEST_DIR, './dir-foo/foo.txt')), './link.txt'], 'file-success', 'file-success'], [[path.resolve(path.join(TEST_DIR, './missing.txt')), './link.txt'], 'file-error', 'file-error'], @@ -55,6 +57,7 @@ describe('fse-ensure-link', () => { fse.mkdirsSync('dir-bar') fs.writeFileSync('dir-bar/bar.txt', 'dir-bar\n') fse.mkdirsSync('real-alpha/real-beta/real-gamma') + fs.linkSync('foo.txt', 'link-foo.txt') }) afterEach(done => fse.emptyDir(TEST_DIR, done)) @@ -107,23 +110,6 @@ describe('fse-ensure-link', () => { }) } - function fileDestExists (args, fn) { - const srcpath = args[0] - const dstpath = args[1] - - it(`should do nothing using src ${srcpath} and dst ${dstpath}`, done => { - const destinationContentBefore = fs.readFileSync(dstpath, 'utf8') - const callback = err => { - if (err) return done(err) - const destinationContentAfter = fs.readFileSync(dstpath, 'utf8') - assert.strictEqual(destinationContentBefore, destinationContentAfter) - return done() - } - args.push(callback) - return fn(...args) - }) - } - function fileSuccessSync (args, fn) { const srcpath = args[0] const dstpath = args[1] @@ -161,18 +147,6 @@ describe('fse-ensure-link', () => { }) } - function fileDestExistsSync (args, fn) { - const srcpath = args[0] - const dstpath = args[1] - - it(`should do nothing using src ${srcpath} and dst ${dstpath}`, () => { - const destinationContentBefore = fs.readFileSync(dstpath, 'utf8') - fn(...args) - const destinationContentAfter = fs.readFileSync(dstpath, 'utf8') - assert.strictEqual(destinationContentBefore, destinationContentAfter) - }) - } - describe('fs.link()', () => { const fn = fs.link tests.forEach(test => { @@ -181,7 +155,6 @@ describe('fse-ensure-link', () => { // const newBehavior = test[2] if (nativeBehavior === 'file-success') fileSuccess(args, fn) if (nativeBehavior === 'file-error') fileError(args, fn) - if (nativeBehavior === 'file-dest-exists') fileDestExists(args, fn) }) }) @@ -193,7 +166,6 @@ describe('fse-ensure-link', () => { const newBehavior = test[2] if (newBehavior === 'file-success') fileSuccess(args, fn) if (newBehavior === 'file-error') fileError(args, fn) - if (newBehavior === 'file-dest-exists') fileDestExists(args, fn) }) }) @@ -229,7 +201,6 @@ describe('fse-ensure-link', () => { // const newBehavior = test[2] if (nativeBehavior === 'file-success') fileSuccessSync(args, fn) if (nativeBehavior === 'file-error') fileErrorSync(args, fn) - if (nativeBehavior === 'file-dest-exists') fileDestExists(args, fn) }) }) @@ -241,7 +212,6 @@ describe('fse-ensure-link', () => { const newBehavior = test[2] if (newBehavior === 'file-success') fileSuccessSync(args, fn) if (newBehavior === 'file-error') fileErrorSync(args, fn) - if (newBehavior === 'file-dest-exists') fileDestExistsSync(args, fn) }) }) }) diff --git a/lib/ensure/__tests__/symlink.test.js b/lib/ensure/__tests__/symlink.test.js index 22a00430f..0c951efa3 100644 --- a/lib/ensure/__tests__/symlink.test.js +++ b/lib/ensure/__tests__/symlink.test.js @@ -32,10 +32,12 @@ describe('fse-ensure-symlink', () => { [['./foo.txt', './alpha/symlink.txt'], 'file-error', 'file-success'], [['./foo.txt', './alpha/beta/symlink.txt'], 'file-error', 'file-success'], [['./foo.txt', './alpha/beta/gamma/symlink.txt'], 'file-error', 'file-success'], + [['./foo.txt', './real-symlink.txt'], 'file-error', 'file-success'], + [['./dir-foo/foo.txt', './real-symlink.txt'], 'file-error', 'file-error'], [['./missing.txt', './symlink.txt'], 'file-broken', 'file-error'], [['./missing.txt', './missing-dir/symlink.txt'], 'file-error', 'file-error'], // error is thrown if destination path exists - [['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-dest-exists'], + [['./foo.txt', './dir-foo/foo.txt'], 'file-error', 'file-error'], [['./dir-foo', './symlink-dir-foo'], 'dir-success', 'dir-success'], [['../dir-bar', './dir-foo/symlink-dir-bar'], 'dir-success', 'dir-success'], [['./dir-bar', './dir-foo/symlink-dir-bar'], 'dir-broken', 'dir-success'], @@ -46,9 +48,11 @@ describe('fse-ensure-symlink', () => { [['./dir-foo', './alpha/dir-foo'], 'dir-error', 'dir-success'], [['./dir-foo', './alpha/beta/dir-foo'], 'dir-error', 'dir-success'], [['./dir-foo', './alpha/beta/gamma/dir-foo'], 'dir-error', 'dir-success'], + [['./dir-foo', './real-symlink-dir-foo'], 'dir-error', 'dir-success'], + [['./dir-bar', './real-symlink-dir-foo'], 'dir-error', 'dir-error'], [['./missing', './dir-foo/symlink-dir-missing'], 'dir-broken', 'dir-error'], // error is thrown if destination path exists - [['./dir-foo', './real-alpha/real-beta'], 'dir-error', 'dir-dest-exists'], + [['./dir-foo', './real-alpha/real-beta'], 'dir-error', 'dir-error'], [[path.resolve(path.join(TEST_DIR, './foo.txt')), './symlink.txt'], 'file-success', 'file-success'], [[path.resolve(path.join(TEST_DIR, './dir-foo/foo.txt')), './symlink.txt'], 'file-success', 'file-success'], [[path.resolve(path.join(TEST_DIR, './missing.txt')), './symlink.txt'], 'file-broken', 'file-error'], @@ -69,6 +73,8 @@ describe('fse-ensure-symlink', () => { fse.mkdirsSync('dir-bar') fs.writeFileSync('dir-bar/bar.txt', 'dir-bar\n') fse.mkdirsSync('real-alpha/real-beta/real-gamma') + fs.symlinkSync('foo.txt', 'real-symlink.txt') + fs.symlinkSync('dir-foo', 'real-symlink-dir-foo') }) afterEach(done => fse.emptyDir(TEST_DIR, done)) @@ -138,22 +144,6 @@ describe('fse-ensure-symlink', () => { }) } - function fileDestExists (args, fn) { - const srcpath = args[0] - const dstpath = args[1] - it(`should do nothing using src ${srcpath} and dst ${dstpath}`, done => { - const destinationContentBefore = fs.readFileSync(dstpath, 'utf8') - const callback = err => { - if (err) return done(err) - const destinationContentAfter = fs.readFileSync(dstpath, 'utf8') - assert.strictEqual(destinationContentBefore, destinationContentAfter) - return done() - } - args.push(callback) - return fn(...args) - }) - } - function dirSuccess (args, fn) { const srcpath = args[0] const dstpath = args[1] @@ -214,22 +204,6 @@ describe('fse-ensure-symlink', () => { }) } - function dirDestExists (args, fn) { - const srcpath = args[0] - const dstpath = args[1] - it(`should do nothing using src ${srcpath} and dst ${dstpath}`, done => { - const destinationContentBefore = fs.readdirSync(dstpath) - const callback = err => { - if (err) return done(err) - const destinationContentAfter = fs.readdirSync(dstpath) - assert.deepStrictEqual(destinationContentBefore, destinationContentAfter) - return done() - } - args.push(callback) - return fn(...args) - }) - } - function fileSuccessSync (args, fn) { const srcpath = args[0] const dstpath = args[1] @@ -280,17 +254,6 @@ describe('fse-ensure-symlink', () => { }) } - function fileDestExistsSync (args, fn) { - const srcpath = args[0] - const dstpath = args[1] - it(`should do nothing using src ${srcpath} and dst ${dstpath}`, () => { - const destinationContentBefore = fs.readFileSync(dstpath, 'utf8') - fn(...args) - const destinationContentAfter = fs.readFileSync(dstpath, 'utf8') - assert.strictEqual(destinationContentBefore, destinationContentAfter) - }) - } - function dirSuccessSync (args, fn) { const srcpath = args[0] const dstpath = args[1] @@ -341,17 +304,6 @@ describe('fse-ensure-symlink', () => { }) } - function dirDestExistsSync (args, fn) { - const srcpath = args[0] - const dstpath = args[1] - it(`should do nothing using src ${srcpath} and dst ${dstpath}`, () => { - const destinationContentBefore = fs.readdirSync(dstpath) - fn(...args) - const destinationContentAfter = fs.readdirSync(dstpath) - assert.deepStrictEqual(destinationContentBefore, destinationContentAfter) - }) - } - describe('fs.symlink()', () => { const fn = fs.symlink tests.forEach(test => { @@ -361,12 +313,10 @@ describe('fse-ensure-symlink', () => { if (nativeBehavior === 'file-success') fileSuccess(args, fn) if (nativeBehavior === 'file-broken') fileBroken(args, fn) if (nativeBehavior === 'file-error') fileError(args, fn) - if (nativeBehavior === 'file-dest-exists') fileDestExists(args, fn) args.push('dir') if (nativeBehavior === 'dir-success') dirSuccess(args, fn) if (nativeBehavior === 'dir-broken') dirBroken(args, fn) if (nativeBehavior === 'dir-error') dirError(args, fn) - if (nativeBehavior === 'dir-dest-exists') dirDestExists(args, fn) }) }) @@ -379,11 +329,9 @@ describe('fse-ensure-symlink', () => { if (newBehavior === 'file-success') fileSuccess(args, fn) if (newBehavior === 'file-broken') fileBroken(args, fn) if (newBehavior === 'file-error') fileError(args, fn) - if (newBehavior === 'file-dest-exists') fileDestExists(args, fn) if (newBehavior === 'dir-success') dirSuccess(args, fn) if (newBehavior === 'dir-broken') dirBroken(args, fn) if (newBehavior === 'dir-error') dirError(args, fn) - if (newBehavior === 'dir-dest-exists') dirDestExists(args, fn) }) }) @@ -419,12 +367,10 @@ describe('fse-ensure-symlink', () => { if (nativeBehavior === 'file-success') fileSuccessSync(args, fn) if (nativeBehavior === 'file-broken') fileBrokenSync(args, fn) if (nativeBehavior === 'file-error') fileErrorSync(args, fn) - if (nativeBehavior === 'file-dest-exists') fileDestExistsSync(args, fn) args.push('dir') if (nativeBehavior === 'dir-success') dirSuccessSync(args, fn) if (nativeBehavior === 'dir-broken') dirBrokenSync(args, fn) if (nativeBehavior === 'dir-error') dirErrorSync(args, fn) - if (nativeBehavior === 'dir-dest-exists') dirDestExistsSync(args, fn) }) }) @@ -437,11 +383,9 @@ describe('fse-ensure-symlink', () => { if (newBehavior === 'file-success') fileSuccessSync(args, fn) if (newBehavior === 'file-broken') fileBrokenSync(args, fn) if (newBehavior === 'file-error') fileErrorSync(args, fn) - if (newBehavior === 'file-dest-exists') fileDestExistsSync(args, fn) if (newBehavior === 'dir-success') dirSuccessSync(args, fn) if (newBehavior === 'dir-broken') dirBrokenSync(args, fn) if (newBehavior === 'dir-error') dirErrorSync(args, fn) - if (newBehavior === 'dir-dest-exists') dirDestExistsSync(args, fn) }) }) }) diff --git a/lib/ensure/link.js b/lib/ensure/link.js index 2cd419626..f6d67486b 100644 --- a/lib/ensure/link.js +++ b/lib/ensure/link.js @@ -5,6 +5,7 @@ const path = require('path') const fs = require('graceful-fs') const mkdir = require('../mkdirs') const pathExists = require('../path-exists').pathExists +const { areIdentical } = require('../util/stat') function createLink (srcpath, dstpath, callback) { function makeLink (srcpath, dstpath) { @@ -14,14 +15,13 @@ function createLink (srcpath, dstpath, callback) { }) } - pathExists(dstpath, (err, destinationExists) => { - if (err) return callback(err) - if (destinationExists) return callback(null) - fs.lstat(srcpath, (err) => { + fs.lstat(dstpath, (_, dstStat) => { + fs.lstat(srcpath, (err, srcStat) => { if (err) { err.message = err.message.replace('lstat', 'ensureLink') return callback(err) } + if (dstStat && areIdentical(srcStat, dstStat)) return callback(null) const dir = path.dirname(dstpath) pathExists(dir, (err, dirExists) => { @@ -37,11 +37,14 @@ function createLink (srcpath, dstpath, callback) { } function createLinkSync (srcpath, dstpath) { - const destinationExists = fs.existsSync(dstpath) - if (destinationExists) return undefined + let dstStat + try { + dstStat = fs.lstatSync(dstpath) + } catch {} try { - fs.lstatSync(srcpath) + const srcStat = fs.lstatSync(srcpath) + if (dstStat && areIdentical(srcStat, dstStat)) return } catch (err) { err.message = err.message.replace('lstat', 'ensureLink') throw err diff --git a/lib/ensure/symlink.js b/lib/ensure/symlink.js index fe68b7997..2b93052f0 100644 --- a/lib/ensure/symlink.js +++ b/lib/ensure/symlink.js @@ -2,7 +2,7 @@ const u = require('universalify').fromCallback const path = require('path') -const fs = require('graceful-fs') +const fs = require('../fs') const _mkdirs = require('../mkdirs') const mkdirs = _mkdirs.mkdirs const mkdirsSync = _mkdirs.mkdirsSync @@ -17,26 +17,38 @@ const symlinkTypeSync = _symlinkType.symlinkTypeSync const pathExists = require('../path-exists').pathExists +const { areIdentical } = require('../util/stat') + function createSymlink (srcpath, dstpath, type, callback) { callback = (typeof type === 'function') ? type : callback type = (typeof type === 'function') ? false : type - pathExists(dstpath, (err, destinationExists) => { + fs.lstat(dstpath, (err, stats) => { + if (!err && stats.isSymbolicLink()) { + Promise.all([ + fs.stat(srcpath), + fs.stat(dstpath) + ]).then(([srcStat, dstStat]) => { + if (areIdentical(srcStat, dstStat)) return callback(null) + _createSymlink(srcpath, dstpath, type, callback) + }) + } else _createSymlink(srcpath, dstpath, type, callback) + }) +} + +function _createSymlink (srcpath, dstpath, type, callback) { + symlinkPaths(srcpath, dstpath, (err, relative) => { if (err) return callback(err) - if (destinationExists) return callback(null) - symlinkPaths(srcpath, dstpath, (err, relative) => { + srcpath = relative.toDst + symlinkType(relative.toCwd, type, (err, type) => { if (err) return callback(err) - srcpath = relative.toDst - symlinkType(relative.toCwd, type, (err, type) => { + const dir = path.dirname(dstpath) + pathExists(dir, (err, dirExists) => { if (err) return callback(err) - const dir = path.dirname(dstpath) - pathExists(dir, (err, dirExists) => { + if (dirExists) return fs.symlink(srcpath, dstpath, type, callback) + mkdirs(dir, err => { if (err) return callback(err) - if (dirExists) return fs.symlink(srcpath, dstpath, type, callback) - mkdirs(dir, err => { - if (err) return callback(err) - fs.symlink(srcpath, dstpath, type, callback) - }) + fs.symlink(srcpath, dstpath, type, callback) }) }) }) @@ -44,8 +56,15 @@ function createSymlink (srcpath, dstpath, type, callback) { } function createSymlinkSync (srcpath, dstpath, type) { - const destinationExists = fs.existsSync(dstpath) - if (destinationExists) return undefined + let stats + try { + stats = fs.lstatSync(dstpath) + } catch {} + if (stats && stats.isSymbolicLink()) { + const srcStat = fs.statSync(srcpath) + const dstStat = fs.statSync(dstpath) + if (areIdentical(srcStat, dstStat)) return + } const relative = symlinkPathsSync(srcpath, dstpath) srcpath = relative.toDst diff --git a/lib/util/stat.js b/lib/util/stat.js index 0b1c1b096..e4e6a4cef 100644 --- a/lib/util/stat.js +++ b/lib/util/stat.js @@ -135,5 +135,6 @@ module.exports = { checkPathsSync, checkParentPaths, checkParentPathsSync, - isSrcSubdir + isSrcSubdir, + areIdentical }