Emstricten libcn #223
Emstricten libcn #223
Conversation
|
(The branch is based on #220, which should be merged before.) |
| #!/bin/bash | ||
|
|
||
| FLAGS="-O2 " | ||
| if [[ -n "${GITHUB_ACTIONS+isset}" ]]; then |
There was a problem hiding this comment.
Without this, if you have a newer version of a compiler, builds can break locally, but pass CI. See #85.
So I would not remove this.
There was a problem hiding this comment.
Note that there is no -Werror, and only hand-picked warning are promoted to errors.
I can re-add -Werror for the CI. But the additional warnings should be local, otherwise they go unfixed.
There was a problem hiding this comment.
(Or, we can use dune-idiomatic thing and add it for dev profile only.)
There was a problem hiding this comment.
Removing -Werror weakens CI, no? I'd rather not...
There was a problem hiding this comment.
I can re-add -Werror for the CI. But the additional warnings should be local, otherwise they go unfixed.
Perhaps we do this then?
There was a problem hiding this comment.
@pqwy Zain seems to be happy with re-adding -Werror for the CI and making additional warnings local, so once you update the branch I'm happy to merge
3 participants
Increase the C compiler strictness for libcn, to mirror the Linux build environment more closely. Then fix the obvious fallout. In particular:
runtime/libcn/lib/compile.sh;-Werror=strict-prototypestree-wide;-Wimplicit-fallthroughin Fulminate.This helps avoid a few C foot guns, and keeps the C sources in a good shape to build as part of Linux/pKVM. There are no downsides to adhering to a slightly stricter programming discipline, either.
One functional change I made is not making warnings conditional on CI. Having them pop out during development helps fixing things early; and conversely, having the CI stricter than the normal workflow is bound to cause frustration.