Skip to content

Commit

Permalink
BREAKING: Ensure existing dest correct type/target in ensureLink/Syml…
Browse files Browse the repository at this point in the history 
…ink (#826)

Fixes #786
Fixes #870
  • Loading branch information
@RyanZim
RyanZim committed Apr 2, 2021
1 parent c8bd383 commit 4523526
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 121 deletions.
38 changes: 4 additions & 34 deletions lib/ensure/__tests__/link.test.js
View file
Expand Up @@ -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'],
Expand All @@ -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'],
Expand All @@ -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))
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 => {
Expand All @@ -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)
})
})

Expand All @@ -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)
})
})

Expand Down Expand Up @@ -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)
})
})

Expand All @@ -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)
})
})
})
72 changes: 8 additions & 64 deletions lib/ensure/__tests__/symlink.test.js
View file
Expand Up @@ -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'],
Expand All @@ -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'],
Expand All @@ -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))
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 => {
Expand All @@ -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)
})
})

Expand All @@ -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)
})
})

Expand Down Expand Up @@ -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)
})
})

Expand All @@ -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)
})
})
})
17 changes: 10 additions & 7 deletions lib/ensure/link.js
View file
Expand Up @@ -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) {
Expand All @@ -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) => {
Expand All @@ -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
Expand Down
49 changes: 34 additions & 15 deletions lib/ensure/symlink.js
View file
Expand Up @@ -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
Expand All @@ -17,35 +17,54 @@ 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)
})
})
})
})
}

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
Expand Down
3 changes: 2 additions & 1 deletion lib/util/stat.js
View file
Expand Up @@ -173,5 +173,6 @@ module.exports = {
checkPathsSync,
checkParentPaths,
checkParentPathsSync,
isSrcSubdir
isSrcSubdir,
areIdentical
}

0 comments on commit 4523526

Please sign in to comment.