Run CI on more machines

[mitchellwrosen]

Overview

This PR adds a few more machines to the CI build, fixes an issue with the ~/.stack-work caching logic (by including the branch name in the cache key), and adds some documentation to the ci.yaml file so that others may find it easier to tweak in the future.

[pchiusano]

Thanks! Can you just comment out the macOS-11.0 one for now and maybe the 16.04 ubuntu? And file a ticket to fix the build for macOS 11.0

[mitchellwrosen]

@pchiusano Removed ubuntu-16.04 from CI (and marked this PR as ready - it was just a WIP before)

About fixing the build on macOS 11.10, I'm not sure it's a unison issue at the moment. The error was weird ("Cocoa not found"), and it might just be that stack or perhaps ghc don't work on Big Sur yet. 😬

[mitchellwrosen]

Hmm, now the macOS build's failing, so something's gone wrong with the config!

[mitchellwrosen]

Another thing I noticed: it's building every push and every pull request. Perhaps we only need to build every pull request?

[pchiusano]

Build on every push... same as Travis used to do. It's handy for looking at a PR and seeing what commits passed CI without needing to like bisect.

[mitchellwrosen]

Build on every push... same as Travis used to do. It's handy for looking at a PR and seeing what commits passed CI without needing to like bisect.

Ok. Do you think we should also build on pull request? (As far as I can tell, it just ends up building each commit in a PR twice with both "push" and "pull_request" set as triggers).

[pchiusano]

I mean, you want the CI to run on the pushed branch and what will be the merged branch. So if it's doing that, then that's what we want.

If it's duplicating work to do that and have the build on every commit, I guess that's not great but I'd probably still want to keep the builds on each commit.

[mitchellwrosen]

I mean, you want the CI to run on the pushed branch and what will be the merged branch. So if it's doing that, then that's what we want.

Hmm... I'm not sure I understand the distinction here. Which is which?

From the docs, the "pull request" events (by default) occur when a pull request is opened, synchronized, or reopened. Synchronized means the source branch of the pull request was updated - that explains the double-builds I was seeing earlier (one for push, one for pull_request).

If it's duplicating work to do that and have the build on every commit, I guess that's not great but I'd probably still want to keep the builds on each commit.

Ah! I'm not actually sure there's an option to build every single commit in CI. I believe if you push multiple commits up, only the latest would be built.

[pchiusano]

Hmm... I'm not sure I understand the distinction here. Which is which?

Like if I have a PR proposing merging topic/blah to trunk, there should be a CI of the latest commit of topic/blah and one for the codebase that will result from merging topic/blah into trunk, which will be the new trunk. They are different codebases possibly so CI should be run on each. Travis did this by default, I think the former was called 'push' and the latter... 'pr'?

Ah! I'm not actually sure there's an option to build every single commit in CI. I believe if you push multiple commits up, only the latest would be built.

Right, that's totally fine (and that's what Travis would do too).

[mitchellwrosen]

Like if I have a PR proposing merging topic/blah to trunk, there should be a CI of the latest commit of topic/blah and one for the codebase that will result from merging topic/blah into trunk, which will be the new trunk. They are different codebases possibly so CI should be run on each. Travis did this by default, I think the former was called 'push' and the latter... 'pr'?

Ohh, I understand what you're saying now. I wasn't aware that Travis CI did this automatically! At work we use bors to merge all patches into dev, which works as follows:

• CI runs on pushed/PR'd code (hopefully relatively up-to-date with trunk)
• When it's passing and cleanly mergeable, we summon bors to make a merge commit (usuallly combining multiple PRs), re-run the tests on that, and if it passes, push to trunk.

I just briefly looked at the Mergify docs, which I hadn't seen before. It looks like you can make it reject PRs that are not strictly ahead of trunk, so that merge commits that create a yet-untested version of the code cannot be merged into trunk. This would require contributors to stay ahead of trunk to get their code merged, of course.

I'm not aware of how to accomplish this using only GitHub Actions, but I'm also not an expert or anything :)

[mitchellwrosen]

Oh, it looks to me like pull requests actually do create a merge commit. I wasn't aware of that. So, I think you're right that we do want to enable both in CI.

[mitchellwrosen]

@pchiusano Oops, one change I accidentally made when pulling this ci.yaml over from another project of mine is the addition of --fast (i.e. -O0) to stack build. The old TravisCI was just running stack build (so -O1). Should I remove --fast?

[aryairani]

I don't think we care about the -O level here unless you think it could affect the runtime behavior apart from speed. --fast seems good to me.

[aryairani]

Thanks for getting this lined up, @mitchellwrosen!

[aryairani]

LGTM when you're ready.

[mitchellwrosen]

I can't merge because the build (ubuntu-latest) check says "Expected - Waiting for status to be reported". Might be a bit of a bug because that CI check has been deleted in this branch. Can you override that?