drivers: console: Kconfig: Remove redundant 'default n' properties

[ulfalizer]

Bool symbols implicitly default to 'n'.

A 'default n' can make sense e.g. in a Kconfig.defconfig file, if you
want to override a 'default y' on the base definition of the symbol. It
isn't used like that on any of these symbols though.

[codecov-io]

Codecov Report

Merging #8577 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8577      +/-   ##
==========================================
- Coverage   63.97%   63.96%   -0.01%     
==========================================
  Files         429      429              
  Lines       41290    41290              
  Branches     6957     6957              
==========================================
- Hits        26414    26413       -1     
- Misses      11719    11720       +1     
  Partials     3157     3157

Impacted Files	Coverage Δ
samples/philosophers/src/main.c	96.87% <0%> (-1.57%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80e02a9...7f2b106. Read the comment docs.

[pfalcon]

Bool symbols implicitly default to 'n'.

This may be obvious to the maintainer of kconfiglib, but not to mere humans, users of Zephyr. I don't think that we should optimize for the tool, but instead for humans, and those "default n" are useful documentation for them.

[ulfalizer]

Bool symbols implicitly default to 'n'.

This may be obvious to the maintainer of kconfiglib, but not to mere humans, users of Zephyr. I don't think that we should optimize for the tool, but instead for humans, and those "default n" are useful documentation for them.

It's inconsistent now, which I think confuses people about how it works ("there's a default n here, but not here... wonder if I need it. Might as well drop it in, just in case...")

I think that's how those default ns got started in Zephyr. Don't see much of them in other projects.

I've updated the auto-generated documentation to say "No defaults. Implicitly defaults to n." as well by the way. That's much less confusing than default n if FOO && BAR && BAZ (which a default n turns into after dependencies have been propagated).

Another motivation is wanting to get rid of Zephyr's prefer-later-defaults behavior and go back to standard Kconfig behavior. That will be smoother to implement with less redundant defaults.

[ulfalizer]

Put another way, with no default ns, it's easy to pick up that that must be the default (that's documented as well, and mentioned in the autogenerated docs now). With a random mix of no defaults and default ns, you're gonna believe there's some significance to them, and walk away thinking that Kconfig is more complex than it actually is.

[pfalcon]

@ulfalizer :

It's inconsistent now, which I think confuses people about how it works ("there's a default n here, but not here... wonder if I need it. Might as well drop it in, just in case...")

Yes, that's how it works, with people. Having too much implicit stuff (to keep in mind) is counter-human. Humans try to deconfuse it with "explicit is better than implicit" principle.

With a random mix of no defaults and default ns, you're gonna believe there's some significance to them, and walk away thinking that Kconfig is more complex than it actually is.

Again, for humans, too much implicit stuff is more complex. Why for example you don't propose an alternative of having explicit "default n" and "default y" everywhere?

Anyway, technical Kconfig-specific reasons you give seems to make sense. But nobody knows about them. You remove "default n", and here're are recent new patches which have them again: https://github.com/zephyrproject-rtos/zephyr/pull/8620/files#diff-29344602dce088feb8b8d20ad0d70ba0R46 , #7118 , etc.

[ulfalizer]

@pfalcon
In this case I think it adds more confusion than it helps. The generated documentation gets confusing too.

I was thinking of turning it into a style warning later. For the moment, it's mostly to investigate all the defaults to see which ones are essential. That will make it easier to turn them around later when getting rid of the prefer-later-defaults behavior.

[pfalcon]

@ulfalizer: But you don't answer the main concern there - that you, effectively, make the changes behind other people's back, so they're not aware of your changes and keep doing it in the "has-been-normal" way.

when getting rid of the prefer-later-defaults behavior

I dread hearing this. We rely on ability to override one setting with another. And it's already worrying that a warning is issued in this case currently. I'm not sure how "defaults" relate to being able to override one value of an option to another, but in my list, directly, so it's all very worrying.

Again, please consider keeping the Zephyr community in loop on your anticipated changes (the standard way to do that is sending RFC mail(s) to the devel mailing list).

[ulfalizer]

@pfalcon
You'll still be able to override properties. It's just going back to the standard Kconfig behavior where earlier defaults override later defaults, instead of the other way around. You can view the Kconfiglib version of the patch here.

Instead of including the Kconfig.defconfig files last, they'd be included first, making the behavior the same as before. All other symbols with multiple defaults would need to have their defaults reversed as well.

I'm still not sure what motivated that patch in the first place. Might've been a Kconfig misunderstanding, or some limitation in the C tools.

Whoever originally added that patch to the C tools forgot to make the behavior consistent for other properties as well, so it's a bit of a mess now. default on choices still prefers early defaults, and earlier ranges are preferred to.

At least @carlescufi and @SebastianBoe are in on the patch-removal thing. We'll make sure to tell people before the switch (unlike the original patch, which was completely undocumented until I documented it :P).

[ulfalizer]

Re. the default ns, it's no biggie if people add some in the meantime. The behavior is the same as before.

[pfalcon]

@ulfalizer : I appreciate your detailed responses. And while your changes look as bordering on causing issues in cases you might not anticipate (e.g. in third-party projects integrating with Zephyr), I have to admit that so far they caused need to clean up stuff and didn't break anything. I trust that it'll be ok too.

My suggestion to try to keep wider developer audience in loop via posting RFCs to the devel mailing list remains though. (You may not even get a reply, but you'll know that you did everything from your side - e.g. after such a post and no objections, you can go and give -1 to new PR introducing "default n").

Anyway, I'm removing my -1 here (especially given that this change was split among few PRs, the rest of which was already merged).

Dismissing based on the discussion and the fact that this change is already done in other places.

[ulfalizer]

@pfalcon
Yeah, third-party projects are a bit of a mess. I was thinking of adding some temporary compatibilty hack for them. Might get tricky though... would need to use different behavior depending on the Kconfig file, and probably assume some stuff.

@SebastianBoe suggested ignoring compatibility and telling people to switch the defaults around before upgrading to the Zephyr version that gets rid of the patch. That would make things a whole lot easier for sure.

[ulfalizer]

Hmm... one pretty simple solution would be to print a warning about the changed behavior whenever a third-party project is detected (should be possible somehow), and make the warning easy to disable (e.g. by touching a .defaults-fixed file).

Could keep the warning around for a release or two. That should be enough for people to see it, even if they don't check the mailing list.

[pfalcon]

@ulfalizer :

Hmm... one pretty simple solution would be to print a warning about the changed behavior whenever a third-party project is detected

To clarify, my reference about 3rd-party projects is related to overriding of defaults rather than the topic of this patch. And in a 3rd-party project, I want to be able to put anything in prj.conf and don't get an error (e.g. because it overrides something which was "set" before). This is especially important, because many 3rd-party projects are interested in the general "OS" nature of Zephyr, not peculiarities like support of particular hardware or hardware's feature. As a third-party software developer, I want to provide my user with the ability to build a project for any hardware supported by Zephyr - and with a single config. (And this config may override option set for a particular hardware in Zephyr.) Hope that clarifies my concern. Anyway, that's a bit offtopic to discuss in this ticket.

[pfalcon]

As I'm set as the only reviewer so far, approving.

[ulfalizer]

@pfalcon
Yep, that will keep working (and Kconfig.defconfig files too). I disabled the "set more than once" warning so that people can override stuff in .config files without generating warnings.

I added some documentation that explains the difference between prj.conf and Kconfig.defconfig files earlier btw, including motivation: http://docs.zephyrproject.org/porting/board_porting.html#setting-configuration-values

Had to reverse-engineer it myself. Not exactly obvious.