Upgrade the build process via GitHub Actions

[rpsene]

This commit upgrades the build process by replacing the previous approach with a new one, using a single GitHub Action.

The updated build process is generating the same results and artifacts as the previous one but using a simpler approach, which is easier to maintain.

[jrtc27]

Has something changed to mean that GitHub actions run in a pull request have permission to comment and edit those comments? That’s why the split exists.

[Alasdair]

I don't think so, if you want actions to comment on a PR you still need two workflows.

[jrtc27]

As expected:

00:47:35 +0000 - publish - INFO - This action is running on a pull_request event for a fork repository. Pull request comments and check runs cannot be created, so disabling these features. To fully run the action on fork repository pull requests, see https://github.com/EnricoMi/publish-unit-test-result-action/blob/v2.16.1/README.md#support-fork-repositories-and-dependabot-branches

[Alasdair]

I wonder if there is a better alternative to manually using github-script these days though?

I looked at the error bill reported for the action earlier, and I think it may just be a temporary network failure. Sometimes github actions just stops working due to issues on github's end.

[jrtc27]

I don't know what error you're referring to, but if GitHub's network goes kaput it doesn't matter what programming language you're using. And besides, GitHub Actions are written in JavaScript, the github-script step is just a way to write that JavaScript directly rather than making a separate plugin with it that you then call into. AFAIK you get the same API.

[Alasdair]

The two parts of my previous message were not intended to be related in any way, sorry for the confusion.

[Alasdair]

The error that was reported was about publish-tests action failing with a 403 error when fetching the artifact zip file on forks of this repository. After looking some more I think the issue actually occurs when the master branch (on the fork) has not been kept updated - the PR/push action runs on a recent feature branch, then the download/publish results action runs using the master version. If the Node versions don't match between the two it fails.

[rpsene]

@jrtc27 why you closed this? We can add a step to comment on PRs.

[jrtc27]

Because if you're adding another action back you just end up where we already are?

[rpsene]

@jrtc27 this is an upgrade to simplify the maintenance of this Actions. We do not need 2 files to handle the simple build process and results publication.

[jrtc27]

They have different triggers, so the best fit for GitHub Actions's model is to put each action in its own file. Otherwise you trigger both sets of jobs for each trigger, and have one skipped each time, which is pretty ugly. Unless there's some other alternative you're proposing, I don't see how you can do it in one file.

[rpsene]

You can easily filter when to run an action... that's not a big issue.

[jrtc27]

If you have a proposal you're welcome to push to the reopened PR. I closed it because what you had was broken by design and I do not see how there's a good alternative to the current approach, which is what the linked documentation above describes as the way to do it, albeit with a different implementation of the separate action (maybe it changed since, or maybe I got it from somewhere else).

[rpsene]

I closed it because what you had was broken by design

it was not, I was expecting discussion not you to close it. Anyways, I will check the desired behavior and adjust what is missing...

[Alasdair]

From my understanding, the issue with combining both actions into a single action is right now, when you create a PR on our github the actions will post a comment with the test results like https://github.com/riscv/sail-riscv/pull/491#issuecomment-2135784399 which is a nice feature to have.

If you combine the actions into a single action this does not work. The pull request action that runs the tests is run on the external repository where the PR lives before being merged. The action that checks the results and publishes the results runs on this repository and is triggered when it detects a PR workflow finishing, and it has to do so for permission reasons in order to post the comment. I don't know if there is a good way to avoid having two workflows while keeping this feature.

[rpsene]

@Alasdair we just need to add

jobs: build: if: github.repository == 'sail/sail-riscv'

to ensure it does not run on forks, when a PR is raised.

[jrtc27]

Well then you don't get the comment on the PR for the common case of people filing PRs from forks, which is a worse UX, or entire build job even being run?

[rpsene]

You get the comment on the PR that is raised at sail/sail-riscv ...

[jrtc27]

You get the comment on the PR that is raised at sail/sail-riscv ...

Right, that's the repo the PR is targeting. But that won't magically give it permissions that it didn't before, and that's a feature, because otherwise the PR could change the workflow file to do something else. By having it be the file that's in the repo's own master branch it's under the control of the repo owner, not the PR submitter.