Add cloning support #40
Conversation
This commit makes COPYFILE_FICLONE the default. A lot of tests may need to be changed...
| test('clone a big file', async t => { | ||
| const buffer = crypto.randomBytes(THREE_HUNDRED_KILO); | ||
| fs.writeFileSync(t.context.source, buffer); | ||
| await cpFile(t.context.source, t.context.destination, {clone: true}); |
There was a problem hiding this comment.
Can't make this force yet -- I can't figure out how to use different install scripts for different OS. Since cmd is super incompatible with sh I don't think I can get away with some if statements. Otherwise I would be able to make a known-fs disk image and stuff.
There was a problem hiding this comment.
Oh, travis on Windows uses mingw sh. Guess I am free to do whatever.
|
Eugh, |
I am dumb enough to require this. A few kludges still need some ignore; the others indicate my laziness.
|
|
||
| /** | ||
| Perform a fast copy-on-write clone (reflink) if possible. | ||
|
|
There was a problem hiding this comment.
Not clear from the docs here what force does.
Maybe also mention some use-cases for needing to set it to false or 'force' as I cannot think of anything reason to not have this be just true all the time.
There was a problem hiding this comment.
This is analogous to gnu cp's --reflink=auto vs --reflink. From a usability standpoint, it is usually preferable to have a fallback (hence true), but when someone comes onto a new platform it's nice to have a test for whether reflink is supported (hence "force").
| @@ -54,6 +55,9 @@ | |||
| "sinon": "^9.0.0", | |||
| "tsd": "^0.11.0", | |||
| "uuid": "^7.0.2", | |||
| "xo": "^0.27.2" | |||
| "xo": "^0.27.2", | |||
There was a problem hiding this comment.
Try upgrading it to the latest version. That version has built-in TS support.
There was a problem hiding this comment.
The current version already does have the commit for TS support. And to quote a common saying, "it works on my machine"...
Could be travis CI caching an old version in the node_modules for some other dep though.
There was a problem hiding this comment.
Sure, but the latest XO version upgrades the TS parsers and such.
| * @param {string} destination | ||
| * @param {import('.').Options} options | ||
| * @param {ProgressEmitter} progressEmitter | ||
| */ |
There was a problem hiding this comment.
I think all the JSDoc comments should have been added in a separate PR. Try to keep that in mind in the future. It makes it much easier to review without all the unrelated noise.
|
Thanks for working on this 🙌🏻 |
| 'use strict'; | ||
| const EventEmitter = require('events'); | ||
| const { EventEmitter } = require('events'); |
| @param {string} destinationPath | ||
| @param {import('.').Options} options | ||
| @returns {Promise<void> & import('.').ProgressEmitter} | ||
| */ |
There was a problem hiding this comment.
| */ | |
| */ |
Applies everywhere.
|
@Artoria2e5 Bump |
|
@Artoria2e5 Bump |
This commit makes COPYFILE_FICLONE the default. A lot of tests may need to be changed...
Fixes #27