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

ci: add NVC jobs #917

Merged
merged 4 commits into from
Mar 9, 2023
Merged

ci: add NVC jobs #917

merged 4 commits into from
Mar 9, 2023

Conversation

umarcor
Copy link
Member

@umarcor umarcor commented Mar 8, 2023

This PR adds NVC jobs to CI.

Yesterday, I added a container to hdl.github.io/containers, which builds the master branch of NVC (see hdl/containers@367faa2).

In this PR, the Images workflow is modified to build container image ghcr.io/vunit/dev/nvc based on gcr.io/hdl-containers/nvc. Then, in the Push workflow, a two job matrix is added to run acceptance and vcomponents tests using ghcr.io/vunit/dev/nvc.

By the way, two tests were not being executed by NVC because they were marked to be run with GHDL only. It seems that NVC can handle one of them (JSON in generics). Hence, I updated the xfail mark condition. The other one is related to PSL: https://github.com/VUnit/vunit/blob/master/examples/vhdl/array_axis_vcs/src/fifo.vhd#L38-L48. Maybe NVC can handle enough of PSL through some switch/option?

@LarsAsplund it seems that the image pushed by github actions was set to private by default (https://github.com/orgs/VUnit/packages). That's making CI fail when trying to pull with the default token (funny, since that was used for pushing it XD). Can you check whether you can change it to public? Maybe somewhere in https://github.com/VUnit/vunit/pkgs/container/dev%2Fnvc? BTW, I think we can remove https://github.com/VUnit/vunit/pkgs/container/vunit%2Fdev.

@nickg at first I used the same dependencies for building and runtime. However, in order to reduce the size of the container, I try reduce runtime dependencies. This is the list I came with through trial and error, since I could not find an explicit list: https://github.com/hdl/containers/blob/main/debian-bullseye/nvc/HDLC#L49-L54. As you can see in https://gcr.io/hdl-containers/nvc, I could reduce from 324 MB to 160 MB. For reference, the equivalent container with GHDL LLVM takes 148 MB (https://hdl.github.io/containers/ToolsAndImages.html). The pkg/ghdl is 10MB and pkg/nvc is 18MB, so that's where the difference is coming from. I think it is good enough, but please let me know if you would add or remove any.

@eine eine added this to the v4.7.0 milestone Mar 8, 2023
@nickg
Copy link
Contributor

nickg commented Mar 8, 2023

Shouldn't need clang as a runtime dependeny (ld from binutils is enough on Linux) and don't need libdwarf at all (we use libdw instead on Linux which you already have). But you do need the llvm runtime libs, maybe that's what clang is bringing in as a dependency?

@umarcor
Copy link
Member Author

umarcor commented Mar 8, 2023

@nickg thanks! hdl/containers@fd54c4e reduced the size of the container image to 100 MB, and all VUnit tests do pass!

@LarsAsplund
Copy link
Collaborator

@umarcor I made the package public and yes, I think you can remove that old package.

@umarcor umarcor marked this pull request as ready for review March 9, 2023 01:59
@umarcor
Copy link
Member Author

umarcor commented Mar 9, 2023

@LarsAsplund, thanks! Making the package public let CI run. This PR is now ready for merge!

With regard to the old package, I don't have permissions to do that. Nonetheless, it's not important.

@LarsAsplund LarsAsplund merged commit 1bc4d06 into master Mar 9, 2023
@eine eine deleted the nvc branch March 9, 2023 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants