tidev / node-ios-device

Queries connected iOS devices and installs apps
Other
122 stars 37 forks source link

build: move to using prebuildify #66

Closed ewanharris closed 3 years ago

ewanharris commented 3 years ago

This replace the node-pre-gyp setup with prebuildify which will build the binaries before publish and include them in the npm package, replacing the need for the S3 bucket that currently serves the binaries.

This does unfortunately have the side effect that if a node version is changed and the binary does not exist locally, then it will no longer fallback to building a fresh binary. I assume this is probably something we could create our own package for if required, it's just that it does not exist today.

Currently:

sgtcoolguy commented 3 years ago
ewanharris commented 3 years ago
  1. Currently it uses --all when building which does all non-deprecated targets.If we wanted build for a specific set I think we'd have to provide a list of versions via --target
  2. I'm not sure how to really to get the arm64 builds going, it looks like prebuildify has some solutions for building and then combining things together before publishing but GitHub actions doesn't offer arm64 builds yet to my knowledge
  3. Honestly I'm not sure, looking at their readme they recommend node-pre-gyp-github if you don't want to use S3, so maybe it would work but they don't recommend it? I'd figured we were better off moving away from node-pre-gyp to avoid needing to publish artifacts anywhere (GH releases or S3) and as development had seem to have stopped recently (although it seems like they've picked it up again recently and are now published under a different package)
cb1kenobi commented 3 years ago

Taking a step back, we know we now have 2 architectures we need to support: x64 and ARM.

If you do not have the binary for your Node.js + platform + architecture, it will blow up. Prebuilds do not solve this problem. Bundling all the binaries into the SDK build does not solve the problem. We need something at runtime that does the rebuild if needed which is what node-pre-gyp-init does.

So, I propose 3 solutions:

  1. Write a new package that is similar to node-pre-gyp-init, but rebuilds the package as needed. We do not need prebuilds. We do not need to download all the binaries when building the SDK. The building happens in the SDK directory.

  2. Use prebuild + prebuild-install instead of prebuildify, then have a CI for each architecture that prebuilds the binary and puts it on GitHub. The SDK build could then grab all the prebuilds for all the architectures and bundle them into the SDK.

  3. At runtime, resolve node-ios-device from a user dot directory for the given platform and architecture, then directly require() it from there and NOT from the SDK. If the required version of node-ios-device is not present, npm install it which will trigger the compile. When we build the SDK, node-ios-device will have no binaries because it'll be built on Linux which is not a target and there will be no prebuilds.

ewanharris commented 3 years ago

The only problem being solved here was the one of removing the S3 bucket, the PR acknowledges that the replacement is not ideal as we will lose the recompile functionality provided by node-pre-gyp-init.

Given that we download all the binaries at SDK build time anyways, I figured that doing that upfront in node-ios-device just made sense. Out of your 3 proposals I'd probably vote for 1 or 2, I think 3 is needlessly complex

cb1kenobi commented 3 years ago

Proposal 2 still falls over if the user installs a new Node.js version that we don't have prebuilds for. We could port 1.x to N-API, then we'd be future proofed.

I wonder if it's just easier to forget about node-ios-device v1 and see about upgrading ioslib 1.x to use node-ios-device v3?

ewanharris commented 3 years ago

@sgtcoolguy's alternative of moving to serving off GitHub releases seems a better idea #69

ewanharris commented 3 years ago

Closing in favour of #69