Work around CFLAGS overriding --enable-werror.#2103
Conversation
If we have CFLAGS set then ./configure --enable-werror has no impact, as it just sets this in CFLAGS rather than another variable. In theory we should do ./configure --enable-werror CFLAGS=xyz. However that means parameterising the <<:*COMPILE template. It's easiest to do this via an environment given the matrix, but we have to be careful to avoid it undoing itself again. We also need to take care to not do --enable-werror when using ubsan. This is a long-standing issue of removal of -Werror that's been present right since the first day we used cirrus-CI.
|
Note this is using However. I'm not sure it's needed, as htslib checks itself anyway and samtools is primarily doing CI on samtools and not so much htslib. Secondly if we get a new warning or new compiler that breaks htslib, then fixing it in htslib isn't sufficient. All samtools PRs will start bugging out too, which seems disconnected given the error isn't possible to fix there. This PR is an example of this infact: htslib currently has problems because it rewrites the htscodecs build and hence is broken until samtools/htslib#1828 is merged. (It works in my branch, but in PRs we're not correctly copying the branch over I think for the htslib clone) If you feel it's necessary however, the trivial CFLAGS to _CFLAGS rename does solve this. But you'll need to merge the htslib PR first in that case. |
|
The |
|
I added that change as a separate commit as I still had it around. As expected it's failing atm, but this is an issue of cirrus-ci and branches I think. It treats PRs different from own branching. |
"make -e" causes variables to be set from the environment, overriding those from configure, which is not what we want. Where set, CFLAGS, CPPFLAGS and LDFLAGS are all now passed to configure. CPPFLAGS is set for -fsanitize=address,undefined to prevent unaligned access in HTSlib. Adjusts warning options to work around false positives when using undefined behaviour sanitizer, and to enable full format checking on the gcc build with -Wextra.
After option handling, samtools import was looking for up to four file names, which if present would overflow the opts->fn[] array used to store them. It only makes sense to look for two (for R1, R2).
|
Comparing with HTSlib, it seems the problem was due to using |
Interesting. This seems to have been inherited from the old Travis config, all the way back to 050bc54. I'm not really sure what it's there as it's specifying the variable on the make command line rather than the environment, but maybe it's to pick up something else. Did you try building without the |
ed32ad0 to
24c3e8b
Compare
|
Non |
If we have CFLAGS set then ./configure --enable-werror has no impact, as it just sets this in CFLAGS rather than another variable.
In theory we should do ./configure --enable-werror CFLAGS=xyz. However that means parameterising the <<:*COMPILE template. It's easiest to do this via an environment given the matrix, but we have to be careful to avoid it undoing itself again. We also need to take care to not do --enable-werror when using ubsan.
This is a long-standing issue of removal of -Werror that's been present right since the first day we used cirrus-CI.