ci: move CI to GitHub actions and get prebuilds working again #152

mcous · 2021-09-26T03:01:44Z

Overview

(Diff ignoring lockfile: 141 insertions(+), 203 deletions(-))

As noted in #134, #140, #151, the existing CI setup has started to have issues, resulting in prebuilds not being correctly published to GitHub Releases.

From what I can tell, the problem may be that CI setup is configured somewhat redundantly. The prebuild utility is able to generate prebuilds for all available ABI versions while running on a single version of Node. However, CI is configured to run a matrix of different Node.js versions, too many of which are configured to run prebuild --all. Prebuilds only need to be generated once per OS/arch combination, and the compilation works most smoothly on later versions of Node.

Additionally, there's a few other snags and updates since these problems started appearing:

The travis-ci.org infrastructure the current CI pipeline is set up on has been shut down
The oldest non-EOL'd Node.js version is now v12

Rather than try to work around Travis' .org shutdown and figure out the .com migration, I decided to see what this CI would look like on GitHub Actions. Over at Opentrons/opentrons, where we use usb-detection, we switched from a Travis/AppVeyor combo to GH Actions, and it's gone pretty well. If the switch to GH Actions isn't too presumptuous, I think this is a good change!

IMO the config is easier to understand (caveat: GH Actions is my current default-choice for CI, so I'm used to it)
It means AppVeyor isn't needed, because GH Actions has macOS, Windows, and Linux
- That being said, I believe Travis also has Windows support these days
It's a little easier for external contributors to inspect CI logs because they're in the repository
Since all builds happen in one CI pipeline, auto-publish to npm is possible because you can halt the autopublish if any OS/arch build fails

Relevant outputs from my fork running this CI:

Heavy caveat: I don't write native Node.js modules. This is not my area of expertise, though I'm pretty handy with a CI config. I haven't actually tested any these prebuilds; the only thing I know about these builds right now is that they are compiling without errors.

Open questions / TODO

Is switching to GitHub Actions a good idea for this repository?
- Resolution: Upsides outweigh the downsides
Should the engines.node field in package.json be bumped from 4 to 12?
- All Node.js versions prior to 12 are EOL'd
- At least one dependeny requires Node >= 6, so spec'ing >=4 is already inaccurate
- This would be a breaking change, but I think it's a good idea to ease potential maintenance burden
- Resolution: Probably, but not in this PR
Should CI be configured to publish tags to npm?
- I think this is a good idea given the single multi-OS build pipeline
- I couldn't tell how usb-detection is currently published, so I'm guess the answer is "manually"
- Resolution: Yes, secrets have been added to the repo
Make sure older Node versions are in the build matrix to ensure regular compile from source works

Change log

Replace TravisCI and AppVeyor with GitHub Actions
- Bumped Node.js in CI to 12, 14, and 16
- Bumped recommended dev version to 14
  - Note: this also bumps npm to v7, resulting in some serious lockfile churn; sorry about this diff
- Dropped Linux x86 from CI, since Node.js stopped officially publishing Linux x86 builds after v8
Bump various dependencies to keep things building
Set 'MACOSX_DEPLOYMENT_TARGET': '10.9' in node-gyp config
- Without this change, I was unable to build this project on macOS locally
- I believe this has to do with the switch from GCC to Clang that first happened in OSX 10.9
Update and simply development section in README, accordingly
- Remove Python 2 requirement, because node-gyp now only supports Python 3 and Python 2 has been EOL'd
- Remove some outdated links and instructions in favor of linking to node-gyp documentation

How does this CI pipeline work?

The CI pipeline starts by kicking off several jobs in parallel:

A job to run linting
- Only run on Ubuntu, because there's no need for a matrix build for linting
A matrix of jobs to run npm ci --build-from-source on all the OS's and architectures we want
- macOS x64, Linux x64, Windows x64, Windows x86
- Node 12, 14, and 16
- Within this matrix, one job per OS/arch combo will run prebuild --all and upload prebuilds to GH Actions artifact storage

If all those jobs complete and the CI run was triggered by a tag, then a publish job will run, which will:

Download all prebuilt artifacts
Upload those artifacts to the GH release for the built version
(TODO) Publish the release to npm

mcous

@MadLittleMods this is ready for your review. I'm going to keep it listed as a draft while there are still TODO items to address:

Is switching to GitHub Actions actually a path you would like to take with this repository?
Should this PR bump minimum required Node version to 12?
Should this PR enable auto-publishing to npm?

mcous · 2021-09-26T17:01:11Z

.github/workflows/ci.yml

+      # - name: 'Publish to npm'
+      #   run: npm publish
+      #   env:
+      #     NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}


By uncommenting this and adding an NPM_TOKEN secret to the repository, the CI would automatically publish tagged builds to npm if and only if all builds and prebuilds succeed

Suggested change

# - name: 'Publish to npm'

# run: npm publish

# env:

# NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

- name: 'Publish to npm'

run: npm publish

env:

NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}

Should CI be configured to publish tags to npm?

I think this is a good idea given the single multi-OS build pipeline

I couldn't tell how usb-detection is currently published, so I'm guess the answer is "manually"

Added secret to the repo ✅ Seems good to me 👍

mcous · 2021-10-04T16:01:52Z

@MadLittleMods gentle ping on this one again. Is GitHub Actions a path you'd be willing to take on this repository?

.github/workflows/ci.yml

README.md

MadLittleMods · 2021-10-13T08:22:54Z

package.json

@@ -41,18 +42,18 @@
    "node": ">=4"


Should the engines.node field in package.json be bumped from 4 to 12?

All Node.js versions prior to 12 are EOL'd

At least one dependeny requires Node >= 6, so spec'ing >=4 is already inaccurate

This would be a breaking change, but I think it's a good idea to ease potential maintenance burden

Sounds good to update this but we can do this in another PR 👍

MadLittleMods · 2021-10-13T08:31:19Z

.github/workflows/ci.yml

@@ -0,0 +1,112 @@
+name: 'ci'


Is switching to GitHub Actions a good idea for this repository?

@mcous and I discussed this a bit at https://gitter.im/MadLittleMods/node-usb-detection?at=6165a80e29ddcd02931b22cc

Some pain points with GitHub actions but probably a good way forward for this project ⏩

MadLittleMods · 2021-10-13T08:36:03Z

.github/workflows/ci.yml

+      #     NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
+
+      - name: 'Upload prebuilt artifacts to GitHub release'
+        run: npm run prebuild-upload -- ${{ secrets.GITHUB_TOKEN }}


@mcous Thanks for this great migration and moving this project in the new up-to-date and more sane direction instead of fiddling with the old stuff! ♥

Sorry for the delay to get some eyes on this 🙇

MadLittleMods · 2021-10-14T04:52:43Z

.github/workflows/ci.yml

+      - name: 'Publish to npm'
+        run: npm publish
+        env:
+          NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}


Also to note the sanity check, I see it running on your fork ✅

https://github.com/mcous/node-usb-detection/actions/runs/1338151169

https://github.com/mcous/node-usb-detection/actions/runs/1275586665

(also linked in issue description)

MadLittleMods · 2021-10-14T06:48:15Z

Woot woot! https://github.com/MadLittleMods/node-usb-detection/releases/tag/v4.13.0 with a multitude of prebuilds and available on npm https://www.npmjs.com/package/usb-detection/v/4.13.0 🚀

Thanks again @mcous 🥰

mcous added 11 commits September 25, 2021 20:46

wip: binary builds on GH actions

222e1d6

fixup: indentation error

2ead1e5

fixup: libudev on linux

94196c2

fixup: drop linux x86

67794df

fixup: see if we can get GH release uploads

9079d62

fixup: get a little fancier here

f574af1

fixup: once more with feeling

58661f7

fixup: try to get this token working

428a52a

fixup: start finalizing

a2d1f02

fixup: add Node 12 and 16 into the build matrix for testing

84b51f2

fixup: oops, actually run on Node 12 and 16

151caad

mcous commented Sep 26, 2021

View reviewed changes

mcous added 2 commits September 27, 2021 10:19

fixup: remove redundant if

de61db4

fixup: tweak node installation step title

901029e

MadLittleMods reviewed Oct 13, 2021

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

MadLittleMods reviewed Oct 13, 2021

View reviewed changes

README.md Show resolved Hide resolved

MadLittleMods reviewed Oct 13, 2021

View reviewed changes

MadLittleMods added dependencies Pull requests that update a dependency file enhancement bug and removed dependencies Pull requests that update a dependency file labels Oct 13, 2021

MadLittleMods reviewed Oct 13, 2021

View reviewed changes

fixup: incorporate review feedback and enable npm publish

d277080

mcous marked this pull request as ready for review October 13, 2021 15:30

MadLittleMods reviewed Oct 14, 2021

View reviewed changes

MadLittleMods merged commit bb95f17 into MadLittleMods:master Oct 14, 2021

MadLittleMods added a commit that referenced this pull request Oct 14, 2021

Prepare changelog with #152

8e7213e

This was referenced Oct 14, 2021

4.11 tag not published to npm #151

Closed

GitHub release URL not found when installing - prebuild #140

Closed

MadLittleMods added the prebuild Issues related to missing and broken prebuilt binaries when installing (`prebuild` npm package) label Oct 14, 2021

mcous mentioned this pull request Oct 14, 2021

fix(app-shell): upgrade usb-detection to latest Opentrons/opentrons#8524

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: move CI to GitHub actions and get prebuilds working again #152

ci: move CI to GitHub actions and get prebuilds working again #152

mcous commented Sep 26, 2021 •

edited

Loading

mcous left a comment

mcous Sep 26, 2021 •

edited

Loading

MadLittleMods Oct 13, 2021

mcous commented Oct 4, 2021

MadLittleMods Oct 13, 2021

MadLittleMods Oct 13, 2021

MadLittleMods Oct 13, 2021 •

edited

Loading

MadLittleMods Oct 14, 2021 •

edited

Loading

MadLittleMods commented Oct 14, 2021

ci: move CI to GitHub actions and get prebuilds working again #152

ci: move CI to GitHub actions and get prebuilds working again #152

Conversation

mcous commented Sep 26, 2021 • edited Loading

Overview

Open questions / TODO

Change log

How does this CI pipeline work?

mcous left a comment

Choose a reason for hiding this comment

mcous Sep 26, 2021 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Oct 13, 2021

Choose a reason for hiding this comment

mcous commented Oct 4, 2021

MadLittleMods Oct 13, 2021

Choose a reason for hiding this comment

MadLittleMods Oct 13, 2021

Choose a reason for hiding this comment

MadLittleMods Oct 13, 2021 • edited Loading

Choose a reason for hiding this comment

MadLittleMods Oct 14, 2021 • edited Loading

Choose a reason for hiding this comment

MadLittleMods commented Oct 14, 2021

mcous commented Sep 26, 2021 •

edited

Loading

mcous Sep 26, 2021 •

edited

Loading

MadLittleMods Oct 13, 2021 •

edited

Loading

MadLittleMods Oct 14, 2021 •

edited

Loading