From 56fbf5c414fb24f50df58c70dcefaf4b74352dce Mon Sep 17 00:00:00 2001 From: Nate Fischer Date: Fri, 8 Apr 2016 15:34:54 -0700 Subject: [PATCH] feat(cp): -P option, plus better handling of symlinks (#421) --- README.md | 3 +- src/cp.js | 94 ++++++++++++++-------- src/mkdir.js | 7 +- src/rm.js | 8 +- test/common.js | 5 ++ test/cp.js | 115 ++++++++++++++++++++++++--- test/ls.js | 7 ++ test/mkdir.js | 8 ++ test/resources/cp/fakeLinks/file.txt | 1 + test/resources/cp/fakeLinks/sym.lnk | 1 + test/resources/cp/links/file.txt | 1 + test/resources/cp/links/sym.lnk | 1 + test/resources/cp/symFolder | 1 + test/resources/rm/fake.lnk | 1 + test/rm.js | 13 +++ test/touch.js | 9 +++ 16 files changed, 221 insertions(+), 54 deletions(-) create mode 100644 test/resources/cp/fakeLinks/file.txt create mode 100644 test/resources/cp/fakeLinks/sym.lnk create mode 100644 test/resources/cp/links/file.txt create mode 120000 test/resources/cp/links/sym.lnk create mode 120000 test/resources/cp/symFolder create mode 120000 test/resources/rm/fake.lnk diff --git a/README.md b/README.md index b6ea269..f65c533 100644 --- a/README.md +++ b/README.md @@ -202,7 +202,8 @@ Available options: + `-f`: force (default behavior) + `-n`: no-clobber + `-r`, `-R`: recursive -+ `-L`: followsymlink ++ `-L`: follow symlinks ++ `-P`: don't follow symlinks Examples: diff --git a/src/cp.js b/src/cp.js index 69b7cd9..628e9ad 100644 --- a/src/cp.js +++ b/src/cp.js @@ -6,39 +6,51 @@ var os = require('os'); // Buffered file copy, synchronous // (Using readFileSync() + writeFileSync() could easily cause a memory overflow // with large files) -function copyFileSync(srcFile, destFile) { +function copyFileSync(srcFile, destFile, options) { if (!fs.existsSync(srcFile)) common.error('copyFileSync: no such file or directory: ' + srcFile); - var BUF_LENGTH = 64*1024, - buf = new Buffer(BUF_LENGTH), - bytesRead = BUF_LENGTH, - pos = 0, - fdr = null, - fdw = null; + if (fs.lstatSync(srcFile).isSymbolicLink() && !options.followsymlink) { + try { + fs.lstatSync(destFile); + common.unlinkSync(destFile); // re-link it + } catch (e) { + // it doesn't exist, so no work needs to be done + } - try { - fdr = fs.openSync(srcFile, 'r'); - } catch(e) { - common.error('copyFileSync: could not read src file ('+srcFile+')'); + var symlinkFull = fs.readlinkSync(srcFile); + fs.symlinkSync(symlinkFull, destFile, os.platform() === "win32" ? "junction" : null); + } else { + var BUF_LENGTH = 64*1024, + buf = new Buffer(BUF_LENGTH), + bytesRead = BUF_LENGTH, + pos = 0, + fdr = null, + fdw = null; + + try { + fdr = fs.openSync(srcFile, 'r'); + } catch(e) { + common.error('copyFileSync: could not read src file ('+srcFile+')'); + } + + try { + fdw = fs.openSync(destFile, 'w'); + } catch(e) { + common.error('copyFileSync: could not write to dest file (code='+e.code+'):'+destFile); + } + + while (bytesRead === BUF_LENGTH) { + bytesRead = fs.readSync(fdr, buf, 0, BUF_LENGTH, pos); + fs.writeSync(fdw, buf, 0, bytesRead); + pos += bytesRead; + } + + fs.closeSync(fdr); + fs.closeSync(fdw); + + fs.chmodSync(destFile, fs.statSync(srcFile).mode); } - - try { - fdw = fs.openSync(destFile, 'w'); - } catch(e) { - common.error('copyFileSync: could not write to dest file (code='+e.code+'):'+destFile); - } - - while (bytesRead === BUF_LENGTH) { - bytesRead = fs.readSync(fdr, buf, 0, BUF_LENGTH, pos); - fs.writeSync(fdw, buf, 0, bytesRead); - pos += bytesRead; - } - - fs.closeSync(fdr); - fs.closeSync(fdw); - - fs.chmodSync(destFile, fs.statSync(srcFile).mode); } // Recursively copies 'sourceDir' into 'destDir' @@ -83,7 +95,7 @@ function cpdirSyncRecursive(sourceDir, destDir, opts) { if (opts.followsymlink) { if (cpcheckcycle(sourceDir, srcFile)) { // Cycle link found. - console.log('Cycle link found.'); + console.error('Cycle link found.'); symlinkFull = fs.readlinkSync(srcFile); fs.symlinkSync(symlinkFull, destFile, os.platform() === "win32" ? "junction" : null); continue; @@ -94,20 +106,26 @@ function cpdirSyncRecursive(sourceDir, destDir, opts) { cpdirSyncRecursive(srcFile, destFile, opts); } else if (srcFileStat.isSymbolicLink() && !opts.followsymlink) { symlinkFull = fs.readlinkSync(srcFile); + try { + fs.lstatSync(destFile); + common.unlinkSync(destFile); // re-link it + } catch (e) { + // it doesn't exist, so no work needs to be done + } fs.symlinkSync(symlinkFull, destFile, os.platform() === "win32" ? "junction" : null); } else if (srcFileStat.isSymbolicLink() && opts.followsymlink) { srcFileStat = fs.statSync(srcFile); if (srcFileStat.isDirectory()) { cpdirSyncRecursive(srcFile, destFile, opts); } else { - copyFileSync(srcFile, destFile); + copyFileSync(srcFile, destFile, opts); } } else { /* At this point, we've hit a file actually worth copying... so copy it on over. */ if (fs.existsSync(destFile) && opts.no_force) { common.log('skipping existing file: ' + files[i]); } else { - copyFileSync(srcFile, destFile); + copyFileSync(srcFile, destFile, opts); } } @@ -139,7 +157,8 @@ function cpcheckcycle(sourceDir, srcFile) { //@ + `-f`: force (default behavior) //@ + `-n`: no-clobber //@ + `-r`, `-R`: recursive -//@ + `-L`: followsymlink +//@ + `-L`: follow symlinks +//@ + `-P`: don't follow symlinks //@ //@ Examples: //@ @@ -158,8 +177,15 @@ function _cp(options, sources, dest) { 'R': 'recursive', 'r': 'recursive', 'L': 'followsymlink', + 'P': 'noFollowsymlink', }); + // If we're missing -R, it actually implies -L (unless -P is explicit) + if (options.followsymlink) + options.noFollowsymlink = false; + if (!options.recursive && !options.noFollowsymlink) + options.followsymlink = true; + // Get sources, dest if (arguments.length < 3) { common.error('missing and/or '); @@ -185,7 +211,7 @@ function _cp(options, sources, dest) { return; // skip file } var srcStat = fs.statSync(src); - if (srcStat.isDirectory()) { + if (!options.noFollowsymlink && srcStat.isDirectory()) { if (!options.recursive) { // Non-Recursive common.error("omitting directory '" + src + "'", true); @@ -218,7 +244,7 @@ function _cp(options, sources, dest) { return; // skip file } - copyFileSync(src, thisDest); + copyFileSync(src, thisDest, options); } }); // forEach(src) return new common.ShellString('', common.state.error, common.state.errorCode); diff --git a/src/mkdir.js b/src/mkdir.js index 9171be9..48c341f 100644 --- a/src/mkdir.js +++ b/src/mkdir.js @@ -46,10 +46,13 @@ function _mkdir(options, dirs) { // if it's array leave it as it is dirs.forEach(function(dir) { - if (fs.existsSync(dir)) { + try { + fs.lstatSync(dir); if (!options.fullpath) - common.error('path already exists: ' + dir, true); + common.error('path already exists: ' + dir, true); return; // skip dir + } catch (e) { + // do nothing } // Base dir does not exist, and no -p option given diff --git a/src/rm.js b/src/rm.js index 18f2afb..2780285 100644 --- a/src/rm.js +++ b/src/rm.js @@ -107,17 +107,17 @@ function _rm(options, files) { files = [].slice.call(arguments, 1); files.forEach(function(file) { - if (!fs.existsSync(file)) { + var stats; + try { + stats = fs.lstatSync(file); // test for existence + } catch (e) { // Path does not exist, no force flag given if (!options.force) common.error('no such file or directory: '+file, true); - return; // skip file } // If here, path exists - - var stats = fs.lstatSync(file); if (stats.isFile() || stats.isSymbolicLink()) { // Do not check for file writing permissions diff --git a/test/common.js b/test/common.js index b450ae2..014ed2c 100644 --- a/test/common.js +++ b/test/common.js @@ -46,6 +46,11 @@ var result = common.expand(['**/file*.js']); assert.equal(shell.error(), null); assert.deepEqual(result.sort(), ["resources/file1.js","resources/file2.js","resources/ls/file1.js","resources/ls/file2.js"].sort()); +// broken links still expand +var result = common.expand(['resources/b*dlink']); +assert.equal(shell.error(), null); +assert.deepEqual(result, ['resources/badlink']); + // common.parseOptions (normal case) var result = common.parseOptions('-Rf', { 'R': 'recursive', diff --git a/test/cp.js b/test/cp.js index 28d4d3e..ee5c29e 100644 --- a/test/cp.js +++ b/test/cp.js @@ -231,6 +231,39 @@ assert.equal(shell.error(), null); // crash test only assert.ok(!result.stderr); assert.equal(result.code, 0); +if (process.platform !== 'win32') { + // Recursive, everything exists, overwrite a real file with a link (if same name) + // Because -R implies to not follow links! + shell.rm('-rf', 'tmp/*'); + shell.cp('-R', 'resources/cp/*', 'tmp'); + assert.ok(fs.lstatSync('tmp/links/sym.lnk').isSymbolicLink()); // this one is a link + assert.ok(!(fs.lstatSync('tmp/fakeLinks/sym.lnk').isSymbolicLink())); // this one isn't + assert.notEqual(shell.cat('tmp/links/sym.lnk').toString(), shell.cat('tmp/fakeLinks/sym.lnk').toString()); + result = shell.cp('-R', 'tmp/links/*', 'tmp/fakeLinks'); + assert.equal(shell.error(), null); + assert.ok(!result.stderr); + assert.equal(result.code, 0); + assert.ok(fs.lstatSync('tmp/links/sym.lnk').isSymbolicLink()); // this one is a link + assert.ok(fs.lstatSync('tmp/fakeLinks/sym.lnk').isSymbolicLink()); // this one is now a link + assert.equal(shell.cat('tmp/links/sym.lnk').toString(), shell.cat('tmp/fakeLinks/sym.lnk').toString()); + + // Recursive, everything exists, overwrite a real file *by following a link* + // Because missing the -R implies -L. + shell.rm('-rf', 'tmp/*'); + shell.cp('-R', 'resources/cp/*', 'tmp'); + assert.ok(fs.lstatSync('tmp/links/sym.lnk').isSymbolicLink()); // this one is a link + assert.ok(!(fs.lstatSync('tmp/fakeLinks/sym.lnk').isSymbolicLink())); // this one isn't + assert.notEqual(shell.cat('tmp/links/sym.lnk').toString(), shell.cat('tmp/fakeLinks/sym.lnk').toString()); + result = shell.cp('tmp/links/*', 'tmp/fakeLinks'); // don't use -R + assert.equal(shell.error(), null); + assert.ok(!result.stderr); + assert.equal(result.code, 0); + assert.ok(fs.lstatSync('tmp/links/sym.lnk').isSymbolicLink()); // this one is a link + assert.ok(!fs.lstatSync('tmp/fakeLinks/sym.lnk').isSymbolicLink()); // this one is still not a link + // But it still follows the link + assert.equal(shell.cat('tmp/links/sym.lnk').toString(), shell.cat('tmp/fakeLinks/sym.lnk').toString()); +} + //recursive, everything exists, with force flag shell.rm('-rf', 'tmp/*'); result = shell.cp('-R', 'resources/cp', 'tmp'); @@ -275,12 +308,12 @@ assert.equal(fs.existsSync('tmp/dest/z'), true); // On Windows, permission bits are quite different so skip those tests for now if (common.platform !== 'win') { - //preserve mode bits - shell.rm('-rf', 'tmp/*'); - var execBit = parseInt('001', 8); - assert.equal(fs.statSync('resources/cp-mode-bits/executable').mode & execBit, execBit); - shell.cp('resources/cp-mode-bits/executable', 'tmp/executable'); - assert.equal(fs.statSync('resources/cp-mode-bits/executable').mode, fs.statSync('tmp/executable').mode); + //preserve mode bits + shell.rm('-rf', 'tmp/*'); + var execBit = parseInt('001', 8); + assert.equal(fs.statSync('resources/cp-mode-bits/executable').mode & execBit, execBit); + shell.cp('resources/cp-mode-bits/executable', 'tmp/executable'); + assert.equal(fs.statSync('resources/cp-mode-bits/executable').mode, fs.statSync('tmp/executable').mode); } // Make sure hidden files are copied recursively @@ -304,7 +337,7 @@ assert.ok(fs.existsSync('tmp/file1.txt')); shell.rm('-rf', 'tmp/'); shell.mkdir('tmp/'); result = shell.cp('resources/file1.txt', 'resources/file2.txt', 'resources/cp', - 'resources/ls/', 'tmp/'); + 'resources/ls/', 'tmp/'); assert.ok(shell.error()); assert.ok(!fs.existsSync('tmp/.hidden_file')); // doesn't copy dir contents assert.ok(!fs.existsSync('tmp/ls')); // doesn't copy dir itself @@ -313,13 +346,69 @@ assert.ok(!fs.existsSync('tmp/cp')); // doesn't copy dir itself assert.ok(fs.existsSync('tmp/file1.txt')); assert.ok(fs.existsSync('tmp/file2.txt')); -// Recursive, copies entire directory with no symlinks and -L option does not cause change in behavior. +if (process.platform !== 'win32') { + // -R implies -P + shell.rm('-rf', 'tmp/*'); + shell.cp('-R', 'resources/cp/links/sym.lnk', 'tmp'); + assert.ok(fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); + + // using -P explicitly works + shell.rm('-rf', 'tmp/*'); + shell.cp('-P', 'resources/cp/links/sym.lnk', 'tmp'); + assert.ok(fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); + + // using -PR on a link to a folder does not follow the link + shell.rm('-rf', 'tmp/*'); + shell.cp('-PR', 'resources/cp/symFolder', 'tmp'); + assert.ok(fs.lstatSync('tmp/symFolder').isSymbolicLink()); + + // -L overrides -P for copying directory + shell.rm('-rf', 'tmp/*'); + shell.cp('-LPR', 'resources/cp/symFolder', 'tmp'); + assert.ok(!fs.lstatSync('tmp/symFolder').isSymbolicLink()); + assert.ok(!fs.lstatSync('tmp/symFolder/sym.lnk').isSymbolicLink()); + + // Recursive, copies entire directory with no symlinks and -L option does not cause change in behavior. + shell.rm('-rf', 'tmp/*'); + result = shell.cp('-rL', 'resources/cp/dir_a', 'tmp/dest'); + assert.equal(shell.error(), null); + assert.ok(!result.stderr); + assert.equal(result.code, 0); + assert.equal(fs.existsSync('tmp/dest/z'), true); +} + +// using -R on a link to a folder *does* follow the link shell.rm('-rf', 'tmp/*'); -result = shell.cp('-rL', 'resources/cp/dir_a', 'tmp/dest'); -assert.equal(shell.error(), null); -assert.ok(!result.stderr); -assert.equal(result.code, 0); -assert.equal(fs.existsSync('tmp/dest/z'), true); +shell.cp('-R', 'resources/cp/symFolder', 'tmp'); +assert.ok(!fs.lstatSync('tmp/symFolder').isSymbolicLink()); + +// Without -R, -L is implied +shell.rm('-rf', 'tmp/*'); +shell.cp('resources/cp/links/sym.lnk', 'tmp'); +assert.ok(!fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); + +// -L explicitly works +shell.rm('-rf', 'tmp/*'); +shell.cp('-L', 'resources/cp/links/sym.lnk', 'tmp'); +assert.ok(!fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); + +// using -LR does not imply -P +shell.rm('-rf', 'tmp/*'); +shell.cp('-LR', 'resources/cp/links/sym.lnk', 'tmp'); +assert.ok(!fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); + +// using -LR also works recursively on directories containing links +shell.rm('-rf', 'tmp/*'); +shell.cp('-LR', 'resources/cp/links', 'tmp'); +assert.ok(!fs.lstatSync('tmp/links/sym.lnk').isSymbolicLink()); + +// -L always overrides a -P +shell.rm('-rf', 'tmp/*'); +shell.cp('-LP', 'resources/cp/links/sym.lnk', 'tmp'); +assert.ok(!fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); +shell.rm('-rf', 'tmp/*'); +shell.cp('-LPR', 'resources/cp/links/sym.lnk', 'tmp'); +assert.ok(!fs.lstatSync('tmp/sym.lnk').isSymbolicLink()); // Test max depth. shell.rm('-rf', 'tmp/'); diff --git a/test/ls.js b/test/ls.js index 841df58..2f563ba 100644 --- a/test/ls.js +++ b/test/ls.js @@ -370,6 +370,13 @@ assert.ok(result.atime); // check that these keys exist assert.ok(result.ctime); // check that these keys exist assert.ok(result.toString().match(/^(\d+ +){5}.*$/)); +// still lists broken links +result = shell.ls('resources/badlink'); +assert.equal(shell.error(), null); +assert.equal(result.code, 0); +assert.equal(result.indexOf('resources/badlink') > -1, true); +assert.equal(result.length, 1); + // Test new ShellString-like attributes result = shell.ls('resources/ls'); assert.equal(shell.error(), null); diff --git a/test/mkdir.js b/test/mkdir.js index df14f6b..7d51af2 100644 --- a/test/mkdir.js +++ b/test/mkdir.js @@ -25,6 +25,14 @@ assert.equal(result.code, 1); assert.equal(result.stderr, 'mkdir: path already exists: tmp'); assert.equal(fs.statSync('tmp').mtime.toString(), mtime); // didn't mess with dir +// Can't overwrite a broken link +mtime = fs.lstatSync('resources/badlink').mtime.toString(); +result = shell.mkdir('resources/badlink'); +assert.ok(shell.error()); +assert.equal(result.code, 1); +assert.equal(result.stderr, 'mkdir: path already exists: resources/badlink'); +assert.equal(fs.lstatSync('resources/badlink').mtime.toString(), mtime); // didn't mess with file + assert.equal(fs.existsSync('/asdfasdf'), false); // sanity check result = shell.mkdir('/asdfasdf/foobar'); // root path does not exist assert.ok(shell.error()); diff --git a/test/resources/cp/fakeLinks/file.txt b/test/resources/cp/fakeLinks/file.txt new file mode 100644 index 0000000..0637880 --- /dev/null +++ b/test/resources/cp/fakeLinks/file.txt @@ -0,0 +1 @@ +This is a file diff --git a/test/resources/cp/fakeLinks/sym.lnk b/test/resources/cp/fakeLinks/sym.lnk new file mode 100644 index 0000000..de8e66c --- /dev/null +++ b/test/resources/cp/fakeLinks/sym.lnk @@ -0,0 +1 @@ +This is not a link diff --git a/test/resources/cp/links/file.txt b/test/resources/cp/links/file.txt new file mode 100644 index 0000000..0637880 --- /dev/null +++ b/test/resources/cp/links/file.txt @@ -0,0 +1 @@ +This is a file diff --git a/test/resources/cp/links/sym.lnk b/test/resources/cp/links/sym.lnk new file mode 120000 index 0000000..4c33073 --- /dev/null +++ b/test/resources/cp/links/sym.lnk @@ -0,0 +1 @@ +file.txt \ No newline at end of file diff --git a/test/resources/cp/symFolder b/test/resources/cp/symFolder new file mode 120000 index 0000000..ce866dd --- /dev/null +++ b/test/resources/cp/symFolder @@ -0,0 +1 @@ +links/ \ No newline at end of file diff --git a/test/resources/rm/fake.lnk b/test/resources/rm/fake.lnk new file mode 120000 index 0000000..6eab79a --- /dev/null +++ b/test/resources/rm/fake.lnk @@ -0,0 +1 @@ +missing \ No newline at end of file diff --git a/test/rm.js b/test/rm.js index bdef49b..d149edf 100644 --- a/test/rm.js +++ b/test/rm.js @@ -225,4 +225,17 @@ assert.equal(result.code, 0); assert.equal(fs.existsSync('tmp/rm/link_to_a_dir'), false); assert.equal(fs.existsSync('tmp/rm/a_dir'), true); +// remove broken symbolic link +if (process.platform !== 'win32') { + result = shell.rm('-rf', 'tmp'); + shell.mkdir('tmp'); + shell.cp('-R', 'resources/rm', 'tmp'); + assert.ok(shell.test('-L', 'tmp/rm/fake.lnk')); + result = shell.rm('tmp/rm/fake.lnk'); + assert.equal(shell.error(), null); + assert.equal(result.code, 0); + assert.ok(!shell.test('-L', 'tmp/rm/fake.lnk')); + assert.equal(fs.existsSync('tmp/rm/fake.lnk'), false); +} + shell.exit(123); diff --git a/test/touch.js b/test/touch.js index 54929b9..074e649 100644 --- a/test/touch.js +++ b/test/touch.js @@ -109,6 +109,15 @@ assert.equal(result.code, 0); assert(fs.existsSync(testFile)); assert(fs.existsSync(testFile2)); +// touching broken link creates a new file +if (process.platform !== 'win32') { + result = shell.touch('resources/badlink'); + assert.equal(result.code, 0); + assert.ok(!shell.error()); + assert.ok(fs.existsSync('resources/not_existed_file')); + shell.rm('resources/not_existed_file'); +} + function resetUtimes(f) { var d = new Date(); d.setYear(2000);