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

Build CI fixes/improvements #2262

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

Conversation

lesshonor
Copy link
Contributor

@lesshonor lesshonor commented Apr 7, 2024

All push event runs of the Build action since 2024-01-25 have failed in part1 because actions/checkout is not being run prior to tj-actions/changed-files.

This primarily presents a problem for novice end-users looking for settings_reset firmware as linked from the documentation. The outdated binaries appear to be entirely ineffective on Zephyr 3.5 firmware2. All artifacts containing the outdated firmware have now expired.

Remaining commits are general improvements to security and efficiency:

concurrency

Group builds by workflow and github ref.

The fully-formed ref of the branch or tag that triggered the workflow run. For workflows triggered by push, this is the branch or tag ref that was pushed. For workflows triggered by pull_request, this is the pull request merge branch.

ref_name is another possibility, but could collide with any eventual tags.

These groupings allow us to...

cancel-in-progress

...cancel workflow runs which are subsequently obsoleted by later pushes.

In an ideal world, anyway. Unfortunately, the way ZMK currently handles the final Build and Upload Artifact steps means the cancellation of those particular steps is not immediate. If the five-minute post-cancellation timeout passes, something will error out. If not: the job will grind on to completion and display as "Cancelled".

This is something worth examining as part of a re-evaluation of CI generally, but it's out of scope for now. Any kind of "stop doing unnecessary work" constitutes improvement.

permissions

Given what this workflow currently does, its GITHUB_TOKEN doesn't need anything more than read-only access to metadata.

persist-credentials

It also doesn't need to keep the zmkfirmware credentials handy.

etc

  • If the compile-matrix job has no inputs because the previous jobs were canceled, might as well cancel it.
  • I don't see why forks should run nightly builds.

Suggestions welcome. I have been testing these changes in my own fork.

Footnotes

  1. It might be "in total"; I haven't checked every run.

  2. [SOLVED] Right side of Corne suddenly dead. #2210, discord, discord

@lesshonor lesshonor requested a review from a team as a code owner April 7, 2024 23:19
@caksoylar
Copy link
Contributor

Might be worth pointing out this is a fix, maybe it will get higher priority during review?

@lesshonor lesshonor changed the title Build CI improvements Build CI fixes/improvements Apr 8, 2024
* Limit unnecessary permissions.

* Avoid storing credentials.
* Cancel redundant non-nightly runs and jobs dependent on canceled jobs.

* Limit scheduled runs to zmkfirmware-owned repos.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants