-
Notifications
You must be signed in to change notification settings - Fork 71
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
Review state of canary in CI #220
Comments
node-test-commit-osx-arm is green 😇 |
A lot of failures are:
Any idea? |
A missing include? I think the memory mapped stuff is in |
Seems right: https://man7.org/linux/man-pages/man2/memfd_create.2.html The problem is that the include is already there:
|
The linked man page also has #define _GNU_SOURCE before the include, which doesn't appear to be in either the |
V8 added the call in v8/v8@7b2b3af |
The weird thing is that most linux hosts compiled without errors: https://ci.nodejs.org/job/node-test-commit-linux/44849/ |
It looks like most (all?) the failures of that type are centos 7/RHEL 7. I'm wondering if it's linked to the glibc version as there are other passing platforms using gcc 8. FWIW I'm currently setting up RHEL 8 s390x machines and kicked off a canary build in the job I've been using to test them and it looks like the rhel8 machine is building this where the rhel7 one failed: https://ci.nodejs.org/job/richardlau-node-test-commit-linuxone/19/ |
From https://man7.org/linux/man-pages/man2/memfd_create.2.html#VERSIONS:
CentOS 7/RHEL 7 is old (first released in 2014) and has glibc 2.17 [1]. The good news there is that we are planning to move building binaries for Node.js 18 onwards from CentOS 7 to RHEL 8 (nodejs/build#2815, nodejs/build#2741) so this may resolve itself over the next few weeks as I get RHEL 8 hosts added to the CI. [1] Refs: https://distrowatch.com/dwres.php?resource=package-in-distro&pkg=glibc |
Does it mean we have to increase the minimum glibc for v18.0.0? |
@nodejs/v8 does Chromium document somewhere the minimum required versions of the Linux kernel and glibc? |
Yes, for the running the release binaries at least we'll need to raise the minimum glibc to 2.28 (as that is what is used by RHEL 8). It may be possible to still build against glibc versions earlier than that, but it looks like you'd need to be on at least 2.27 if the change on canary lands, according to the documentation. |
Apropos that memfd_create error, that's in a function that's only used for testing. If you really wanted to keep compatibility with old kernels, you could float a patch that changes it to: PlatformSharedMemoryHandle OS::CreateSharedMemoryHandleForTesting(size_t size) {
return kInvalidSharedMemoryHandle;
} |
The function that references it is only used for V8 testing. Refs: nodejs/node-v8#220
@bnoordhuis good point. I added the following patch: nodejs/node@72f0932 |
@targos Compile-time feature checking means the binary won't run on older systems when it's compiled on a newer system. There are ways around that but I wouldn't bother in this particular case and just stub it out. |
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: nodejs/node-v8#220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: nodejs/node-v8#220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: nodejs/node-v8#220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
Remove call to `memfd_create`. The function that references it is only used for V8 testing. Refs: #220
It looks like they didn't intend to bump glibc requirements: v8/v8@4e81f25 |
I finally found the time to update and fix canary, at least for my system (macOS, arm64)
Latest commit: 0a3560b
CI: https://ci.nodejs.org/job/node-test-commit/52045/
The text was updated successfully, but these errors were encountered: