-
Notifications
You must be signed in to change notification settings - Fork 23
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 warning if requesting x64 on apple silicon runners #300
Conversation
582765a
to
138418b
Compare
138418b
to
59f78c1
Compare
e212682
to
ee74250
Compare
@@ -580,6 +580,9 @@ function run() { | |||
if (!originalArchInput) { // if `originalArchInput` is an empty string | |||
throw new Error(`Arch input must not be null`); | |||
} | |||
if (originalArchInput == 'x64' && os.platform() == 'darwin' && os.arch() == 'arm64') { | |||
core.warning('[setup-julia] x64 arch has been requested on a macOS runner that has an arm64 (Apple Silicon) architecture. You may have meant to use the "aarch64" arch instead (or left it unspecified for the correct default).'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems misleading, since leaving it unspecified causes it to throw an exception and fail:
setup-julia/src/setup-julia.ts
Lines 61 to 63 in 5c9647d
if (!originalArchInput) { // if `originalArchInput` is an empty string | |
throw new Error(`Arch input must not be null`) | |
} |
https://github.com/vtjnash/Glob.jl/actions/runs/11898442357/job/33154951261?pr=44
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unspecified as in not an empty string, just not set at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do this:
- uses: julia-actions/setup-julia@desiredtag
with:
version: '1.10'
Then setup-julia will default to the architecture of the runner.
In contrast, if you do this:
- uses: julia-actions/setup-julia@desiredtag
with:
version: '1.10'
arch: ''
Then setup-julia will (correctly) throw an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use a matrix and don't specify arch, then it will get an empty string here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following two are equivalent:
- uses: julia-actions/setup-julia@desiredtag
with:
version: '1.10'
- uses: julia-actions/setup-julia@desiredtag
with:
version: '1.10'
arch: 'default'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation says it defaults to ''
, though I agree that using "default"
instead works
Lines 56 to 57 in 5c9647d
# Defaults to the architecture of the runner executing the job. | |
arch: '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use a matrix, you shouldn't be passing an empty string ''
. You should pass 'default'
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the readme is misleading. They're all like that, note the default behaviors are written above the empty string examples. Not sure what the clearest way to write them all is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should fix up the README. For inputs that are optional (like arch
), we should put a reasonable default in there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be happening quite a bit in the wild.
i.e. JuliaLang/julia#55878