-
Notifications
You must be signed in to change notification settings - Fork 663
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
Expand npm installer to include ARM64 Linux #2234
base: master
Are you sure you want to change the base?
Conversation
Thanks for suggesting these code changes. To set expectations:
Finally, please be patient with the core team. They are trying their best with limited resources. |
Hi @evancz, could we please merge this fix? This is becoming a real issue as more developers are moving to Macs M1 and using Docker. |
How is a linux-arm64 docker container able to run linux-x64 binaries? Is that somehow using Rosetta inside the docker container? Does it only work on MacOS? Does your docker image have qemu-user-binfmt configured, or can all linux-arm64 docker images somehow run linux-x64 binaries (including on a linux-arm64 host)? If this working relies on Rosetta and/or qemu-user-binfmt, then I think this is probably incorrect to merge since it will get and try to run binaries for the wrong architecture on machines that don't have emulation services configured. Also for the record, imo we ought to make official linux-arm64 binaries and then update the npm installer to use those when appropriate. |
@avh4 We tried downloading the binary for linux manually from releases and run it inside Docker on a M1. It works, but it is very slow. Unsure what is going on, but yes, doesn't seem like the right approach. |
@sporto Which Docker image do you use? I don’t have an M1, but some colleagues do, and as far as I remember running the Linux x64 binary in (Btw, elm-tooling currently does not have any arch checks, so you should be able to use it to install the (only) Linux binary in Docker.) |
@lydell We used The dockerfile is something like
|
Perhaps adding a check to see if an |
Hey @evancz, any chance you can chime in here as the change in to add verification in the postinstall breaks the I'd raise a PR with the changes myself, but from the GitHub pulse it seems this project is no longer maintained? |
@evancz Any way we could get this merged? Of course we can download the binary manually but I came to a point where that doesn't work either. I'm using |
@nicklayb This PR has merge conflicts, because the code looks nothing like this anymore since #2287. So it won’t be merged (as-is). Also, this PR just points to the regular x86 binary for Linux ARM, which is not going to work (in most cases). People with real Linux ARM computers need a Linux ARM binary (as far as I understood from people testing my PR), and people with macOS ARM running in Docker also need a Linux ARM binary. The @lydell/elm package exists for this reason. You can force npm to use my package instead, something like this: {
"overrides": {
"elm": "npm:@lydell/[email protected]"
},
"dependencies": {
"create-elm-app": "^5.22.0"
}
} (Btw, create-elm-app isn’t maintained, so you might want to use some other tool like Vite, Parcel 2, Elm Land or elm-watch.) |
A quickfix to #2232 (see comments there), though it may be re-written better or removed entirely (the only main architecture/OS combo this seems to leave out now is ARM Windows).
Or maybe we want to un-do 759bf04 in its entirety?
(Note that this lets Apple Silicon users with Elm in docker containers use the npm installer)
I can't find any documentation about how to run the installers, so I can't test this. But
v0.19.1-4
works andv0.19.1-5
does not.