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.

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

fs: change default mode to COPYFILE_FICLONE #47951

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HinataKah0
Copy link
Contributor

@HinataKah0 HinataKah0 commented May 10, 2023

Change default value of mode to COPYFILE_FICLONE in
fs.copyFile and fs.copyFileSync.

If copy-on-write is not supported, fallback copy mechanism
will be used.

Fixes: #47861

I am aware that the discussion is still very nascent. If we are not going to do this or if we are going in different direction, feel free to close this PR.

TODO:

  • Update commit message

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels May 10, 2023
@@ -61,6 +62,9 @@ fs.unlinkSync(dest);
fs.copyFileSync(src, dest, UV_FS_COPYFILE_FICLONE);
verify(src, dest);

// Verify that default mode is COPYFILE_FICLONE.
assert.strictEqual(getValidMode(null, 'copyFile'), COPYFILE_FICLONE);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I am not really sure how to test it.

Input is welcomed! 😄

@HinataKah0 HinataKah0 force-pushed the default-COPYFILE_FICLONE branch from 843999c to 03af9a5 Compare May 11, 2023 13:06
@HinataKah0 HinataKah0 changed the title fs: Change default mode to COPYFILE_FICLONE fs: change default mode to COPYFILE_FICLONE May 11, 2023
@aduh95
Copy link
Contributor

aduh95 commented Sep 20, 2023

Ping @nodejs/fs for reviews

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fs: Change the default value of mode argument of fs.copyFile() to fs.constants.COPYFILE_FICLONE
3 participants