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

macOS binaries: UV_FS_COPYFILE_FICLONE not supported #24521

Closed
cristianl opened this issue Nov 20, 2018 · 9 comments
Closed

macOS binaries: UV_FS_COPYFILE_FICLONE not supported #24521

cristianl opened this issue Nov 20, 2018 · 9 comments
Labels
confirmed-bug Issues with confirmed bugs. macos Issues and PRs related to the macOS platform / OSX.

Comments

@cristianl
Copy link

Node binaries from the official pkg distribution do not support copy-on-write (UV_FS_COPYFILE_FICLONE) on apfs-formatted volumes.

  • Version: v11.2.0
  • Platform: Darwin iMac.local 17.7.0 Darwin Kernel Version 17.7.0: Wed Oct 10 23:06:14 PDT 2018; root:xnu-4570.71.13~1/RELEASE_X86_64 x86_64
  • Subsystem: fs

Steps to reproduce

Generate a file (3MB here, size is not important) to clone later

mkdir ~/apfs-cow-test && cd ~/apfs-cow-test
head -c 3000000 </dev/urandom >origfile
node

In the node REPL, run:

const fs = require('fs')
fs.copyFile('origfile', 'newfile', fs.constants.UV_FS_COPYFILE_FICLONE_FORCE, err => { if (err) { console.log(err) }})

In the official node binary, this error is raised:

> { [Error: ENOSYS: function not implemented, copyfile 'origfile' -> 'newfile']
  errno: -78,
  code: 'ENOSYS',
  syscall: 'copyfile',
  path: 'origfile',
  dest: 'newfile' }

With a binary that supports this feature, the file is cloned normally. You can verify that the clone call works as intended by cloning a large file and checking the volume size on Disk Utility. It doesn't create a hardlink. Changes to the clone don't affect the original file.

Implications

This bug also affects users of nvm and n who install prebuilt binaries (the default behavior). brew users are not impacted as the prebuilt binary (bottle) is built with support for this flag.

Yarn is directly affected by this. UV_FS_COPYFILE_FICLONE falls back to normal copying, negating the performance and disk space benefits of cloning.

@bnoordhuis
Copy link
Member

With a binary that supports this feature

I'm unclear on what you mean by that: another node binary, or another program entirely? If it's the former, where did you get it from?

@cristianl
Copy link
Author

Another node binary. 11.2.0, installed via brew install node. I'd like to try building from source but I'm afraid V8 would take some time.

@bnoordhuis bnoordhuis added the macos Issues and PRs related to the macOS platform / OSX. label Nov 20, 2018
@bnoordhuis
Copy link
Member

Okay, I understand what you mean now. It sounds like the release binaries were built on a system whose headers don't define COPYFILE_CLONE_FORCE. The ENOSYS comes from an #ifdef guard in libuv.

@nodejs/releasers Can one of you confirm? I'm having a hard time tracking down the machine used to build them. The define should be in /usr/include/copyfile.h (that's where it is on my system anyway.)

@targos
Copy link
Member

targos commented Nov 20, 2018

@bnoordhuis version 11.2.0 was built on release-macstadium-macos10.11-x64-1.
I do not have access to the machine so I can only give you a few lines from the build output that contain version numbers:

21:12:59 Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
21:12:59 Apple LLVM version 8.0.0 (clang-800.0.42.1)
21:12:59 Target: x86_64-apple-darwin15.0.0

Version 15.0.0 seems to be OS X El Capitan.

@rvagg
Copy link
Member

rvagg commented Nov 21, 2018

Yep, it's missing COPYFILE_CLONE_FORCE. Header says /* version 0.1 */ if that helps.

Darwin release-macstadium-macos10.11-x64-1.nodejs.org 15.0.0 Darwin Kernel Version 15.0.0: Sat Sep 19 15:53:46 PDT 2015; root:xnu-3247.10.11~1/RELEASE_X86_64 x86_64

@bnoordhuis
Copy link
Member

bnoordhuis commented Nov 22, 2018

I had a look and I think it's unfixable short-term. COPYFILE_CLONE_FORCE was added in macos 10.12 and the release machine is 10.11.

Libuv can't hard-code the value because copyfile() blissfully ignores flags it doesn't know about. That would break the API contract that states UV_FS_COPYFILE_FICLONE_FORCE fails with an error when the file system doesn't support cloning.

Theoretically, libuv could call clonefileat/fclonefileat() directly instead of going through copyfile() but that's basically reimplementing copyfile(), and probably poorly.

edit: Perhaps libuv can turn the compile-time guard into a runtime OS version check. Needs investigation.

@rvagg
Copy link
Member

rvagg commented Nov 23, 2018

So, does this call for switching to 10.12+ for Node 12 release builds?

bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Nov 24, 2018
Using #ifdef guards is not reliable because libuv may have been compiled
on a newer system than what it runs on, or vice versa.

Fixes: libuv#2088
Fixes: nodejs/node#24521
@bnoordhuis
Copy link
Member

@rvagg I think we can work around this in libuv, see libuv/libuv#2092.

bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Nov 25, 2018
Using #ifdef guards is not reliable because libuv may have been compiled
on a newer system than what it runs on, or vice versa.

Fixes: libuv#2088
Fixes: nodejs/node#24521
PR-URL: libuv#2092
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
@bnoordhuis
Copy link
Member

This should be fixed the next time libuv is upgraded, probably sometime next month.

cjihrig added a commit to cjihrig/node that referenced this issue Dec 19, 2018
PR-URL: nodejs#25078
Fixes: nodejs#24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue Dec 25, 2018
PR-URL: #25078
Fixes: #24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
refack pushed a commit to refack/node that referenced this issue Jan 14, 2019
PR-URL: nodejs#25078
Fixes: nodejs#24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
njlr pushed a commit to buckaroo-pm/libuv that referenced this issue Apr 5, 2019
Using #ifdef guards is not reliable because libuv may have been compiled
on a newer system than what it runs on, or vice versa.

Fixes: libuv#2088
Fixes: nodejs/node#24521
PR-URL: libuv#2092
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
BethGriggs pushed a commit that referenced this issue Apr 17, 2019
PR-URL: #25078
Fixes: #24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BethGriggs pushed a commit that referenced this issue Apr 28, 2019
PR-URL: #25078
Fixes: #24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
BethGriggs pushed a commit that referenced this issue May 10, 2019
PR-URL: #25078
Fixes: #24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
MylesBorins pushed a commit that referenced this issue May 16, 2019
PR-URL: #25078
Fixes: #24521
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Development

No branches or pull requests

4 participants