Add CI with github actions

[domenkozar]

This builds nix on macos and linux, but avoids running three tests that require a VM.

It currently fails the build so that's a proof it adds value :)

In the future I plan to add installer script as an artifact.

I commit to maintain the CI, but I'd welcome anyone to help me out.

[gilligan]

I’m in favor of merging this. As I laid out in the RFC (linked above) i would love to see something that adds value now instead of delaying further to reach perfection.

This change is small and easy to revert whenever something else or better has been established.

Let’s start having mandatory CI tests now and improve later. This will also mean that no further changes will be merged until the currently broken tests have been fixed. That definitely seems right to me.

What’s not to like?

[edolstra]

This will not work once the flakes branch is merged because flake.nix (which replaces release.nix) does not accept arguments, and builtins.currentSystem does not exist.

[edolstra]

No need to build the coverage job. It's currently broken, takes a long time to build and nobody ever looks at it anyway.

[andir]

This will not work once the flakes branch is merged because flake.nix (which replaces release.nix) does not accept arguments, and builtins.currentSystem does not exist.

Why cant' the release.nix stay for compatibility with non-flake users? Couldn't we just do an import of release.nix in the flake?

[edolstra]

That's what I did originally, but it's much more convenient to have one file rather than a release.nix, shell.nix, release-common.nix, ...

[edolstra]

Let’s start having mandatory CI tests now and improve later. This will also mean that no further changes will be merged until the currently broken tests have been fixed.

I disagree. It means that updating the github actions (e.g. when merging the flakes branch) becomes my responsibility, even though it already works fine in Hydra. That's why I've always objected to having multiple CI systems.

[flokli]

Hydra isn't well-intergrated into GitHub PRs, Github Actions is. I see a lot of benefit from having CI results for each PR immediately visible inside the PR, as a status.

I think it's the right thing, and this also means CI descriptions need to be updated if the underlying assumptions change.

[edolstra]

I agree, but then we should get rid of the Hydra jobset so that we don't have to deal with two CI systems. The main issue with that is the VM tests, but they're not that important (and the nix-copy-closure and remote build tests can probably be rewritten as regular tests if we avoid going through SSH).

[gilligan]

I disagree. It means that updating the github actions (e.g. when merging the flakes branch) becomes my responsibility, even though it already works fine in Hydra. That's why I've always objected to having multiple CI systems.

You pointed out a conflict with the flakes fork and @andi offered a simple way to work around this by sticking to the release.nix file.

I won't disagree that it is desirable and preferable to have one definitive CI system but doesn't it make sense to look at what we have and what we can do right now ? My summary would be:

• A RFC decision to have CI for each Nix PR
• A RFC decision to use @grahamc 's ofborg for Nix
• A Hydra PR plugin which cannot set the GitHub PR status
• ofborg isn't currently ready for this (no darwin builds is one reason)
• Nix with failing tests in master
• This PR which provides CI via GitHub actions building on linux and darwin
• This PR which as-is won't work once the flakes PR is merged
• A suggestion on how the GitHub Actions PR can be made to work with the flakes branch

I understand that you don't want additional burden but to finally get all Nix PRs checked in the first place is something that I would argue removes burden. Furthermore we are talking about a very small test.yaml it's not like additional infrastructure needs to be managed.

A lot of people are very eager to get Nix PRs CI checked and i'm sure would be willing to help out to refactor a small GitHub actions yaml file when that becomes necessary (I don't want to question the results of the RFC process but I would argue this approach should be less maintenance than using ofborg).

Once ofborg is ready, the test.yaml can be dropped but - wouldn't it be nice to already get all PRs tested (and block further changes until the tests are fixed) until we actually get there? IMHO that would be nice indeed ;-)

[gilligan]

PS: One more thought concerning the use of hydra - We could use https://github.com/nlewo/hydra-cli to integrate hydra.

https://github.com/nlewo/hydra-cli/blob/master/.travis.yml

- language: nix
  nix: 2.2.2
  if: type = pull_request
  script: 
    # This is because the unstable channel has not been updated yet
    - nix-env -f https://github.com/NixOS/nixpkgs/archive/1f5fa9a8298ec7411431da981b4f1a79e10f2a8e.tar.gz -i hydra-cli
    - hydra-cli -H https://hydra.nix.corp.cloudwatt.com jobset-wait hydra-cli $TRAVIS_PULL_REQUEST

Note that the above is currently disfunctional because the hydra instance doesn't exist anymore iirc.

[flokli]

@gilligan I think we should move long-term discussions to a separate issue - this PR is about adding the GitHub Action.

@edolstra what about keeping the VM tests in hydra for now, until we have a better alternative, but drop everything from Hydra that's covered by this PR?

We don't currently require a passing Hydra for Nix PRs anyways, but keeping to run the VM test we'd at least still have ways to quickly spot when we broke a test only visible in a VM test.

Edit: With the command propsed from @gilligan we might even be able to expose the hydra status for the VM tests.

[gilligan]

@gilligan I don't think we should move long-term discussions to a separate issue - this PR is about adding the GitHub Action.

Fair enough. Sorry @edolstra didn't want to blow this up but it started to move in a direction of more general concern ✌️

[domenkozar]

About flakes branch: we can build nix and run flakes to do all the checks.

[domenkozar]

Thank you Eelco ❤️

[gilligan]

Thank you indeed!