Kconfig should not produce warnings

[SebastianBoe]

I believe that the Kconfig warnings that are produced today are creating more confusion for users than they are creating value by assisting users with diagnostic information.

I would suggest to rectify this by following this procedure:

    for warning in warning_types:
        should_treat_as_error = determine_if_the_warning_should_be_treated_as_an_error()
        feasible_to_resolve   = determine_if_it_is_feasible_to_resolve_all_instances_in_testsuite()

        if should_treat_as_error:
            if feasible_to_resolve:
                resolve()
                change_warning_into_an_error()
            else:
                turn_off_warning_for_non_verbose_builds()
        else:
            turn_off_warning_for_non_verbose_builds()

Turning off warnings that should be treated as errors is unfortunate, but I believe that it is better to live with this technical debt than to have the system behave as it does today.

Alternatively, we could simply have Kconfig follow gcc's warning policy. -Werror on sanitycheck, warnings otherwise. But I would prefer it if the CI environment was not different from the desktop environment.

[ulfalizer]

@SebastianBoe
Thinking of any warnings in particular?

Hiding warnings by default would probably make them pile up. My experience has been that people start completely ignoring warnings once there are more than a few, and then there's no going back.

I think it's better to commit to a warning-free build, with checks during testing. It's not a huge deal if something slips through, as long as you test the stuff that can easily be tested.

Particular warnings can be poorly designed though, of course.

[ulfalizer]

The tests could just grep kconfig.py's stderr for "warning" to see if any warnings were generated by the way.

[ulfalizer]

Alternatively, we could simply have Kconfig follow gcc's warning policy. -Werror on sanitycheck, warnings otherwise. But I would prefer it if the CI environment was not different from the desktop environment.

This seems reasonable to me. You don't want to break stuff for users for relatively harmless warnings, but you don't want to integrate new warnings either.

[SebastianBoe]

"I think it's better to commit to a warning-free build"

That may be appropriate.

"Thinking of any warnings in particular?"

One example is redundant CONFIG_BT=y in Bluetooth samples. I'm sure running the testsuite with -Werror will demonstrate many more.

"I think it's better to commit to a warning-free build, with checks during testing. "
Right, that would match how we avoid noise from gcc's warningset. This would resolve this issue.

[ulfalizer]

@SebastianBoe
Yeah, that redundant assignment warning might not be great in all cases. Kinda depends on how you organize your .config fragments, and it just suggests tiny cleanups anyway (since mismatched assignments are caught either way).

If it's more annoying than helpful, it can be disabled with kconf.disable_redun_warnings() (which you added ;)).

[ulfalizer]

@SebastianBoe
I wonder what would be the easiest way to turn all the Kconfig warnings fatal during testing. Could you pass a flag to CMake that gets forwarded to kconfig.py (--fatal-warnings or the like)?

[SebastianBoe]

"If it's more annoying than helpful, it can be disabled with kconf.disable_redun_warnings() (which you added ;))."

Right, so for that particular case, I would say the warning is exposing a design-flaw. This design flaw to be precise: #5454

But presumably it is not feasible to get that fixed. So in this case we must create some technical debt by disabling the warning. Later on we can pay the debt (with interest).

Resolving this issue involves going over every warning produced by sanitycheck and doing a similair analysis.

[ulfalizer]

Yeah, I agree that the best approach is probably to disable it and turn all the other warnings into errors during testing, just to get that out.

Maybe you could make an exception for it and have it always be a warning too, by calling kconf.disable_redun_warnings() during testing, but that might go against the point of the issue...

[SebastianBoe]

"but that might go against the point of the issue..."
Yep.

[SebastianBoe]

"I wonder what would be the easiest way to turn all the Kconfig warnings fatal during testing. Could you pass a flag to CMake that gets forwarded to kconfig.py (--fatal-warnings or the like)?"

Hard to say without digging more into things. But I would think using the same mechanism as we do with gcc makes sense.

To do this with gcc we have this code in sanitycheck:

\t\t-DEXTRA_CFLAGS="-Werror {cflags}"\\

So with kconfig.py we just need to introduce a CMake variable EXTRA_KCONFIG_FLAGS,
and a kconfig.py argument --fatal-warnings.

[ioannisg]

I believe that the Kconfig warnings that are produced today are creating more confusion for users than they are creating value by assisting users with diagnostic information.

I think that, at least, the K-option unsatisfied-dependency warnings are valuable as they help revealing possible build configuration bugs.

[ulfalizer]

@ioannisg
Yeah, wouldn't want to get rid of that one.

I discovered that there are some non-obvious cases with turning that warning into an error though, like a shared configuration fragment setting CONFIG_FLOAT=y, which depends on CONFIG_CPU_HAS_FLOAT. Dunno how tricky that particular one is, but there seems to be more of them...

The new plan is to turn all the easy-to-fix warnings into errors in kconfig.py and have a "whitelist" of warnings that don't trigger errors (but still print the warning). That'd be a good start I think. kconfig.py could always work like that, regardless of whether it's run by scripts/sanitycheck or not.

It's similar to what @SebastianBoe proposed, just with a whitelist.