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

Use node-pre-gyp to publish binaries for windows (fixes #186) #593

Closed
wants to merge 1 commit into from

Conversation

kondi
Copy link

@kondi kondi commented Oct 5, 2016

This solution only works on windows because on windows the ./build/Release folder contains all the required binary dependencies. On linux and osx the ./lib folder is required as well, but they are excluded from the node-pre-gyp precompiled binary package. We might want to move the logic which prepares the ./lib folder from the binding.gyp file to a preinstall script. Would not be so complicates as some part is already in binding.js file.

However I think the binary precompilation is much more important on windows than on linux. This is already a big a improvement in my opinion.

I have updated to appveyor.yml configuration to automatically publish the precompiled binary when the commit message contains the "[publish binary]" string.

Important notes

In my branch I am using my S3 bucket (kondi-sharp) and my encrypted keys to upload the binaries. You have to change the bucket in package.json binary/host entry and update the keys in appveyor.yml file. More details here: Create an S3 bucket and Create secure appveyor variables. If you do not update these, the [publish binary] builds will fail on your AppVeyor account because the encryption is unique per project instance.

@kondi
Copy link
Author

kondi commented Oct 5, 2016

Alternatively you can remove the publish call from AppVeyor and manage manually the binary publication with locally stored S3 access keys.

@lovell
Copy link
Owner

lovell commented Oct 6, 2016

Hello, thank you for starting work on this. Removing the need to install MSVC (even the newer, "lightweight" version) on Windows is going to make a huge improvement for users of sharp.

  • How do we control which build artefacts are bundled? For Windows I think we need only the sharp.node and libvips-cpp.dll files within build\Release as all of the other DLL files are placed there by the binding.js script.
  • Is it possible to publish build artefacts only for tagged builds (via git tag vX.Y.Z) rather than based on commit message?

I'd like the pre-compiled sharp binaries to be served via Bintray alongside the pre-compiled dependent binaries because (a) S3 bandwidth charges (US$90/TB) are too high and (b) Bintray is Akamai-fronted, which I've found provides a better experience for users in Asia and Oceania. The docs suggest this should be possible.

@kondi
Copy link
Author

kondi commented Oct 6, 2016

If you want to make it tag based then it makes sense to use node-pre-gyp-github instead which can use github releases for free file serving (instead of Bintray or S3). I can help to set it up if you want. If you still prefer bintray, you need your own way of uploading the output instead using the standard publish command of node-pre-gyp or node-pre-gyp-github.

Controlling the bundle content in this case is very tricky. node-pre-gyp needs one folder with all the required binaries. If you do not want to have everything from the /build/Release, it is common practice to create a new folder (like /bindings) and copy there the needed file(s) as a last step of gyp build and use this folder for the precompiled binary bundle. On windows it is required to have the dlls next to the sharp.node, so they have to be in the same folder. If you want to exclude some files from the pre-gyp bundle, you have to delete them before the node-pre-gyp package step. But you have to copy them back after to make it working. I think this is just too fragile, it is not a good idea to control this directory from two sources in this case.

I would suggest to get all the required files in one request using the standard pre-gyp procedure instead of having the custom bindings.js logic. It is enough to use the bindings.js when you really want to (or have to) build from sources. Currently the bindings.js is called from gyp file, so anyway it will be executed only when no pre-gyp bundle have been downloaded.

@lovell
Copy link
Owner

lovell commented Oct 7, 2016

"the bindings.js is called from gyp file, so anyway it will be executed only when no pre-gyp bundle have been downloaded"

This is a very good point. I'm going to experiment a little with how this can work cross-platform and, as you say, avoid it becoming (any more) fragile. node-pre-gyp separates packaging from publishing, which might provide the right hook.

Thank you for your work so far getting this started - I'm sure Windows users will rejoice when this all "just works".

@wtgtybhertgeghgtwtg
Copy link

wtgtybhertgeghgtwtg commented Oct 17, 2016

I am probably missing something, but would this remove the need for the request dependency? Granted, request is a dependency of node-pre-gyp, so it would be there, anyway.

@KyleAMathews
Copy link

Very excited for this! This will be excellent for mac/linux users as well as it adds compiling sharp adds 1+ minute to installation currently :-)

@lovell
Copy link
Owner

lovell commented Feb 8, 2017

Closing this as there remains work to extract the libvips-downloading logic from the binding.* files first. Please subscribe to #186 for updates. Thanks for your work on this @kondi.

@lovell lovell closed this Feb 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants