search

chipsalliance / verible-linter-action

Automatic SystemVerilog linting in github actions with the help of Verible
Apache License 2.0
24 stars 11 forks source link

Convert to Container Action #4

Closed umarcor closed 3 years ago

umarcor commented 3 years ago

Close #3

Unfortunately, when the image is built automatically as part of a Container Action, it seems that global environment variables are not inherited. Therefore, enabling BuildKit globally through DOCKER_BUILDKIT does not work. That would be desirable in order to use https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/syntax.md#build-mounts-run---mount in the dockerfile. For now, I worked around it by using a COPY statement (a layer) instead. Nonetheless, I kept the commit isolated, because it might be reverted when building the image is decoupled from the execution.

I added a very simple CI workflow for testing the execution of the Action. Yet, since there is no "demo" project here, execution is rather useless. It would be interesting if someone familiar with Verible would contribute an example project in a follow-up PR.

NOTE: Since GitHub Actions is not enabled in this repository yet, results are not shown below. See https://github.com/umarcor/verible-linter-action/actions

tgorochowik commented 3 years ago

Nice, thanks!

How often is this container built? Is it built each time the action is used, or when is the image pushed to the registry you mentioned earlier?

As for the actions - they are enabled for this repo, I am not sure why it didn't start here

umarcor commented 3 years ago

How often is this container built? Is it built each time the action is used, or when is the image pushed to the registry you mentioned earlier?

Yes, the container is built each time the action is used. I didn't change that in this PR yet. Here, I just moved the installation steps from the composite action into a Dockerfile. Apart from that, there is no difference.

Pushing the image to the registry and consuming it in the action should be done in an upcoming PR, to avoid making this too complex to review/follow.

As for the actions - they are enabled for this repo, I am not sure why it didn't start here

Maybe it's because there is no workflow in the main branch yet, so they are enabled for pushes but not for PRs. That might be configurable in the settings of this repo, but I'm unsure because those settings are being changed/improved lately.

tgorochowik commented 3 years ago

@umarcor would you be willing to continue with the topics you suggested?

It would also be quite nice if we could keep the approach consistent here and in the other verible formatter action: https://github.com/chipsalliance/verible-formatter-action

umarcor commented 3 years ago

@tgorochowik, see #6.

Unfortunately, I cannot take care of the Actions related to Verible. There are some other CI issues I do still need to sort out (chipsalliance/verible#622), and then I'd like to improve the integration with hdl/containers (verible is not available there yet). That's as much I might handle. Actually, I hesitated to implement #4 and #6 myself, but I thought it'd be worth doing it because those can be used as a reference by others. Nevertheless, I can review similar implementations in other repos, in order to help whoever get confident with the procedure.