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

Workflows: Drop Windows release builds and use more powerful runners for others #117111

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

tstellar
Copy link
Collaborator

@tstellar tstellar commented Nov 21, 2024

We have community provided Windows builds that are better than what we can build on GitHub. For the Linux/X86 builds and Mac/Aarch64 builds we will use depot runners, for Mac/X86 we will use the larger GitHub runners.

@tstellar tstellar marked this pull request as ready for review December 20, 2024 14:49
@llvmbot
Copy link
Member

llvmbot commented Dec 20, 2024

@llvm/pr-subscribers-github-workflow

Author: Tom Stellard (tstellar)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/117111.diff

1 Files Affected:

  • (modified) .github/workflows/release-binaries.yml (+75-15)
diff --git a/.github/workflows/release-binaries.yml b/.github/workflows/release-binaries.yml
index 1cde628d3f66c3..7a3f6870ebe291 100644
--- a/.github/workflows/release-binaries.yml
+++ b/.github/workflows/release-binaries.yml
@@ -60,6 +60,8 @@ jobs:
       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
@@ -144,12 +146,25 @@ jobs:
 
         echo "target-cmake-flags=$target_cmake_flags" >> $GITHUB_OUTPUT
         echo "build-flang=$build_flang" >> $GITHUB_OUTPUT
+        case "${{ inputs.runs-on }}" in
+          ubuntu-22.04)
+            runs_on="depot-${{ inputs.runs-on }}-16"
+            multi_stage="false"
+            ;;
+          *)
+            runs_on="${{ inputs.runs-on }}"
+            multi_stage="true"
+            ;;
+        esac
+        echo "runs-on=$runs_on" >> $GITHUB_OUTPUT
+        echo "multi-stage=$multi_stage" >> $GITHUB_OUTPUT
 
   build-stage1:
     name: "Build Stage 1"
     needs: prepare
-    if: github.repository == 'llvm/llvm-project'
-    runs-on: ${{ inputs.runs-on }}
+    if: >-
+      github.repository == 'llvm/llvm-project'
+    runs-on: ${{ needs.prepare.outputs.runs-on }}
     steps:
 
     - name: Checkout Actions
@@ -195,7 +210,7 @@ jobs:
         key: sccache-${{ runner.os }}-${{ runner.arch }}-release
         variant: sccache
 
-    - name: Build Stage 1 Clang
+    - name: Configure Stage 1 Clang
       id: build
       shell: bash
       run: |
@@ -208,10 +223,35 @@ jobs:
             -DBOOTSTRAP_CPACK_PACKAGE_FILE_NAME="${{ needs.prepare.outputs.release-binary-basename }}" \
             -DCMAKE_C_COMPILER_LAUNCHER=sccache \
             -DCMAKE_CXX_COMPILER_LAUNCHER=sccache
-        ninja -v -C ${{ steps.setup-stage.outputs.build-prefix }}/build
-        # There is a race condition on the MacOS builders and this command is here
-        # to help debug that when it happens.
-        ls -ltr ${{ steps.setup-stage.outputs.build-prefix }}/build
+    - name: Build Stage 1 Clang
+      shell: bash
+      run: |
+        if [ "${{ needs.prepare.outputs.multi-stage}}" = "false" ]; then
+          ninja -v -C ${{ steps.setup-stage.outputs.build-prefix }}/build stage2-package
+          mv ${{ steps.setup-stage.outputs.build-prefix  }}/build/tools/clang/stage2-bins/${{ needs.prepare.outputs.release-binary-filename }} .
+        else
+          ninja -v -C ${{ steps.setup-stage.outputs.build-prefix }}/build
+          # There is a race condition on the MacOS builders and this command is here
+          # to help debug that when it happens.
+          ls -ltr ${{ steps.setup-stage.outputs.build-prefix }}/build
+        fi
+    
+    - uses: actions/upload-artifact@26f96dfa697d77e81fd5907df203aa23a56210a8 #v4.3.0
+      if: needs.prepare.outputs.multi-stage == 'false'
+      with:
+        name: ${{ runner.os }}-${{ runner.arch }}-release-binary
+        # Due to path differences on Windows when running in bash vs running on node,
+        # we need to search for files in the current workspace.
+        path: |
+          ${{ needs.prepare.outputs.release-binary-filename }}
+
+    # Clean up some build files to reduce size of artifact.
+    - name: Clean Up Build Directory
+      if: needs.prepare.outputs.multi-stage == 'false'
+      shell: bash
+      run: |
+        find ${{ steps.setup-stage.outputs.build-prefix }}/build -iname ${{ needs.prepare.outputs.release-binary-filename }} -delete
+        rm -Rf ${{ steps.setup-stage.outputs.build-prefix }}/build/tools/clang/stage2-bins/_CPack_Packages
     
     - name: Save Stage
       uses: ./workflows-main/.github/workflows/release-binaries-save-stage
@@ -223,8 +263,10 @@ jobs:
     needs:
       - prepare
       - build-stage1
-    if: github.repository == 'llvm/llvm-project'
-    runs-on: ${{ inputs.runs-on }}
+    if: >- 
+      github.repository == 'llvm/llvm-project' &&
+      needs.prepare.outputs.multi-stage == 'true'
+    runs-on: ${{ needs.prepare.outputs.runs-on }}
     steps:
     - name: Checkout Actions
       uses: actions/checkout@v4
@@ -242,7 +284,9 @@ jobs:
 
     - name: Build Stage 2
       # Re-enable once PGO builds are supported.
-      if: needs.prepare.outputs.enable-pgo == 'true'
+      if: >-
+        needs.prepare.outputs.enable-pgo == 'true' &&
+        needs.prepare.outputs.multi-stage == 'true'
       shell: bash
       run: |
         ninja -C ${{ steps.setup-stage.outputs.build-prefix}}/build stage2-instrumented
@@ -257,8 +301,10 @@ jobs:
     needs:
       - prepare
       - build-stage2
-    if: github.repository == 'llvm/llvm-project'
-    runs-on: ${{ inputs.runs-on }}
+    if: >- 
+      github.repository == 'llvm/llvm-project' &&
+      needs.prepare.outputs.multi-stage == 'true'
+    runs-on: ${{ needs.prepare.outputs.runs-on }}
     steps:
     - name: Checkout Actions
       uses: actions/checkout@v4
@@ -307,7 +353,9 @@ jobs:
     needs:
       - prepare
       - build-stage3-clang
-    runs-on: ${{ inputs.runs-on }}
+    if: >- 
+      needs.prepare.outputs.multi-stage == 'true'
+    runs-on: ${{ needs.prepare.outputs.runs-on }}
     steps:
     - name: Checkout Actions
       uses: actions/checkout@v4
@@ -357,7 +405,7 @@ jobs:
     needs:
       - prepare
       - build-stage3-flang
-    runs-on: ${{ inputs.runs-on }}
+    runs-on: ${{ needs.prepare.outputs.runs-on }}
     steps:
     - name: Checkout Actions
       uses: actions/checkout@v4
@@ -409,6 +457,7 @@ jobs:
     needs:
       - prepare
       - build-stage3-all
+      - build-stage1
     if: >-
       always() &&
       github.event_name != 'pull_request' &&
@@ -469,6 +518,7 @@ jobs:
       - prepare
       - build-stage3-all
     if: >-
+      always() &&
       github.repository == 'llvm/llvm-project'
     runs-on: ${{ inputs.runs-on }}
     steps:
@@ -484,7 +534,17 @@ jobs:
       id: setup-stage
       uses: ./workflows/.github/workflows/release-binaries-setup-stage
       with:
-        previous-artifact: build-stage3-all
+        previous-artifact: ${{ (needs.prepare.outputs.multi-stage == 'false' && 'build-stage1') || 'build-stage3-all' }}
+
+    # Need sccache installed, because some stage1 objects are being built for the tests.
+    # FIXME: This probably shouldn't be happening.
+    - name: Setup sccache
+      uses: hendrikmuhs/ccache-action@ca3acd2731eef11f1572ccb126356c2f9298d35e # v1.2.9
+      with:
+        # Default to 2G to workaround: https://github.com/hendrikmuhs/ccache-action/issues/174
+        max-size: 2G
+        key: sccache-${{ runner.os }}-${{ runner.arch }}-release
+        variant: sccache
 
     - name: Run Tests
       shell: bash

@tstellar tstellar mentioned this pull request Dec 21, 2024
Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits so far.

It seems like there's a lot of additional complexity from having to support both single stage and multi stage builds? Is it possible to just do single stage builds on depot like we did for the container build?

.github/workflows/release-binaries.yml Outdated Show resolved Hide resolved
.github/workflows/release-binaries.yml Outdated Show resolved Hide resolved
.github/workflows/release-binaries.yml Outdated Show resolved Hide resolved
@tstellar
Copy link
Collaborator Author

Some nits so far.

It seems like there's a lot of additional complexity from having to support both single stage and multi stage builds? Is it possible to just do single stage builds on depot like we did for the container build?

I agree, but we can't drop the multi-stage builds until we get more powerful mac/Windows runners to use. I thought about splitting the workflow, but the main reason I didn't is that I thought there would be too much duplication between the two workflows. I could try to factor out the common stuff and put it in a composite action, and I can do that if you think it will be better.

@boomanaiden154
Copy link
Contributor

I agree, but we can't drop the multi-stage builds until we get more powerful mac/Windows runners to use. I thought about splitting the workflow, but the main reason I didn't is that I thought there would be too much duplication between the two workflows. I could try to factor out the common stuff and put it in a composite action, and I can do that if you think it will be better.

Does Depot support MacOS/Windows jobs? I know AWS at least has instances for both.

@tstellar
Copy link
Collaborator Author

tstellar commented Jan 3, 2025

I agree, but we can't drop the multi-stage builds until we get more powerful mac/Windows runners to use. I thought about splitting the workflow, but the main reason I didn't is that I thought there would be too much duplication between the two workflows. I could try to factor out the common stuff and put it in a composite action, and I can do that if you think it will be better.

Does Depot support MacOS/Windows jobs? I know AWS at least has instances for both.

Depot supports AArch64 macOS, I'm not sure they support X86 though. Windows is not supported yet.

@tstellar tstellar marked this pull request as draft January 4, 2025 06:45
@tstellar tstellar changed the title Workflows: Use new depot runners for x86 Linux release builds Workflows: Drop Windows release builds and use more powerful runners for others Jan 6, 2025
@tstellar tstellar marked this pull request as ready for review January 6, 2025 10:06
@tstellar
Copy link
Collaborator Author

tstellar commented Jan 6, 2025

I went ahead and dropped the multi-stage builds, so this workflow file is much more simple now. Linux/X86 and Mac/AArch64 will use depot runners and Mac/X86 will use larger github runners. However, to save resources, the Mac builds will use the free runners when testing pull requests.

Copy link
Contributor

@boomanaiden154 boomanaiden154 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late review on this one.

.github/workflows/release-binaries.yml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants