Github Actions configuration #123
Conversation
Also fix freebsd to be up to date as it broke CI.
jkbonfield
force-pushed
the
windows-actions-tmp
branch
from
1346a4b to
ff102be
Compare
jkbonfield
force-pushed
the
windows-actions-tmp
branch
2 times, most recently
from
8cb0fd4 to
3a9f336
Compare
There was a problem hiding this comment.
I've suggested changes to the Check sections that write out tests/test-suite.log on failure while ensuring the failed state is reported correctly. I would have pushed these myself, but I'm running into some GitHub permissions problems around changing workflows...
jkbonfield
force-pushed
the
windows-actions-tmp
branch
from
3a9f336 to
4519f18
Compare
.github/workflows/windows-build.yml
Outdated
| export PATH=/mingw64/bin:$PATH | ||
| export MSYSTEM=MINGW64 | ||
| autoreconf -i | ||
| ./configure --disable-shared CFLAGS='-g -O3 -D_XOPEN_SOURCE=600 -Wno-char-subscripts' |
There was a problem hiding this comment.
Why is -Wno-char-subscripts here? It's not the sort of error that should be ignored. Also, all builds should use -Wall -Werror as we know from experience that warnings get ignored unless they cause the whole test to fail.
There was a problem hiding this comment.
It was added because there were a large number of errors coming from the original (non mingw-w64) compiler that the github action was using on Windows. None of the unix ones reported these, so for now I disabled it so I could break the problem down into one thing at a time - in this case getting windows working.
However I suspect they've now gone again as I realised my other errors came down to PATH and finding the wrong compiler.
I think the errors came from differing implementations of ctype, with the other compiler using an array based lookup. Eg see https://github.com/jkbonfield/htscodecs/actions/runs/9644267159/job/26596006462
It's probably a genuine thing, but best solved via another PR.
There was a problem hiding this comment.
I've confirmed that -Wno-char-subscripts is no longer needed anyway even with the ctype problems unpatched. However I fixed the ctype checking in #124. Note I found the same problems in htslib (once), samtools(~10) and bcftools (100ish). This is no doubt why it's worked around on linux, and I suspect several on many other platforms, as it's just an endemic problem and frankly a sign of a terrible clash of API design in C ctype vs str* functions.
I enabled -Wall -Werror. (These were absent simply because we missed them off before in appveyor and I was copying that config.) I could try using configure --enable-werror but I'm not sure when that appeared (we don't use it in htslib, but it's perhaps an automake thing) and apparently unlike htslib we don't explode if we attempt to define -Werror during configure anyway, presumably due to the limited range of tests we use. We can add it later if it's a problem for e.g. cross-compiling.
These are replaced by GitHub actions, so we now distribute a few jobs between GitHub Actions and Cirrus-CI which hopefully will give us a fast turnaround. Also tweak test-suite.log catting on failure to ensure tests still fail.
jkbonfield
force-pushed
the
windows-actions-tmp
branch
from
4519f18 to
d869736
Compare
NB this is a PR from samtools/htscodecs itself as a test to see if it then permits the new CI to run as validation. (If not, you can see the results of the CI here anyway.)