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

add recipe to compile arm64-musl version #59

Merged
merged 3 commits into from
Feb 19, 2023

Conversation

mollux
Copy link
Contributor

@mollux mollux commented May 16, 2022

Configuration to allow compiling a arm64 musl version (on amd64 hardware)

I tested this by building the last version of node 8, 10, 12, 14 and 16 on amd64 hardware, and doing the basic steps in a frontend theme project (npm install, grunt watch, ...), and it works without issues.

This addresses #50

I updated the readme.md best effort, but feel free to update if needed

@nschonni
Copy link
Member

@rvagg
Copy link
Member

rvagg commented May 17, 2022

Would @nodejs/docker images consume these binaries if we produced them here?

And, is there much of an audience for this? I already probed at this question in #50. It seems reasonable to me that there might be a growing audience for this kind of thing. Maybe we have Docker image numbers to suggest audience size?

@mollux I assume you're one of those people, can you talk about your use-case a bit?

recipes/arm64-musl/run.sh Outdated Show resolved Hide resolved
recipes/arm64-musl/run.sh Outdated Show resolved Hide resolved
@nschonni
Copy link
Member

Currently, only the x64 builds from here are special cased https://github.com/nodejs/docker-node/blob/1b70b5dfb424d67c8e0b0badb985aedf43a2227a/Dockerfile-alpine.template#L11-L26 but it's possible it could be done for arm64. Not sure if it would need to be arm64v8 vs arm64 or if that's implied

@nschonni
Copy link
Member

@mollux
Copy link
Contributor Author

mollux commented May 18, 2022

@rvagg

our use case

Our use case is that we have a custom image (based on php:cli-alpine) where we install the theia editor to have an in-browser IDE to develop / compile / without a local IDE.

One of the tasks to perform is to compile frontend themes, where we require nodejs.
We use nvm to install multiple versions (for some old projects we are updating we still need nodejs 10).

On amd64 we use the pre-build version from unofficial builds, hence the PR to be able to do the same for arm64.
We have this use case as we are switching more and more of our laptops to Macbook M1.

I currently use a self-managed location where we host the compiled versions and the index.tab, but ideally they are all hosted on an official unofficial location :)

use the binaries in the nodejs-docker images

It looks like there is not much change needed to use the arm64 binaries if they are available.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

fine by me, just needs someone to deploy and test the deployment; I'll try and get to it soon but there are others who might like to hop in too

@mollux
Copy link
Contributor Author

mollux commented Jun 10, 2022

hi @rvagg

is there anything I can do on my side to move this PR forward?

@acifani
Copy link

acifani commented Aug 25, 2022

hey @rvagg, any news on this? anything we can do to help?

in our company, we use docker with a custom alpine base image. having prebuilt binaries would help us have multiarch images rather sooner & quicker than building from source

@nschonni
Copy link
Member

@acifani there are already prebuilt Alpine images on dockerhub through the official node image

@acifani
Copy link

acifani commented Aug 25, 2022

@acifani there are already prebuilt Alpine images on dockerhub through the official node image

Hey @nschonni, thanks for the heads up! Unfortunately that doesn't work for us because we need to use our own Alpine base image, so we can't depend on the node alpine image directly 😞

@jacobdr jacobdr mentioned this pull request Nov 3, 2022
@PeterBurner
Copy link

PeterBurner commented Nov 30, 2022

I am facing a similar scenario as @acifani. I am required to use a specific alpine base image and need to install node 18 with musl support.
@rvagg can you make some time to merge this?

@aral
Copy link

aral commented Feb 1, 2023

This would enable Node.js to run on mobile devices like the PinePhone and PinePhone Pro running Alpine-based PostmarketOS and open up new possibilities for the use of such devices in education and elsewhere.

Is there anything we can do to help get this merged @rvagg? :)

@mcollina
Copy link
Member

mcollina commented Feb 1, 2023

@rvagg @nodejs/build @nodejs/docker could you take a look?

@wenerme
Copy link

wenerme commented Feb 18, 2023

@rvagg need this for cloud provided arm64 server

@rvagg
Copy link
Member

rvagg commented Feb 19, 2023

let's just deploy and see what happens eh? will report back if there are errors and this needs to be backed out, disabled or whatever and has to be fixed

@rvagg rvagg merged commit 2037d02 into nodejs:main Feb 19, 2023
@@ -0,0 +1,39 @@
FROM alpine:3.15
Copy link
Member

Choose a reason for hiding this comment

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

btw, why are we doing 3.15? wouldn't it be better to go for the latest so we don't have so much catch-up work to do next time we have to revisit this?

Copy link

Choose a reason for hiding this comment

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

Prefer 3.17

@rvagg
Copy link
Member

rvagg commented Feb 20, 2023

Nope:

I think I can leave this in place for now while it gets fixed as it shouldn't disrupt other builds, but it'll need to get fixed or reverted soon please. @mollux are you able to take a look at this and figure out how to address it?

@richardlau
Copy link
Member

Maybe nodejs/node#45756?

@rvagg
Copy link
Member

rvagg commented Mar 2, 2023

reverting in #70

@nikolaik
Copy link

I believe this can be reopened after nodejs/node#51256 got merged. Probably have to wait for the fix to land in the release branches first though. Do you want me to create a separate issue?

@mcollina
Copy link
Member

It would be best if you would open a fresh PR

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.

10 participants