Skip to content

[test] Only run build_and_deploy once for PRs from upstream #80329

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 9, 2025
+4 −1
Merged

[test] Only run build_and_deploy once for PRs from upstream #80329

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
08603cb
[not for merge] Test build_and_deploy on forks
eps1lon Jun 9, 2025
30ce276
Don't mark build as passed from skipped build_and_deploy
eps1lon Jun 9, 2025
11487c5
Progress
eps1lon Jun 9, 2025
2e45ffd
Simple wins
eps1lon Jun 9, 2025
ba35963
Don't mark `buildPassed` as skipped on upstream pull_request events
eps1lon Jun 9, 2025
f6f4876
Give up on this
eps1lon Jun 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/build_and_deploy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ env:
jobs:
deploy-target:
runs-on: ubuntu-latest
# Don't trigger this job on `pull_request` events from upstream branches.
# Those would already run this job on the `push` event
if: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork }}
outputs:
value: ${{ steps.deploy-target.outputs.value }}
steps:
Expand Down Expand Up @@ -693,7 +696,7 @@ jobs:

buildPassed:
needs: ['deploy-target', 'build', 'build-wasm', 'build-native']
if: always()
if: ${{ always() && needs.deploy-target.outputs.value != '' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

The condition needs.deploy-target.outputs.value != '' will cause buildPassed to be skipped when deploy-target is skipped, as skipped jobs don't produce outputs. This means buildPassed won't run for upstream PRs, potentially breaking required status checks.

Consider using needs.deploy-target.result != 'skipped' instead to run buildPassed when deploy-target has actually executed. Alternatively, if the goal is to always run buildPassed when dependent jobs complete (regardless of whether they were skipped), the additional condition could be removed entirely.

Suggested change
if: ${{ always() && needs.deploy-target.outputs.value != '' }}
if: ${{ always() && needs.deploy-target.result != 'skipped' }}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tradeoff correctly identified but that's intended as per PR description. Though wouldn't be surprised we actually missed some edge case. Time will tell.

name: thank you, build
runs-on: ubuntu-latest
steps:
Expand Down
Loading