factor out common workflows #332
Conversation
|
Have now also tested |
.github/workflows/sel4test-sim.yml
Outdated
| # PR. Ok on the workflow level here, because this workflow does not re-trigger | ||
| # on "labeled" events. | ||
| concurrency: | ||
| group: ${{ github.workflow }}-sel4sim-pr-${{ github.event.number }} |
There was a problem hiding this comment.
Debugging gives me seL4Test-sel4sim-pr- here when a never job cancels an old run. Debugging says
"event_name": "pull_request",
"event": {
"action": "synchronize",
...
"number": 140,
...
so I'm a but puzzled why the PR number is missing there.
There was a problem hiding this comment.
Seems the whole github.event does not exist a this stage.
There was a problem hiding this comment.
In which scenario is that happening? I.e. are you seeing this in the old job that is being cancelled or in the new job that is the reason for cancellation?
There was a problem hiding this comment.
I'm seeing this in the old job, where github gives the cancellation reason. This is the only place where the group name is ever printed. It seems impossible to see the concurrency group for a running job anywhere. You can print it in a job/step, but the context there seems slightly different, because this runs on the runner. The workflow control and concurrency seems to run on an "action server", which does not see some dynamic parts of the contexts. The github documentation is quite thin here, but that is what I found in some github community postings.
There was a problem hiding this comment.
Ok, the docs do say that github, inputs, and vars are available at the workflow level, and additionally needs, strategy, and matrix on the job level. So I think we are fine with this expression, it's just hard to debug as usual on GitHub.
There was a problem hiding this comment.
Let's see if I get some answers here: https://github.com/orgs/community/discussions/107552
There was a problem hiding this comment.
I'm going to put it into the job level for now, so that we can merge this. Would still be good to figure out what exactly is available, though.
There was a problem hiding this comment.
The other option could be adding another callable workflow layer here. The the job level concurrency group there will cover everything. It could also filter the events and invoke this workflow with a simple bool parameter for enabling hw-test.
These can now be called from other workflows, reducing duplication. Signed-off-by: Gerwin Klein <[email protected]>
|
I think this is ready to merge now. We can move the concurrency group for |
- use job concurrency instead of workflow concurrency, because on the workflow level a label trigger event will already call all current runs, whereas the job level is guarded by `if` conditions. - disambiguate concurrency group names, because the workflow name is inherited from the caller. Signed-off-by: Gerwin Klein <[email protected]>
The workflow is intended to only run for pull_request_target. Signed-off-by: Gerwin Klein <[email protected]>
This makes sure all build jobs run on the same repo manifest state. Signed-off-by: Gerwin Klein <[email protected]>
Signed-off-by: Gerwin Klein <[email protected]>
Signed-off-by: Gerwin Klein <[email protected]>
Signed-off-by: Gerwin Klein <[email protected]>
Signed-off-by: Gerwin Klein <[email protected]>
Add the strategy.job-index back into the concurrency key, now that these are back at the job level. Co-authored-by: Axel Heider <[email protected]> Signed-off-by: Gerwin Klein <[email protected]>
It looks like ${{ github.event.number }} might not exist yet at the
workflow level when the workflow is called, so we move it to the job
level where we know that this info exists.
See also <https://github.com/orgs/community/discussions/107552>
Signed-off-by: Gerwin Klein <[email protected]>
Since multiple workflows can now be called from the same caller workflow, these will all end up in the same UI, so we should make sure the artifact names are not ambiguous. Signed-off-by: Gerwin Klein <[email protected]>
With reusable GitHub workflows we can now factor out common workflow files and maintain them only once here in the
ci-actionsrepo.Should also make it easier to deploy additional PR label actions to many/all of the seL4 org repos.
Unfortunately they have to live in the somewhat hidden
.github/workflowsdirectory, which I find a bit awkward, but it's still better than having to replicate everything.I have so far only tested the workflows that don't need secrets and trigger on
pull_request_target, but those work. The display name changes, which mean we will have to update the names of required checks in all repos, but the benefit shoudl be worth the one-time effort.