Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add '-p' (preserve) option to 'cp', fixes #771 #841

Closed
wants to merge 4 commits into from
Closed

Add '-p' (preserve) option to 'cp', fixes #771 #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bafc9a9
Add '-p' (preserve) option to 'cp'
dwi2 May 2, 2018
d84236d
Address review comment and add sleep in test to workaround possible r…
dwi2 May 6, 2018
3ede418
Update time before fd close
dwi2 May 6, 2018
4760a76
Use resources/cp/file1 and add sleep to workaround possible racing
dwi2 May 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 8 additions & 3 deletions src/cp.js
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@ common.register('cp', _cp, {
'r': 'recursive',
'L': 'followsymlink',
'P': 'noFollowsymlink',
'p': 'preserve',
},
wrapOutput: false,
});
Expand Down Expand Up @@ -51,6 +52,7 @@ function copyFileSync(srcFile, destFile, options) {
var pos = 0;
var fdr = null;
var fdw = null;
var srcStat = common.statFollowLinks(srcFile);

try {
fdr = fs.openSync(srcFile, 'r');
Expand All @@ -60,7 +62,7 @@ function copyFileSync(srcFile, destFile, options) {
}

try {
fdw = fs.openSync(destFile, 'w');
fdw = fs.openSync(destFile, 'w', srcStat.mode);
} catch (e) {
/* istanbul ignore next */
common.error('copyFileSync: could not write to dest file (code=' + e.code + '):' + destFile);
Expand All @@ -72,10 +74,13 @@ function copyFileSync(srcFile, destFile, options) {
pos += bytesRead;
}

if (options.preserve) {
fs.fchownSync(fdw, srcStat.uid, srcStat.gid);
fs.futimesSync(fdw, srcStat.atime, srcStat.mtime);
Copy link
Contributor Author

@dwi2 dwi2 May 6, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On linux, fs.utimesSync() always produces timestamps rounded off to seconds but original files could have timestamps of millisecond level. But I still don't why is that so I probably need more time to figure it out.
However fs.futimesSync() could produce timestamps of millisecond level. So I put it this way for now.

Also I tried to add sleep in test but it didn't work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, ok. This implementation is fine. I will add a comment explaining that utimesSync doesn't work but futimesSync does.

}

fs.closeSync(fdr);
fs.closeSync(fdw);

fs.chmodSync(destFile, common.statFollowLinks(srcFile).mode);
}
}

Expand Down
27 changes: 27 additions & 0 deletions test/cp.js
View file
Open in desktop
Expand Up @@ -756,3 +756,30 @@ test('should not overwrite recently created files (not give error no-force mode)
// Ensure First file is copied
t.is(shell.cat(`${t.context.tmp}/file1`).toString(), 'test1');
});

test('cp -p should preserve mode, ownership, and timestamp', t => {
const result = shell.cp('-p', 'test/resources/cp/file1', `${t.context.tmp}/preservedFile1`);
const stat = common.statFollowLinks('test/resources/cp/file1');
const statOfResult = common.statFollowLinks(`${t.context.tmp}/preservedFile1`);

t.is(result.code, 0);
t.is(stat.mtime.getTime(), statOfResult.mtime.getTime());
t.is(stat.atime.getTime(), statOfResult.atime.getTime());
t.is(stat.mode, statOfResult.mode);
t.is(stat.uid, statOfResult.uid);
t.is(stat.gid, statOfResult.gid);
});

test('cp -p should preserve mode, ownership, and timestamp of symlink', t => {
const result = shell.cp('-p', 'test/resources/link', `${t.context.tmp}/copiedLink`);
utils.sleep(1000);
const stat = common.statFollowLinks('test/resources/link');
const statOfResult = common.statFollowLinks(`${t.context.tmp}/copiedLink`);

t.is(result.code, 0);
t.is(stat.mtime.getTime(), statOfResult.mtime.getTime());
t.is(stat.atime.getTime(), statOfResult.atime.getTime());
t.is(stat.mode, statOfResult.mode);
t.is(stat.uid, statOfResult.uid);
t.is(stat.rid, statOfResult.rid);
});