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

fs: expose copy-on-write flags for fs.copyFile() #19759

Closed
wants to merge 1 commit into from
Closed

fs: expose copy-on-write flags for fs.copyFile() #19759

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Apr 2, 2018

This commit exposes the UV_FS_COPYFILE_FICLONE and UV_FS_COPYFILE_FICLONE_FORCE flags added in libuv 1.20.0.

Note, the libuv update hasn't landed yet (#19758), so the first commit is only temporary.

Fixes: #19152

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Apr 2, 2018
@joyeecheung joyeecheung added the fs Issues and PRs related to the fs subsystem / file system. label Apr 2, 2018
joyeecheung
joyeecheung approved these changes Apr 2, 2018
View reviewed changes
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

The second commit LGTM

@mscdex mscdex added the blocked PRs that are blocked by other issues or PRs. label Apr 2, 2018
jasnell
jasnell reviewed Apr 3, 2018
View reviewed changes
doc/api/fs.md
exists.
* `fs.constants.COPYFILE_FICLONE` - The copy operation will attempt to create a
copy-on-write reflink. If the platform does not support copy-on-write, then a
fallback copy mechanism is used.
Copy link
Member

Choose a reason for hiding this comment

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

which fallback copy mechanism? Is it system specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. It falls back to uv_fs_copyfile()'s default behavior for that platform.

@cjihrig cjihrig force-pushed the ficlone branch 2 times, most recently from d50a567 to cb7f50c Compare April 6, 2018 02:19
@cjihrig
fs: expose copy-on-write flags for fs.copyFile()
31e63ca 
This commit exposes the UV_FS_COPYFILE_FICLONE and
UV_FS_COPYFILE_FICLONE_FORCE flags added in libuv 1.20.0.

Fixes: nodejs#19152
PR-URL: nodejs#19759
Fixes: nodejs#19152
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 6, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/14085/

cjihrig added a commit that referenced this pull request Apr 6, 2018
@cjihrig
fs: expose copy-on-write flags for fs.copyFile()
a16d88d 
This commit exposes the UV_FS_COPYFILE_FICLONE and
UV_FS_COPYFILE_FICLONE_FORCE flags added in libuv 1.20.0.

Fixes: #19152
PR-URL: #19759
Fixes: #19152
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig
Copy link
Contributor Author

cjihrig commented Apr 6, 2018

Landed in a16d88d.

@cjihrig cjihrig closed this Apr 6, 2018
@cjihrig cjihrig deleted the ficlone branch April 6, 2018 03:23
@targos targos added semver-minor PRs that contain new features and should be released in the next minor version. and removed blocked PRs that are blocked by other issues or PRs. labels Apr 6, 2018
targos pushed a commit that referenced this pull request Apr 6, 2018
@cjihrig @targos
fs: expose copy-on-write flags for fs.copyFile()
5ecd82d 
This commit exposes the UV_FS_COPYFILE_FICLONE and
UV_FS_COPYFILE_FICLONE_FORCE flags added in libuv 1.20.0.

Fixes: #19152
PR-URL: #19759
Fixes: #19152
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this pull request May 1, 2018
@cjihrig @BridgeAR
fs: expose copy-on-write flags for fs.copyFile()
3bdc9ef 
This commit exposes the UV_FS_COPYFILE_FICLONE and
UV_FS_COPYFILE_FICLONE_FORCE flags added in libuv 1.20.0.

Fixes: nodejs#19152
PR-URL: nodejs#19759
Fixes: nodejs#19152
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Member

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@jasnell jasnell jasnell approved these changes

@santigimeno santigimeno santigimeno approved these changes

@joyeecheung joyeecheung joyeecheung approved these changes

Assignees
No one assigned
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@cjihrig @MylesBorins @bnoordhuis @jasnell @santigimeno @joyeecheung @mscdex @targos @nodejs-github-bot