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

Implement GitHub Actions based CI. #118

Merged
merged 1 commit into from
Oct 9, 2020

Conversation

jchv
Copy link
Contributor

@jchv jchv commented Oct 8, 2020

Summary

This pull request implements a continuous build process for building Higan.

Features

  • Implements native builds for Ubuntu, Windows and macOS.
  • Builds Higan piece-wise by running a matrix over the programs (higan-ui, byuu, icarus) and the OS (for a total of 9 jobs.)
  • Produces a "nightly" build release that is continuously updated with the latest binaries. This gives a consistent URL to get the latest nightly build. See, for example, on my fork: https://github.com/jchw-forks/higan/releases/tag/nightly
  • Produces a version release on each tag starting with v. For example, see https://github.com/jchw-forks/higan/releases/tag/v116. (We could make this a draft so release notes can be edited and a release could be manually published, too, but right now it just publishes the release immediately.)

Design

The Matrix

The build matrix has two dimensions: the programs (higan-ui, byuu, icarus) and the OS (ubuntu, windows, and macos.) The builder always passes build=performance and local=false to the make invocation. Unlike Cirrus CI, we do not set the profile value at all, since it defaults to what is expected for each UI anyways.

local=false is needed because some workers arbitrarily have a newer Xeon that supports AVX512, which has two undesirable side-effects:

  • Versions of GCC earlier than 8.4 fail due to an issue with handling the XMM16-31 registers. The builders have GCC 8.1.
  • Binaries produced by these workers will not run on machines without AVX512, which is a significant portion of users.

In any case, this was already set by the Cirrus CI runner, and is idiomatically the correct thing to do, but those caveats are especially worth mentioning here.

Packaging

GitHub's artifacts mechanism is a little tricky at first. The first interesting behavior is that within a single workflow, artifacts with the same name are merged into one unit. The second interesting behavior is that we can download all artifacts at once, and each will appear in a subdirectory with the name of the artifact in the path we request (it defaults to $GITHUB_WORKSPACE which is, coincidentally, also the default $PWD.)

Packaging varies between higan-ui and byuu, but it doesn't actually vary by much between platforms. In fact, here the packaging code is actually identical across platforms, and so we just loop over each platform and do the same exact thing.

Release Step

GitHub has some helper actions, written using their platform-agnostic JavaScript action runner, that can perform certain tasks for you, like create a new release. Unfortunately, we need to do some things that do not fit well within the capabilities available from existing actions. (I will detail exactly what those are in a moment.) In interest of broader maintainability, my initial code which did work in TypeScript has been ported to shell script and subsequently merged into the workflow itself.

Much of what is needed to make GitHub REST API calls across platforms is pretty consistent across different endpoints, so we define a function github_rest which takes an HTTP method (sometimes known as a verb) and a path, and then forwards remaining arguments to curl. Authentication and API versioning are handled within github_rest to reduce duplication. We use cURL's --fail mode to make server errors return errors, which, using errquit, should propagate up to a build failure.

Helper functions do actual GitHub API calls, like github_get_release_id_for_tag. We parse JSON using jq which is preinstalled on GitHub Action's Ubuntu runner. Since pipelines effectively bypass errquit, we capture the output into a variable and run jq separately; it might be possible to do this in a less awful fashion. Some other errquit workaround shenanigans appear in the code as well; when in doubt, assume the weird thing I'm doing is working around subtle shell behavior.

Finally, the release step passes the upload URL for artifacts into UPLOAD_URL using the GitHub Runner environment file mechanism, and this can then be used by subsequent stages to upload the artifacts.

Nightly Release

GitHub Actions artifacts are useful, but they come with caveats.

  • They have a retention of 6 months. (This is not actually bad, but it's worth noting.)
  • They are inaccessible by users who are not authenticated with GitHub.
  • There is no consistent URL to get the latest build for a branch.

We would ideally like to have a static URL for the latest build, and also, it would be ideal if said builds were available even to people without GitHub accounts. Therefore, it is ideal to have a 'rolling' nightly release that is updated on every push to master.

To facilitate this, the bash code aforementioned deletes and recreates a release with the nightly tag on every build, and then new build artifacts are attached to that.

This does have the downside of being very noisy for users who are trying to watch the repository using the "Releases Only" option, although it has little effect for ordinary watchers who would see pull request merges anyways.

Performance

Compared to Cirrus CI, builds are faster. There is also slightly more parallelism, as Icarus is compiled in a separate VM, although this is more a move to simplify the code than it is to improve performance.

Since GitHub Actions limits concurrency to 8 simultaneous jobs, one job will start stalled, but it will only take a minute or two for it to pick up. Typical runtimes look like around 17 minutes from start to finish. macOS jobs are the fastest, although also seem to be fairly variable.

Comparing to Cirrus CI

This setup has some disadvantages to the current Cirrus CI setup:

  • We lack a path to support FreeBSD, at least easily. Cross-compilation is a potential avenue, but people seem skeptical about whether or not a FreeBSD CI is desirable without byuu around. I would like to support it just to ensure the code stays FreeBSD compatible, since there is little sense in not.
  • Only the latest nightly build is kept. There are build artifacts for the remaining builds, but they are not nicely packaged binaries like the release downloads, and they require authentication. That said, it should be possible to reconcile some of this, with a little bit uglier code.
  • Currently the release is all-or-nothing, whereas if some targets failed on Cirrus CI it would be easy to grab the latest build for at least the successful jobs.

It has some advantages as well:

  • It performs better; almost twice as fast seemingly.
  • It is built-in on GitHub, and therefore governance is somewhat simplified and a bit more community-controlled.
  • It offers some better integration with GitHub.

Above all, the primary advantage that seems to make it appealing is just the simplicity. Cirrus CI is fine, but it is another 'thing.' If it can be dropped, it is one less 'thing' to worry about.

Currently, I believe we should continue running both CIs simultaneously for a period of time.

@Screwtapello
Copy link
Contributor

Random notes as I look at the built binaries:

  • byuu-ubuntu.zip doesn't have +x on the executable, weirdly
  • the default bindings shipped with byuu don't work anymore, but that's not your fault - filed Fix default bindings for byuu UI #119
  • The higan and icarus executables aren't executable either

Apart from that, the resulting Ubuntu artefacts look good to me.

@Screwtapello
Copy link
Contributor

Produces a version release on each tag starting with v. For example, see https://github.com/jchw-forks/higan/releases/tag/v116.

The example you link to only has higan binaries, not byuu binaries, but the nightly has both?

Copy link
Contributor

@Screwtapello Screwtapello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks pretty straightforward, even the status-code stuff.

I definitely want the executable bit set on Ubuntu and macOS binaries; nightlies are a whole lot less useful without it. The other things aren't a big deal - if they're difficult or tedious, don't worry about 'em.

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
@jchv
Copy link
Contributor Author

jchv commented Oct 8, 2020

Produces a version release on each tag starting with v. For example, see https://github.com/jchw-forks/higan/releases/tag/v116.

The example you link to only has higan binaries, not byuu binaries, but the nightly has both?

Ah, that's because it was produced with an earlier version of the script. I can go remake that tag to produce a newer build, but the build process is the same for nightlies, so it doesn't make a huge difference.

@Screwtapello
Copy link
Contributor

I can go remake that tag to produce a newer build...

That's OK, as long as there's an explanation for why the tag is different from the nightly.

@Screwtapello
Copy link
Contributor

It's 3AM and I may have missed something — but that last force-push fixes everything besides the "linux binaries lack the executable bit" thing, right?

@jchv
Copy link
Contributor Author

jchv commented Oct 8, 2020

It's 3AM and I may have missed something — but that last force-push fixes everything besides the "linux binaries lack the executable bit" thing, right?

Sorry, I went AFK (and I'm now at work) but, the last thing I did added a small hack to try to restore the executable bit for the ubuntu binaries. So if that didn't work, then maybe something is wrong with my zip invocation.

@jchv
Copy link
Contributor Author

jchv commented Oct 8, 2020

OK, just took a quick look.

image

This should be good, although macOS may need a similar hack. Frustratingly, GitHub's own docs recommend just using a tarball artifact for passing data instead. I'll have to think more about this problem a bit more later on when I get back.

@jchv
Copy link
Contributor Author

jchv commented Oct 9, 2020

I extended the permissions hack to macOS binaries as well. Probably could be cleaner but this should be OK for now.

@Screwtapello Screwtapello merged commit 4bb6555 into higan-emu:master Oct 9, 2020
@Screwtapello
Copy link
Contributor

Thanks for working on this! It'll be good to have GitHub releases for everything from now on.

jchv added a commit to jchw-forks/bsnes that referenced this pull request Oct 18, 2020
This is lightly adapted from the implementation in Higan. For more
information, see that PR in higan-emu/higan#118.

The main difference is that we only compile one binary here, and
packaging is similar to (but adapted from) packaging byuu.
jchv added a commit to jchw-forks/bsnes that referenced this pull request Oct 18, 2020
This is lightly adapted from the implementation in Higan. For more
information, see that PR in higan-emu/higan#118.

The main difference is that we only compile one binary here, and
packaging is similar to (but adapted from) packaging byuu.
jchv added a commit to jchw-forks/bsnes that referenced this pull request Oct 18, 2020
This is lightly adapted from the implementation in Higan. For more
information, see that PR in higan-emu/higan#118.

The main difference is that we only compile one binary here, and
packaging is similar to (but adapted from) packaging byuu.
Screwtapello pushed a commit to bsnes-emu/bsnes that referenced this pull request Oct 19, 2020
This is lightly adapted from the implementation in Higan. For more
information, see that PR in higan-emu/higan#118.

The main difference is that we only compile one binary here, and
packaging is similar to (but adapted from) packaging byuu.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants