non-visible options vs menuconfigs

PATCH Mike Frysinger 3 participants 7 messages Apr 22, 2013 Kbuild

Share & tools

Mike Frysinger

author

2013-04-22 16:51 UTC #0 lore raw

the current EXPERT menuconfig is broken by some new options that happen to be sprinkled into the wrong place. seems like if a node is unprintable, it should get skipped for menuconfig purposes ? otherwise, this is a constantly losing battle where someone inserts new Kconfig options and forgets this nuance, and then it stays broken for a while until someone notices. this particular bug wrt EXPERT has been linux-3.2.

for example, in the General setup section, you currently see:

	[ ] Configure standard kernel features (expert users)  --->
	[ ] Embedded system

if you enable EXPERT there, the options get dumped into the same level instead of being under that menuconfig:
[*] Configure standard kernel features (expert users) ---> [ ] Sysctl syscall support
[*] Load all symbols for debugging/ksymoops
...
[ ] Embedded system

is this feasible in the kconfig code ?
-mike

init/Kconfig +29 ~~-29~~

@@ -1177,6 +1177,35 @@ config SYSCTL
 config ANON_INODES
 	bool
 
+config HAVE_UID16
+	bool
+
+config SYSCTL_EXCEPTION_TRACE
+	bool
+	help
+	  Enable support for /proc/sys/debug/exception-trace.
+
+config SYSCTL_ARCH_UNALIGN_NO_WARN
+	bool
+	help
+	  Enable support for /proc/sys/kernel/ignore-unaligned-usertrap
+	  Allows arch to define/use @no_unaligned_warning to possibly warn
+	  about unaligned access emulation going on under the hood.
+
+config SYSCTL_ARCH_UNALIGN_ALLOW
+	bool
+	help
+	  Enable support for /proc/sys/kernel/unaligned-trap
+	  Allows arches to define/use @unaligned_enabled to runtime toggle
+	  the unaligned access emulation.
+	  see arch/parisc/kernel/unaligned.c for reference
+
+config HOTPLUG
+	def_bool y
+
+config HAVE_PCSPKR_PLATFORM
+	bool
+
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
 	# Unhide debug options, to make the on-by-default options visible

@@ -1187,9 +1216,6 @@ menuconfig EXPERT
           environments which can tolerate a "non-standard" kernel.
           Only use this if you really know what you are doing.
 
-config HAVE_UID16
-	bool
-
 config UID16
 	bool "Enable 16-bit UID system calls" if EXPERT
 	depends on HAVE_UID16

@@ -1214,26 +1240,6 @@ config SYSCTL_SYSCALL
 
 	  If unsure say N here.
 
-config SYSCTL_EXCEPTION_TRACE
-	bool
-	help
-	  Enable support for /proc/sys/debug/exception-trace.
-
-config SYSCTL_ARCH_UNALIGN_NO_WARN
-	bool
-	help
-	  Enable support for /proc/sys/kernel/ignore-unaligned-usertrap
-	  Allows arch to define/use @no_unaligned_warning to possibly warn
-	  about unaligned access emulation going on under the hood.
-
-config SYSCTL_ARCH_UNALIGN_ALLOW
-	bool
-	help
-	  Enable support for /proc/sys/kernel/unaligned-trap
-	  Allows arches to define/use @unaligned_enabled to runtime toggle
-	  the unaligned access emulation.
-	  see arch/parisc/kernel/unaligned.c for reference
-
 config KALLSYMS
 	 bool "Load all symbols for debugging/ksymoops" if EXPERT
 	 default y

@@ -1259,9 +1265,6 @@ config KALLSYMS_ALL
 
 	   Say N unless you really need all symbols.
 
-config HOTPLUG
-	def_bool y
-
 config PRINTK
 	default y
 	bool "Enable support for printk" if EXPERT

@@ -1300,9 +1303,6 @@ config PCSPKR_PLATFORM
           This option allows to disable the internal PC-Speaker
           support, saving some memory.
 
-config HAVE_PCSPKR_PLATFORM
-	bool
-
 config BASE_FULL
 	default y
 	bool "Enable full-sized data structures for core" if EXPERT

Randy Dunlap

Acked-by

2013-04-22 17:08 UTC #1 lore raw

Quoted 6 lines · Mike Frysinger

the current EXPERT menuconfig is broken by some new options that happen to be sprinkled into the wrong place. seems like if a node is unprintable, it should get skipped for menuconfig purposes ? otherwise, this is a constantly losing battle where someone inserts new Kconfig options and forgets this nuance, and then it stays broken for a while until someone notices. this particular bug wrt EXPERT has been linux-3.2.

I only noticed a few days ago and then forgot to send a patch for it.

Quoted 12 lines

for example, in the General setup section, you currently see:
	[ ] Configure standard kernel features (expert users)  --->
	[ ] Embedded system
if you enable EXPERT there, the options get dumped into the same level instead of being under that menuconfig:
[*] Configure standard kernel features (expert users) ---> [ ] Sysctl syscall support
[*] Load all symbols for debugging/ksymoops
...
[ ] Embedded system

Quoted 1 line

is this feasible in the kconfig code ?

A kconfig fix would be very nice.

for the patch below:

Acked-by Randy Dunlap <[email protected]>

Thanks.

Quoted 98 lines

(quoted diff) +0 -1

-mike

init/Kconfig +29 ~~-29~~

@@ -1177,6 +1177,35 @@ config SYSCTL
 config ANON_INODES
 	bool
 
+config HAVE_UID16
+	bool
+
+config SYSCTL_EXCEPTION_TRACE
+	bool
+	help
+	  Enable support for /proc/sys/debug/exception-trace.
+
+config SYSCTL_ARCH_UNALIGN_NO_WARN
+	bool
+	help
+	  Enable support for /proc/sys/kernel/ignore-unaligned-usertrap
+	  Allows arch to define/use @no_unaligned_warning to possibly warn
+	  about unaligned access emulation going on under the hood.
+
+config SYSCTL_ARCH_UNALIGN_ALLOW
+	bool
+	help
+	  Enable support for /proc/sys/kernel/unaligned-trap
+	  Allows arches to define/use @unaligned_enabled to runtime toggle
+	  the unaligned access emulation.
+	  see arch/parisc/kernel/unaligned.c for reference
+
+config HOTPLUG
+	def_bool y
+
+config HAVE_PCSPKR_PLATFORM
+	bool
+
 menuconfig EXPERT
 	bool "Configure standard kernel features (expert users)"
 	# Unhide debug options, to make the on-by-default options visible

@@ -1187,9 +1216,6 @@ menuconfig EXPERT
           environments which can tolerate a "non-standard" kernel.
           Only use this if you really know what you are doing.
 
-config HAVE_UID16
-	bool
-
 config UID16
 	bool "Enable 16-bit UID system calls" if EXPERT
 	depends on HAVE_UID16

@@ -1214,26 +1240,6 @@ config SYSCTL_SYSCALL
 
 	  If unsure say N here.
 
-config SYSCTL_EXCEPTION_TRACE
-	bool
-	help
-	  Enable support for /proc/sys/debug/exception-trace.
-
-config SYSCTL_ARCH_UNALIGN_NO_WARN
-	bool
-	help
-	  Enable support for /proc/sys/kernel/ignore-unaligned-usertrap
-	  Allows arch to define/use @no_unaligned_warning to possibly warn
-	  about unaligned access emulation going on under the hood.
-
-config SYSCTL_ARCH_UNALIGN_ALLOW
-	bool
-	help
-	  Enable support for /proc/sys/kernel/unaligned-trap
-	  Allows arches to define/use @unaligned_enabled to runtime toggle
-	  the unaligned access emulation.
-	  see arch/parisc/kernel/unaligned.c for reference
-
 config KALLSYMS
 	 bool "Load all symbols for debugging/ksymoops" if EXPERT
 	 default y

@@ -1259,9 +1265,6 @@ config KALLSYMS_ALL
 
 	   Say N unless you really need all symbols.
 
-config HOTPLUG
-	def_bool y
-
 config PRINTK
 	default y
 	bool "Enable support for printk" if EXPERT

@@ -1300,9 +1303,6 @@ config PCSPKR_PLATFORM
           This option allows to disable the internal PC-Speaker
           support, saving some memory.
 
-config HAVE_PCSPKR_PLATFORM
-	bool
-
 config BASE_FULL
 	default y
 	bool "Enable full-sized data structures for core" if EXPERT

Signature · 1 line

~Randy

Mike Frysinger

author

2013-04-22 18:03 UTC #2 lore raw

Quoted 2 lines · On Monday 22 April 2013 13:08:37 Randy Dunlap wrote:

for the patch below:
Acked-by Randy Dunlap <[email protected]>

ok, i posted it for real to lkml
-mike

Yann E. MORIN

Reviewed-by

2013-04-22 18:00 UTC #3 lore raw

In reply to Mike Frysinger

Mike, All,

Quoted 11 lines · Mike Frysinger

the current EXPERT menuconfig is broken by some new options that happen to be sprinkled into the wrong place. seems like if a node is unprintable, it should get skipped for menuconfig purposes ? otherwise, this is a constantly losing battle where someone inserts new Kconfig options and forgets this nuance, and then it stays broken for a while until someone notices. this particular bug wrt EXPERT has been linux-3.2.
for example, in the General setup section, you currently see:
	[ ] Configure standard kernel features (expert users)  --->
	[ ] Embedded system

Quoted 7 lines

if you enable EXPERT there, the options get dumped into the same level instead of being under that menuconfig:
[*] Configure standard kernel features (expert users) ---> [ ] Sysctl syscall support
[*] Load all symbols for debugging/ksymoops
...
[ ] Embedded system

Yes, this is indeed disturbing.

Reviewed-by "Yann E. MORIN" <[email protected]>

Quoted 1 line

is this feasible in the kconfig code ?

From Documentation/kbuild/kconfig-language.txt:

menuconfig:
"menuconfig" <symbol>
<config options>

This is similar to the simple config entry above, but it also gives a hint to front ends, that all suboptions should be displayed as a separate list of options.

This text does not explicit what a "sub-options" is. In fact, it is an option that depends on the menuconfig, and that is not separated from it from a non-dependent option. Thus:

menuconfig MENU_A
bool "A"

config OPT_B
bool "B"
depends on MENU_A

config OPT_C
bool "C"
depends on MENU_A

config OPT_D
bool "D"

config OPT_E
bool "E"
depends on MENU_A

OPT_B and OPT_C are sub-options of MENU_A, as they depend on it, and directly follow it. OPT_D is not a sub-option of MENU_A, as it does not depend on it. OPT_E, although it depends on MENU_A, is not a sub-option as it does not directly follow it.

As far as I remember, this has always been the behaviour of menuconfig.

What do you suggest the frontends do in this case, short of re-ordering the options (which I think is not correct) ?

On the other hand, we could consider dependent options as candidates for being sub-options until we see the first non-dependent option with a prompt. But I do not think this is sane behaviour, as as soon a new option with a prompt is added in-between sub-options, we again break the relation to the menuconfig.

What I always do to ensure consistency is I explicitly use a conditional around the options I consider as sub-options, which forces me to properly order the options in the Kconfig file, eg. (with the same options as above):

menuconfig MENU_A
bool "A"

if MENU_A

config OPT_B
bool "B"

config OPT_C
bool "C"

config OPT_E
bool "E"

endif # MENU_A

config OPT_D
bool "D"

Maybe we could change the semantic of menuconfig so that, for an option to be considered as a sub-option, it shall be enclosed in an 'if' block.

Another option is to explicitly require and 'endmenu' statement to close the menuconfig.

Either way, that would mean we'd have to audit and fix quite a bunch of Kconfig files:

    $ grep --exclude-dir=.git --exclude-dir=Documentation \
           -r -E -e '^menuconfig' .                       \
      |wc -l
    215

So, 215 occurences of 'menuconfig' (in 189 Kconfig files).

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Mike Frysinger

author

2013-04-22 18:19 UTC #4 lore raw

Quoted 23 lines · On Monday 22 April 2013 14:00:46 Yann E. MORIN wrote:

Quoted 15 lines · Mike Frysinger
the current EXPERT menuconfig is broken by some new options that happen to be sprinkled into the wrong place. seems like if a node is unprintable, it should get skipped for menuconfig purposes ? otherwise, this is a constantly losing battle where someone inserts new Kconfig options and forgets this nuance, and then it stays broken for a while until someone notices. this particular bug wrt EXPERT has been linux-3.2.
for example, in the General setup section, you currently see:
	[ ] Configure standard kernel features (expert users)  --->
	[ ] Embedded system
if you enable EXPERT there, the options get dumped into the same level instead
Quoted 6 lines
of being under that menuconfig:
[*] Configure standard kernel features (expert users) ---> [ ] Sysctl syscall support
[*] Load all symbols for debugging/ksymoops
...
[ ] Embedded system

Quoted 7 lines

Yes, this is indeed disturbing.
Reviewed-by "Yann E. MORIN" <[email protected]>
Quoted 1 line
is this feasible in the kconfig code ?
From Documentation/kbuild/kconfig-language.txt:

when i said "kconfig code", i was referring to making changes to the parsing source, not the kconfig language.

Quoted 4 lines

As far as I remember, this has always been the behaviour of menuconfig.
What do you suggest the frontends do in this case, short of re-ordering the options (which I think is not correct) ?

i think you missed a critical part of my proposal:
seems like *if a node is unprintable*, it should get skipped for menuconfig purposes

to modify your example, i'm proposing a change for:
menuconfig MENU_A
bool "A"

config OPT_B
bool

config OPT_C
bool "C"
depends on MENU_A

OPT_B is not shown to the user (there is no option text), therefore it should automatically get skipped when searching for options to collapse into the menuconfig MENU_A.

i do agree that if OPT_B had text, then the existing behavior would be unchanged.
-mike

Yann E. MORIN

2013-04-22 20:26 UTC #5 lore raw

Mike, All,

Quoted 9 lines · Mike Frysinger

Quoted 4 lines · On Monday 22 April 2013 14:00:46 Yann E. MORIN wrote:
As far as I remember, this has always been the behaviour of menuconfig.
What do you suggest the frontends do in this case, short of re-ordering the options (which I think is not correct) ?
i think you missed a critical part of my proposal:
seems like *if a node is unprintable*, it should get skipped for menuconfig purposes

Ah, yes, I missed it. And it is part of the solutions I exposed, too:

On the other hand, we could consider dependent options as candidates for being sub-options until we see the first non-dependent option with a prompt.

Quoted 11 lines

to modify your example, i'm proposing a change for:
menuconfig MENU_A
bool "A"
config OPT_B
bool
config OPT_C
bool "C"
depends on MENU_A

Quoted 6 lines

OPT_B is not shown to the user (there is no option text), therefore it should automatically get skipped when searching for options to collapse into the menuconfig MENU_A.
i do agree that if OPT_B had text, then the existing behavior would be unchanged.

And as I pointed out, this would break as soon as an option with a prompt (which you call 'option text') is added in-between. For example, starting with:
menuconfig MENU_A
bool "A"

config OPT_B
bool

config OPT_C
bool "C"
depends on MENU_A

And then modifying into:
menuconfig MENU_A
bool "A"

config OPT_D
bool "D"

config OPT_B
bool

config OPT_C
bool "C"
depends on MENU_A

(which would be trivivaly noticed here, but can be more complex in real life, as you havee seen in the current init/Kconfig).

This would change the semantic of the language anyway, where currently your example disrupts the dependendcy chain. Granted, it looks like a good change, but I think we should not encourage people to be lazy, and we better want them to be careful about what they intend to do, about what they review and ack (myself largely included! :-] ).

I'll see if I can coerce the kconfig parser and frontends (yes, frontends are probably impacted, too) to behave the way we discussed in this thread, and see how good it works when confronted to real life.

Thank you!

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Mike Frysinger

author

2013-04-22 21:12 UTC #6 lore raw

Quoted 32 lines · On Monday 22 April 2013 16:26:49 Yann E. MORIN wrote:

Quoted 9 lines · Mike Frysinger
Quoted 4 lines · On Monday 22 April 2013 14:00:46 Yann E. MORIN wrote:
As far as I remember, this has always been the behaviour of menuconfig.
What do you suggest the frontends do in this case, short of re-ordering the options (which I think is not correct) ?
i think you missed a critical part of my proposal:
seems like *if a node is unprintable*, it should get skipped for menuconfig purposes
Ah, yes, I missed it. And it is part of the solutions I exposed, too:
On the other hand, we could consider dependent options as candidates
for being sub-options until we see the first non-dependent option with a prompt.
Quoted 15 lines
to modify your example, i'm proposing a change for:
menuconfig MENU_A
bool "A"
config OPT_B
bool
config OPT_C
bool "C"
depends on MENU_A
OPT_B is not shown to the user (there is no option text), therefore it should automatically get skipped when searching for options to collapse into the menuconfig MENU_A.

Quoted 6 lines

Quoted 2 lines
i do agree that if OPT_B had text, then the existing behavior would be unchanged.
And as I pointed out, this would break as soon as an option with a prompt (which you call 'option text') is added in-between. For example, starting with:

sure. i'm focusing on the non-displayed case since it wouldn't be a behavioral change, and because that's the only reason that EXPERT broke. there aren't EXPERT & non-EXPERT options there (at least, unintentional). i don't think we should worry about this case because, as you pointed out, we already have kconfig language to enforce the behavior if truly desired.

using the EXPERT case as an example, i don't think we want the re-ordering as this is a general knob. we've got specific items which make more sense when grouped elsewhere, and we've got a "dumping bucket" which the current EXPERT menuconfig is designed to hold.

for example, checkpoint/restore makes more sense when grouped with cgroups/namespace (and it actually appears *before* the EXPERT menuconfig, so you'd have to build the entire tree first before doing possible re- ordering/processing).

another example, the vmcounters/slub debug better fits with the non-expert memory settings (like "choose your allocator").

Quoted 5 lines

This would change the semantic of the language anyway, where currently your example disrupts the dependendcy chain. Granted, it looks like a good change, but I think we should not encourage people to be lazy, and we better want them to be careful about what they intend to do, about what they review and ack (myself largely included! :-] ).

sure, but i think we can segment this. i don't think we should hassle developers with details that make 0 difference to the end result. that's why i think automatically handling options that don't get displayed is OK. but resorting options and possible sticking them in different menus than what the source Kconfig files declare violates KISS.
-mike