Update htscodecs and add more warnings #1828
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These find an old bug-let in hts_parse_format(): The buffer size is 8, but it includes \0 so the string would have been truncated, causing the strcmp to fail for "fastq.gz". Curiously gcc 10 and 11 spot this, but not gcc 12 or 13, nor clang. Some builds didn't have -Werror enabled, such as those without ./configure and on Windows.
jmarshall
reviewed
Aug 23, 2024
test/test-bcf-sr.c
Outdated
| @@ -179,6 +179,7 @@ int main(int argc, char *argv[]) | |||
| break; | |||
| case 'h': | |||
| usage(EXIT_SUCCESS); | |||
| // fall-through | |||
There was a problem hiding this comment.
This isn't really a fall-through situation. usage() (and error()) wants HTS_NORETURN.
Change hts_close() to hts_close_abruptly() in bgzip, to fix an empty body warning, allowing removal of -Wno-empty-body in CI tests. Fix signed compare in htslib/bgzf.h, which caused a warning in the maintainer-check usepublic.cpp check. It appears that -Wall does not allow signed-compare in c++. Make CFLAGS settings more consistent in .cirrus.yml, and ensure $MAKE_OPTS is consistently applied in all make invocations. The latter is necessary because "make cc-version" and "make maintainer-check" can trigger production of some .mk files, and we want them to be made in the same way as if we ran the full build. Switch MacOS build to Sonoma as Ventura is no longer supported by Cirrus.
Hat-tip to John Marshall
It's already in $(LIBS) and the MacOS linker complains if it appears more than once
|
I've pushed a few adjustments:
Additionally, the MacOS builds were getting a banner saying "Only ghcr.io/cirruslabs/macos-runner:sonoma is allowed. Automatically upgraded." so the MacOS image has been changed to the Sonoma one. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The extra warnings do spot a few buglets, such as in hts_parse_format().
Also resurrect
-Werrorwhich was absent from some builds. (It's still absent from the configure-less macos builds, but the recommended configure route has it which is sufficient.)