Skip to content

Add rust+cargo to musl toolchain#228

Merged
sxa merged 3 commits intonodejs:mainfrom
nikeee:fix/add-rust-cargo
May 7, 2026
Merged

Add rust+cargo to musl toolchain#228
sxa merged 3 commits intonodejs:mainfrom
nikeee:fix/add-rust-cargo

Conversation

@nikeee
Copy link
Copy Markdown
Contributor

@nikeee nikeee commented May 6, 2026

See #227
See nodejs/docker-node#2486

This probably affects all architectures that are built here.

The only one that is used in the official docker images seems to be x64:
nodejs/docker-node@6027bae

Merging this would prevent the new docker images from being blocked. We probably can fix the cross-compilation later.

@nikeee nikeee force-pushed the fix/add-rust-cargo branch 2 times, most recently from 74ae38b to 6fada9d Compare May 6, 2026 20:36
@nikeee nikeee marked this pull request as ready for review May 6, 2026 20:37
@sxa
Copy link
Copy Markdown
Member

sxa commented May 7, 2026

Merging this would prevent the new docker images from being blocked. We probably can fix the cross-compilation later.

Noting that the presence of rust will likely cause the the arm builds to fail. A similar solution to nodejs/build#4316 may be all that's required but it would need someone to verify it.

Perhaps @mollux would like to take a look at this?

@nikeee
Copy link
Copy Markdown
Contributor Author

nikeee commented May 7, 2026

Noting that the presence of rust will likely cause the the arm builds to fail.

This PR only touches x64+musl. For arm, the docker image doesn't pull in a binary from unofficial-builds for some reason, but uses a multi-stage-build to build them from source (see: nodejs/docker-node#2488). There the docker image seems to be built on arm64 (or being emulated To arm64) because there don't seem to be any cross-compilations going on.

Not sure if there would be a better solution.

@sxa
Copy link
Copy Markdown
Member

sxa commented May 7, 2026

Noting that the presence of rust will likely cause the the arm builds to fail.

This PR only touches x64+musl. For arm, the docker image doesn't pull in a binary from unofficial-builds for some reason, but uses a multi-stage-build to build them from source (see: nodejs/docker-node#2488). There the docker image seems to be built on arm64 (or being emulated To arm64) because there don't seem to be any cross-compilations going on.

Not sure if there would be a better solution.

Correct but the standalone unofficial-build for Alpine/arm64 (not the one that goes into the docker image at present) will likely be affected if we put this in as it would install the x64 version of rust which I expect to fail as per the build issue I mentioned above which was a problem for cross-compiled macos builds.

@nikeee
Copy link
Copy Markdown
Contributor Author

nikeee commented May 7, 2026

Maybe I'm getting this wrong, but i thought musl-arm is built solely from https://github.com/nodejs/unofficial-builds/blob/main/recipes/arm64-musl and not this one. At least that is where some cross-compilation is set up. If this PR breaks the arm build, then we'd probably do it somehow different like you mentioned.

@sxa
Copy link
Copy Markdown
Member

sxa commented May 7, 2026

Yeah you're right - my bad (I wish that directory was explicitly musl-x64!). Have you tested these changes to confirm that it definitely builds ok on Alpine/x64?

@nikeee
Copy link
Copy Markdown
Contributor Author

nikeee commented May 7, 2026

I ran ./bin/local_build.sh -r musl -v v26.0.0. After applying the patch, ./configure doesn't print out the two warnings regarding cargo/rust any more. cargo is also invoked, as it prints out some rust-compiler output. Haven't been able to finish an entire build yet, because I ran out of ram before finishing.
It's basically the same change as in nodejs/docker-node#2488 (but for x64 instead of arm) and I was able to confirm that it worked.

My local build is still running and I'll reach back when it finishes.

@sxa
Copy link
Copy Markdown
Member

sxa commented May 7, 2026

Just a note on this - adjusting the dockerfile to Alpine 3.23 may cause problems on the Alpine 3.22 image so we'll want to verify that too. FYI @MikeMcC399
It might be better to drop this to 3.22 for now.

@nikeee
Copy link
Copy Markdown
Contributor Author

nikeee commented May 7, 2026

Good point. Done in 67f2cd8

Update: Build went through. Tested it:
image

@sxa
Copy link
Copy Markdown
Member

sxa commented May 7, 2026

Thanks. I think I'm happy to merge this now :-)
If it causes a problem we can adjust later, but this should unblock the container production for Node 26.1.0

Copy link
Copy Markdown
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

LGTM

@sxa sxa merged commit 7159cd7 into nodejs:main May 7, 2026
@nikeee nikeee deleted the fix/add-rust-cargo branch May 7, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants