From 3fe1a67cfb0c027b44846df13622e3369b80b9a6 Mon Sep 17 00:00:00 2001 From: Loilo Date: Thu, 5 Apr 2018 11:49:12 +0200 Subject: [PATCH 1/7] Allow ln destination to be an existing directory --- src/ln.js | 4 ++++ test/ln.js | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/ln.js b/src/ln.js index 2cf87cd8..fe8800fc 100644 --- a/src/ln.js +++ b/src/ln.js @@ -35,6 +35,10 @@ function _ln(options, source, dest) { var isAbsolute = (path.resolve(source) === sourcePath); dest = path.resolve(process.cwd(), String(dest)); + if (fs.existsSync(dest) && common.statFollowLinks(dest).isDirectory(dest)) { + dest = path.resolve(dest, path.basename(sourcePath)) + } + if (fs.existsSync(dest)) { if (!options.force) { common.error('Destination file exists', { continue: true }); diff --git a/test/ln.js b/test/ln.js index b1682058..8ee3a3d6 100644 --- a/test/ln.js +++ b/test/ln.js @@ -48,6 +48,14 @@ test('destination already exists', t => { t.is(result.code, 1); }); +test('destination already exists inside directory', t => { + shell.cd(t.context.tmp); + let result = shell.ln('-s', 'file1', './'); + t.truthy(shell.error()); + t.is(result.code, 1); + shell.cd('..'); +}); + test('non-existent source', t => { const result = shell.ln(`${t.context.tmp}/noexist`, `${t.context.tmp}/linkfile1`); t.truthy(shell.error()); @@ -137,6 +145,19 @@ test('To current directory', t => { shell.cd('..'); }); +test('Inside existing directory', t => { + shell.cd(t.context.tmp); + let result = shell.ln('-s', 'external/node_script.js', './'); + t.is(result.code, 0); + t.truthy(fs.existsSync('node_script.js')); + t.is( + fs.readFileSync('external/node_script.js').toString(), + fs.readFileSync('node_script.js').toString() + ); + shell.rm('node_script.js'); + shell.cd('..'); +}); + test('-f option', t => { const result = shell.ln('-f', `${t.context.tmp}/file1.js`, `${t.context.tmp}/file2.js`); t.is(result.code, 0); From 400859622b8c5326fb93c4bf1765c79c8a89b4be Mon Sep 17 00:00:00 2001 From: Loilo Date: Thu, 5 Apr 2018 11:59:15 +0200 Subject: [PATCH 2/7] Fix coding style --- src/ln.js | 2 +- test/ln.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ln.js b/src/ln.js index fe8800fc..c754ff25 100644 --- a/src/ln.js +++ b/src/ln.js @@ -36,7 +36,7 @@ function _ln(options, source, dest) { dest = path.resolve(process.cwd(), String(dest)); if (fs.existsSync(dest) && common.statFollowLinks(dest).isDirectory(dest)) { - dest = path.resolve(dest, path.basename(sourcePath)) + dest = path.resolve(dest, path.basename(sourcePath)); } if (fs.existsSync(dest)) { diff --git a/test/ln.js b/test/ln.js index 8ee3a3d6..7ab98467 100644 --- a/test/ln.js +++ b/test/ln.js @@ -50,7 +50,7 @@ test('destination already exists', t => { test('destination already exists inside directory', t => { shell.cd(t.context.tmp); - let result = shell.ln('-s', 'file1', './'); + const result = shell.ln('-s', 'file1', './'); t.truthy(shell.error()); t.is(result.code, 1); shell.cd('..'); @@ -147,7 +147,7 @@ test('To current directory', t => { test('Inside existing directory', t => { shell.cd(t.context.tmp); - let result = shell.ln('-s', 'external/node_script.js', './'); + const result = shell.ln('-s', 'external/node_script.js', './'); t.is(result.code, 0); t.truthy(fs.existsSync('node_script.js')); t.is( From 50e6ca8da49a489ef13f05aab8dfe16d7c7803fc Mon Sep 17 00:00:00 2001 From: Loilo Date: Fri, 6 Apr 2018 01:41:32 +0200 Subject: [PATCH 3/7] Implement first review feedback --- src/ln.js | 2 +- test/ln.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ln.js b/src/ln.js index c754ff25..9f334fee 100644 --- a/src/ln.js +++ b/src/ln.js @@ -36,7 +36,7 @@ function _ln(options, source, dest) { dest = path.resolve(process.cwd(), String(dest)); if (fs.existsSync(dest) && common.statFollowLinks(dest).isDirectory(dest)) { - dest = path.resolve(dest, path.basename(sourcePath)); + dest = path.join(dest, path.basename(sourcePath)); } if (fs.existsSync(dest)) { diff --git a/test/ln.js b/test/ln.js index 7ab98467..991dc191 100644 --- a/test/ln.js +++ b/test/ln.js @@ -53,7 +53,6 @@ test('destination already exists inside directory', t => { const result = shell.ln('-s', 'file1', './'); t.truthy(shell.error()); t.is(result.code, 1); - shell.cd('..'); }); test('non-existent source', t => { @@ -149,12 +148,13 @@ test('Inside existing directory', t => { shell.cd(t.context.tmp); const result = shell.ln('-s', 'external/node_script.js', './'); t.is(result.code, 0); + t.falsy(result.stderr); + t.falsy(shell.error()); t.truthy(fs.existsSync('node_script.js')); t.is( fs.readFileSync('external/node_script.js').toString(), fs.readFileSync('node_script.js').toString() ); - shell.rm('node_script.js'); shell.cd('..'); }); From 582414d2d1a62b33245ad8aff912ffe0295bcb77 Mon Sep 17 00:00:00 2001 From: Loilo Date: Fri, 6 Apr 2018 01:43:11 +0200 Subject: [PATCH 4/7] Remove incorrect parameter from stat.isDirectory() --- src/ln.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ln.js b/src/ln.js index 9f334fee..867cc775 100644 --- a/src/ln.js +++ b/src/ln.js @@ -35,7 +35,7 @@ function _ln(options, source, dest) { var isAbsolute = (path.resolve(source) === sourcePath); dest = path.resolve(process.cwd(), String(dest)); - if (fs.existsSync(dest) && common.statFollowLinks(dest).isDirectory(dest)) { + if (fs.existsSync(dest) && common.statFollowLinks(dest).isDirectory()) { dest = path.join(dest, path.basename(sourcePath)); } From 9392c558d29e30798d151831de017472334aa170 Mon Sep 17 00:00:00 2001 From: Loilo Date: Fri, 6 Apr 2018 01:52:20 +0200 Subject: [PATCH 5/7] Re-add the working directory cleanup in test It somehow caused temporary test folders not to get cleaned up correctly. --- test/ln.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/ln.js b/test/ln.js index 991dc191..32ae2108 100644 --- a/test/ln.js +++ b/test/ln.js @@ -53,6 +53,7 @@ test('destination already exists inside directory', t => { const result = shell.ln('-s', 'file1', './'); t.truthy(shell.error()); t.is(result.code, 1); + shell.cd('..'); }); test('non-existent source', t => { From 946ab48bf5cf9c8aac03407c3608b2144485a91d Mon Sep 17 00:00:00 2001 From: Loilo Date: Fri, 6 Apr 2018 09:22:17 +0200 Subject: [PATCH 6/7] cd to the original CWD after each test --- test/ln.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/ln.js b/test/ln.js index 32ae2108..fee55652 100644 --- a/test/ln.js +++ b/test/ln.js @@ -16,6 +16,7 @@ test.beforeEach(t => { }); test.afterEach.always(t => { + process.chdir(CWD); shell.rm('-rf', t.context.tmp); }); @@ -53,7 +54,6 @@ test('destination already exists inside directory', t => { const result = shell.ln('-s', 'file1', './'); t.truthy(shell.error()); t.is(result.code, 1); - shell.cd('..'); }); test('non-existent source', t => { @@ -142,7 +142,6 @@ test('To current directory', t => { t.truthy(fs.existsSync('dest/testfile.txt')); t.truthy(fs.existsSync('dir1/insideDir.txt')); t.falsy(fs.existsSync('dest/insideDir.txt')); - shell.cd('..'); }); test('Inside existing directory', t => { @@ -156,7 +155,6 @@ test('Inside existing directory', t => { fs.readFileSync('external/node_script.js').toString(), fs.readFileSync('node_script.js').toString() ); - shell.cd('..'); }); test('-f option', t => { From c57f7ffc3ffb8ab828b9032ce6effb477dc5cc30 Mon Sep 17 00:00:00 2001 From: Loilo Date: Fri, 6 Apr 2018 10:17:04 +0200 Subject: [PATCH 7/7] Allow dead symlinks for source/destination, fixes #829 --- src/ln.js | 21 +++++++++++++++++++-- test/ln.js | 21 +++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/ln.js b/src/ln.js index 867cc775..f1f4a73f 100644 --- a/src/ln.js +++ b/src/ln.js @@ -39,7 +39,15 @@ function _ln(options, source, dest) { dest = path.join(dest, path.basename(sourcePath)); } - if (fs.existsSync(dest)) { + var destinationExists; + try { + fs.lstatSync(dest); + destinationExists = true; + } catch (err) { + destinationExists = false; + } + + if (destinationExists) { if (!options.force) { common.error('Destination file exists', { continue: true }); } @@ -51,7 +59,16 @@ function _ln(options, source, dest) { var isWindows = process.platform === 'win32'; var linkType = isWindows ? 'file' : null; var resolvedSourcePath = isAbsolute ? sourcePath : path.resolve(process.cwd(), path.dirname(dest), source); - if (!fs.existsSync(resolvedSourcePath)) { + + var resolvedSourceExists; + try { + fs.lstatSync(resolvedSourcePath); + resolvedSourceExists = true; + } catch (err) { + resolvedSourceExists = false; + } + + if (!resolvedSourceExists) { common.error('Source file does not exist', { continue: true }); } else if (isWindows && common.statFollowLinks(resolvedSourcePath).isDirectory()) { linkType = 'junction'; diff --git a/test/ln.js b/test/ln.js index fee55652..2f00f5a9 100644 --- a/test/ln.js +++ b/test/ln.js @@ -157,6 +157,15 @@ test('Inside existing directory', t => { ); }); +test('Link to dead source links', t => { + shell.cd(t.context.tmp); + const result = shell.ln('-s', 'badlink', 'link-to-dead-link'); + t.is(result.code, 0); + t.falsy(result.stderr); + t.falsy(shell.error()); + fs.lstatSync('link-to-dead-link'); +}); + test('-f option', t => { const result = shell.ln('-f', `${t.context.tmp}/file1.js`, `${t.context.tmp}/file2.js`); t.is(result.code, 0); @@ -181,6 +190,18 @@ test('-sf option', t => { }); }); +test('Override dead destination links with -sf', t => { + shell.cd(t.context.tmp); + const result = shell.ln('-sf', 'file1.txt', 'badlink'); + t.is(result.code, 0); + t.falsy(result.stderr); + t.falsy(shell.error()); + t.is( + fs.readFileSync('file1.txt').toString(), + fs.readFileSync('badlink').toString() + ); +}); + test('Abspath regression', t => { utils.skipOnWinForEPERM(shell.ln.bind(shell, '-sf', 'file1', path.resolve(`${t.context.tmp}/abspath`)), () => { t.truthy(fs.existsSync(`${t.context.tmp}/abspath`));