Skip to content

Commit

Permalink
Fix #631 throw error when overwriting recently created file (#702)
Browse files Browse the repository at this point in the history
* cp: add error to not overwrite recently created files #631

* cp: add tests for errors not overwrite recently created files #631

* mv: show error when overwriting recently created file #631

* mv: add tests for error on recently created files #631

* mv: test remove unnecessary steps #631
  • Loading branch information
uttpal authored and nfischer committed Apr 15, 2017
1 parent d8ac4d3 commit bbe521b
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 3 deletions.
21 changes: 19 additions & 2 deletions src/cp.js
Expand Up @@ -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()) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
20 changes: 19 additions & 1 deletion src/mv.js
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
36 changes: 36 additions & 0 deletions test/cp.js
Expand Up @@ -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');
});
27 changes: 27 additions & 0 deletions test/mv.js
Expand Up @@ -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'));
});
1 change: 1 addition & 0 deletions test/resources/cp/file1
@@ -0,0 +1 @@
test2

0 comments on commit bbe521b

Please sign in to comment.