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

Expand npm installer to include ARM64 Linux #2234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cllns
Copy link

@cllns cllns commented Dec 1, 2021

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 and v0.19.1-5 does not.

@github-actions
Copy link

github-actions bot commented Dec 1, 2021

Thanks for suggesting these code changes. To set expectations:

  • Pull requests are reviewed in batches, so it can take some time to get a response.
  • Smaller pull requests are easier to review. To fix nine typos, nine specific issues will always go faster than one big one. Learn why here.
  • Reviewers may not know as much as you about certain situations, so add links to supporting evidence for important claims, especially regarding standards for CSS, HTTP, URI, etc.

Finally, please be patient with the core team. They are trying their best with limited resources.

@sporto
Copy link

sporto commented May 25, 2022

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.

@avh4
Copy link
Member

avh4 commented May 25, 2022

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.

@sporto
Copy link

sporto commented May 25, 2022

@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.

@lydell
Copy link
Contributor

lydell commented May 25, 2022

@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 node:16-bullseye failed for them. If you have time, creating a minimal Dockerfile that demonstrates successfully running Elm on an M1 would be great!

(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.)

@sporto
Copy link

sporto commented May 25, 2022

@lydell We used node:lts-buster. One of my collegues tried this as I don't have an M1 myself.

The dockerfile is something like

FROM node:lts-buster@sha256:a6c217d7c8f001dc6fc081d55c2dd7fb3fefe871d5aa7be9c0c16bd62bea8e0c

RUN apt-get update

RUN curl -L -o elm.gz https://github.com/elm/compiler/releases/download/0.19.1/binary-for-linux-64-bit.gz \
    && gunzip elm.gz \
    && chmod +x elm \
    && mv elm /usr/local/bin/

@alshdavid
Copy link

Perhaps adding a check to see if an elm binary is available before deciding to download would be better. That way people with Rosetta2 and Box64 can manually install the amd64 binary knowing its limitations while the project maintainers can add darwin-arm64 and linux-arm64 binaries later

@alshdavid
Copy link

Hey @evancz, any chance you can chime in here as the change in to add verification in the postinstall breaks the npm install on my project when compiled in Docker on MacOS / arm Linux.

I'd raise a PR with the changes myself, but from the GitHub pulse it seems this project is no longer maintained?

@nicklayb
Copy link

@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 create-elm-app and has elm as a npm dependency, so it tries to download the binary anyway

@lydell
Copy link
Contributor

lydell commented Apr 22, 2024

@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/elm@0.19.1-14"
  },
  "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.)

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.

6 participants