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

Arm64 with prebuilds #238

Merged
merged 5 commits into from
Mar 4, 2024
Merged

Arm64 with prebuilds #238

merged 5 commits into from
Mar 4, 2024

Conversation

jsumners-nr
Copy link
Contributor

@jsumners-nr jsumners-nr commented Feb 27, 2024

This PR resolves #205.

Proposed Release Notes

Migrated to using prebuildify and node-gyp-install.

Links

Details

Utilizing prebuildify and node-gyp-install allows us to remove all of the custom code around checking for a local binary and downloading one from an S3 bucket if a binary isn't found. This is accomplished by bundling a binary for all known supported platforms within the npm package. If the module is installed on a platform we have not bundled a binary for, then node-gyp-install will run the build locally as per normal node-gyp. Additionally, if anyone wants to force a build, they can npm install --build-from-source.

Notice: the create-release.yml workflow does not directly use the one from newrelic/node-newrelic. We need to do some extra steps to 1. build the binaries, 2. grab them from the GHA artifacts cache, 3. construct a project structure with the binaries and npm pack to build the shippable module, and 4. grab the packed module from the GHA actions cache and then publish it to npmjs.com.

The process can be seen in this dry run. The npm-module artifact from that run can be downloaded and:

  1. unzip npm-module.zip
  2. mkdir foo && cd foo && npm init && npm install ../newrelic-*.tgz
  3. echo -e "const nr = require('@newrelic/native-metrics')\nconsole.log(nr)" > index.js
  4. node index.js

You'll see the serialized representation of the exported module since the prebuilt binary gets loaded from the require line 😁

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.81%. Comparing base (02f7bce) to head (e51abcd).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #238      +/-   ##
==========================================
+ Coverage   97.67%   97.81%   +0.14%     
==========================================
  Files           5        1       -4     
  Lines         644      183     -461     
==========================================
- Hits          629      179     -450     
+ Misses         15        4      -11     
Flag Coverage Δ
integration-tests-16-macos-14-arm64 85.24% <100.00%> (?)
integration-tests-16-ubuntu-latest-x64 85.24% <100.00%> (?)
integration-tests-16-windows-latest-x64 85.24% <100.00%> (?)
integration-tests-16-windows-latest-x86 85.24% <100.00%> (?)
integration-tests-16.x-linux ?
integration-tests-16.x-windows-2019 ?
integration-tests-16.x-windows-latest ?
integration-tests-18-macos-14-arm64 84.15% <100.00%> (?)
integration-tests-18-ubuntu-latest-x64 84.15% <100.00%> (?)
integration-tests-18-windows-latest-x64 84.15% <100.00%> (?)
integration-tests-18-windows-latest-x86 84.15% <100.00%> (?)
integration-tests-18.x-linux ?
integration-tests-18.x-windows-2019 ?
integration-tests-18.x-windows-latest ?
integration-tests-20-macos-14-arm64 84.15% <100.00%> (?)
integration-tests-20-ubuntu-latest-x64 84.15% <100.00%> (?)
integration-tests-20-windows-latest-x64 84.15% <100.00%> (?)
integration-tests-20-windows-latest-x86 84.15% <100.00%> (?)
integration-tests-20.x-linux ?
integration-tests-20.x-windows-2019 ?
integration-tests-20.x-windows-latest ?
unit-tests-16-macos-14-arm64 93.44% <100.00%> (?)
unit-tests-16-ubuntu-latest-x64 93.44% <100.00%> (?)
unit-tests-16-windows-latest-x64 93.44% <100.00%> (?)
unit-tests-16-windows-latest-x86 93.44% <100.00%> (?)
unit-tests-16.x-linux ?
unit-tests-16.x-windows-2019 ?
unit-tests-16.x-windows-latest ?
unit-tests-18-macos-14-arm64 92.34% <100.00%> (?)
unit-tests-18-ubuntu-latest-x64 92.34% <100.00%> (?)
unit-tests-18-windows-latest-x64 92.34% <100.00%> (?)
unit-tests-18-windows-latest-x86 92.34% <100.00%> (?)
unit-tests-18.x-linux ?
unit-tests-18.x-windows-2019 ?
unit-tests-18.x-windows-latest ?
unit-tests-20-macos-14-arm64 92.34% <100.00%> (?)
unit-tests-20-ubuntu-latest-x64 92.34% <100.00%> (?)
unit-tests-20-windows-latest-x64 92.34% <100.00%> (?)
unit-tests-20-windows-latest-x86 92.34% <100.00%> (?)
unit-tests-20.x-linux ?
unit-tests-20.x-windows-2019 ?
unit-tests-20.x-windows-latest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jsumners-nr jsumners-nr marked this pull request as ready for review February 28, 2024 20:56
@jsumners-nr jsumners-nr marked this pull request as draft February 28, 2024 21:00
@jsumners-nr jsumners-nr marked this pull request as ready for review February 29, 2024 15:28
@bizob2828 bizob2828 self-assigned this Mar 1, 2024
Copy link
Member

@bizob2828 bizob2828 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👏🏻 . A few questions and some comments around lack of testing in build.js. This reduces so much manual code to pre build, upload, etc, so much better. I did verify that I could install the .tgz built from the CI and tested the npm run build and instal from source flows.

This PR also addresses or by making irrelevant:

npm install --verbose --maxsockets 1
cp -R node_modules /host/
npm run unit
# Skipping integration until we can get native arm64 runners
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have a ticket for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not yet. We can file one once we merge this.

README.md Show resolved Hide resolved
tests/integration/loop-performance.tap.js Show resolved Hide resolved
build.js Show resolved Hide resolved
.github/workflows/create-release.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Member

@bizob2828 bizob2828 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. This reduces a lot of bespoke code and also adds more pre-builts 🚀

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

Successfully merging this pull request may close these issues.

Add ARM 64 test runs and pre-built downloads
2 participants