search

noble / node-bluetooth-hci-socket

MIT License
154 stars 188 forks source link

Make compatible with node-pre-gyp so the binaries are pre-built so they are easier to install #41

Closed manixate closed 7 years ago

manixate commented 7 years ago

It makes it easier to install because of the hassle to download build tools for the platforms. Specially on Windows where node-gyp requires around 3GB of download of build tools.

To make it work you need to do the following:

Create personal access token for both CIs

Windows

  1. Create account on Appveyor which is free.
  2. Import your project.
  3. Go to Settings of your project and set NODE_PRE_GYP_GITHUB_TOKEN environment variable to the value of the access token.
  4. Now you are done.
  5. Just push a new tag and the binaries will be uploaded properly.

Linux

  1. Create account on Travis which is free.
  2. Import your project.
  3. Follow steps 3 and onwards of above.

If you need any help you can contact me

sandeepmistry commented 7 years ago

Hi @manixate,

Thanks for submitting this!

A few questions:

manixate commented 7 years ago

I will try to answer your questions as best as I can:

sandeepmistry commented 7 years ago

@manixate thanks!

Please remove lib/binding/binding.node from this pull request, you can see it as a new added file.

manixate commented 7 years ago

Accidentally added that file. Now removed.

manixate commented 7 years ago

Hi @sandeepmistry, did you get any time to look into this?

sandeepmistry commented 7 years ago

@manixate thanks!

I'll make a 0.5.2 tag, can you review the release artifacts once the CI builds are done? Then I can publish to npm.

I've removed the badge from the read me for now, because the build status was only valid for tags.

manixate commented 7 years ago

@sandeepmistry it is working. See this: https://ci.appveyor.com/project/manixate/node-bluetooth-hci-socket/build/1.0.31 for Windows artifacts and this: https://travis-ci.org/manixate/node-bluetooth-hci-socket/builds/169988903 for Linux artifacts.

sandeepmistry commented 7 years ago

@manixate well those files are for your account right?

Please take a look at these ones: https://github.com/sandeepmistry/node-bluetooth-hci-socket/releases/tag/0.5.2

Also, if you run npm install from GitHub master, does node-pre-gyp try to pull the versions from Github Releases?

manixate commented 7 years ago

@sandeepmistry I tested on Linux and Windows and it works. It downloads the prebuilt binaries instead of building them. It does have to build node-usb package though but that is only on Linux because it doesn't provide linux built binaries.

sandeepmistry commented 7 years ago

@manixate awesome, thanks for trying it out!

What do you think about taking a similar approach to node-usb for Linux, and only providing pre-built binaries for Linux?

I think it's easier for them to get the build deps and in general they a bit more technical. Currently, the Travis CI only provides 64-bit builds, not 32-bit and ARM.

oroce commented 7 years ago

actually I've been working on building docker images for arm* architectures which can be (probably) reused for building noble for different archiectures: https://github.com/oroce/docker-alpine-armhf (it's in early stage but it is achievable).

Also we can try the hypriot's docker image.

sandeepmistry commented 7 years ago

@oroce cool, thanks for the heads up!

Would this be compatible with Travis CI?

oroce commented 7 years ago

@sandeepmistry All we need is docker, and since TravisCI does support docker it should work. The solution I posted above uses qemu to emulate certain architectures, which makes the process slow but we can build release for as many architectures and operating systems as needed.

sandeepmistry commented 7 years ago

@oroce great!

Do you have time to try this out and submit a PR?

I'll look into 32-bit Linux support, node-serialport supports this: https://github.com/EmergingTechnologyAdvisors/node-serialport/blob/master/.travis.yml

oroce commented 7 years ago

@sandeepmistry sure, I'm running some experiments with the docker stuff and I'm gonna come back with the results.

sandeepmistry commented 7 years ago

@oroce cool!

I've been testing out cross-compiling in the following branch: https://github.com/sandeepmistry/node-bluetooth-hci-socket/blob/pre-gyp-32-bit/.travis.yml

I'm not sure if the ARM version works yet, we need Travis CI to whitelist the g++-4.8-arm-linux-gnueabihf package to test. Only Node.js 0.12 for ARM builds with the regular g++-arm-linux-gnueabihf. Request to whitelist can be found here: https://github.com/travis-ci/apt-package-whitelist/issues/3464

oroce commented 7 years ago

quick update from me.

I'm able to run custom architecture on travisci (arm64 and armv7l): https://travis-ci.org/oroce/node-bluetooth-hci-socket-fork/builds/172748607#L328-L336

I'm gonna restructure the build script to be able to compile the source on those archs.

oroce commented 7 years ago

Wow, it was faster than I though it'll be, I managed to have arm build:

The code can be found here: https://github.com/sandeepmistry/node-bluetooth-hci-socket/compare/master...oroce:docker-arm-build (the number of commits and log are mess, it's really hard to test this:()

The process is pretty slow right now, but it works. It can be enhanced fortunately:

My biggest concern right now is that node-pre-gyp uses the architecture arm, but I thought the goal is having multiple arm builds (for example rpi1 is armv6 while rpi2 is armhf). Unfortunately I'm not a pre-gyp expert, @manixate don't you have experience with pre-gyp and arm builds?

sandeepmistry commented 7 years ago

@oroce cool, thanks for sharing your progress!

@manixate looks like node-pre-gyp-github now requires Node.js 4 or higher, so we'll have to disable pre-gyp for 0.12

manixate commented 7 years ago

Sorry I was pretty busy in my schedule so couldn't get into the thread.

@oroce I also don't know much about the internals of node-pre-gyp but from what I can see if the binary is compiled on the target arch it will build it. See this node-serialport. Apparently it is able to run on armv6 and armv7 without any different configuration, only the code needs to be compatible.

@sandeepmistry Where did you find that node-pre-gyp-github requires node.js 4 or higher? From what I know, node-pre-gyp-github is just required to publish the binaries to github releases tag so it should not be too much dependent on node.js version.

sandeepmistry commented 7 years ago

@manixate

Where did you find that node-pre-gyp-github requires node.js 4 or higher?

See: https://ci.appveyor.com/project/sandeepmistry/node-bluetooth-hci-socket/build/1.0.8/job/wmeeud2rdyuuop0m

npm install -g node-gyp node-pre-gyp-github
C:\Users\appveyor\AppData\Roaming\npm\node-pre-gyp-github -> C:\Users\appveyor\AppData\Roaming\npm\node_modules\node-pre-gyp-github\bin\node-pre-gyp-github.js
npm : npm WARN engine request@2.78.0: wanted: {"node":">= 4"} (current: {"node":"0.12.16","npm":"2.15.1"})
At line:1 char:1
+ npm install -g node-gyp node-pre-gyp-github
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (npm WARN engine...npm":"2.15.1"}):String) [], RemoteException
    + FullyQualifiedErrorId : NativeCommandError

From what I know, node-pre-gyp-github is just required to publish the binaries to github releases tag so it should not be too much dependent on node.js version.

Any suggestions on how to move forward then?

manixate commented 7 years ago

@sandeepmistry I don't know why you are not able to install it, may be the warning is set to cause an error on your system. I used this docker image for npm to install it and it installed.

/ # npm --version
2.15.11
/ # node --version
v0.12.17
/ # npm install -g node-pre-gyp
npm WARN engine request@2.78.0: wanted: {"node":">= 4"} (current: {"node":"0.12.17","npm":"2.15.11"})
/usr/bin/node-pre-gyp -> /usr/lib/node_modules/node-pre-gyp/bin/node-pre-gyp
node-pre-gyp@0.6.31 /usr/lib/node_modules/node-pre-gyp
├── semver@5.3.0
├── mkdirp@0.5.1 (minimist@0.0.8)
├── nopt@3.0.6 (abbrev@1.0.9)
├── rc@1.1.6 (ini@1.3.4, strip-json-comments@1.0.4, deep-extend@0.4.1, minimist@1.2.0)
├── tar@2.2.1 (inherits@2.0.3, block-stream@0.0.9, fstream@1.0.10)
├── tar-pack@3.3.0 (uid-number@0.0.6, once@1.3.3, debug@2.2.0, fstream@1.0.10, readable-stream@2.1.5, fstream-ignore@1.0.5)
├── rimraf@2.5.4 (glob@7.1.1)
├── npmlog@4.0.0 (set-blocking@2.0.0, console-control-strings@1.1.0, are-we-there-yet@1.1.2, gauge@2.6.0)
└── request@2.78.0 (is-typedarray@1.0.0, aws-sign2@0.6.0, oauth-sign@0.8.2, forever-agent@0.6.1, tunnel-agent@0.4.3, caseless@0.11.0, stringstream@0.0.5, isstream@0.1.2, json-stringify-safe@5.0.1, extend@3.0.0, aws4@1.5.0, node-uuid@1.4.7, qs@6.3.0, combined-stream@1.0.5, tough-cookie@2.3.2, form-data@2.1.1, mime-types@2.1.12, hawk@3.1.3, http-signature@1.1.1, har-validator@2.0.6)
/ # node-pre-gyp --version
v0.6.31
/ # npm install -g node-pre-gyp-github
/usr/bin/node-pre-gyp-github -> /usr/lib/node_modules/node-pre-gyp-github/bin/node-pre-gyp-github.js
node-pre-gyp-github@1.3.1 /usr/lib/node_modules/node-pre-gyp-github
├── commander@2.9.0 (graceful-readlink@1.0.1)
└── github@0.2.4 (mime@1.3.4)

But after checking the code of https://github.com/request/request I found that it will indeed not working properly on node.js v0.12.x so I created a pull request for node-pre-gyp to support older version of request because node-pre-gyp is still expecting their project to be compatible with node versions >=0.10.x. The pull request is here: https://github.com/mapbox/node-pre-gyp/pull/252