Add windows arm and bump version #49
Conversation
WalkthroughBump artifact version to 0.7.1 across manifests and native-binding checks; add description metadata to platform packages; introduce a Windows ARM64 MSVC package + README; update CI build steps and docker build workflows; bump napi-related deps and Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
JonoPrest
force-pushed
the
jp/release-windows-arm
branch
from
8e46178 to
791d090
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish_to_npm.yaml (1)
71-76: New platform aarch64-pc-windows-msvc lacks test coverage before publishing.The new ARM64 Windows target is built (lines 71–76) but excluded from the
test-macOS-windows-bindingtest job, which only testsx86_64-apple-darwin,aarch64-apple-darwin, andx86_64-pc-windows-msvc. This means the Windows ARM64 artifacts are generated but never verified before being published to npm.Add
aarch64-pc-windows-msvcto the test matrix intest-macOS-windows-binding:- host: windows-latest target: aarch64-pc-windows-msvc architecture: arm64Alternatively, if Windows ARM64 testing is not feasible in CI, create a separate test job or defer publishing this target until manual verification is complete.
🧹 Nitpick comments (2)
.github/workflows/publish_to_npm.yaml (2)
28-28: Inconsistent rustup command for target installation.Line 28 uses
rustup target installbut lines 34 and 75 userustup target add. While both work,addis the preferred command in modern Rust toolchains. Useaddconsistently across all Windows and macOS builds for clarity.Apply this diff to standardize the rustup command:
- host: macos-15 target: x86_64-apple-darwin build: | yarn set version 3 - rustup target install x86_64-apple-darwin + rustup target add x86_64-apple-darwin yarn build --target x86_64-apple-darwinAlso applies to: 34-35, 75-76
58-59: Duplicateyarn set version 3command.Line 58 and 59 both execute
yarn set version 3, which is redundant.Remove the duplicate line:
- host: macos-latest target: aarch64-apple-darwin build: | yarn set version 3 - yarn set version 3 yarn build --target aarch64-apple-darwin strip -x *.node
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.github/workflows/publish_to_npm.yaml(2 hunks)Cargo.toml(1 hunks)package.json(2 hunks)
🔇 Additional comments (1)
package.json (1)
10-18: Consistent napi target configuration.The napi targets array correctly includes the new
aarch64-pc-windows-msvcplatform, aligning with the workflow build matrix additions. This ensures npm package manifests will include the ARM64 Windows bindings.
| [dependencies] | ||
| # Default enable napi4 feature, see https://nodejs.org/api/n-api.html#node-api-version-matrix | ||
| napi = { version = "3.5.2", default-features = false, features = [ | ||
| napi = { version = "3.6.0", default-features = false, features = [ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
napi crate 3.6.0 crates.io breaking changes
💡 Result:
Do you mean the napi crate version 3.6.0 (napi v3.6.0)? I can't find a release/tag for "napi 3.6.0" in the napi-rs changelogs or on crates.io — do you want breaking changes for napi v3 (the v3 migration) or for a different specific version (please paste a link or exact version)?
🌐 Web query:
napi-derive crate 3.4.0 crates.io breaking changes
💡 Result:
I couldn't find a napi-derive 3.4.0 release on crates.io or in the official changelogs — latest documented napi-derive releases in the official changelog are 3.0.0-beta.* (and site tools show 3.1.1 as a recent stable). See the napi-derive changelog and package listings for available versions. (napi.rs)
Citations:
🌐 Web query:
napi-rs 3.6.0 3.4.0 compatibility @napi-rs/cli 3.5.0
💡 Result:
Short answer: yes — @napi-rs/cli@3.5.0 is intended to work across napi-rs v3.x releases, so napi-rs 3.4.0 and 3.6.0 are compatible in practice. The v3 line (napi v3 + @napi-rs/cli v3) is the intended compatibility surface; earlier strict version-enforcement checks were relaxed in the CLI. [1][2]
Sources:
- napi-rs changelog (v3 series). [1]
- @napi-rs/cli docs / changelog (CLI v3, and note about CLI version-check behavior). [2]
The specified napi and napi-derive versions do not exist on crates.io.
The Cargo.toml references napi 3.6.0 and napi-derive 3.4.0, but these versions are not available on crates.io. The latest documented napi-derive release is 3.1.1, and napi 3.6.0 does not appear in official changelogs. Verify the correct version numbers and update Cargo.toml accordingly, or confirm these are intentional pre-releases/local versions.
🤖 Prompt for AI Agents
In Cargo.toml around line 11, the specified napi = "3.6.0" and napi-derive =
"3.4.0" are not present on crates.io; update the dependency declarations to
point to published, compatible versions (for example set napi-derive to the
latest released 3.1.1 or another verified napi-derive release and pick a
matching napi release that exists), or if these are intentional
pre-release/local builds replace the version entries with the appropriate git =
"...", tag = "...", or path = "..." specification so Cargo can resolve them;
ensure both napi and napi-derive versions are compatible with each other and run
cargo update / cargo check to verify resolution.
| "name": "@envio-dev/hypersync-client", | ||
| "description": "HyperSync Client by Envio", | ||
| "version": "0.7.0", | ||
| "version": "0.7.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@napi-rs/cli version 3.5.0 npm compatibility napi
💡 Result:
Short answer: There is no published @napi-rs/cli v3.5.0 on npm. The npm package shows v3.1.5 as the latest; the project changelog shows N-API (napi) crates at v3.5.0 (released 2025-11-06) but that is the napi crate/changelog, not an @napi-rs/cli npm release. [1][2][3]
Sources:
- npm package page for @napi-rs/cli (latest: 3.1.5). [1]
- napi-rs changelog showing napi v3.5.0 (2025-11-06). [2]
- napi-rs CLI changelog / releases (shows CLI v3.0–3.4.x history). [3]
🌐 Web query:
@napi-rs/cli 3.5.0 napi 3.6.0 compatibility
💡 Result:
Do you mean @napi-rs/cli v3.5.0 (the CLI package) and napi v3.6.0 (the Rust crate)? I can't find published releases with those exact version numbers in the napi-rs changelogs — the project recently moved to a v3 series (see changelog). Please confirm which exact npm/crate packages and versions you mean (or paste your package.json/Cargo.toml), and I’ll check compatibility and any required upgrades. [1][3]
Sources:
- @napi-rs changelogs (CLI / napi): https://napi.rs/changelog/napi-cli, https://napi.rs/changelog/napi. [1][3]
🏁 Script executed:
git ls-files package.jsonRepository: enviodev/hypersync-client-node
Length of output: 86
🏁 Script executed:
cat -n package.json | head -30Repository: enviodev/hypersync-client-node
Length of output: 999
@napi-rs/cli version 3.5.0 does not exist on npm—this will break installation.
The package.json specifies "@napi-rs/cli": "3.5.0", but the latest published version on npm is 3.1.5. The napi Rust crate has a v3.5.0 release (napi crate, not the CLI npm package). Use the correct @napi-rs/cli version available on npm, or verify the intended version number.
🤖 Prompt for AI Agents
In package.json around line 4, the dependency "@napi-rs/cli": "3.5.0" references
a non-existent npm package version; change it to a valid published CLI version
(for example "3.1.5") or the intended correct version, then run npm install to
verify resolution; update the lockfile (package-lock.json or yarn.lock) and
commit the lockfile change so CI reproduces the correct dependency; if the 3.5.0
target was actually the Rust crate, replace the CLI entry with the correct
package name/version or remove it and document the intended dependency.
9f3a088 to
a5ebbfc
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/build-docker-images.yaml (1)
84-97: Clarify the libc++ host path setup as a documented prerequisite.This step installs libc++ libraries into the host filesystem (via volume mount to
${{ github.workspace }}/lib/llvm-18), which are later consumed by the debian-aarch64.Dockerfile build viaADD ./lib/llvm-18. While this setup pattern works, it creates a hidden prerequisite and makes the workflow less self-contained.Add a comment documenting why this step is necessary and what files it produces, so that future maintainers understand the dependency chain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build-docker-images.yaml(1 hunks).github/workflows/publish_to_npm.yaml(4 hunks)docker/alpine.Dockerfile(1 hunks)docker/debian-aarch64.Dockerfile(1 hunks)docker/debian.Dockerfile(1 hunks)
🧰 Additional context used
🪛 Hadolint (2.14.0)
docker/debian-aarch64.Dockerfile
[error] 15-15: Use COPY instead of ADD for files and folders
(DL3020)
🔇 Additional comments (6)
docker/alpine.Dockerfile (3)
3-7: LGTM on environment setup.The cross-compilation environment variables are correctly configured for Alpine's musl libc, with RUSTFLAGS appropriately disabling crt-static to enable dynamic C runtime linking.
9-18: Acknowledge Alpine edge repository usage.The Dockerfile uses Alpine edge/testing repositories (line 10-11) for rustup, gn, and ninja. While this provides newer tooling, edge packages are pre-release and may be less stable. This is acceptable for a build environment but document the trade-off if instability occurs.
20-28: LGTM on Rust and cross-toolchain setup.Rust initialization and target installation are standard. The pre-built cross-toolchain from napi-rs is appropriate and tarball cleanup is performed.
.github/workflows/publish_to_npm.yaml (1)
31-36: LGTM on Windows build targets.The explicit Windows x86_64 target addition and new Windows ARM64 (aarch64-pc-windows-msvc) build configuration follow the established pattern. The PR objective of adding Windows ARM64 support is satisfied.
Also applies to: 71-76
docker/debian.Dockerfile (1)
1-59: LGTM on Debian cross-compilation environment.The Dockerfile properly establishes a comprehensive cross-compilation environment using manylinux2014-cross base, LLVM 18 tooling, and correct sysroot configuration. NASM version parameterization and source builds are appropriate. Environment variables for LDFLAGS, CFLAGS, CXXFLAGS are properly aligned with sysroot paths.
docker/debian-aarch64.Dockerfile (1)
3-47: LGTM on aarch64 cross-compilation setup.Environment variables, LLVM 18 tooling, and Rust target configuration are properly aligned for aarch64 cross-compilation. Setup mirrors debian.Dockerfile pattern appropriately with aarch64-specific sysroot paths.
| name: Build Docker Images | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| push: | ||
| branches: | ||
| - main | ||
| paths: | ||
| - 'docker/**' | ||
|
|
||
| permissions: | ||
| contents: read | ||
| packages: write | ||
|
|
||
| jobs: | ||
| build-alpine: | ||
| name: Build Alpine image | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Log in to GitHub Container Registry | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Build and push Alpine image | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: docker/alpine.Dockerfile | ||
| platforms: linux/amd64,linux/arm64 | ||
| push: true | ||
| tags: ghcr.io/${{ github.repository }}/nodejs-rust:alpine | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
|
|
||
| build-debian: | ||
| name: Build Debian image | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Log in to GitHub Container Registry | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Build and push Debian image | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: docker/debian.Dockerfile | ||
| platforms: linux/amd64 | ||
| push: true | ||
| tags: ghcr.io/${{ github.repository }}/nodejs-rust:debian | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
|
|
||
| build-debian-aarch64: | ||
| name: Build Debian ARM64 image | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v3 | ||
|
|
||
| - name: Set up QEMU | ||
| uses: docker/setup-qemu-action@v3 | ||
| with: | ||
| platforms: arm64 | ||
|
|
||
| - name: Install latest libc++-dev for cross build | ||
| uses: addnab/docker-run-action@v3 | ||
| with: | ||
| image: node:lts-slim | ||
| options: '--platform linux/arm64 --user 0:0 -v ${{ github.workspace }}/lib/llvm-18:/usr/lib/llvm-18' | ||
| run: >- | ||
| apt-get update && | ||
| apt-get install -y wget gnupg2 && | ||
| wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - && | ||
| echo "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-18 main" >> /etc/apt/sources.list && | ||
| echo "deb-src http://apt.llvm.org/focal/ llvm-toolchain-focal-18 main" >> /etc/apt/sources.list && | ||
| apt-get update && | ||
| apt-get install libc++-18-dev libc++abi-18-dev -y --fix-missing --no-install-recommends && | ||
| rm /usr/lib/llvm-18/lib/libc++abi.so | ||
| - name: Log in to GitHub Container Registry | ||
| uses: docker/login-action@v3 | ||
| with: | ||
| registry: ghcr.io | ||
| username: ${{ github.actor }} | ||
| password: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Build and push Debian ARM64 image | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: docker/debian-aarch64.Dockerfile | ||
| platforms: linux/amd64 | ||
| push: true | ||
| tags: ghcr.io/${{ github.repository }}/nodejs-rust:debian-aarch64 | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd publish_to_npm.yamlRepository: enviodev/hypersync-client-node
Length of output: 56
🏁 Script executed:
find .github/workflows -type f -name "*.yaml" -o -name "*.yml"Repository: enviodev/hypersync-client-node
Length of output: 154
🏁 Script executed:
cat -n .github/workflows/publish_to_npm.yamlRepository: enviodev/hypersync-client-node
Length of output: 12495
Add explicit workflow dependency between build and publish workflows.
The publish_to_npm.yaml workflow references three Docker images built by build-docker-images.yaml (lines 39, 48, 64). However, publish_to_npm.yaml is triggered only via workflow_dispatch (manual trigger), while build-docker-images.yaml triggers on either workflow_dispatch or push to main affecting docker/**. This creates a scenario where a user could manually trigger publish_to_npm.yaml without first ensuring the required Docker images exist in GHCR, causing the build steps to fail when attempting to pull non-existent images.
Consider adding a workflow_dispatch input to publish_to_npm.yaml that allows optionally triggering build-docker-images.yaml first, or document the required manual sequencing.
🤖 Prompt for AI Agents
.github/workflows/build-docker-images.yaml lines 1-115: The publish workflow can
be manually triggered before required Docker images exist; add an explicit
optional dependency in publish_to_npm.yaml by adding a workflow_dispatch input
like "build_images: boolean" and, when true, add an initial step that triggers
the build-docker-images workflow via the GitHub REST workflow_dispatch API
(using GITHUB_TOKEN) and polls/waits for its completion (or use workflow_run if
you prefer event-based chaining) before continuing to pull images; ensure the
step fails the job if the build workflow fails so publish won't proceed without
images.
| - host: ubuntu-latest | ||
| target: x86_64-unknown-linux-gnu | ||
| docker: ghcr.io/napi-rs/napi-rs/nodejs-rust:lts-debian | ||
| docker: ghcr.io/${{ github.repository }}/nodejs-rust:debian |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the publish_to_npm.yaml file to understand the workflow structure
git ls-files | grep -E "workflows|\.github"Repository: enviodev/hypersync-client-node
Length of output: 154
🏁 Script executed:
# Read the publish_to_npm.yaml file to examine the workflow structure and dependencies
cat -n .github/workflows/publish_to_npm.yamlRepository: enviodev/hypersync-client-node
Length of output: 12495
🏁 Script executed:
# Read the build-docker-images.yaml file to understand how images are built/pushed
cat -n .github/workflows/build-docker-images.yamlRepository: enviodev/hypersync-client-node
Length of output: 4346
🏁 Script executed:
# Check for README or documentation files that might explain workflow ordering
fd -e md | head -20Repository: enviodev/hypersync-client-node
Length of output: 667
🏁 Script executed:
# Verify if there's any documentation in the repo about this workflow dependency
rg "build-docker-images|publish_to_npm" --type md -B 2 -A 2Repository: enviodev/hypersync-client-node
Length of output: 56
Docker image references depend on undocumented workflow prerequisite.
Lines 39, 48, and 64 reference Docker images at ghcr.io/${{ github.repository }}/nodejs-rust:* that are built and pushed by the build-docker-images.yaml workflow. However, publish_to_npm.yaml is triggered only by workflow_dispatch with no explicit dependency on build-docker-images.yaml. If that workflow hasn't run recently or if images haven't been pushed to the container registry, the build will fail at the "Build in docker" step (line 127) when attempting to pull non-existent images.
Add a workflow_run trigger to ensure build-docker-images.yaml completes successfully before publish_to_npm.yaml runs, or document this prerequisite explicitly.
🤖 Prompt for AI Agents
.github/workflows/publish_to_npm.yaml lines 39, 48, 64 (and build step at line
127): the workflow references Docker images ghcr.io/${{ github.repository
}}/nodejs-rust:* that are produced by build-docker-images.yaml but
publish_to_npm.yaml is only workflow_dispatch and may run before those images
exist; update the workflow to add a workflow_run trigger that waits for
build-docker-images.yaml to complete successfully (or alternatively add a clear
documented prerequisite in the workflow/README stating the image build workflow
must be run/pushed before using publish_to_npm.yaml) so the publish job can
reliably pull the images.
| C_INCLUDE_PATH="/usr/aarch64-unknown-linux-gnu/aarch64-unknown-linux-gnu/sysroot/usr/include" \ | ||
| LDFLAGS="-L/usr/aarch64-unknown-linux-gnu/lib/llvm-18/lib" | ||
|
|
||
| ADD ./lib/llvm-18 /usr/aarch64-unknown-linux-gnu/lib/llvm-18 |
There was a problem hiding this comment.
Replace ADD with COPY per Dockerfile best practices.
Line 15 uses ADD for a local directory, but ADD has unnecessary extra behavior (tar extraction, remote URL support). For local files/directories, use COPY instead per Hadolint (DL3020) and Docker best practices.
-ADD ./lib/llvm-18 /usr/aarch64-unknown-linux-gnu/lib/llvm-18
+COPY ./lib/llvm-18 /usr/aarch64-unknown-linux-gnu/lib/llvm-18📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ADD ./lib/llvm-18 /usr/aarch64-unknown-linux-gnu/lib/llvm-18 | |
| COPY ./lib/llvm-18 /usr/aarch64-unknown-linux-gnu/lib/llvm-18 |
🧰 Tools
🪛 Hadolint (2.14.0)
[error] 15-15: Use COPY instead of ADD for files and folders
(DL3020)
🤖 Prompt for AI Agents
docker/debian-aarch64.Dockerfile around line 15: the Dockerfile uses ADD for a
local directory which can trigger unintended behaviors; replace the ADD
instruction with COPY for the local path (i.e., use COPY ./lib/llvm-18
/usr/aarch64-unknown-linux-gnu/lib/llvm-18) to follow Docker best practices and
satisfy Hadolint DL3020.
9f65ea3 to
6c929d9
Compare
|
Merging to get the docker workflow. Will re-open |
Summary by CodeRabbit
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.