-
Notifications
You must be signed in to change notification settings - Fork 32
Add Windows, macOS, and Stack Support (#1) #12
Conversation
* Re-base onto upstream master * Use os over operating-system per json schema suggestion * Upgrade dependencies * Update action.yml syntax * Restructure repo on latest typescript-action * Update workflow.yml to use same node version as action.yml * Pull in improvements from open PRs. Fix package.json scripts * Type function return arguments * Don't eslint automatic formatting changes * Initial implementation of ghc+cabal install for linux/mac/windows * Update README * Fix cabal version in action.yml * Implement initial simplistic test suite * Test action with CI * Use chocolatey directly to install ghc and cabal * Try pre-installed tool on linux first * Clean up documentation * Expand README * Test super old GHC on ubuntu * Implement support for stack * Update documentation about Stack support * Test stack install in Github Actions CI authored-by: Jared Weakly <jweakly@galois.com>
|
@jared-w thanks for all the great work here! Let me track down someone working directly on actions every day to help review and answer your other questions about being a maintainer and getting this action in the marketplace (I'm a contributor here just out of needing a haskell build environment 😄).
If you haven't found it already, that will happen in https://github.com/actions/virtual-environments |
This patch adds functionality to * Test cabal build and cabal run in the CI workflow * Test GHC 7.10.3 on Linux and ensures that it works * On Linux, try hvr's PPA before ghcup
|
@jared-w, when you get a chance will you merge latest master into your branch here (and fix workflow.yml conflicts)? I enabled running this repo's tests on pull requests. |
tclem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just gave this a quick review. Again, really appreciate the contributions here @jared-w!
I think the first thing to tackle is to move any installing of haskell tools over to https://github.com/actions/virtual-environments. This is critical for build time performance. This action (setup-haskell) should simply put the right tools on the path. Does that make sense?
README.md
Outdated
| [](https://github.com/actions/setup-haskell) | ||
|
|
||
| This action sets up a Haskell environment for use in actions by: | ||
| This action installs a specified version of GHC and Cabal. It can also install [Stack](https://haskellstack.org). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These setup actions shouldn't install anything (they just switch between versions of already installed tools). Installing to the base vm needs to happen in https://github.com/actions/virtual-environments. Knowing that, i think the language here should still say "sets up" instead of "installs".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does this sound?:
This action sets up a Haskell environment for use in actions by:
- optionally installing a version of ghc and cabal and adding to PATH.
- optionally installing a version of Stack and adding to PATH.
Once actions/virtual-environments#645 is addressed, the bottom part of the intro can read:
The GitHub runners come with some pre-installed versions of GHC and Cabal. Those will be used whenever possible.
For all other versions, this action utilizes ppa:hvr/ghc, ghcup, and chocolatey.
README.md
Outdated
| This action installs a specified version of GHC and Cabal. It can also install [Stack](https://haskellstack.org). | ||
|
|
||
| - optionally installing a version of ghc and cabal and adding to PATH. Note that this action only uses versions of ghc and cabal already installed in the cache. The action will fail if no matching versions are found. | ||
| Currently only Linux comes with [pre-installed versions of GHC and Cabal](https://github.com/actions/virtual-environments/blob/master/images/linux/Ubuntu1804-README.md). Those will be used whenever possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize that the images came with GHC 8 all the way up to 8.8. I think this should be okay, though we'll need to keep track of when we need to upgrade (since 8.10 just landed). Is there a way to force an image to use ghcup?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just gave this a quick review. Again, really appreciate the contributions here @jared-w!
I think the first thing to tackle is to move any installing of haskell tools over to https://github.com/actions/virtual-environments. This is critical for build time performance. This action (
setup-haskell) should simply put the right tools on the path. Does that make sense?
Makes total sense. I've opened actions/virtual-environments#645 to address this. I do know that the setup-node action will fall back and download a full version of node if the version specified doesn't exist, so there's precedent for said fallback behavior.
The intention of most of this code was always to become the fallback behavior and have the images come with GHC across all supported operating systems. Since this supports installing as a fallback, it also means github could drop about 8 of their current GHC installs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize that the images came with GHC 8 all the way up to 8.8. I think this should be okay, though we'll need to keep track of when we need to upgrade (since 8.10 just landed). Is there a way to force an image to use
ghcup?
The action currently doesn't have any code for forcing the usage of ghcup over the PPA, but will fall back if GHC wasn't already installed and trying the PPA didn't work. I'm open to suggestions on how to tweak/change that.
As for the actual virtual environment for the runners, I'm not sure how the upgrade process currently works; I'm assuming you don't want to automatically download the "three latest major releases" when building the images and would rather specify them explicitly?
tclem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opening up actions/runner-images#645, just a couple more comments.
@patrickt do you want to try this out on one of our projects? Should able able to do something like:
uses: actions/setup-haskell@c4e863e92fcd479e2fcec8fe81921baa76000fca
| __tests__/runner/* | ||
| lib/**/* | ||
|
|
||
| dist-newstyle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep .gitignore streamlined to just this project's needs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I pulled in the default gitignore from the typescript-action template, but it's quite large. I'll pair it down.
src/installer.ts
Outdated
| const p = join('/opt', tool, v, 'bin'); | ||
| const installed = await fs | ||
| .access(p) | ||
| .then(() => true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be nice to have a message in the build logs to let you know that the tool was found in the cache and maybe another message if it's not pointing to documentation about which tools are in the vm by default. That documentation might include something like your suggested policy of N versions of ghc, with instructions or links about how to contribute to https://github.com/actions/virtual-environments (e.g., in the case of a new release).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This brings up a small but interesting question. Cabal by default is installed with version x.x on Linux (because that's what hvr's PPA uses). This means cabal v3.0.0.0 is installed in /opt/cabal/3.0/bin and cabal v2.4.0.1 is installed in /opt/cabal/2.4/bin. It's fine, but also an odd design choice.
I currently break backwards compatibility and force an exact Cabal version only for implementation simplicity and secretly "undo" that when checking to see if I can locate a pre-installed version of Cabal.
This will lead to a somewhat confusing info message where we let people know we located cabal version "2.4" when they entered "2.4.1.0", necessitating a second message explaining that this is intended behavior.
Do we want to do that? If we don't, it seems like we might want to fully support "semver-like" versions (or at least major.major and have the "latest" point release be automatically selected).
But if we do that, it makes sense to also support GHC as well. If we went that route, it'd be more important to make sure that the code deferred to what the github virtual-environment thought was the latest point release rather than what it actually was, lest something get out of sync and we see CI build times spike right after a GHC release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh cabal, how fun.
I think my vote right now would be to support both major.major.minor.patch and major.major (maybe just for cabal). So 3.0.0.0 and 3.0 would both work. Info messages should say something like "Found cabal 3.0.0.0 in /opt/cabal/3.0/bin/".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I was thinking as well.
I also looked at the state of how the code was structured and disliked how it special cased the happy path for linux. It'll be messy to update things to add fallback install methods and have the right "just add to path" behavior for pre-installed versions. And it doesn't cleanly handle non-exact versions. So I've restructured the code quite a bit over the last day and I'm much happier with how it looks.
I'll have that up soon-ish and that should have more informative messages and a cleaner way forward to setting the path for pre-installed versions once windows and macOS get pre-installed versions of GHC. The version handling will also be more flexible.
|
I am looking to use this to get newer versions of ghc on my project to test a bug I may have found in ghc (https://gitlab.haskell.org/ghc/ghc/issues/18038). Am I correct in reading the .md file that even after this is merged, there will still be no support for 8.10.1 nor for HEAD? |
|
@JasonGross I think 8.10 should work, since it's installed on the runners and present in the hvr package. GHC HEAD is a trickier proposition, since we'd need to figure out how to build and distribute it and keep it updated. |
* Add better version support. Restructure code. Implement OS agnostic fallback * Support latest for cabal and stack. Add enable-stack option. * Add outputs * Update documentation. Change default for GHC to latest.
|
I've merged in the updates I've been working on for a bit to the haskell-CI/setup master branch. At this point I think I've finished implementing the functionality in a way that'll make it much easier to add support for recognizing pre-installed binaries on macOS and Windows (for when actions/virtual-environments#645 is implemented. The setup should now be much more ergonomic by default. GHC 8.10 should indeed work. Support for HEAD will be trickier as the underlying tools don't always have the best support for it. (In fact, chocolatey, ghcup, and the PPA don't have any support for installing HEAD versions of haskell currently). I don't really know how I could ergonomically add support for that but I'm open to suggestions. |
I don’t think we need to worry about HEAD. Given the difficulty of even installing it, I feel justified in saying that we can pass on that complexity to the people who absolutely need it (and I’m not sure how many people who need GHC HEAD are gonna use GH Actions anyway). |
This is my thinking as well. The infrastructure around being able to obtain a nightly version of the compiler isn't super fleshed out at the moment as Haskell is catching up to the tooling game. That being said, I'm not opposed to revisiting that should the windows situation change, but such a change would likely require that ghcup and chocolatey have support for downloading nightlies. So, as it stands, it looks like this branch is essentially feature complete. Are there any remaining blockers to getting it in? The latest status of actions/runner-images#645 looks like it's going to be a while before that gets merged in. Is this completely dependent on that? |
* Version ghcup * Don't cache something that needs to be tested for correctness
|
This is looking good. I've done a local run through and am now going to test out on one of our haskell projects that was already using the setup-haskell action. @jared-w, I think we should rev the version here to v1.1 (would be OK to do in a follow up PR) and make sure the README and other documentation reflects that. Also, what do you think about adding a list of contributors to the README? |
|
Build looks good on https://github.com/github/semantic/pull/545/checks?check_run_id=626680941. @jared-w, I'm going to merge this. Will you open a new PR to do a version rev? I'm happy to cut the release on github if you'll help me craft some release notes. |
|
@tclem absolutely. I'll open the PR and draft some release notes as well. Thanks for all your help! |
This commit represents the work done on haskell-CI/setup, specifically the
add-windows-macosbranch. It closes several issues and pull requests. In doing so, it is effectively a full rewrite of the repo (although certain pains have been taken to keep the git history as sane as possible)Linux will check for existing installs of GHC/Cabal and use those when possible. Thus, this offers the benefit of having zero regression in functionality and CPU cycles over the current state while being a strict addition of functionality.
Additional configuration inputs have been added based on my experiences writing CI systems for Galois and common annoyances people have in "real world projects" that make use of Cabal, Nix, Stack, or multiple combinations thereof. All inputs are fully documented and tested.
I will also be following up with GitHub to see if we can get common versions of GHC/Cabal/Stack baked into the environments for all operating systems. I have some suggestions that should save a decent amount of space in their images.
Lastly, I am willing and able to step up to help maintain and improve this project if you so wish. One thing I am very interested in is getting this action on the marketplace.
Fixes #1
Fixes #7
Fixes #10
Fixes #11