zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.02k stars 6.17k forks source link

Kconfig: (All) choices should be named #31824

Open ghost opened 3 years ago

ghost commented 3 years ago

Is your enhancement proposal related to a problem? Please describe. Kconfig allows to select one option from a list using choices, e.g. selecting the C++ standard from a list of C+11,17 & 20. While regular 'config'-options can be modified/amended, e.g. in Kconfig.board or an application-specific Kconfig-file, most choices in zephyr cannot be modified. The reason for that is that choices need to have a name to be referenced in another file but zephyr does not provide a name in most cases.

Describe the solution you'd like Introduce names for existing choices and recommend them for new choice or even make them mandatory. Kconfig/lint.py might be extended to produce a warning in case a name is missing.

Describe alternatives you've considered There are multiple possible workaround, depending on use case. One might directly patch the change into the zephyr repository or use a fork of zephyr with the changes needed. In some cases, simply adding a CONF_FILE might be sufficient.

Additional context Example choice without a name as it is currently in use:

choice
    prompt "C++ Standard"
    default STD_CPP11

config STD_CPP98
    #...
#...
endchoice

The same choice with a name:

choice CPLUSPLUS_STANDARD
    prompt "C++ Standard"
    default STD_CPP11

config STD_CPP98
    #...
#...
endchoice
cfriedt commented 2 years ago

After reading #38293 this sounds like less of an enhancement and more of a bug (albeit one that presents itself at config time). What is the overhead of just adding names for all choices now? Currently 176 Kconfig choice instances are named while 293 kconfig items are unnamed cc @emillindq

emillindq commented 2 years ago

After reading #38293 this sounds like less of an enhancement and more of a bug (albeit one that presents itself at config time). What is the overhead of just adding names for all choices now? Currently 176 Kconfig choice instances are named while 293 kconfig items are unnamed cc @emillindq

In what way is this a bug? Naming Kconfig choices allows "overlaying" and making a default choice according to other Kconfig symbols, which in my experience (so far) works well. Should it be implemented in some other way?

cfriedt commented 2 years ago

After reading #38293 this sounds like less of an enhancement and more of a bug (albeit one that presents itself at config time). What is the overhead of just adding names for all choices now? Currently 176 Kconfig choice instances are named while 293 kconfig items are unnamed cc @emillindq

In what way is this a bug? Naming Kconfig choices allows "overlaying" and making a default choice according to other Kconfig symbols, which in my experience (so far) works well. Should it be implemented in some other way?

@emillindq - you misunderstood me. I am pro-naming kconfig choices. it sounds as though not naming them causes several issues (bugs, but really compile-time bugs) that could be avoided if choices were named. If this ticket fixes a bug, it can make it into LTS, and we would have the preferred method established with LTS v2

emillindq commented 2 years ago

After reading #38293 this sounds like less of an enhancement and more of a bug (albeit one that presents itself at config time). What is the overhead of just adding names for all choices now? Currently 176 Kconfig choice instances are named while 293 kconfig items are unnamed cc @emillindq

In what way is this a bug? Naming Kconfig choices allows "overlaying" and making a default choice according to other Kconfig symbols, which in my experience (so far) works well. Should it be implemented in some other way?

@emillindq - you misunderstood me. I am pro-naming kconfig choices. it sounds as though not naming them causes several issues (bugs, but really compile-time bugs) that could be avoided if choices were named. If this ticket fixes a bug, it can make it into LTS, and we would have the preferred method established with LTS v2

Ah yes I understand what you mean! The way I understand choices is that it's a way to easily tell Kconfig that "one and only one of these symbols shall be defined". I'm just thinking, would it be possible to "select" a choice? I imagine it would require a change in Kconfig script/compiler but I just imagine that's the way a user would like it to be. My entry point to this discussion is that a *.prj conf file for a common app should be as small as possible since it's 100% static. Obviously there would be one default "choice", and then if multiple choices where "selected" from a "select" then it would throw an error. Maybe I'm completely wrong here but in my use case we define our own Kconfig symbols (as in our own "config OUR_BLAHA select FS...) and then select different components of Zephyr

Personally I don't really like the overlayed *.prj files we can see in the samples. One example is the echo_client/server which has a ton of overlays. I feel like the better way would be to have a Kconfig file in the sample which acts as the default entry point Kconfig and then define its own symbols like "USE_TLS", "USE_811154" which in itself "selects" required symbols and then finally includes zephyr/Kconfig. In this case, there would be one prj.conf file where the user woud uncomment desired configurations (or make choices in menuconfig GUI???). And like I said, choices would need to be able to be "selected" from the Konconfig symbol for this to be possible.

In other words, my feedback on this is:

emillindq commented 2 years ago

Forgot to tag @cfriedt