Fix build warnings, so Ubuntu CI can pass --werror to meson #13317
Conversation
ddstreet
changed the title
|
The first two commits looks nice. But I don't think we should enable -Werror by default. It is OK as a developer tool, to catch any issues early in a controlled environment, but it is unsuitable to distributed code or code that is compiled in changing environments. In particular, it creates a "time bomb" of code which compiles fine but might stop compiling after the next glibc or compiler update. |
ack; I can move the meson -werror=true param to Ubuntu CI's call to meson, so it would only run during Ubuntu CI, if that sounds ok to you. I'm not sure if the -Werror addition to the EFI build can be done from Ubuntu CI, though. |
This attribute is x86-only, so when building on non-intel archs it generates a compiler warning. When building with -Werror this turns into an error, so only include the attribute on intel archs.
This attribute is x86_32-only, so when building on non-intel archs it generates a compiler warning. When building with -Werror this turns into an error, so only include the attribute on i386 arch builds.
This part of the build does not use the normal meson parameters, so we need to explicitly check for the meson --werror parameter, and if it's true, set the gcc -Werror parameter for this subdir's build.
ddstreet
changed the title
|
Ok, force pushed to remove the meson --werror commit, and updated the commit to the src/boot/efi/meson.build so it only passes -Werror to gcc if --werror has been passed to the main meson build. |
|
Maybe, meson_options.txt should be also updated? |
|
|
Oh, I did not know that. Thanks. |
|
LGTM. |
UPDATE: removed commit to always pass werror to meson; this can be done separately from Ubuntu CI, so normal builds don't use the param. Also, the src/boot/efi/ build is changed so instead of always passing -Werror to gcc, it only does so if the meson get_option('werror') is true.
Original description:
The first 2 commits restrict the use of arch-specific gcc attributes to only the archs they are valid for.
The 3rd commit adds 'werror=true' to the project default meson params, which adds the gcc option -Werror to the entire build, except EFI which builds by calling gcc manually.
The last commit adds -Werror to the EFI build calls to gcc.
@evverx suggested this here:
#13174 (comment)
#13137 (comment)
though that suggestion was to add it to the Ubuntu CI builds, not all builds. If you think it's better to allow warnings in the build by default, and enable werror only for CI runs, I can change it to pass meson the werror flag only for Ubuntu CI (though it might not be possible for Ubuntu CI to add the param for the EFI build, only the top-level meson call).