Add msys2 CI on github actions

[jeromekelleher]

To support msys2 on Windows as a first class platform for C development, we'd like to add it to CI. This is an experiment to see if that'll work.

Based on CMake example: https://github.com/msys2/setup-msys2/blob/master/examples/cmake.yml

[codecov]

Codecov Report

Merging #1742 (c63228f) into main (cbc593e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head c63228f differs from pull request most recent head cb62997. Consider uploading reports for the commit cb62997 to get more accurate results

@@           Coverage Diff           @@
##             main    #1742   +/-   ##
=======================================
  Coverage   93.36%   93.36%           
=======================================
  Files          27       27           
  Lines       24405    24421   +16     
  Branches     1095     1095           
=======================================
+ Hits        22785    22801   +16     
  Misses       1585     1585           
  Partials       35       35           

Flag	Coverage Δ
c-tests	92.08% <100.00%> (+0.01%)	⬆️
lwt-tests	89.25% <ø> (ø)
python-c-tests	94.52% <ø> (+<0.01%)	⬆️
python-tests	98.76% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
c/tskit/core.h	100.00% <ø> (ø)
c/tskit/core.c	97.84% <100.00%> (+0.05%)	⬆️
c/tskit/tables.c	90.14% <100.00%> (ø)
c/tskit/trees.c	94.87% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbc593e...cb62997. Read the comment docs.

[jeromekelleher]

This is ready to go I think. Most of the problems I encountered were because of that annoying problem we hit in #721, which showed up in a slightly different way and took ages to track down (again). I think the solution here is better (using tsk_isfinite, and doing the typdefing workaround within the function definition) because it doesn't require any fiddling with symbols and we don't have to remember to define a particular macro in any file we want to use isfinite in.

The only other thing that's potentially controversial is the tsk_set/get_debug_stream functions and they way they are implemented. I think it's probably fine for an internal development tool - but see #1744 for a discussion of what we should do with the warn_stream. This was a good thing to change for 1.0 anyway, as hard-coding in stdout for debug is pretty nasty (and the way we were testing it was very nasty).

But yeah - C library code and tests building and passing native-ishly on Windows!

Any chance you could take a look please @bhaller and @molpopgen?

[bhaller]

Any chance you could take a look please @bhaller and @molpopgen?

I don't use or have access to Windows, and don't even know what msys2 is. :-> But perhaps @rdinnager or @bernard-kim would like to have a look?

[rdinnager]

I mean, looks good to me, but I have very little C experience, so I am not the best person to look at it. The gh actions MSYS2 setup looks fine to me though. Thanks for spending the effort on this! Just for the record, if it builds with MinGW, then I believe it is native Windows (not 'native-ish').

I will probably use this as reference for setting up gh actions tests for SLiM too, once it is ready. So thanks again!

[jeromekelleher]

Thanks @bhaller. I was actually mainly interested in seeing if you're OK with the new workarounds for #721 - using builtins on gcc and clang for isfinite and isnan seem all right?

Thanks for taking a look @rdinnager! Hopefully this will provide a good basis for SLiM CI - I think you should be more-or-less able to replace meson with Cmake and it should work.

[bhaller]

Thanks @bhaller. I was actually mainly interested in seeing if you're OK with the new workarounds for #721 - using builtins on gcc and clang for isfinite and isnan seem all right?

Ah, I see, sorry. #721 wasn't on my radar, I don't think I've seen those warnings or had them reported to me. Your fix seems OK I think? I don't know much about builtins really. The one drawback to your approach that I can see is that these functions will now entail actual function calls, rather than getting inlined by the compiler, but unless they get used in a spot that is a performance bottleneck, that shouldn't matter (and they might get inlined by the linker anyway, if it's smart enough). But you could define them as macros instead to avoid that, e.g.:

#if defined(__GNUC__)
#define tsk_isnan(val) __builtin_isnan(val);
#else
#define tsk_isnan(val) isnan(val);
#endif

or something like that. Anyhow, this all seems fine, I don't anticipate it causing any issues for SLiM.

[jeromekelleher]

Thanks @bhaller - you're right about the function calls. If they start showing up on our profiles we can inline/macro-fy them or something, but I'm guessing they won't end up being significant so will keep it simple for now.

[benjeffery]

LGTM, some nice cleanup there