-
Notifications
You must be signed in to change notification settings - Fork 15.1k
workflows/release-binaries: Enable builds on Linux/AArch64 #120786
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
Changes from 18 commits
cb87398
e0373d6
6568444
1702aa6
72ff71f
e2ac546
137ed98
605e497
c60000d
76675cb
2fca063
5aa1a47
dc72a8a
cf8f566
065341f
232248b
4a39852
23244ea
38ef06d
6b97ea4
712789d
f0f4e87
9843b3d
ec799e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ on: | |||||
| type: choice | ||||||
| options: | ||||||
| - ubuntu-22.04 | ||||||
| - depot-ubuntu-22.04-arm-small | ||||||
| - windows-2022 | ||||||
| - macos-13 | ||||||
| - macos-14 | ||||||
|
|
@@ -56,10 +57,13 @@ jobs: | |||||
| ref: ${{ steps.vars.outputs.ref }} | ||||||
| upload: ${{ steps.vars.outputs.upload }} | ||||||
| target-cmake-flags: ${{ steps.vars.outputs.target-cmake-flags }} | ||||||
| ccache: ${{ steps.vars.outputs.ccache }} | ||||||
| build-flang: ${{ steps.vars.outputs.build-flang }} | ||||||
| enable-pgo: ${{ steps.vars.outputs.enable-pgo }} | ||||||
| release-binary-basename: ${{ steps.vars.outputs.release-binary-basename }} | ||||||
| release-binary-filename: ${{ steps.vars.outputs.release-binary-filename }} | ||||||
| runs-on: ${{ steps.vars.outputs.runs-on }} | ||||||
| multi-stage: ${{ steps.vars.outputs.multi-stage }} | ||||||
|
|
||||||
| steps: | ||||||
| # It's good practice to use setup-python, but this is also required on macos-14 | ||||||
|
|
@@ -120,8 +124,21 @@ jobs: | |||||
|
|
||||||
| # Detect necessary CMake flags | ||||||
| target="${{ runner.os }}-${{ runner.arch }}" | ||||||
| echo "enable-pgo=false" >> $GITHUB_OUTPUT | ||||||
| target_cmake_flags="-DLLVM_RELEASE_ENABLE_PGO=OFF" | ||||||
|
|
||||||
| # The hendrikmuhs/ccache-action action does not support installing sccache | ||||||
| # on arm64 Linux. | ||||||
| if [ "$target" = "Linux-ARM64" ]; then | ||||||
| echo ccache=ccache >> $GITHUB_OUTPUT | ||||||
| else | ||||||
| echo ccache=sccache >> $GITHUB_OUTPUT | ||||||
| fi | ||||||
|
|
||||||
| if [ "${{ runner.os }}" = "Linux" ]; then | ||||||
|
||||||
| if [ "${{ runner.os }}" = "Linux" ]; then | |
| if [ "$RUNNER_OS" = "Linux" ]; then |
We don't need workflow interpolation here, and it's better to avoid it if we don't need it: https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-an-intermediate-environment-variable
In general I'd prefer to remove all workflow interpolation (even in instances where it is safe) to make auditing easier, but that would require plenty of changes across all 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.
Here is a patch to clean up the file: #120860
I'll update this patch with your suggestions too.
Outdated
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.
| github.repository == 'llvm/llvm-project' | |
| github.repository_owner == 'llvm' |
Might make it easier to reuse workflow snippets in other org repos, but this is a very mild preference.
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've put this change in a separate PR: #123797
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 seems like this should work still, no?
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 is working now, so I dropped this part of the change.