Update GHC versions on windows

[thboop]

So, this is a fairly lengthy pr to update GHC versions on windows.

Context

To begin with, we install GHC differently depending on OS. For windows, we use the tool Choco. Which adds GHC to path using the add-path command, which was just recently disabled. This causes the workflow run to fail.

So, working with GhcChoco, we made some new versions that no longer use the disabled command. However, these only exist on choco, not the other platforms. Previously, the version list of GHC for all os's was the same. So thats our first major change:

• GHC now has different versions available for windows and (linux/mac)

However, when we did that, we ran into some other issues which we have fixed:

• the new add-path environment file occurs at the end of the step, not during like the add-path command did, so when we check that we have added to the path mid step, that failed. We now manually add it to the path as well using the core.addPath command. This is best effort (we assume the path we are adding is correct, if the generated path is wrong we may encounter issues, but its the simplest way to solve this issue)
• The format for the GHC path used to be solely based on the version number. But now that we have added patch versions, some parts of the path include the patch number, others do not, so we had to change how we compute the path to correctly match.
• We also updated the docs to make this clear to users and regenerated the dist.

Resolves #44

[tclem]

👍

What's the user experience for a matrix build where a ghc version is supported by one platform and not another? As long as the docs and error messages are clear (and clear about what to do to fix) this seems like an ok solution. Is there anyway to support the same version list across max/linux/windows? Even it it means supporting fewer GHCs, I think that would be a superior user experience.

[tclem]

@jared-w if you've got a free moment, would appreciate your thoughts on this as well.

[hazelweakly]

@tclem yeah, I have a few thoughts, thanks for the mention.

There's a few subtle things happening here that make it to where I am unconvinced the PR needs to take the approach it currently does.

1. When Tamar is talking about github action compatibility in the chocolatey scripts, that is only for adding the GHC binaries to the path. This is so that when chocolatey is used directly, it "just works". I have never relied on that behavior and have always added the path manually, myself.

   As such, the supported versions of GHC on windows do not change because it still works the same way it used to. What has changed is if one installs GHC 8.10.X, or 8.8.X, it will use the patch version which does not call the removed command any longer.

   But, setup-actions has never relied on that behavior. So it is sufficient to "resolve" a version like 8.8.4 to 8.8.4.1 on windows, and just add that path; unaffected versions do not need to change, so there doesn't need to be a split between windows and macos/linux version support of GHC.

2. The version difference exists only because chocolatey doesn't allow revisions of uploaded versions (which is the correct thing to do). If someone manually installs GHC 8.4.4 through chocolatey, for example, it'll use the patch version automatically (as far as I can figure out). We should be explicit because it makes generating the path easier, but fundamentally we haven't semantically changed anything here.

3. The timing of the add-path file vs the add-path environment file is a breaking change and should be addressed if it causes the action to break on Windows. The added path logic to isInstalled is indeed the best place to fix that, imo.

   I am a bit grumpy that this is effectively impossible to unit test, but that's outside of the scope of the PR.

In summary:

• The supported versions doesn't need to change
• The getChocoPath logic looks correct
• There's no need for the note about we "believe" the generated path is correct. It is correct; we use the same logic that GHC's chocoInstall itself uses to generate that directory and add it to the path.
• I am now slightly suspicious of whether or not we need to add paths manually for other tools mid-step, but it seems like we don't so far.

[hazelweakly]

And as a general note, while I don't myself use Windows on my personal machines, I see it as important to have it on equal footing whenever possible.

Even if it were necessary to change a lot of the internal reworkings of how we invoke chocolatey (or install GHC/cabal on windows in general) in order to accommodate other GHC versions and keep the version specification the same, I would be much more willing to do that; particularly since the most core API of setup-haskell is the ghc-version option. Absolutely every other bit of logic in the action is replaceable in semi-trivial amounts of code except for that.

Having a visible breaking change to how Windows users specify GHC versions solely due to github-action implementation implementation details is truly the worst of both worlds. Luckily, it doesn't seem to be necessary. This is not to diminish Thomas's work; chasing tooling breakages across several layers of abstraction and coming up with a working solution is no small feat.

[thboop]

Hey @jared-w thanks for the feedback! I've laid out what I think our options are below, but I'm open to other ideas! I would love to hear your thoughts!

If someone manually installs GHC 8.4.4 through chocolatey, for example, it'll use the patch version automatically (as far as I can figure out)

In my testing, this did not occur.
For example the workflow

name: CI
on:
  push
jobs:
  test:
    runs-on: windows-latest
    steps:
     - run: "choco install ghc --version 8.10.2 -m --no-progress -r" # this will fail
       continue-on-error: true
     - run: "choco install ghc --version 8.10.2.2 -m --no-progress -r" # this works

The first step fails while the second works

When Tamar is talking about github action compatibility in the chocolatey scripts, that is only for adding the GHC binaries to the path. This is so that when chocolatey is used directly, it "just works". I have never relied on that behavior and have always added the path manually, myself.

This action appears to have relied on the functionality. Hence we had to add the change to core.addPath to manually add it to the path if choco does not.

We fail the step if we detect any usage of add-path so installing any of these older (non-patch) versions instantly fails on windows. This was done to give instant actionable feedback to users, rather than allowing the action to continue without setting the path which would have be a confusing, somewhat invisible error if they don't check the logs or annotations. So any user installing any of these non specific patch versions of ghc on windows will fail. Hence we separate out the windows list here.

The way I see it, we have 4 options:

• Add a way for the actions runner to skip failing this step due to the invocation of add-path
• We figure out how ghcChoco is determining to invoke add-path and bypass it. Maybe setting GITHUB_ACTION to null would work? We would have to manually add to path in this case.
  • code
• Separate out the version list for windows (what this pr does)
• Work with ghc choco to update existing versions, or automatically install the patch versions if they are not specified.

4 is probably the best solution, but is out of our hands and likely would not be available for a while.
3 is what this pr implemented, but may cause minor breakage for users reliant on specific versions that have not been patched.
2 feels somewhat hacky to rely on, but if we could get it working would likely be the least disruption.
1 requires a coordinated rollout across a lot of systems, which can be slow.

[Mistuke]

Hi folks,

just my 2 cents..

Hey @jared-w thanks for the feedback! I've laid out what I think our options are below, but I'm open to other ideas! I would love to hear your thoughts!

If someone manually installs GHC 8.4.4 through chocolatey, for example, it'll use the patch version automatically (as far as I can >> figure out)

In my testing, this did not occur.
For example the workflow

Correct, using a specific version resolves to exactly that version. There is an open feature request for this but it's not available yet chocolatey/choco#800

So it is sufficient to "resolve" a version like 8.8.4 to 8.8.4.1 on windows

I think this is the right thing to do. As @jared-w said the point releases are just because chocolatey won't let me revise the packages. Now the point releases will never contain functionality changes wrt to the compiler itself. They are always just packaging related.

as such there will never be a functional difference, only a difference in what the installer does, since a change in functionality will be a bump in minor version from upstream not the revision number. For instance the 8.10.2.1 revision fixes the hardcoded linker issue with the upstream binary, without touching the package they provide. It just post-processes the settings file after install.

As such I think it makes more sense to ignore the revision number and just map them internally to the latest one. my intention with the revisions are that users shouldn't care about which number it is. This will allow you to treat Windows the same as Linux etc w.r.t version numbers.

[thboop]

Thank you everyone for jumping in here on such short notice.

Based on the feedback, I believe our goals should be:

• consistent behavior and versions across every platform
• a seamless update of the v1 action so users who are using @actions/setup-haskell@v1 just start working again
• supported versions should not change (no breaking changes)
• release an update asap

We can accomplish that by:

• disabling workflow command execution before installing from choco (so that add-path does not fail the action) and re-enabling it after the choco install completes)
  • this is done via the ::stop-commands::{token} command
• manually adding paths when installing via choco (already done and signed off in this pr)
• releasing a new patch version (and sliding the major tag along)

I'll go ahead and update this pr and get this version released. Since this is a minor version with no breaking changes, if we decide on a solution that we feel is better in the future (that may take longer to implement), we can major version the action and spend the time to do that. However, I would like to get this fixed asap.

Thanks again folks!

[hazelweakly]

Disabling command execution is a good way to accomplish that, nice; I hadn't thought of it initially.

We should, however, eventually map all patch releases on chocolatey internally so that we use the "most correct" version of the installer for any given GHC version. But that can be in a follow up.

[phadej]

Do I interpret these changes right: I need to do

      - uses: actions/[email protected]
        with:
          ghc-version: '8.10.2.2' # 8.10.2 isn't automatically remapped to 8.10.2.2?
          cabal-version: '3.2.0.0'

[thboop]

Do I interpret these changes right: I need to do

      - uses: actions/[email protected]
        with:
          ghc-version: '8.10.2.2' # 8.10.2 isn't automatically remapped to 8.10.2.2?
          cabal-version: '3.2.0.0'

@phadej You should just use the existing versions

       - uses: actions/[email protected]
         with:
          ghc-version: '8.10.2

[Mistuke]

Do I interpret these changes right: I need to do

      - uses: actions/[email protected]
        with:
          ghc-version: '8.10.2.2' # 8.10.2 isn't automatically remapped to 8.10.2.2?
          cabal-version: '3.2.0.0'

@phadej You should just use the existing versions

       - uses: actions/[email protected]
         with:
          ghc-version: '8.10.2

Unless 8.10.2 is internally mapped to 8.10.2.1 or 8.10.2.2 he shouldn't though... as 8.10.2 is broken as released by GHC HQ.

[hazelweakly]

It will be internally mapped from 8.10.2 -> 8.10.2.2 but currently that hasn't been done yet. I'm hoping to write up that functionality and get it merged shortly so that 8.10.2 will "just work".

In the meantime, versions like 8.10.2.2 and other unrecognized versions will transparently pass through to the underlying tool. So 8.10.2.2 will work on windows but will break on linux.