search

node-gfx / node-canvas-prebuilt

Repo used to build binaries for node-canvas on CI
170 stars 31 forks source link

Use --target instead of building with each node version #80

Closed zbjornson closed 4 years ago

zbjornson commented 5 years ago

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 full vX.Y.Z string, not just X. If you're only using X you could change it to --target=v$ver.0.0.

chearon commented 5 years ago
  1. I was wrong about ci/*/preinstall.sh being ready for NVM, that was true for Linux but not macOS, I've made the fix to .travis.yml for the latter.
  2. I just remembered the reason why I didn't do this. In order to test if the build worked we need node.

https://github.com/node-gfx/node-canvas-prebuilt/blob/b5aea2987c3ccc560e1456da21129d92bf68aaea/ci/install.sh#L50-L53

So we probably do need ci/*/node_version.sh so it can use Install-Product or whatever it does in master, and it can now just do a simple nvm install/nvm use in macOS and Linux. Then I guess we'll have to add something that skips the testing if a node version isn't available 😕 I'm not sure what else we can do unless there's some alternate to NVM for Windows.

zbjornson commented 5 years ago

Ahh. What about downloading the portable node executables from https://nodejs.org/dist/vX.Y.Z for testing?

e.g.

https://nodejs.org/dist/v10.16.0/win-x64/node.exe

chearon commented 5 years ago

That should work yeah, ci/win/node_version.sh can use wget, then probably has to do alias node ='./vX.Y.Z.exe' (hopefully that doesn't have side effects 😬). Note that I changed your example wiki code a bit, the version variable doesn't have the v in it.

I still like using node-gyp's version argument though, we can leave that.

fpauser commented 5 years ago

Any news on this?

zbjornson commented 4 years ago

Moot since https://github.com/Automattic/node-canvas/pull/1568