-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
CI: add Node.js 15 #2415
CI: add Node.js 15 #2415
Conversation
@@ -1,4 +1,4 @@ | |||
os: Visual Studio 2017 | |||
os: Visual Studio 2019 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the changelog:
build: drop support for VS2017
I guess it's still too early. |
Node.js 15 ships with npm v7, which now assumes the user permission of the working directory when running install scripts. This means we need to ensure the working directory is owned by root via |
This PR depends on this now: docker-library/official-images#8933 |
@lovell Looks like we're good to go now, the FreeBSD CI is still failing, but it is failing on master as well so I guess it's expected. |
Merci beaucoup! |
Thanks! When can we expect |
The use of N-API means the existing binaries already work with Node.js 15. $ nvm i 15
Downloading and installing node v15.0.1...
Downloading https://nodejs.org/dist/v15.0.1/node-v15.0.1-linux-x64.tar.xz...
######################################################################### 100.0%
Computing checksum with sha256sum
Checksums matched!
now using node v15.0.1 (npm v7.0.3)
$ npm i sharp
added 71 packages, and audited 71 packages in 16s
6 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
$ node -p "require('sharp').format"
{
jpeg: {
id: 'jpeg',
input: { file: true, buffer: true, stream: true },
output: { file: true, buffer: true, stream: true }
},
... |
Mmh, unfortunately it's not working in our CI. Any idea why the prebuilt binaries might not be used?
|
I suspect you've run into this breaking change of npm v7 - check the user permissions on all directories involved. |
We've done some investigation, it seems like |
npm v7 is far less verbose during installation - please can you trying adding a non-zero exit code here to see if it might be swallowing an error message: diff --git a/install/dll-copy.js b/install/dll-copy.js
index 1e88b1d..0fcadda 100644
--- a/install/dll-copy.js
+++ b/install/dll-copy.js
@@ -33,5 +33,6 @@ if (platform === 'win32') {
});
} catch (err) {
npmLog.error('sharp', err.message);
+ process.exit(1);
} |
I've been combing through Process Monitor-captured events captured while running I haven't figured out why |
Running
Running the
Given that the install is running It looks like prebuild-install added npm 7 support in 6.0.0. I'll add a PR. |
Shamelessly based on this commit: edf3fe2