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/release-binaries: Always pull composite actions from main branch #100805

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

tstellar
Copy link
Collaborator

If we pull from the release tag, then if there is a bug in one of the actions on the release tag, then we can never do a build for that tag. Pulling from main will allows us to fix bugs in the actions we use to build the releases.

…ranch

If we pull from the release tag, then if there is a bug in one of the
actions on the release tag, then we can never do a build for that tag.
Pulling from main will allows us to fix bugs in the actions we use to
build the releases.
@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2024

@llvm/pr-subscribers-github-workflow

Author: Tom Stellard (tstellar)

Changes

If we pull from the release tag, then if there is a bug in one of the actions on the release tag, then we can never do a build for that tag. Pulling from main will allows us to fix bugs in the actions we use to build the releases.


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

1 Files Affected:

  • (modified) .github/workflows/release-binaries.yml (+15-5)
diff --git a/.github/workflows/release-binaries.yml b/.github/workflows/release-binaries.yml
index 0a800fd006e9d..5250a9fd4cae1 100644
--- a/.github/workflows/release-binaries.yml
+++ b/.github/workflows/release-binaries.yml
@@ -148,6 +148,20 @@ jobs:
     if: github.repository == 'llvm/llvm-project'
     runs-on: ${{ inputs.runs-on }}
     steps:
+
+    - name: Checkout Actions
+      uses: actions/checkout@v4
+      with:
+        ref: ${{ (github.event_name == 'pull_request' && github.sha) || 'main' }}
+        sparse-checkout: |
+          .github/workflows/
+        sparse-checkout-cone-mode: false
+        path: workflows
+
+    - name: Setup Stage
+      id: setup-stage
+      uses: ./workflows/.github/workflows/release-binaries-setup-stage
+
     - name: Checkout LLVM
       uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
       with:
@@ -161,10 +175,6 @@ jobs:
         key: sccache-${{ runner.os }}-${{ runner.arch }}-release
         variant: sccache
 
-    - name: Setup Stage
-      id: setup-stage
-      uses: ./.github/workflows/release-binaries-setup-stage
-
     - name: Build Stage 1 Clang
       id: build
       shell: bash
@@ -184,7 +194,7 @@ jobs:
         ls -ltr ${{ steps.setup-stage.outputs.build-prefix }}/build
     
     - name: Save Stage
-      uses: ./.github/workflows/release-binaries-save-stage
+      uses: ./workflows/.github/workflows/release-binaries-save-stage
       with:
         build-prefix: ${{ steps.setup-stage.outputs.build-prefix }}
 

sparse-checkout-cone-mode: false
path: workflows

- name: Setup Stage
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming this moved up here so it can use the just pulled in action definition?

The subsequent LLVM checkout ends up overriding this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The next checkout won't override this, so I didn't have to move this. The reason I did was to match the other stages which have these two steps next to each other.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense.

For some reason I missed the path argument in my initial readover. This makes more sense now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was wrong, the checkout does overwrite this. See #100845.

@@ -148,6 +148,20 @@ jobs:
if: github.repository == 'llvm/llvm-project'
runs-on: ${{ inputs.runs-on }}
steps:

- name: Checkout Actions
uses: actions/checkout@v4
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably should be hash pinned to the same version as below.

@tstellar tstellar merged commit b0860b2 into llvm:main Jul 26, 2024
5 checks passed
@tstellar
Copy link
Collaborator Author

Should have looked closer, the test failure didn't show up in the checks list, I think because there was a bug in the yml. I'll push a fix-up commit.

tstellar added a commit to tstellar/llvm-project that referenced this pull request Aug 3, 2024
…ranch (llvm#100805)

If we pull from the release tag, then if there is a bug in one of the
actions on the release tag, then we can never do a build for that tag.
Pulling from main will allows us to fix bugs in the actions we use to
build the releases.

(cherry picked from commit b0860b2)
tru pushed a commit to tstellar/llvm-project that referenced this pull request Aug 5, 2024
…ranch (llvm#100805)

If we pull from the release tag, then if there is a bug in one of the
actions on the release tag, then we can never do a build for that tag.
Pulling from main will allows us to fix bugs in the actions we use to
build the releases.

(cherry picked from commit b0860b2)
FASOOS1

This comment was marked as spam.

FASOOS1

This comment was marked as spam.

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.

4 participants