-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
ci: pin GitHub Actions macos runner version and build for darwin/amd64 #6721
ci: pin GitHub Actions macos runner version and build for darwin/amd64 #6721
Conversation
.github/workflows/post-merge.yaml
Outdated
run: make ci-build-darwin ci-build-darwin-arm64-static | ||
run: | | ||
make ci-build-darwin GOARCH=amd64 | ||
make ci-build-darwin GOARCH=arm64 |
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 can skip this in-line with previous releases and build only static binary for arm64.
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.
Also we need to update the pull-request
and post-tag
workflows.
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.
✅ Deploy Preview for openpolicyagent ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
- os: darwin | ||
run: macos-latest | ||
run: macos-13 | ||
exec: opa_darwin_amd64 | ||
arch: amd64 |
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.
CPU architecture of macos-13 is amd64.
Should we test opa_darwin_arm64_static too?
opa/.github/workflows/pull-request.yaml
Line 268 in f76246f
# Note(philipc): We only run the amd64 targets. |
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.
It would be good if we were consistent with the macos version. If we just used macos-13
then we don't need to change anything else.
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.
To test opa_darwin_amd64, we have to use macos-13.
And to test test opa_darwin_arm64_static, we have to use macos-14.
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 added a test of opa_darwin_arm64_static.
This means to run smoke test of both opa_darwin_amd64 and opa_darwin_arm64_static.
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.
LGTM 👍
Thank you! 😃
Signed-off-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Signed-off-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Signed-off-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Signed-off-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Signed-off-by: Shunsuke Suzuki <suzuki.shunsuke.1989@gmail.com>
Why the changes in this PR are needed?
Close #6720
To release pre built binaries for darwin/amd64.
What are the changes in this PR?
macos-14
macos-latest
was changed from amd64 to arm64. To avoid the same issue, we should pin the versionNotes to assist PR review:
Please see #6720 .
Further comments: