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

Verify that all-jobs-succeeded CI job depends on all other jobs #444

Closed
5 tasks
joshlf opened this issue Oct 2, 2023 · 4 comments · Fixed by #720
Closed
5 tasks

Verify that all-jobs-succeeded CI job depends on all other jobs #444

joshlf opened this issue Oct 2, 2023 · 4 comments · Fixed by #720
Assignees
Labels
experience-medium This issue is of medium difficulty, and requires some experience help wanted Extra attention is needed

Comments

@joshlf
Copy link
Member

joshlf commented Oct 2, 2023

Due to a limitation in GitHub's merge queue support, we need to manually list all jobs that are required to pass before a PR in the merge queue can be considered to have succeeded. In order to avoid having to list all jobs manually in the GitHub settings page for this repository, we instead provide the following job which depends on all other jobs:

# Used to signal to branch protections that all other jobs have succeeded.
all-jobs-succeed:
name: All checks succeeded
if: success()
runs-on: ubuntu-latest
needs: [build_test, kani, check_fmt, check_readme, check_msrv, check_versions, generate_cache]
steps:
- name: Mark the job as successful
run: exit 0

This works, but is still error-prone: there's nothing that ensures that this job won't fall out of sync if we add new CI jobs in the future.

Similar to jobs like check_versions, we should write a new CI job that automatically extracts all jobs which should block a PR merging, and make sure that all-jobs-succeed depends on each such job.

Mentoring instructions

The job should support the following behaviors:

  • Enumerate every YAML file under .github
  • For each such file:
    • Parse it as YAML with yq
    • Extract any jobs which run for each PR (at a minimum, it must support this syntax, but you should also confirm that there aren't other ways of indicating to GitHub that an action should be run for each PR)
  • Ensure that all-jobs-succeeded depends on each job found in the previous step
@joshlf joshlf added help wanted Extra attention is needed Hacktoberfest experience-medium This issue is of medium difficulty, and requires some experience labels Oct 2, 2023
@tommy-gilligan
Copy link
Contributor

This issue is in the latest This Week in Rust. I would like to help if that's OK

@joshlf
Copy link
Member Author

joshlf commented Dec 12, 2023

Sounds good, I'll assign it to you, thanks! Let me know ow if you have any questions or run into any issues.

@tommy-gilligan
Copy link
Contributor

I'm happy using yq but AFAICT it is something that will need to be installed and it is not included in CI's apt repository.
Luckily there are many ways to install it outside of apt https://github.com/mikefarah/yq/#install . Do you have a preference?

Alternatively:

  • use more basic ubiquitous tools eg. awk/sed/grep (downside is that these tools do not really 'get' YAML) or
  • use a small Rust tool as part of CI ie. xtask

@tommy-gilligan
Copy link
Contributor

I've gone with using go install for now

tommy-gilligan added a commit to tommy-gilligan/zerocopy that referenced this issue Dec 13, 2023
tommy-gilligan added a commit to tommy-gilligan/zerocopy that referenced this issue Dec 13, 2023
tommy-gilligan added a commit to tommy-gilligan/zerocopy that referenced this issue Dec 13, 2023
github-merge-queue bot pushed a commit that referenced this issue Dec 13, 2023
* [ci] check all-jobs-succeed depends on all jobs

Closes #444

* [ci] Print message when check-job-dependencies fails

This also changes the logic somewhat (hopefully in a way that improves
readability).  There was a bit of a self-check in place before that has been
removed because it made the code hard to explain (too many double negatives).

* [ci] Improve message for check-job-dependencies

* [ci] Use less obscure line filter

* [ci] Update comment to reflect change in code

* [ci] Be more specific on ignoring exit status
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experience-medium This issue is of medium difficulty, and requires some experience help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants