ci: Add GitHub Actions workflow for lni_nodejs multi-platform builds#28
ci: Add GitHub Actions workflow for lni_nodejs multi-platform builds#28
Conversation
Builds native Node.js bindings for: - Linux x64 (gnu) - Linux ARM64 (gnu) - macOS x64 - macOS ARM64 - macOS Universal (lipo) - Windows x64 (msvc) Features: - Runs on push to master/main and tags - Creates GitHub Release on version tags (v*) - Uploads platform-specific .node binaries as release assets - Optional npm publish (requires NPM_TOKEN secret) - Test step to verify bindings load correctly
📝 WalkthroughWalkthroughAdds a new GitHub Actions workflow to build, test, and release multi-platform Node.js native bindings and updates Cargo config to add macOS (Darwin) Rust target settings for dynamic linking. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
actor Developer
participant GitHubActions as "GitHub Actions"
participant Runner as "Platform Runner\n(Linux/macOS/Windows)"
participant Toolchain as "Node & Rust Toolchain"
participant Build as "Build (yarn napi / cargo)"
participant Artifacts as "Artifact Storage"
participant TestJob as "Test Job"
participant ReleaseJob as "Release Job"
participant GitHubRelease as "GitHub Release"
participant NPM as "npm Registry"
Developer->>GitHubActions: push or tag (v*)
GitHubActions->>Runner: start matrix build jobs
Runner->>Toolchain: setup Node.js and Rust (target toolchains)
Runner->>Build: install deps and build native addon
Build->>Artifacts: upload platform-specific artifacts
GitHubActions->>TestJob: trigger tests (download artifacts)
TestJob->>Artifacts: download artifacts
TestJob->>Runner: run module load & verification
alt tag triggered
GitHubActions->>ReleaseJob: gather all artifacts
ReleaseJob->>Artifacts: download platform artifacts
ReleaseJob->>GitHubRelease: create release and attach assets
alt NPM_TOKEN provided
ReleaseJob->>NPM: publish package to npm
end
end
GitHubRelease-->>Developer: release created with assets
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
- Install protobuf-compiler on all platforms (needed for prost-build) - Use 'yarn napi build' with --target flag directly - Add brew/choco install steps for macOS/Windows
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build-nodejs.yml:
- Around line 43-75: Replace all mutable GitHub Action references (e.g.,
actions/checkout@v4, actions/setup-node@v4, dtolnay/rust-toolchain@stable,
actions/upload-artifact@v4 and any other uses: entries across the workflow jobs)
with their reproducible commit SHA pins; for each replacement keep the original
tag in a comment immediately above the uses: line for clarity. Update every job
that uses a third-party action (build, universal-macos, test, release, publish)
to use the full commit SHA instead of the mutable tag, ensuring the same
behavior by selecting the SHA that corresponds to the current tag, and commit
those changes.
🧹 Nitpick comments (2)
.github/workflows/build-nodejs.yml (2)
115-152: Consider adding coverage for darwin‑x64 and linux‑arm64 artifacts.Those outputs are currently untested. If they’re shipped, add an Intel macOS runner (or Rosetta) and an arm64 Linux runner/QEMU to catch loader regressions early.
💡 Example: add an Intel macOS test lane
include: - os: ubuntu-latest artifact: bindings-linux-x64-gnu + - os: macos-13 + artifact: bindings-darwin-x64 - os: macos-latest artifact: bindings-darwin-arm64 - os: windows-latest artifact: bindings-win32-x64-msvc
195-235: Skip the publish job whenNPM_TOKENis absent.Gating the job keeps tag workflows clean and avoids a no‑op publish step.
✅ Suggested gating
- publish: - if: startsWith(github.ref, 'refs/tags/v') + publish: + if: startsWith(github.ref, 'refs/tags/v') && secrets.NPM_TOKEN != '' @@ - - name: Publish to npm - working-directory: bindings/lni_nodejs - run: | - # Only publish if NPM_TOKEN is set - if [ -n "${{ secrets.NPM_TOKEN }}" ]; then - npm publish --access public - else - echo "NPM_TOKEN not set, skipping publish" - fi + - name: Publish to npm + working-directory: bindings/lni_nodejs + run: npm publish --access public
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build-nodejs.yml:
- Around line 240-250: The workflow step named "Publish to npm" currently tests
the secret by interpolating ${{ secrets.NPM_TOKEN }} into the shell; change the
guard to check the environment variable instead (use NODE_AUTH_TOKEN already set
in env) and rely on NODE_AUTH_TOKEN in the shell check (e.g., if [ -n
"${NODE_AUTH_TOKEN}" ]), avoid echoing or printing the token, and keep npm
publish using the existing env variable rather than interpolating the secret
directly.
- Around line 200-208: Update the GitHub Actions step named "Create Release" to
use softprops/action-gh-release@v2 instead of `@v1` by changing the uses reference
(softprops/action-gh-release@v1 → softprops/action-gh-release@v2); keep the
existing inputs (files, generate_release_notes, draft, prerelease) and env
GITHUB_TOKEN as-is, and verify the v2 action supports the same fields in the
"Create Release" step to ensure compatibility.
🧹 Nitpick comments (2)
.github/workflows/build-nodejs.yml (2)
116-127: Add error handling for universal binary creation.Two concerns with this job:
- The
lipocommand uses wildcards which could fail unexpectedly if there are multiple.nodefiles or if the glob doesn't match.- The upload step lacks
if-no-files-found: error, unlike the build job, which could mask failures silently.♻️ Suggested improvements
- name: Create universal binary run: | + # Verify exactly one .node file in each directory + x64_file=$(ls artifacts/x64/*.node) + arm64_file=$(ls artifacts/arm64/*.node) lipo -create \ - artifacts/x64/*.node \ - artifacts/arm64/*.node \ + "$x64_file" \ + "$arm64_file" \ -output lni_js.darwin-universal.node - name: Upload universal artifact uses: actions/upload-artifact@v4 with: name: bindings-darwin-universal path: lni_js.darwin-universal.node + if-no-files-found: error
231-238: Avoid silently suppressing copy errors during npm publish preparation.The
2>/dev/null || truepattern hides all errors, which could result in publishing an npm package with missing binaries. Consider validating that files were actually copied.♻️ Suggested improvement
- name: Prepare npm package working-directory: bindings/lni_nodejs run: | # Copy platform-specific binaries for dir in ../../artifacts/bindings-*; do - cp "$dir"/*.node . 2>/dev/null || true + if ls "$dir"/*.node 1>/dev/null 2>&1; then + cp "$dir"/*.node . + fi done ls -la *.node + # Verify at least one binary was copied + if ! ls *.node 1>/dev/null 2>&1; then + echo "Error: No .node binaries found" + exit 1 + fi
Replace mutable tag references with reproducible commit SHA pins: - actions/checkout@v4 → @34e114876b0b11c390a56381ad16ebd13914f8d5 - actions/setup-node@v4 → @49933ea5288caeca8642d1e84afbd3f7d6820020 - dtolnay/rust-toolchain@stable → @4be9e76fd7c4901c61fb841f559994984270fce7 - actions/upload-artifact@v4 → @ea165f8d65b6e75b540449e92b4886f43607fa02 - actions/download-artifact@v4 → @d3f86a106a0bac45b974a628896c90dbdf5c8093 - softprops/action-gh-release@v1 → @de2c0eb89ae2a093876385947365aca7b0e5f844 Original tags preserved in comments above each uses: line for clarity.
- darwin-x64: Use macos-13 (Intel runner) for native x86_64 build - darwin-arm64: Use macos-14 (Apple Silicon runner) for native aarch64 build - Update test and universal-macos jobs to use macos-14 macos-latest now defaults to arm64, causing cross-compilation issues.
…secret
Avoid interpolating ${{ secrets.NPM_TOKEN }} directly into shell script.
Instead, check the NODE_AUTH_TOKEN environment variable which is already
set via the env block. This prevents potential secret exposure in logs.
- v1 → v2 (SHA: a06a81a03ee405af7f2048a818ed3f03bbf83c7b) - v2 uses node20 runtime (vs node16 in v1) - All existing inputs (files, generate_release_notes, draft, prerelease) are compatible
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build-nodejs.yml:
- Around line 28-34: The GitHub Actions job uses an invalid macOS runner label
for the darwin-x64 entry; update the runner value currently set to "macos-13" to
a supported Intel runner such as "macos-15-intel" (or
"macos-14-large"/"macos-15-large") so the darwin-x64 job (target:
x86_64-apple-darwin, name: darwin-x64) runs on a valid hosted runner.
The darwin builds were failing because the linker couldn't find N-API symbols (_napi_create_error, _napi_create_function, etc.). These symbols are provided by Node.js at runtime, so we need -undefined dynamic_lookup to tell the linker to defer resolution.
- macos-13 (Intel) → macos-15-intel - macos-14 (ARM64) → macos-latest Per GitHub deprecation notice: actions/runner-images#13046
✅ Created GitHub Actions workflow for Builds native Node.js bindings!
Branch: ci/nodejs-build
Build Targets:
Triggers:
• Push to master/main → Build & test
• Push tag v* → Build, test, create Release
• Pull requests → Build & test
To create a release:
Features:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.