On Tue, Oct 22, 2024 at 08:13:40PM +0200, Simon Glass wrote:
Hi Andy,
On Tue, 22 Oct 2024 at 15:23, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
On Mon, Oct 21, 2024 at 06:32:21PM +0200, Simon Glass wrote:
On Mon, 21 Oct 2024 at 16:27, Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
looking at the redness of the output of `make W=1` here is the question: isn't it a good time to enable `make W=1` by default. Yes, I understand the impact, but at least we can do it mandatory for a _new_ code submitted to U-Boot, right?
Ideally I would have what Linux kernel has for a few releases already, i.e. Werror by default and getting close to make a clean builds with that and make W=1` at least against default configurations (yeah, with U-Boot there is probably no default, but sandbox one).
Warnings should be warnings...
Yes, and ideally the code should not have warnings, right?
Otherwise how can we do better? It's quite similar to what you wrote WRT documenting the function prototypes, the same applies to the new contribution WRT `make W=1`.
if you would like to enable it for CI that is fine by me,
Yes, that's the idea, but I'm not the owner of any U-Boot CIs, hence it's a proposal.
You can still do a patch...but I see we already use 'buildman -E' in CI, so perhaps it is already working?
In general, yes, it is.
but the U-Boot makefile shouldn't do it. It defeats the purpose of having a distinction between errors and warnings.
While it's not what I wanted, I disagree on your comment. The idea is to make rules stricter (for new code) to make it better and that's why Linus enabled Werror by default in the Linux kernel. And personally I consider that as a good thing to follow.
I'll note that coreboot enabled this and it is a right pain. Since coreboot always produces copious amounts of pointless output, there is then a warning hidden somewhere in the middle and the build then fails inexplicably. At least with U-Boot we can use -s and only get messages which require user action.
Personally, I find -Werror very helpful when developing code because it means I either (a) made a mistake or (b) forgot to if-out some code I didn't intend. This is also personal preference. But I think "make KCFLAGS=-Werror -sj" is great, personally. And I'll re-run without the -j if I can't see what failed right there.
But anyway, so long as we have this enabled in CI, we are not introducing new warnings. But another point is that we tend to get more warnings when moving to a newer toolchain...and devicetree has tons of warnings now.
It doesn't matter at this point, but perhaps if device tree warnings were errors there'd have been motivation these past many years to (a) resync and (b) fix any number of them. Unfortunately it's like migration notices and just noise to ignore.