Fix shelljs#631 throw error when overwriting recently created file (shelljs#702)

uttpal · nfischer · commit bbe521b57dfe · 2017-04-15T02:01:58.000-07:00
* cp: add error to not overwrite recently created files shelljs#631 * cp: add tests for errors not overwrite recently created files shelljs#631 * mv: show error when overwriting recently created file shelljs#631 * mv: add tests for error on recently created files shelljs#631 * mv: test remove unnecessary steps shelljs#631
diff --git a/src/cp.js b/src/cp.js
@@ -153,6 +153,14 @@ function cpdirSyncRecursive(sourceDir, destDir, currentDepth, opts) {
   } // for files
 } // cpdirSyncRecursive
 
+// Checks if cureent file was created recently
+function checkRecentCreated(sources, index) {
+  var lookedSource = sources[index];
+  return sources.slice(0, index).some(function (src) {
+    return path.basename(src) === path.basename(lookedSource);
+  });
+}
+
 function cpcheckcycle(sourceDir, srcFile) {
   var srcFileStat = fs.lstatSync(srcFile);
   if (srcFileStat.isSymbolicLink()) {
@@ -227,7 +235,7 @@ function _cp(options, sources, dest) {
     return new common.ShellString('', '', 0);
   }
 
-  sources.forEach(function (src) {
+  sources.forEach(function (src, srcIndex) {
     if (!fs.existsSync(src)) {
       common.error('no such file or directory: ' + src, { continue: true });
       return; // skip file
@@ -262,7 +270,16 @@ function _cp(options, sources, dest) {
         thisDest = path.normalize(dest + '/' + path.basename(src));
       }
 
-      if (fs.existsSync(thisDest) && options.no_force) {
+      var thisDestExists = fs.existsSync(thisDest);
+      if (thisDestExists && checkRecentCreated(sources, srcIndex)) {
+        // cannot overwrite file created recently in current execution, but we want to continue copying other files
+        if (!options.no_force) {
+          common.error("will not overwrite just-created '" + thisDest + "' with '" + src + "'", { continue: true });
+        }
+        return;
+      }
+
+      if (thisDestExists && options.no_force) {
         return; // skip file
       }
 
diff --git a/src/mv.js b/src/mv.js
@@ -11,6 +11,14 @@ common.register('mv', _mv, {
   },
 });
 
+// Checks if cureent file was created recently
+function checkRecentCreated(sources, index) {
+  var lookedSource = sources[index];
+  return sources.slice(0, index).some(function (src) {
+    return path.basename(src) === path.basename(lookedSource);
+  });
+}
+
 //@
 //@ ### mv([options ,] source [, source ...], dest')
 //@ ### mv([options ,] source_array, dest')
@@ -55,7 +63,7 @@ function _mv(options, sources, dest) {
     common.error('dest file already exists: ' + dest);
   }
 
-  sources.forEach(function (src) {
+  sources.forEach(function (src, srcIndex) {
     if (!fs.existsSync(src)) {
       common.error('no such file or directory: ' + src, { continue: true });
       return; // skip file
@@ -70,6 +78,16 @@ function _mv(options, sources, dest) {
       thisDest = path.normalize(dest + '/' + path.basename(src));
     }
 
+    var thisDestExists = fs.existsSync(thisDest);
+
+    if (thisDestExists && checkRecentCreated(sources, srcIndex)) {
+      // cannot overwrite file created recently in current execution, but we want to continue copying other files
+      if (!options.no_force) {
+        common.error("will not overwrite just-created '" + thisDest + "' with '" + src + "'", { continue: true });
+      }
+      return;
+    }
+
     if (fs.existsSync(thisDest) && options.no_force) {
       common.error('dest file already exists: ' + thisDest, { continue: true });
       return; // skip file
diff --git a/test/cp.js b/test/cp.js
@@ -712,3 +712,39 @@ test('copy mutliple files to same location', t => {
     "cp: 'resources/file2' and 'resources/file2' are the same file"
   );
 });
+
+test('should not overwrite recently created files', t => {
+  const result = shell.cp('resources/file1', 'resources/cp/file1', t.context.tmp);
+  t.truthy(shell.error());
+  t.is(result.code, 1);
+
+  // Ensure First file is copied
+  t.is(shell.cat(`${t.context.tmp}/file1`).toString(), 'test1');
+  t.is(
+    result.stderr,
+    `cp: will not overwrite just-created '${t.context.tmp}/file1' with 'resources/cp/file1'`
+  );
+});
+
+
+test('should not overwrite recently created files (in recursive Mode)', t => {
+  const result = shell.cp('-R', 'resources/file1', 'resources/cp/file1', t.context.tmp);
+  t.truthy(shell.error());
+  t.is(result.code, 1);
+
+  // Ensure First file is copied
+  t.is(shell.cat(`${t.context.tmp}/file1`).toString(), 'test1');
+  t.is(
+    result.stderr,
+    `cp: will not overwrite just-created '${t.context.tmp}/file1' with 'resources/cp/file1'`
+  );
+});
+
+test('should not overwrite recently created files (not give error no-force mode)', t => {
+  const result = shell.cp('-n', 'resources/file1', 'resources/cp/file1', t.context.tmp);
+  t.falsy(shell.error());
+  t.is(result.code, 0);
+
+  // Ensure First file is copied
+  t.is(shell.cat(`${t.context.tmp}/file1`).toString(), 'test1');
+});
diff --git a/test/mv.js b/test/mv.js
@@ -206,3 +206,30 @@ test('dest exists, but -f given', t => {
   t.falsy(fs.existsSync('file1'));
   t.truthy(fs.existsSync('file2'));
 });
+
+test('should not overwrite recently created files', t => {
+  shell.mkdir('-p', 't');
+  const result = shell.mv('file1', 'cp/file1', 't/');
+  t.truthy(shell.error());
+  t.is(result.code, 1);
+
+  // Ensure First file is copied
+  t.is(shell.cat('t/file1').toString(), 'test1');
+  t.is(
+    result.stderr,
+    "mv: will not overwrite just-created 't/file1' with 'cp/file1'"
+  );
+  t.truthy(fs.existsSync('cp/file1'));
+});
+
+
+test('should not overwrite recently created files (not give error no-force mode)', t => {
+  shell.mkdir('-p', 't');
+  const result = shell.mv('-n', 'file1', 'cp/file1', 't/');
+  t.falsy(shell.error());
+  t.is(result.code, 0);
+
+  // Ensure First file is moved
+  t.is(shell.cat('t/file1').toString(), 'test1');
+  t.truthy(fs.existsSync('cp/file1'));
+});
diff --git a/test/resources/cp/file1 b/test/resources/cp/file1
@@ -0,0 +1 @@
+test2
