xmake silently hides compiler warnings

davidchisnall commented 8 months ago

Xmake Version

2.8.5

Operating System Version and Architecture

macOS

Describe Bug

I normally compile with -Werror, so I'm not sure how long this has been an issue, but I recently started using -Wno-error= to make warnings back into warnings in some third-party code and was surprised that xmake did not report the warnings.

Even with xmake -v (which I need to use to make xmake not annoyingly truncate error messages in the middle), I don't get any output.

Expected Behavior

Warnings should be reported to the programmer!

Project Configuration

Minimal example:

ex.c:

#warning xmake hides this!

int main() {}

xmake.lua:

target("example")
    add_files("ex.c")

Running xmake:

$ xmake
checking for platform ... macosx
checking for architecture ... arm64
checking for Xcode directory ... /Applications/Xcode.app
checking for Codesign Identity of Xcode ... no
checking for SDK version of Xcode for macosx (arm64) ... 14.0
checking for Minimal target version of Xcode for macosx (arm64) ... 14.0
[ 25%]: cache compiling.release ex.c
[ 50%]: linking.release example
[100%]: build ok, spent 3.404s

No warnings!

Running xmake -v after deleting .xmake and build:

$ xmake -v
$ xmake -v
checking for platform ... macosx
checking for architecture ... arm64
checking for Xcode directory ... /Applications/Xcode.app
checking for Codesign Identity of Xcode ... no
checking for SDK version of Xcode for macosx (arm64) ... 14.0
checking for Minimal target version of Xcode for macosx (arm64) ... 14.0
checking for zig ... no
checking for zig ... no
checking for /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang ... ok
checking for the c compiler (cc) ... clang
checking for /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang ... ok
checking for flags (-fPIC) ... ok
[ 25%]: cache compiling.release ex.c
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -c -Qunused-arguments -target arm64-apple-macos14.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk -o build/.objs/example/macosx/arm64/release/ex.c.o ex.c
checking for flags (-MMD -MF) ... ok
checking for flags (-fdiagnostics-color=always) ... ok
checking for /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ ... ok
checking for the linker (ld) ... clang++
checking for /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ ... ok
checking for flags (-fPIC) ... ok
[ 50%]: linking.release example
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -o build/macosx/arm64/release/example build/.objs/example/macosx/arm64/release/ex.c.o -target arm64-apple-macos14.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk -stdlib=libc++ -lz
[100%]: build ok, spent 0.224s

No warnings!

But running the command that xmake tells me it ran:

$ /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang -c -Qunused-arguments -target arm64-apple-macos14.0 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.0.sdk -o build/.objs/example/macosx/arm64/release/ex.c.o ex.c
ex.c:1:2: warning: xmake hides this! [-W#warnings]
#warning xmake hides this!
 ^
1 warning generated.

Additional Information and Error Logs

N/A.

SirLynix commented 8 months ago

It's not a bug but a design choice, see this page of the doc.

You can either pass -w to xmake to make it print warnings: xmake -w, enable it globally with xmake g --build_warning=y or enable it for everyone on this project using set_policy("build.warning", true).

waruqi commented 8 months ago

right, you can add set_policy("build.warning", true) in your xmake.lua

davidchisnall commented 8 months ago

It looks as if that policy needs to be in the root file, so I can't turn it on globally in the SDK's xmake.lua. It's a staggeringly dangerous default. I can't imagine any project actually wanting that behaviour. I need to tell every consumer of our SDK 'If you actually want the warnings that you've enabled to be visible, you need to opt into this non-default bahaviour'.

What is the rationale for this? It's even worse than the 'silently throw away valid compiler flags because xmake doesn't know about them' behaviour.

Warnings exist for a reason. If they're enabled, it's because the user wants to know when they do the thing that the warning warns for. You can disable any warnings that you don't want to see with compiler flags (if you remember {force = true}, otherwise xmake silently doesn't pass these flags to the compiler.

waruqi commented 8 months ago

Your sdk/xmake.lua is the project root file, and you should be able to turn it on in the global root scope. It's the same as this configuration.

https://github.com/microsoft/cheriot-rtos/blob/7280f24899042996f395341f011ccd9ef50eff56/sdk/xmake.lua#L6

Only developers need to see the warnings in real time, while for users they simply want to compile your project and then use your framework and library binaries. They don't care about the warnings.

A large number of warnings can lead to frequent output from the terminal, preventing users from viewing progress information. They don't care about the warnings.

For developers, they have the ability to configure set_policy("build.warnings", true) in xmake.lua or use xmake -w to see the warnings.

Also, set_policy("build.warning", true) is perfectly fine for letting your project modify the default warning output behaviour, which only needs to be configured once and doesn't need to be configured every time by another user.

davidchisnall commented 8 months ago

Your sdk/xmake.lua is the project root file, and you should be able to turn it on in the global root scope. It's the same as this configuration.

I tried that and it does not work. The line that you link to also does not work and so I need {force = true} scattered all over the place.

Only developers need to see the warnings in real time, while for users they simply want to compile your project and then use your framework and library binaries. They don't care about the warnings.

Most users do not build software. Two kinds of people build software:

Developers
Packagers

Both of these want to see warnings. Packagers typically disable anything that hides output because they need it for their auditing.

A large number of warnings can lead to frequent output from the terminal, preventing users from viewing progress information. They don't care about the warnings.

That's an incentive to fix the warnings. It's also a reason to fix the output. Ninja does a much better job than xmake here:

The progress line is always at the bottom of the screen
Tool output appears above the progress line.
Tool output is always coupled with the exact command to reproduce that build step and so it can be run.

For developers, they have the ability to configure set_policy("build.warnings", true) in xmake.lua or use xmake -w to see the warnings.

I shouldn't have to do anything explicit to get a sensible default behaviour.

Also, set_policy("build.warning", true) is perfectly fine for letting your project modify the default warning output behaviour, which only needs to be configured once and doesn't need to be configured every time by another user.

It needs to be in every single xmake.lua that consumes our SDK. It's another thing that we have to educate users of our SDK to do. Onboarding people to a new build system (xmake) is already hard enough.

waruqi commented 8 months ago

I tried that and it does not work. The line that you link to also does not work and so I need scattered all over the place.

It should work, unless you're talking about other projects that use your sdk, rather than building the sdk itself.

If it's just building the sdk, it's a global configuration and should be fully working. If it doesn't work, you can open a separate issues to report bugs.

If you want other projects that consume your sdk to be on by default as well. you can set it in function firmware(name).

https://github.com/microsoft/cheriot-rtos/blob/94dca15276348727ff6ffccbafb2c2be91aafe98/sdk/xmake.lua#L740C1-L740C24

function firmware(name)
    set_policy("build.warning", true)
end

Most users do not build software. Two kinds of people build software:

so they can add -w option or use set_policy("build.warning", true) to enable it.

That's an incentive to fix the warnings. It's also a reason to fix the output. Ninja does a much better job than xmake here:

xmake also support it. you need only to switch to ninja theme.

$ xmake g --theme=ninja
$ xmake -w

I shouldn't have to do anything explicit to get a sensible default behaviour. It needs to be in every single xmake.lua that consumes our SDK. It's another thing that we have to educate users of our SDK to do. Onboarding people to a new build system (xmake) is already hard enough.

But not everyone will see this as a default behaviour. Even if I have it enabled by default. There will be other users configuring set_policy("build.warnings", false) to disable it, or opening issues to complain about too many warnings and how to disable them.

It is impossible for xmake to satisfy every user's preferences, but xmake already provides policy configuration to let users control the default behaviour of their projects.

davidchisnall commented 8 months ago

It should work, unless you're talking about other projects that use your sdk, rather than building the sdk itself.

The SDK does not build itself, it is included in other projects.

If you want other projects that consume your sdk to be on by default as well. you can set it in function firmware(name).

Does that then apply to all targets or just to the firmware target?

so they can add -w option or use set_policy("build.warning", true) to enable it.

Sane defaults are about ensuring that users don't have to explicitly do anything unless they have unusual requirements. Hiding warnings is never the right thing to do. If you don't want to see warnings, disable those warnings.

xmake also support it. you need only to switch to ninja theme.

Oh, nice! It doesn't respect NINJA_STATUS (I have NINJA_STATUS=%p [%f:%s/%t] %o/s, %es in my shell config to get nicer output from Ninja) and it still randomly truncates error messages (which also breaks editors that parse the compiler output to provide warnings), is there another magic setting that can turn that off? It's especially bad for C++ because it always manages to remove the bit of the error message that tells you the line that is actually responsible for the bug.

But not everyone will see this as a default behaviour. Even if I have it enabled by default. There will be other users configuring set_policy("build.warnings", false) to disable it, or opening issues to complain about too many warnings and how to disable them.

Have you had feedback about that? Hiding warnings is not something I have ever wanted to do for anything other than a toy project. It's something that most secure coding guidelines and corporate policies have explicitly disabled. At the very least, a status line saying 'warnings are hidden by default, use xmake -w to show them' would reduce the surprise.

waruqi commented 8 months ago

Does that then apply to all targets or just to the firmware target?

for all

Oh, nice! It doesn't respect (I have in my shell config to get nicer output from Ninja) and it still randomly truncates error messages (which also breaks editors that parse the compiler output to provide warnings), is there another magic setting that can turn that off? It's especially bad for C++ because it always manages to remove the bit of the error message that tells you the line that is actually responsible for the bug.NINJA_STATUSNINJA_STATUS=%p [%f:%s/%t] %o/s, %es

? xmake -D will show all messages

Have you had feedback about that? Hiding warnings is not something I have ever wanted to do for anything other than a toy project. It's something that most secure coding guidelines and corporate policies have explicitly disabled. At the very least, a status line saying 'warnings are hidden by default, use xmake -w to show them' would reduce the surprise.

no, you can only use xmake -w or set_policy("build.warning", true) to enable it.

paul-reilly commented 7 months ago

I have to say that not showing warnings seems like the wrong default. Somebody preferring clean output is surely less important than somebody wanting to see the same warnings they get with other build systems, without having to read the manual first.

Hiding warnings by default breaks The Principle of Least Astonishment imo.

waruqi commented 7 months ago

I've started a poll, and if the majority of people agree that warning output should be turned on by default, I'll consider changing the default behaviour.

https://github.com/xmake-io/xmake/discussions/4642

waruqi commented 7 months ago

I have enabled it. https://github.com/xmake-io/xmake/pull/4651

xmake-io / xmake