Install the correct ghcup binary on aarch64 #47
Conversation
This enables native compilations on M1 Mac runners
There was a problem hiding this comment.
Great contribution!
Would you be willing to take this PR to production?
This would entail defining the new option in action.yaml and documenting it in the README.
Also, can arm64 this be tested in our CI (workflow.yml)? In that case (we have access to a suitable runner), please add a case for arm64 there.
src/installer.ts
Outdated
| core.info(`Attempting to install stack ${version}`); | ||
| const binArch = arch === 'arm64' ? 'aarch64' : 'x86_64'; |
There was a problem hiding this comment.
There is a bit of code duplication with some change below (see there).
src/installer.ts
Outdated
| }-ghcup-${ghcup_version}` | ||
| `https://downloads.haskell.org/ghcup/${ghcup_version}/${ | ||
| arch === 'arm64' ? 'aarch64' : 'x86_64' | ||
| }-${os === 'darwin' ? 'apple-darwin' : 'linux'}-ghcup-${ghcup_version}` |
There was a problem hiding this comment.
Could that translation from option strings to arch names be extracted to its own function so that the translation is defined in exactly one place in the code?
(See comment above on code duplication.)
Also, this function should error out on an unknown arch rather than falling back to a default. This would help users find errors. E.g. if they misspelled arm64 as arm46 they would be alerted of the problem clearly.
Update: maybe the type does not allow a value such as arm46, but new architectures could be added by someone later, and not having a fallback would make clear that the translation has to be extended to the new value.
| @@ -25,25 +25,26 @@ export default async function run( | |||
| try { | |||
| core.info('Preparing to setup a Haskell environment'); | |||
| const os = process.platform as OS; | |||
| const arch = process.arch as Arch; | |||
There was a problem hiding this comment.
Ok, the architecture is detected automatically.
|
Hi @andreasabel! thanks for the review and the patience here 😄 I pushed some changes to address your comments. I'm unsure if the CI will work, as I believe the organization needs to have large runners enabled to be able to use the M1 runners (https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/). I can revert that commit if it's not feasible/economical. |
|
ping, just checking if you'd like me to do any further changes here 👍 I'm happy to revert the CI changes if you decide that paying for GitHub's larger runners is not worth it. |
andreasabel
force-pushed
the
m1-support
branch
from
0482e02
to
4c14628
Compare
|
Thanks for the reminder! I removed the commit that adds to CI. We'll publish this as "untested". I have no budget to pay for CI here. |
|
Released in 2.6.0. |
This enables native compilations on M1 Mac runners.
Closes: #46