zmk icon indicating copy to clipboard operation
zmk copied to clipboard

Build CI fixes/improvements

Open lesshonor opened this issue 10 months ago • 1 comments

All push event runs of the Build action since 2024-01-25 have failed in part[^1] 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 firmware[^2].~~ 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.

[^1]: It might be "in total"; I haven't checked every run. [^2]: #2210, discord, discord

lesshonor avatar Apr 07 '24 23:04 lesshonor

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

caksoylar avatar Apr 08 '24 00:04 caksoylar