Build and test in GitHub CI

[runeksvendsen]

@jberthold thank you for creating this package. I wanted to understand which GHC versions and C compiler versions it works with, so I created a GitHub CI config that builds for multiple versions of each — both for Linux and macOS.

The result of running this CI workflow can be seen in my fork of this repo: runeksvendsen#1 (the same PR but targeting the master branch of my fork).

It reveals a couple of bugs:

1. testmthread: Contains an unsupported closure type (whose implementation is missing) #18
2. detailed test suites (alltests, quickchecktest) fail with panic! read @TestSuiteLog "" for GHC 7.10.3 and 8.0.2 #20

Where #20 is rather easy to fix (runeksvendsen@a42fb0e), but the issue behind #18 is not immediately clear to me.

Note: commit dfc0f58 in this PR tests a total of 216 combinations of different operating systems, GHC and C compilers. This has the advantage that it covers a lot of different configurations, but the disadvantage that it's difficult to quickly get an overview of why a given CI run, that has N number of failures, actually failed — e.g. is it the same failure happening multiple times or different errors? For this reason, one could consider reducing the number of different configurations.

TODO

• Enable testing on ARM macOS once aarch64 support #22 is merged

[jberthold]

Thanks for your interest and testing effort @runeksvendsen !
I haven't touched this code in the last few years so I do not expect it to work with ghc-9.x. Therefore I don't think there is much value in enabling github actions for this repository. If we did, I would probably reduce the amount of versions tested - a suitable goal would probably be 3 GHC versions on Mac and Ubuntu.
For Mac, it would be good to test on ARM, alas, I have no idea whether pacman works at all on ARM based MacOS and I have no way to test and debug that.

Happy to merge your shell.nix and the workflow file (with reduced version matrix), though, since that makes it easier for people to work on the library.

[runeksvendsen]

Hi @jberthold, I'm very happy to hear from you. I'm glad you're willing to merge this PR with a reduced version matrix.

I should note that I have not yet observed errors that happen only for GHC 9.x. So it appears to work. But I don't know enough about this library to say for sure. If it doesn't work on GHC 9.x it would nice with a test case that reveals this.

But I'm happy to reduce the number of GHC versions in the matrix. Do you have any preference regarding the three GHC versions you'd like to build with? I'm thinking GHC 8.10 would make sense to include at least.

Regarding ARM Mac builds, I can enable that. I believe I disabled it initially because ARM doesn't work for GHC versions before 8.10. I did a CI run with ARM enabled (link) and it appears building packman fails with the following error for GHC versions >= 8.10 (see e.g. this job).

GHC/Packing/Type.hs:292:2: error:
     warning: Don't know the word size on your machine. [-W#warnings]
    |
292 | #warning Don't know the word size on your machine.
    |  ^
#warning Don't know the word size on your machine.
 ^
1 warning generated.
[2 of 4] Compiling GHC.Packing.Type ( GHC/Packing/Type.hs, /Users/runner/work/packman/packman/dist-newstyle/build/aarch64-osx/ghc-8.10.7/packman-0.5.1/build/GHC/Packing/Type.o, /Users/runner/work/packman/packman/dist-newstyle/build/aarch64-osx/ghc-8.10.7/packman-0.5.1/build/GHC/Packing/Type.dyn_o )

GHC/Packing/Type.hs:115:34: error:
    Variable not in scope: hexWordFmt :: [Char]
    |
115 |                     printf ('\t':hexWordFmt) w
    |                                  ^^^^^^^^^^

[jberthold]

But I'm happy to reduce the number of GHC versions in the matrix. Do you have any preference regarding the three GHC versions you'd like to build with? I'm thinking GHC 8.10 would make sense to include at least.

My suggestion to use three versions would be to declare those three versions supported, supposedly these should be the most recent GHC versions in that case? However, GHC-9.x support is unknown until it is properly inspected.
FWIW in the past I gamified this question, aiming to support all versions of the past using ifdefs... it is certainly not the most important thing to users.

I should note that I have not yet observed errors that happen only for GHC 9.x. So it appears to work. But I don't know enough about this library to say for sure. If it doesn't work on GHC 9.x it would nice with a test case that reveals this.

The library contains code using internal GHC runtime system types, most importantly types in Closures.h so maintenance/updating the library means following GHC changes to these types and what they mean. From the commit history of ClosureTypes.h, it looks like not much has changed since 2018 except moving the file around and commenting it. The substantial change is "native delimited continuations" in ghc-9.6 which adds a new closure type (even if this closure type remains unsupported, the total N_CLOSURE_TYPES has changed and may require changes to the library).

As for the compilation error on ARM Mac, this code needs to be extended as a first step. The word size will probably just be 64 bit but the ARM architecture is not checked for.

[runeksvendsen]

My suggestion to use three versions would be to declare those three versions supported, supposedly these should be the most recent GHC versions in that case? However, GHC-9.x support is unknown until it is properly inspected.

I pushed two commits: one that reduces the number of C compilers use (to four in total: 43d5f85) and another that reduces the tested GHC versions to 8.10, 8.8 and 8.6 (6f22a07).

All the failures that happen for the two runs related to these changes (as can be seen here runeksvendsen#3) are due to #18 .

As for the compilation error on ARM Mac, this code needs to be extended as a first step. The word size will probably just be 64 bit but the ARM architecture is not checked for.

I created a commit for this: runeksvendsen@e75ecdd. This makes everything build and the tests pass in CI. I will create a separate issue and PR for this.

[jberthold]

Thanks for adapting. Feel free to merge it.

All the failures that happen for the two runs related to these changes (as can be seen here runeksvendsen#3) are due to #18 .

I have limited bandwidth until next Monday (am on holiday 🇨🇳 ), and then work is starting again, but I might be able to spend some time to check what is going on in #18 (first thing would be to run with debug logging).

[runeksvendsen]

Great! Thank you @jberthold. Enjoy your holiday :-)