-
Notifications
You must be signed in to change notification settings - Fork 31
Use --target instead of building with each node version #80
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
Conversation
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.
I know I tried this before and I didn't like the fact that node-gyp --target
requires a full version string (edit: ah you said that in the description!). So we have to specify builds as 12.x.x 11.x.x 10.x.x
in the environment variables instead of 12 11 10
.
It'll be another burden but probably worth it.
Hopefully there weren't any other issues I'm not recalling. After the comment is addressed I'll give it a test run.
.travis.yml
Outdated
- >- | ||
source ~/.nvm/nvm.sh | ||
nvm use 10 || { nvm install 10; nvm use 10; } | ||
npm install -g node-gyp |
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.
This looks like it was moved from the old node_version.sh
file, could you do the nvm
commands in ci/linux|osx/preinstall.sh
? At that point NVM is already setup.
I think you would just add
nvm install node # latest
nvm use node # latest
npm install -g node-gyp
Could do Windows node installation in win/preinstall.sh
for consistency, or leave it in appveyor.yml since it was already there.
node-canvas-prebuilt/ci/install.sh Lines 50 to 53 in b5aea29
So we probably do need |
Ahh. What about downloading the portable node executables from https://nodejs.org/dist/vX.Y.Z for testing? e.g. |
That should work yeah, I still like using node-gyp's version argument though, we can leave that. |
Any news on this? |
Moot since Automattic/node-canvas#1568 |
I have not tested this, but this should avoid the issue with Appveyor being slow to update their images and speed up the builds.
I don't know what $NODEJS_VERSIONS looks like; the
--target
option takes a fullvX.Y.Z
string, not justX
. If you're only usingX
you could change it to--target=v$ver.0.0
.