feat(cp): -P option, plus better handling of symlinks (#421)

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:
 

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 <source> and/or <dest>');
@@ -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);

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

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

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',

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/');

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);

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());

test/resources/cp/fakeLinks/file.txt

@@ -0,0 +1 @@
+This is a file

test/resources/cp/fakeLinks/sym.lnk

@@ -0,0 +1 @@
+This is not a link

test/resources/cp/links/file.txt

@@ -0,0 +1 @@
+This is a file

test/resources/cp/links/sym.lnk

@@ -0,0 +1 @@
+file.txt

test/resources/cp/symFolder

@@ -0,0 +1 @@
+links/

test/resources/rm/fake.lnk

@@ -0,0 +1 @@
+missing

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);

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);