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 architecture alias for x86-64: x64 #58

Closed
PiotrSikora opened this issue Aug 2, 2023 · 6 comments
Closed

Add architecture alias for x86-64: x64 #58

PiotrSikora opened this issue Aug 2, 2023 · 6 comments

Comments

@PiotrSikora
Copy link

When using act or self-hosted runners, the host could be aarch64, at which point the cost of the additional virtualization to the default x86_64 architecture is not worth it for someone trying to test if a project builds on a given OS, without caring about the specific architecture.

Also, I've tried a few options to set it explicitly to $RUNNER_ARCH, but for some reason none of them work, even though I can echo the variable just fine:

architecture: $RUNNER_ARCH
architecture: ${{ env.RUNNER_ARCH }}
architecture: ${{ format('{0}', env.RUNNER_ARCH) }}

Any ideas?

@jacob-carlborg
Copy link
Contributor

When using act or self-hosted runners, the host could be aarch64, at which point the cost of the additional virtualization to the default x86_64 architecture is not worth it for someone trying to test if a project builds on a given OS, without caring about the specific architecture.

Do you want the default architecture to follow the host/runner architecture?

Also, I've tried a few options to set it explicitly to $RUNNER_ARCH, but for some reason none of them work, even though I can echo the variable just fine:

Which value is actually passed to the action? You can see that in the log, here's an example:

Screenshot 2023-08-02 at 09 07 09

@PiotrSikora
Copy link
Author

Do you want the default architecture to follow the host/runner architecture?

Yes, I think that's a better default than any hardcoded value (with a caveat that x64_64 is the only available architecture on runners hosted by GitHub, but presumably it was selected for the same reason as I'm suggesting here, i.e. don't virtualize another CPU for no reason).

But of course it's only a suggestion.

Which value is actually passed to the action? You can see that in the log, here's an example:

It was empty in all 3 cases, but that could be an issue with act.

However, right after I gave up and filled this PR, I found a way to use it:

architecture: ${{ runner.arch }}

Note that this evaluates to X64 on x86_64, which isn't supported value, so it doesn't work right now.

From the docs for variables in GitHub Actions:

RUNNER_ARCH | The architecture of the runner executing the job. Possible values are X86X64ARM, or ARM64.

@jacob-carlborg
Copy link
Contributor

jacob-carlborg commented Aug 2, 2023

Yes, I think that's a better default than any hardcoded value

Hmm, I'm not sure if I think that's a good API. It would be a breaking change as well, although unlikely someone would be affected. If someone would run this action on a AArch64 self hosted runner (I don't even know if the action works on that).

From the docs for variables in GitHub Actions:

RUNNER_ARCH | The architecture of the runner executing the job. Possible values are X86, X64, ARM, or ARM64.

X64, that's an odd choice of name. I guess I could add an alias for that name. In the meantime you can use a conditional to map the value, here's an example:

https://github.com/cross-platform-actions/openbsd-builder/blob/d00dce49c326a40f295df380f70e9b7913b954d9/.github/workflows/build.yml#L40-L44

@PiotrSikora
Copy link
Author

Hmm, I'm not sure if I think that's a good API. It would be a breaking change as well, although unlikely someone would be affected. If someone would run this action on a AArch64 self hosted runner

Fair enough!

(I don't even know if the action works on that).

I thought it did, but now I'm consistently getting this weird error on Linux/aarch64 host using act, so perhaps it doesn't:

/tmp/5fe578c4-9d15-494c-a2f3-2d9f154a1775/qemu-img convert -f qcow2 -O raw /tmp/bb159e62-b118-4411-bf00-a57a0c1be7a5 /tmp/5fe578c4-9d15-494c-a2f3-2d9f154a1775/disk.raw
/tmp/5fe578c4-9d15-494c-a2f3-2d9f154a1775/qemu-img: 1: Syntax error: "(" unexpected

I think this issue can be closed, unless you want to keep it open as a reminder to add X64.

@jacob-carlborg
Copy link
Contributor

I thought it did, but now I'm consistently getting this weird error on Linux/aarch64 host using act, so perhaps it doesn't:

Yeah, that error looks weird.

@jacob-carlborg jacob-carlborg changed the title architecture should default to $RUNNER_ARCH Add architecture alias for x86-64: x64 Aug 4, 2023
@PiotrSikora
Copy link
Author

Thanks!

korli added a commit to korli/action that referenced this issue Mar 15, 2024
Release 0.18.0

Added
- Add support for custom image URLs ([cross-platform-actions#13](cross-platform-actions#13))
- Add architecture alias for x86-64: x64 ([cross-platform-actions#58](cross-platform-actions#58))
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

No branches or pull requests

2 participants