Move to ghactions

[tjurtsch]

This submodule implements generic scripts that will be used by CI repos (compilers, misc, prog, eda).

This PR replaced another PR from remote branch #12

[ajelinski]

LGTM now.

[umarcor]

I'd say I saw some intermediate state of this PR, where the README was "properly" updated. However, some modifications seem gone, and not present after the merge. For example:

Travis scripts for Litex-Hub/litex-conda-(compilers|eda|misc|prog).

was

Scripts for Litex-Hub/litex-conda-(compilers|eda|misc|prog) Continuous Integration.

in https://github.com/litex-hub/litex-conda-ci/pull/13/files/ce25d236f590d242de5cb62ae9b1f26c5613f40e#diff-63aa9fac641138cdd078a654078ea278bdf7b39c88d4fc0843304351d6bb3272.

---

Overall, the addition of action.yml without further clarification in the README feels very weird. action.yml defines a "Composite Action" to be used in GitHub Actions Workflows through uses: litex-hub/litex-conda-ci@master. On the other hand, the README says that this repo is to be added as a submodule. Moreover, it seems that the composite action does depend on the submodule being added in a very specific location: https://github.com/litex-hub/litex-conda-ci/blob/master/action.yml#L15-L17. Therefore, I would recommend to convert action.yml into build.sh. There is no added value by using the YAML syntax in this case. It's an unnecessary layer of complexity which introduces a maintenance burden.

---

I would strongly discourage adding Git for Windows' internal tools to the PATH: https://github.com/litex-hub/litex-conda-ci/blob/master/action.yml#L13. See actions/runner-images#1525. What is the motivation for doing so?

[tjurtsch]

Readme clearly needs to be fixed, thank you for pointing that out.
It's also true that this submodule/actions mixture needs to be cleaned up. @ajelinski suggested that we shall drop usage of a submodule and move to full "action" approach instead. It's still not better than a shell script, but it will be cleaner than the current mixed approach. So the litex-conda-ci repo will be only referenced in the workflow as an action. IMHO the only added value will be: no submodule bumping.

[umarcor]

@tjurtsch I'm not sure that's the best strategy... Let me explain:

Since GitHub Actions were created, there have been two types: "JavaScript/TypeScript" and "Docker".

JavaScript/TypeScript Actions can be executed on any environment (Ubuntu, Windows or macOS), because all environments include nodejs and someone decided that was the "best" language for scripting. However, those have one very obvious problem: the JS package ecosystem is a mess that fights against any optimal use of files. Unless you want all users of the Action to retrieve hundreds of dependencies, you need to "compile" them. At the same time, GitHub Actions does not support semver filters when specifying Actions, and it can neither use releases. All "releases" need to be a branch in the repo. As a result, you are forced to setting up some CI for handling the releases of your JS Actions, you cannot just tag and push. Alternatively, you need developers to "compile" locally before pushing (so they need nodejs on their workstations/laptops).

Docker Actions are nicer because you can wrap any scripting language (say bash or python) in a container, and you can avoid using a browser language for CI. Unfortunately, Docker Actions are only supported on Ubuntu (neither Windows nor macOS), which is not acceptable in many projects. Moreover, the docker images are fixed in Docker Actions. You cannot let the user pass the image as an argument.

The interesting feature of Docker Actions is that you can call existing containers using that syntax. See, for instance: https://github.com/tmeissner/formal_hw_verification/blob/master/.github/workflows/Test.yml#L41-L45. However, you can only call containers which existed before the job was started. That is, you cannot build a container image in one step and use it in the next one using this syntax. You need to use the regular docker run ... image_name ... instead.

Obviously, users complained a lot about the hard to understand limitations, and they recently came up with Composite Actions (the type you used). Those are nice... except they don't allow to compose anything. You cannot call other Actions (use uses:) in a Composite Action. Therefore, it's pretty useless, as you can only wrap scripts with it. The only advantage of this type I could find was, precisely, allowing users to customise the container image used in what was earlier a Docker Action: https://github.com/VUnit/vunit_action/blob/master/action.yml.

As a result, I think you should first evaluate whether you'll be able to describe the logic that you currently use in your scripts, considering the constraints above. If you are already following the submodule approach and that works, maybe you should stick with that. I would suggest the following: rework the existing Composite Action for installing this repo in CI and maybe call it setup-litex-ci. There, just copy the resources of this repo to some known location, or to an arbitrary location and set an envvar. At the same time, remove the submodules and use this Action as the first step in CI. However, use the scripts explicitly in all other steps of CI. That will allow you to extend this repo for providing different entrypoints, without a maintenance burden.

[ajelinski]

Wow, this is quite an extensive explanation.

TBH realizing how many limitations this Composite Action has was very disappointing for me too. It's hard to understand why other Actions can't be called.

However, in this case I see the biggest value of using such an action in dumping the submodule usage. Bumping those is pretty annoying if you have to do it in four repositories. Using its branches to test any changes also isn't the best experience.

Because litex-hub-ci isn't a third-party Action we could use its master branch in the workflows. The LiteX-Hub maintainers decide when it changes so there's no security risk. Maintainability is easier and when there's a need to test some litex-hub-ci branch we can easily add an option to the workflows to start the build manually with an input specifying what revision of litex-hub-ci is to be used.

You're right that current situation needs to be fixed, because this Action shouldn't use scripts from the repository it runs in. At least there should be $GITHUB_ACTION_PATH instead of those paths for it to make sense. I thought that after doing so the .github/scripts submodule could be removed from all litex-conda-* repositories but I've just realized we have two entrypoints indeed. One for building a package and the other for moving the packages to main label if all builds succeeded.

I must say that your proposal with an action simply installing all scripts definitely makes sense and would also enhance the experience of the litex-conda-ci usage. However, calling scripts directly isn't an option because there are simply too many jobs in litex-conda-* workflow to add more steps. The only option would be to convert the existing action.yml to a script and call it although it also adds at least one step to each job – use such setup-litex-ci action and then call the build script.

Maybe there could be two actions in this repository:

• build-action
• master-action

It seems to make sense as both actions share some scripts. The problem of accessing two entrypoints without copying the scripts would be solved.

What do you think about such an idea?

[umarcor]

@ajelinski, just to make it clear: you are already doing a nice job and all your ideas are sensible. I've been using GitHub Actions workflows for 18 moths, and I currently maintain JS, Docker and Composite actions. I want to help with this background/know-how I have. However, I help handle CI in several orgs, and am not familiar with all the details of the CI infrastructure in SymbiFlow/litex-hub/antmicro (yet). I might be missing relevant constraints, such as what you said about having too many jobs. Therefore, we need to help each other. Can you please provide a reference of the four repositories you mentioned, so I can have a look and get a better picture? I can guess which repos you mean, but I prefer to be given a specific scope to focus on. Maybe we can rethink some of those steps, in coherence with reshaping this repo.

[ajelinski]

Sorry for the delayed answer @umarcor. Everything is clear, I hope your help will make litex-conda-* CIs even better.

The four repositories that I use litex-conda-* name for are:

• https://github.com/litex-hub/litex-conda-compilers
• https://github.com/litex-hub/litex-conda-eda
• https://github.com/litex-hub/litex-conda-misc
• https://github.com/litex-hub/litex-conda-prog

They all use litex-conda-ci scripts in their CIs. There's indeed the inconsistency that the action litex-hub/litex-conda-ci@master is called and it uses scripts from the calling repository's submodule. Fortunately it doesn't give any negative effects for the package building right now but we'll definitely need to fix it.

BTW. Using separate jobs in the workflows to build each package instead of using a job with a matrix gives us possibility to properly define intra-repository package dependencies. Unfortunately it results in duplicating a similar job multiple times but it seems inevitable if we want to use needs in such a way. GitHub Actions disallowing usage of the yaml anchors also doesn't help.