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

Point to GitHub releases for pre-built bindings #69

Merged
merged 8 commits into from
Mar 12, 2021

Conversation

sgtcoolguy
Copy link
Contributor

Relates to #66, #67

This points at Github releases URLs for this repo to hold the pre-built binaries. It required modifying the download-all script to support handling 301/302 redirects.

This allows us to stop storing bindings in our s3 bucket and instead place formal releases with the bindings.

@ewanharris
Copy link
Contributor

Jenkins error looks to be related to nodejs/node-gyp/issues/1917, I'm guessing as this uses yarn there's an older npm/node-gyp on the box.

Testing this by requiring the module without the binding would always fallback to building for me, after some debugging the request from node-pre-gyp was erroring with Request error Error: Remote end closed socket abruptly. (but only when required, not when running the command myself).

However, after upgrading to @mapbox/node-prep-gyp that request works just fine. So maybe this PR + that upgrade should be enough to get us to pulling from GitHub?

@ewanharris
Copy link
Contributor

Filed cb1kenobi/node-pre-gyp-init/pull/2 to update node-pre-gyp-init to use the mapbox scoped module, once that is merged/published we can update node-pre-gyp-init and node-pre-gyp in this PR and we should be good to go

@ewanharris
Copy link
Contributor

I can repro the error locally, it looks to be an issue when building for Node.js 0.10.X with Python 3, every other version builds fine. Given #68 I don't think it's worth trying to find a workaround to get it building

@sgtcoolguy, @cb1kenobi if ya'll are happy the additions I made to his PR fancy dropping a LGTM?

@ewanharris ewanharris requested a review from cb1kenobi March 9, 2021 21:24
@sgtcoolguy sgtcoolguy merged commit dd8afc1 into tidev:1_X Mar 12, 2021
@sgtcoolguy sgtcoolguy deleted the github-releases branch March 12, 2021 18:11
@drauggres
Copy link

Hi. The README.md still mentions AWS: https://github.com/appcelerator/node-ios-device/tree/1_X#publishing

@ewanharris
Copy link
Contributor

Thanks @drauggres I've fixed that in #74

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants