volta icon indicating copy to clipboard operation
volta copied to clipboard

Fetch Native Windows on ARM Node builds for Node 20+

Open iNViTiON opened this issue 8 months ago • 7 comments

Relate #1270

Info

  • With the release of Node v20, there are now pre-built binaries available for Windows on ARM devices.
  • Where possible, we should fetch and use those binaries, so that users get the best possible experience.
  • However, for older versions of Node, we will need to continue to fetch the x64 binaries and rely on emulator.

Changes

  • Updated the constants section in tool/node/mod.rs to include the constant values for Windows on ARM architecture
  • Ensured that the parsing of the files metadata in the Node index will check for both the native and x64 versions of Node.
  • Customized archive_filename on Windows on ARM builds to check the Node major version and fetch either a native or x64 build of Node depending on whether it is greater than or equal to 20.
  • Change test_node_archive_basename and test_node_archive_filename from version 1.2.3 to 21.2.3 to allow Windows on ARM and Apple Silicon to fetch correct version without constraint

Tested

  • Our test suite currently doesn't execute on a Windows on ARM machine, so we don't have a way to automatically test these changes.
  • I run a full test with cargo test --all and everything pass. Also confirm this properly fetches and run Node 20+ and 19-.

iNViTiON avatar Nov 06 '23 16:11 iNViTiON

I think the release builds are also generated from this repo, so I think you'd need to tweak https://github.com/volta-cli/volta/blob/main/.github/workflows/release.yml to add a Windows + ARM section (in order to generate the required artifacts for a release).

rwjblue avatar Nov 14 '23 19:11 rwjblue

I think the release builds are also generated from this repo, so I think you'd need to tweak https://github.com/volta-cli/volta/blob/main/.github/workflows/release.yml to add a Windows + ARM section (in order to generate the required artifacts for a release).

I believe running Volta x64 with an emulator and using native ARM for NodeJS might work well, as Volta doesn't require heavy CPU usage.

Should we go ahead and release this first, then work on building a native ARM version of Volta for Windows later?

iNViTiON avatar Nov 14 '23 21:11 iNViTiON

I resolved the conflict with the main branch, by rebasing. In commit 1f4585a, all platforms and architectures were tested with version v16.x, the cutoff for Apple Silicon. I updated this to v20.x, the cutoff for Windows on ARM.

Is it advisable to segregate x64, Apple Silicon, and Windows on ARM for these specific version cutoffs? @rwjblue @charlespierce

iNViTiON avatar Jan 14 '24 06:01 iNViTiON

As ARM64 support for Linux is now available, could the target_os requirement in crates/volta-core/src/tool/node/metadata.rs be removed? This is assuming that this code is required for all operating systems that support ARM64.

Fydon avatar Feb 22 '24 09:02 Fydon

As ARM64 support for Linux is now available, could the target_os requirement in crates/volta-core/src/tool/node/metadata.rs be removed? This is assuming that this code is required for all operating systems that support ARM64.

I'm still unsure about Linux's implementation for the ARM architecture. However, isn't macOS continuing to utilize target_os in metadata.rs?

iNViTiON avatar Feb 27 '24 04:02 iNViTiON

The way I read your changes they are adding Windows to target_os in metadata.rs. If a target_os check for Linux is added to metadata.rs as well, it may be equivalent to removing target_os as a check, as I believed that the only supported operating systems are Linux, macOS and Windows. I'm assume that just having a target_arch check for aarch64 is possible.

I don't fully understand how Volta fits together, but I see that there are ARM64 builds (shown as ARMv8) listed on the NodeJS download page, which I assume means that there are native ARM64 builds for Linux. I assume part of your changes is to obtain ARM64 builds for Windows, in addition to what is already being done for macOS and so might also be relevant for Linux.

Fydon avatar Feb 27 '24 10:02 Fydon

The way I read your changes they are adding Windows to target_os in metadata.rs. If a target_os check for Linux is added to metadata.rs as well, it may be equivalent to removing target_os as a check, as I believed that the only supported operating systems are Linux, macOS and Windows. I'm assume that just having a target_arch check for aarch64 is possible.

I don't fully understand how Volta fits together, but I see that there are ARM64 builds (shown as ARMv8) listed on the NodeJS download page, which I assume means that there are native ARM64 builds for Linux. I assume part of your changes is to obtain ARM64 builds for Windows, in addition to what is already being done for macOS and so might also be relevant for Linux.

I suggest we keep the OS check since each OS has Node.js native ARM support in different versions with different file format.

iNViTiON avatar Feb 28 '24 04:02 iNViTiON

For some reason I'm not seeing the CI checks (nor any way for me to approve them to run). I'm going to close and re-open to see if that gets GitHub working 🤷🏻

charlespierce avatar May 13 '24 04:05 charlespierce

The CI failures I think are related to GitHub actions transitioning macos-latest to use an M1 machine, so they're running on an architecture that we don't have fixtures for. I'll create a separate PR to update the fixtures and then that will need to be merged / rebased into this to get CI passing.

charlespierce avatar May 13 '24 05:05 charlespierce

The CI failures I think are related to GitHub actions transitioning macos-latest to use an M1 machine, so they're running on an architecture that we don't have fixtures for. I'll create a separate PR to update the fixtures and then that will need to be merged / rebased into this to get CI passing.

Thanks a lot. Can you ping me again when I need to do the rebase?

iNViTiON avatar May 13 '24 09:05 iNViTiON

@iNViTiON The CI should be working again, so if you get a moment can you rebase the changes? Thanks again!

charlespierce avatar May 13 '24 17:05 charlespierce

@iNViTiON The CI should be working again, so if you get a moment can you rebase the changes? Thanks again!

I use Sync fork button on my forked repo. Is it okay or should I rebase my fork instead?

iNViTiON avatar May 13 '24 17:05 iNViTiON

No need to actually rebase, merging the changes in should be sufficient!

charlespierce avatar May 13 '24 17:05 charlespierce