* [PATCH 0/4] kconfig: rework symbol help text
@ 2019-12-09 8:19 Thomas Hebb
2019-12-09 8:19 ` Thomas Hebb
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw)
To: linux-kernel, linux-kbuild; +Cc: Thomas Hebb
This series fixes several issues with help text generated by Kconfig,
mainly affecting symbols that are defined in multiple places. Although
results of these patches are somewhat visible for the symbols in Linux,
what prompted me to write the series was working on U-Boot, which also
uses Kconfig and makes very heavy use of multiple definitions (e.g. for
overriding defaults). I have provided Linux examples where I could find
them, but the example for the biggest patch (the first one) is taken
from U-Boot because it was more illustrative than anything I could find
in Linux.
Thomas Hebb (4):
kconfig: list all definitions of a symbol in help text
kconfig: don't crash on NULL expressions in expr_eq()
kconfig: distinguish between dependencies and visibility in help text
kconfig: fix nesting of symbol help text
scripts/kconfig/expr.c | 8 +++--
scripts/kconfig/expr.h | 1 +
scripts/kconfig/menu.c | 75 ++++++++++++++++++++++++------------------
3 files changed, 50 insertions(+), 34 deletions(-)
--
2.24.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1/4] kconfig: list all definitions of a symbol in help text 2019-12-09 8:19 [PATCH 0/4] kconfig: rework symbol help text Thomas Hebb 2019-12-09 8:19 ` Thomas Hebb @ 2019-12-09 8:19 ` Thomas Hebb 2019-12-09 8:19 ` [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text Thomas Hebb ` (2 subsequent siblings) 4 siblings, 0 replies; 12+ messages in thread From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw) To: linux-kernel, linux-kbuild Cc: Thomas Hebb, Masahiro Yamada, Palmer Dabbelt, Paul Walmsley, open list:SIFIVE DRIVERS In Kconfig, each symbol (representing a config option) can be defined in multiple places. Each definition may or may not have a prompt, which allows the option to be set via an interface like menuconfig. Each definition has a set of dependencies, which determine whether its prompt is visible and whether other pieces of the definition, like a default value, take effect. Historically, a symbol's help text (i.e. what's shown when a user presses '?' in menuconfig) contained some symbol-wide information not tied to any particular definition (e.g. what other symbols it selects) as well as the location (file name and line number) and dependencies of each prompt. Notably, the help text did not show the location or dependencies of definitions without prompts. Because this made it hard to reason about symbols that had no prompts, bcdedcc1afd6 ("menuconfig: print more info for symbol without prompts") changed the help text so that, instead of containing the location and dependencies of each prompt, it contained the location and dependencies of the symbol's first definition, regardless of whether or not that definition had a prompt. For symbols with only one definition, that change makes sense. However, it breaks down for symbols with multiple definitions: each definition has its own set of dependencies (the `dep` field of `struct menu`), and those dependencies are ORed together to get the symbol's dependency list (the `dir_dep` field of `struct symbol`). By printing only the dependencies of the first definition, the help text misleads users into believing that an option is more narrowly-applicable than it actually is. For an extreme example of this, we can look at the SYS_TEXT_BASE symbol in the Das U-Boot project, which also uses Kconfig. (I could not find an illustrative example in the Linux source, unfortunately). This config option specifies the load address of the built binary and, as such, is applicable to basically every configuration possible. And yet, without this patch, its help text is as follows: Symbol: SYS_TEXT_BASE [=0x00200000] Type : hex Prompt: Text Base Location: -> Boot images Defined at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=y] && ARCH_ASPEED [=n] The help text indicates that the option only applicable for a specific unselected architecture (aspeed), because that architecture's promptless definition (which just sets a default value), happens to be the first one seen. Because source locations and dependencies are fundamentally properties of definitions and not of symbols, we should treat them as such. This patch brings back the pre-bcdedcc1afd6 behavior for definitions with prompts but also separately prints the location and dependencies of those without prompts, solving the original problem in a different way. With this change, our SYS_TEXT_BASE example becomes Symbol: SYS_TEXT_BASE [=0x00200000] Type : hex Defined with prompt at Kconfig:548 Prompt: Text Base Depends on: !NIOS2 [=n] && !XTENSA [=n] && !EFI_APP [=n] Location: -> Boot images Defined without prompt at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=y] && ARCH_ASPEED [=n] Defined without prompt at arch/arm/mach-socfpga/Kconfig:28 Depends on: ARM [=y] && ARCH_SOCFPGA [=n] <snip> Defined without prompt at board/sifive/fu540/Kconfig:15 Depends on: RISCV [=n] && TARGET_SIFIVE_FU540 [=n] which is a much more accurate representation. Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- scripts/kconfig/menu.c | 49 +++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index d9d16469859a..59fead4b8823 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -698,6 +698,15 @@ const char *menu_get_help(struct menu *menu) return ""; } +static void get_dep_str(struct gstr *r, struct expr *expr, const char *prefix) +{ + if (!expr_is_yes(expr)) { + str_append(r, prefix); + expr_gstr_print(expr, r); + str_append(r, "\n"); + } +} + static void get_prompt_str(struct gstr *r, struct property *prop, struct list_head *head) { @@ -705,7 +714,11 @@ static void get_prompt_str(struct gstr *r, struct property *prop, struct menu *submenu[8], *menu, *location = NULL; struct jump_key *jump = NULL; - str_printf(r, "Prompt: %s\n", prop->text); + str_printf(r, "Defined with prompt at %s:%d\n", + prop->menu->file->name, prop->menu->lineno); + str_printf(r, " Prompt: %s\n", prop->text); + + get_dep_str(r, prop->visible.expr, " Depends on: "); menu = prop->menu->parent; for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent) { bool accessible = menu_is_visible(menu); @@ -755,18 +768,6 @@ static void get_prompt_str(struct gstr *r, struct property *prop, } } -/* - * get property of type P_SYMBOL - */ -static struct property *get_symbol_prop(struct symbol *sym) -{ - struct property *prop = NULL; - - for_all_properties(sym, prop, P_SYMBOL) - break; - return prop; -} - static void get_symbol_props_str(struct gstr *r, struct symbol *sym, enum prop_type tok, const char *prefix) { @@ -806,17 +807,17 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, } } } - for_all_prompts(sym, prop) - get_prompt_str(r, prop, head); - - prop = get_symbol_prop(sym); - if (prop) { - str_printf(r, " Defined at %s:%d\n", prop->menu->file->name, - prop->menu->lineno); - if (!expr_is_yes(prop->visible.expr)) { - str_append(r, " Depends on: "); - expr_gstr_print(prop->visible.expr, r); - str_append(r, "\n"); + + /* Print the definitions with prompts before the ones without */ + for_all_properties(sym, prop, P_SYMBOL) + if (prop->menu->prompt) + get_prompt_str(r, prop->menu->prompt, head); + + for_all_properties(sym, prop, P_SYMBOL) { + if (!prop->menu->prompt) { + str_printf(r, "Defined without prompt at %s:%d\n", + prop->menu->file->name, prop->menu->lineno); + get_dep_str(r, prop->menu->dep, " Depends on: "); } } -- 2.24.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/4] kconfig: list all definitions of a symbol in help text @ 2019-12-09 8:19 ` Thomas Hebb 0 siblings, 0 replies; 12+ messages in thread From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw) To: linux-kernel, linux-kbuild Cc: Thomas Hebb, Masahiro Yamada, Palmer Dabbelt, Paul Walmsley, open list:SIFIVE DRIVERS In Kconfig, each symbol (representing a config option) can be defined in multiple places. Each definition may or may not have a prompt, which allows the option to be set via an interface like menuconfig. Each definition has a set of dependencies, which determine whether its prompt is visible and whether other pieces of the definition, like a default value, take effect. Historically, a symbol's help text (i.e. what's shown when a user presses '?' in menuconfig) contained some symbol-wide information not tied to any particular definition (e.g. what other symbols it selects) as well as the location (file name and line number) and dependencies of each prompt. Notably, the help text did not show the location or dependencies of definitions without prompts. Because this made it hard to reason about symbols that had no prompts, bcdedcc1afd6 ("menuconfig: print more info for symbol without prompts") changed the help text so that, instead of containing the location and dependencies of each prompt, it contained the location and dependencies of the symbol's first definition, regardless of whether or not that definition had a prompt. For symbols with only one definition, that change makes sense. However, it breaks down for symbols with multiple definitions: each definition has its own set of dependencies (the `dep` field of `struct menu`), and those dependencies are ORed together to get the symbol's dependency list (the `dir_dep` field of `struct symbol`). By printing only the dependencies of the first definition, the help text misleads users into believing that an option is more narrowly-applicable than it actually is. For an extreme example of this, we can look at the SYS_TEXT_BASE symbol in the Das U-Boot project, which also uses Kconfig. (I could not find an illustrative example in the Linux source, unfortunately). This config option specifies the load address of the built binary and, as such, is applicable to basically every configuration possible. And yet, without this patch, its help text is as follows: Symbol: SYS_TEXT_BASE [=0x00200000] Type : hex Prompt: Text Base Location: -> Boot images Defined at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=y] && ARCH_ASPEED [=n] The help text indicates that the option only applicable for a specific unselected architecture (aspeed), because that architecture's promptless definition (which just sets a default value), happens to be the first one seen. Because source locations and dependencies are fundamentally properties of definitions and not of symbols, we should treat them as such. This patch brings back the pre-bcdedcc1afd6 behavior for definitions with prompts but also separately prints the location and dependencies of those without prompts, solving the original problem in a different way. With this change, our SYS_TEXT_BASE example becomes Symbol: SYS_TEXT_BASE [=0x00200000] Type : hex Defined with prompt at Kconfig:548 Prompt: Text Base Depends on: !NIOS2 [=n] && !XTENSA [=n] && !EFI_APP [=n] Location: -> Boot images Defined without prompt at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=y] && ARCH_ASPEED [=n] Defined without prompt at arch/arm/mach-socfpga/Kconfig:28 Depends on: ARM [=y] && ARCH_SOCFPGA [=n] <snip> Defined without prompt at board/sifive/fu540/Kconfig:15 Depends on: RISCV [=n] && TARGET_SIFIVE_FU540 [=n] which is a much more accurate representation. Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- scripts/kconfig/menu.c | 49 +++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index d9d16469859a..59fead4b8823 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -698,6 +698,15 @@ const char *menu_get_help(struct menu *menu) return ""; } +static void get_dep_str(struct gstr *r, struct expr *expr, const char *prefix) +{ + if (!expr_is_yes(expr)) { + str_append(r, prefix); + expr_gstr_print(expr, r); + str_append(r, "\n"); + } +} + static void get_prompt_str(struct gstr *r, struct property *prop, struct list_head *head) { @@ -705,7 +714,11 @@ static void get_prompt_str(struct gstr *r, struct property *prop, struct menu *submenu[8], *menu, *location = NULL; struct jump_key *jump = NULL; - str_printf(r, "Prompt: %s\n", prop->text); + str_printf(r, "Defined with prompt at %s:%d\n", + prop->menu->file->name, prop->menu->lineno); + str_printf(r, " Prompt: %s\n", prop->text); + + get_dep_str(r, prop->visible.expr, " Depends on: "); menu = prop->menu->parent; for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent) { bool accessible = menu_is_visible(menu); @@ -755,18 +768,6 @@ static void get_prompt_str(struct gstr *r, struct property *prop, } } -/* - * get property of type P_SYMBOL - */ -static struct property *get_symbol_prop(struct symbol *sym) -{ - struct property *prop = NULL; - - for_all_properties(sym, prop, P_SYMBOL) - break; - return prop; -} - static void get_symbol_props_str(struct gstr *r, struct symbol *sym, enum prop_type tok, const char *prefix) { @@ -806,17 +807,17 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, } } } - for_all_prompts(sym, prop) - get_prompt_str(r, prop, head); - - prop = get_symbol_prop(sym); - if (prop) { - str_printf(r, " Defined at %s:%d\n", prop->menu->file->name, - prop->menu->lineno); - if (!expr_is_yes(prop->visible.expr)) { - str_append(r, " Depends on: "); - expr_gstr_print(prop->visible.expr, r); - str_append(r, "\n"); + + /* Print the definitions with prompts before the ones without */ + for_all_properties(sym, prop, P_SYMBOL) + if (prop->menu->prompt) + get_prompt_str(r, prop->menu->prompt, head); + + for_all_properties(sym, prop, P_SYMBOL) { + if (!prop->menu->prompt) { + str_printf(r, "Defined without prompt at %s:%d\n", + prop->menu->file->name, prop->menu->lineno); + get_dep_str(r, prop->menu->dep, " Depends on: "); } } -- 2.24.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/4] kconfig: list all definitions of a symbol in help text @ 2019-12-09 8:19 ` Thomas Hebb 0 siblings, 0 replies; 12+ messages in thread From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw) To: linux-kernel, linux-kbuild Cc: Masahiro Yamada, open list:SIFIVE DRIVERS, Thomas Hebb, Palmer Dabbelt, Paul Walmsley In Kconfig, each symbol (representing a config option) can be defined in multiple places. Each definition may or may not have a prompt, which allows the option to be set via an interface like menuconfig. Each definition has a set of dependencies, which determine whether its prompt is visible and whether other pieces of the definition, like a default value, take effect. Historically, a symbol's help text (i.e. what's shown when a user presses '?' in menuconfig) contained some symbol-wide information not tied to any particular definition (e.g. what other symbols it selects) as well as the location (file name and line number) and dependencies of each prompt. Notably, the help text did not show the location or dependencies of definitions without prompts. Because this made it hard to reason about symbols that had no prompts, bcdedcc1afd6 ("menuconfig: print more info for symbol without prompts") changed the help text so that, instead of containing the location and dependencies of each prompt, it contained the location and dependencies of the symbol's first definition, regardless of whether or not that definition had a prompt. For symbols with only one definition, that change makes sense. However, it breaks down for symbols with multiple definitions: each definition has its own set of dependencies (the `dep` field of `struct menu`), and those dependencies are ORed together to get the symbol's dependency list (the `dir_dep` field of `struct symbol`). By printing only the dependencies of the first definition, the help text misleads users into believing that an option is more narrowly-applicable than it actually is. For an extreme example of this, we can look at the SYS_TEXT_BASE symbol in the Das U-Boot project, which also uses Kconfig. (I could not find an illustrative example in the Linux source, unfortunately). This config option specifies the load address of the built binary and, as such, is applicable to basically every configuration possible. And yet, without this patch, its help text is as follows: Symbol: SYS_TEXT_BASE [=0x00200000] Type : hex Prompt: Text Base Location: -> Boot images Defined at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=y] && ARCH_ASPEED [=n] The help text indicates that the option only applicable for a specific unselected architecture (aspeed), because that architecture's promptless definition (which just sets a default value), happens to be the first one seen. Because source locations and dependencies are fundamentally properties of definitions and not of symbols, we should treat them as such. This patch brings back the pre-bcdedcc1afd6 behavior for definitions with prompts but also separately prints the location and dependencies of those without prompts, solving the original problem in a different way. With this change, our SYS_TEXT_BASE example becomes Symbol: SYS_TEXT_BASE [=0x00200000] Type : hex Defined with prompt at Kconfig:548 Prompt: Text Base Depends on: !NIOS2 [=n] && !XTENSA [=n] && !EFI_APP [=n] Location: -> Boot images Defined without prompt at arch/arm/mach-aspeed/Kconfig:9 Depends on: ARM [=y] && ARCH_ASPEED [=n] Defined without prompt at arch/arm/mach-socfpga/Kconfig:28 Depends on: ARM [=y] && ARCH_SOCFPGA [=n] <snip> Defined without prompt at board/sifive/fu540/Kconfig:15 Depends on: RISCV [=n] && TARGET_SIFIVE_FU540 [=n] which is a much more accurate representation. Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- scripts/kconfig/menu.c | 49 +++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index d9d16469859a..59fead4b8823 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -698,6 +698,15 @@ const char *menu_get_help(struct menu *menu) return ""; } +static void get_dep_str(struct gstr *r, struct expr *expr, const char *prefix) +{ + if (!expr_is_yes(expr)) { + str_append(r, prefix); + expr_gstr_print(expr, r); + str_append(r, "\n"); + } +} + static void get_prompt_str(struct gstr *r, struct property *prop, struct list_head *head) { @@ -705,7 +714,11 @@ static void get_prompt_str(struct gstr *r, struct property *prop, struct menu *submenu[8], *menu, *location = NULL; struct jump_key *jump = NULL; - str_printf(r, "Prompt: %s\n", prop->text); + str_printf(r, "Defined with prompt at %s:%d\n", + prop->menu->file->name, prop->menu->lineno); + str_printf(r, " Prompt: %s\n", prop->text); + + get_dep_str(r, prop->visible.expr, " Depends on: "); menu = prop->menu->parent; for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent) { bool accessible = menu_is_visible(menu); @@ -755,18 +768,6 @@ static void get_prompt_str(struct gstr *r, struct property *prop, } } -/* - * get property of type P_SYMBOL - */ -static struct property *get_symbol_prop(struct symbol *sym) -{ - struct property *prop = NULL; - - for_all_properties(sym, prop, P_SYMBOL) - break; - return prop; -} - static void get_symbol_props_str(struct gstr *r, struct symbol *sym, enum prop_type tok, const char *prefix) { @@ -806,17 +807,17 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, } } } - for_all_prompts(sym, prop) - get_prompt_str(r, prop, head); - - prop = get_symbol_prop(sym); - if (prop) { - str_printf(r, " Defined at %s:%d\n", prop->menu->file->name, - prop->menu->lineno); - if (!expr_is_yes(prop->visible.expr)) { - str_append(r, " Depends on: "); - expr_gstr_print(prop->visible.expr, r); - str_append(r, "\n"); + + /* Print the definitions with prompts before the ones without */ + for_all_properties(sym, prop, P_SYMBOL) + if (prop->menu->prompt) + get_prompt_str(r, prop->menu->prompt, head); + + for_all_properties(sym, prop, P_SYMBOL) { + if (!prop->menu->prompt) { + str_printf(r, "Defined without prompt at %s:%d\n", + prop->menu->file->name, prop->menu->lineno); + get_dep_str(r, prop->menu->dep, " Depends on: "); } } -- 2.24.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] kconfig: list all definitions of a symbol in help text 2019-12-09 8:19 ` Thomas Hebb @ 2019-12-16 4:49 ` Masahiro Yamada -1 siblings, 0 replies; 12+ messages in thread From: Masahiro Yamada @ 2019-12-16 4:49 UTC (permalink / raw) To: Thomas Hebb Cc: Linux Kernel Mailing List, Linux Kbuild mailing list, Palmer Dabbelt, Paul Walmsley, open list:SIFIVE DRIVERS On Mon, Dec 9, 2019 at 5:19 PM Thomas Hebb <tommyhebb@gmail.com> wrote: > > In Kconfig, each symbol (representing a config option) can be defined in > multiple places. Each definition may or may not have a prompt, which > allows the option to be set via an interface like menuconfig. Each > definition has a set of dependencies, which determine whether its prompt > is visible and whether other pieces of the definition, like a default > value, take effect. > > Historically, a symbol's help text (i.e. what's shown when a user > presses '?' in menuconfig) contained some symbol-wide information not > tied to any particular definition (e.g. what other symbols it selects) > as well as the location (file name and line number) and dependencies of > each prompt. Notably, the help text did not show the location or > dependencies of definitions without prompts. > > Because this made it hard to reason about symbols that had no prompts, > bcdedcc1afd6 ("menuconfig: print more info for symbol without prompts") > changed the help text so that, instead of containing the location and > dependencies of each prompt, it contained the location and dependencies > of the symbol's first definition, regardless of whether or not that > definition had a prompt. > > For symbols with only one definition, that change makes sense. However, > it breaks down for symbols with multiple definitions: each definition > has its own set of dependencies (the `dep` field of `struct menu`), and > those dependencies are ORed together to get the symbol's dependency list > (the `dir_dep` field of `struct symbol`). By printing only the > dependencies of the first definition, the help text misleads users into > believing that an option is more narrowly-applicable than it actually > is. > > For an extreme example of this, we can look at the SYS_TEXT_BASE symbol > in the Das U-Boot project, which also uses Kconfig. (I could not find an "Das U-Boot" is a moving reference. Could you explicitly say the release version (e.g. v2019.10) from which you took the example? > illustrative example in the Linux source, unfortunately). This config > option specifies the load address of the built binary and, as such, is > applicable to basically every configuration possible. And yet, without > this patch, its help text is as follows: > > Symbol: SYS_TEXT_BASE [=0x00200000] > Type : hex > Prompt: Text Base > Location: > -> Boot images > Defined at arch/arm/mach-aspeed/Kconfig:9 > Depends on: ARM [=y] && ARCH_ASPEED [=n] > > The help text indicates that the option only applicable for a specific > unselected architecture (aspeed), because that architecture's promptless > definition (which just sets a default value), happens to be the first > one seen. > > Because source locations and dependencies are fundamentally properties > of definitions and not of symbols, we should treat them as such. This > patch brings back the pre-bcdedcc1afd6 behavior for definitions with > prompts but also separately prints the location and dependencies of > those without prompts, solving the original problem in a different way. > With this change, our SYS_TEXT_BASE example becomes > > Symbol: SYS_TEXT_BASE [=0x00200000] > Type : hex > Defined with prompt at Kconfig:548 > Prompt: Text Base > Depends on: !NIOS2 [=n] && !XTENSA [=n] && !EFI_APP [=n] > Location: > -> Boot images > Defined without prompt at arch/arm/mach-aspeed/Kconfig:9 > Depends on: ARM [=y] && ARCH_ASPEED [=n] > Defined without prompt at arch/arm/mach-socfpga/Kconfig:28 > Depends on: ARM [=y] && ARCH_SOCFPGA [=n] > <snip> > Defined without prompt at board/sifive/fu540/Kconfig:15 > Depends on: RISCV [=n] && TARGET_SIFIVE_FU540 [=n] > > which is a much more accurate representation. This is nice improvement (fix). Just a nit about the help format. I think "with prompt" / "without prompt" is redundant information, and a bit annoying. For the definition "with prompt", the next line is always " Prompt: ... ". For the definition "without prompt", the " Prompt: ... " line is missing. So, we can know the presence of the prompt, anyway. To simplify the for-loop, how about the code like this? /* Print the definitions with prompts before the ones without */ for_all_properties(sym, prop, P_SYMBOL) { str_printf(r, "Defined at %s:%d\n", prop->menu->file->name, prop->menu->lineno); if (prop->menu->prompt) get_prompt_str(r, prop->menu->prompt, head); else get_dep_str(r, prop->menu->dep, " Depends on: "); } -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] kconfig: list all definitions of a symbol in help text @ 2019-12-16 4:49 ` Masahiro Yamada 0 siblings, 0 replies; 12+ messages in thread From: Masahiro Yamada @ 2019-12-16 4:49 UTC (permalink / raw) To: Thomas Hebb Cc: open list:SIFIVE DRIVERS, Paul Walmsley, Palmer Dabbelt, Linux Kernel Mailing List, Linux Kbuild mailing list On Mon, Dec 9, 2019 at 5:19 PM Thomas Hebb <tommyhebb@gmail.com> wrote: > > In Kconfig, each symbol (representing a config option) can be defined in > multiple places. Each definition may or may not have a prompt, which > allows the option to be set via an interface like menuconfig. Each > definition has a set of dependencies, which determine whether its prompt > is visible and whether other pieces of the definition, like a default > value, take effect. > > Historically, a symbol's help text (i.e. what's shown when a user > presses '?' in menuconfig) contained some symbol-wide information not > tied to any particular definition (e.g. what other symbols it selects) > as well as the location (file name and line number) and dependencies of > each prompt. Notably, the help text did not show the location or > dependencies of definitions without prompts. > > Because this made it hard to reason about symbols that had no prompts, > bcdedcc1afd6 ("menuconfig: print more info for symbol without prompts") > changed the help text so that, instead of containing the location and > dependencies of each prompt, it contained the location and dependencies > of the symbol's first definition, regardless of whether or not that > definition had a prompt. > > For symbols with only one definition, that change makes sense. However, > it breaks down for symbols with multiple definitions: each definition > has its own set of dependencies (the `dep` field of `struct menu`), and > those dependencies are ORed together to get the symbol's dependency list > (the `dir_dep` field of `struct symbol`). By printing only the > dependencies of the first definition, the help text misleads users into > believing that an option is more narrowly-applicable than it actually > is. > > For an extreme example of this, we can look at the SYS_TEXT_BASE symbol > in the Das U-Boot project, which also uses Kconfig. (I could not find an "Das U-Boot" is a moving reference. Could you explicitly say the release version (e.g. v2019.10) from which you took the example? > illustrative example in the Linux source, unfortunately). This config > option specifies the load address of the built binary and, as such, is > applicable to basically every configuration possible. And yet, without > this patch, its help text is as follows: > > Symbol: SYS_TEXT_BASE [=0x00200000] > Type : hex > Prompt: Text Base > Location: > -> Boot images > Defined at arch/arm/mach-aspeed/Kconfig:9 > Depends on: ARM [=y] && ARCH_ASPEED [=n] > > The help text indicates that the option only applicable for a specific > unselected architecture (aspeed), because that architecture's promptless > definition (which just sets a default value), happens to be the first > one seen. > > Because source locations and dependencies are fundamentally properties > of definitions and not of symbols, we should treat them as such. This > patch brings back the pre-bcdedcc1afd6 behavior for definitions with > prompts but also separately prints the location and dependencies of > those without prompts, solving the original problem in a different way. > With this change, our SYS_TEXT_BASE example becomes > > Symbol: SYS_TEXT_BASE [=0x00200000] > Type : hex > Defined with prompt at Kconfig:548 > Prompt: Text Base > Depends on: !NIOS2 [=n] && !XTENSA [=n] && !EFI_APP [=n] > Location: > -> Boot images > Defined without prompt at arch/arm/mach-aspeed/Kconfig:9 > Depends on: ARM [=y] && ARCH_ASPEED [=n] > Defined without prompt at arch/arm/mach-socfpga/Kconfig:28 > Depends on: ARM [=y] && ARCH_SOCFPGA [=n] > <snip> > Defined without prompt at board/sifive/fu540/Kconfig:15 > Depends on: RISCV [=n] && TARGET_SIFIVE_FU540 [=n] > > which is a much more accurate representation. This is nice improvement (fix). Just a nit about the help format. I think "with prompt" / "without prompt" is redundant information, and a bit annoying. For the definition "with prompt", the next line is always " Prompt: ... ". For the definition "without prompt", the " Prompt: ... " line is missing. So, we can know the presence of the prompt, anyway. To simplify the for-loop, how about the code like this? /* Print the definitions with prompts before the ones without */ for_all_properties(sym, prop, P_SYMBOL) { str_printf(r, "Defined at %s:%d\n", prop->menu->file->name, prop->menu->lineno); if (prop->menu->prompt) get_prompt_str(r, prop->menu->prompt, head); else get_dep_str(r, prop->menu->dep, " Depends on: "); } -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/4] kconfig: don't crash on NULL expressions in expr_eq() 2019-12-09 8:19 [PATCH 0/4] kconfig: rework symbol help text Thomas Hebb 2019-12-09 8:19 ` Thomas Hebb @ 2019-12-09 8:19 ` Thomas Hebb 2019-12-17 10:20 ` Masahiro Yamada 2019-12-09 8:19 ` [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text Thomas Hebb ` (2 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw) To: linux-kernel, linux-kbuild; +Cc: Thomas Hebb, Masahiro Yamada NULL expressions are taken to always be true, as implemented by the expr_is_yes() macro and by several other functions in expr.c. As such, they ought to be valid inputs to expr_eq(), which compares two expressions. Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- scripts/kconfig/expr.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 77ffff3a053c..8284444cc3fa 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -254,6 +254,11 @@ static int expr_eq(struct expr *e1, struct expr *e2) { int res, old_count; + /* A NULL expr is taken to be yes, but there's also a different way to + * represent yes. expr_is_yes() checks for either representation. */ + if (!e1 || !e2) + return expr_is_yes(e1) && expr_is_yes(e2); + if (e1->type != e2->type) return 0; switch (e1->type) { -- 2.24.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/4] kconfig: don't crash on NULL expressions in expr_eq() 2019-12-09 8:19 ` [PATCH 2/4] kconfig: don't crash on NULL expressions in expr_eq() Thomas Hebb @ 2019-12-17 10:20 ` Masahiro Yamada 0 siblings, 0 replies; 12+ messages in thread From: Masahiro Yamada @ 2019-12-17 10:20 UTC (permalink / raw) To: Thomas Hebb; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list On Mon, Dec 9, 2019 at 5:19 PM Thomas Hebb <tommyhebb@gmail.com> wrote: > > NULL expressions are taken to always be true, as implemented by the > expr_is_yes() macro and by several other functions in expr.c. As such, > they ought to be valid inputs to expr_eq(), which compares two > expressions. > > Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> > --- I applied this patch. I fixed up the block comment style. Thanks. diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 8284444cc3fa..9f1de58e9f0c 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -254,8 +254,10 @@ static int expr_eq(struct expr *e1, struct expr *e2) { int res, old_count; - /* A NULL expr is taken to be yes, but there's also a different way to - * represent yes. expr_is_yes() checks for either representation. */ + /* + * A NULL expr is taken to be yes, but there's also a different way to + * represent yes. expr_is_yes() checks for either representation. + */ if (!e1 || !e2) return expr_is_yes(e1) && expr_is_yes(e2); > scripts/kconfig/expr.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > index 77ffff3a053c..8284444cc3fa 100644 > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > @@ -254,6 +254,11 @@ static int expr_eq(struct expr *e1, struct expr *e2) > { > int res, old_count; > > + /* A NULL expr is taken to be yes, but there's also a different way to > + * represent yes. expr_is_yes() checks for either representation. */ > + if (!e1 || !e2) > + return expr_is_yes(e1) && expr_is_yes(e2); > + > if (e1->type != e2->type) > return 0; > switch (e1->type) { > -- > 2.24.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text 2019-12-09 8:19 [PATCH 0/4] kconfig: rework symbol help text Thomas Hebb 2019-12-09 8:19 ` Thomas Hebb 2019-12-09 8:19 ` [PATCH 2/4] kconfig: don't crash on NULL expressions in expr_eq() Thomas Hebb @ 2019-12-09 8:19 ` Thomas Hebb 2019-12-16 4:50 ` Masahiro Yamada 2019-12-09 8:19 ` [PATCH 4/4] kconfig: fix nesting of symbol " Thomas Hebb 2019-12-16 4:57 ` [PATCH 0/4] kconfig: rework " Masahiro Yamada 4 siblings, 1 reply; 12+ messages in thread From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw) To: linux-kernel, linux-kbuild; +Cc: Thomas Hebb, Masahiro Yamada Kconfig makes a distinction between dependencies (defined by "depends on" expressions and enclosing "if" blocks) and visibility (which includes all dependencies, but also includes inline "if" expressions of individual properties as well as, for prompts, "visible if" expressions of enclosing menus). Before bcdedcc1afd6 ("menuconfig: print more info for symbol without prompts", the "Depends on" lines of a symbol's help text indicated the visibility of the prompt property they appeared under. After bcdedcc1afd, there was always only a single "Depends on" line, which indicated the visibility of the first P_SYMBOL property of the symbol. Since P_SYMBOLs never have inline if expressions, this was in effect the same as the dependencies of the menu item that the P_SYMBOL was attached to. Neither of these situations accurately conveyed the dependencies of a symbol--the first because it was actually the visibility, and the second because it only showed the dependencies from a single definition. Now that we print a "Depends on" line for every definition (regardless of whether or not it has a prompt), we can do better: this patch switches the "Depends on" line for prompts to show the real dependencies of the corresponding menu item and additionally adds a "Visible if" line that shows the visibility only if the visibility is different from the dependencies (which it isn't for most prompts in Linux). Before: Symbol: THUMB2_KERNEL [=n] Type : bool Defined with prompt at arch/arm/Kconfig:1417 Prompt: Compile the kernel in Thumb-2 mode Depends on: (CPU_V7 [=y] || CPU_V7M [=n]) && !CPU_V6 [=n] && !CPU_V6K [=n] && !CPU_THUMBONLY [=n] Location: -> Kernel Features Selects: ARM_UNWIND [=n] After: Symbol: THUMB2_KERNEL [=n] Type : bool Defined with prompt at arch/arm/Kconfig:1417 Prompt: Compile the kernel in Thumb-2 mode Depends on: (CPU_V7 [=y] || CPU_V7M [=n]) && !CPU_V6 [=n] && !CPU_V6K [=n] Visible if: (CPU_V7 [=y] || CPU_V7M [=n]) && !CPU_V6 [=n] && !CPU_V6K [=n] && !CPU_THUMBONLY [=n] Location: -> Kernel Features Selects: ARM_UNWIND [=n] Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- scripts/kconfig/expr.c | 3 +-- scripts/kconfig/expr.h | 1 + scripts/kconfig/menu.c | 12 +++++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 8284444cc3fa..849c574a28d5 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -13,7 +13,6 @@ #define DEBUG_EXPR 0 -static int expr_eq(struct expr *e1, struct expr *e2); static struct expr *expr_eliminate_yn(struct expr *e); struct expr *expr_alloc_symbol(struct symbol *sym) @@ -250,7 +249,7 @@ void expr_eliminate_eq(struct expr **ep1, struct expr **ep2) * equals some operand in the other (operands do not need to appear in the same * order), recursively. */ -static int expr_eq(struct expr *e1, struct expr *e2) +int expr_eq(struct expr *e1, struct expr *e2) { int res, old_count; diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index 017843c9a4f4..d0f17bc9c4ef 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -301,6 +301,7 @@ struct expr *expr_alloc_or(struct expr *e1, struct expr *e2); struct expr *expr_copy(const struct expr *org); void expr_free(struct expr *e); void expr_eliminate_eq(struct expr **ep1, struct expr **ep2); +int expr_eq(struct expr *e1, struct expr *e2); tristate expr_calc_value(struct expr *e); struct expr *expr_trans_bool(struct expr *e); struct expr *expr_eliminate_dups(struct expr *e); diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index 59fead4b8823..4d0542875d70 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -718,7 +718,17 @@ static void get_prompt_str(struct gstr *r, struct property *prop, prop->menu->file->name, prop->menu->lineno); str_printf(r, " Prompt: %s\n", prop->text); - get_dep_str(r, prop->visible.expr, " Depends on: "); + get_dep_str(r, prop->menu->dep, " Depends on: "); + /* Most prompts in Linux have visibility that exactly matches their + * dependencies. For these, we print only the dependencies to improve + * readability. However, prompts with inline "if" expressions and + * prompts with a parent that has a "visible if" expression have + * differing dependencies and visibility. In these rare cases, we + * print both. */ + if (!expr_eq(prop->menu->dep, prop->visible.expr)) { + get_dep_str(r, prop->visible.expr, " Visible if: "); + } + menu = prop->menu->parent; for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent) { bool accessible = menu_is_visible(menu); -- 2.24.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text 2019-12-09 8:19 ` [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text Thomas Hebb @ 2019-12-16 4:50 ` Masahiro Yamada 0 siblings, 0 replies; 12+ messages in thread From: Masahiro Yamada @ 2019-12-16 4:50 UTC (permalink / raw) To: Thomas Hebb; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list On Mon, Dec 9, 2019 at 5:19 PM Thomas Hebb <tommyhebb@gmail.com> wrote: > > Kconfig makes a distinction between dependencies (defined by "depends > on" expressions and enclosing "if" blocks) and visibility (which > includes all dependencies, but also includes inline "if" expressions of > individual properties as well as, for prompts, "visible if" expressions > of enclosing menus). > > Before bcdedcc1afd6 ("menuconfig: print more info for symbol without > prompts", the "Depends on" lines of a symbol's help text indicated the > visibility of the prompt property they appeared under. After > bcdedcc1afd, there was always only a single "Depends on" line, which > indicated the visibility of the first P_SYMBOL property of the symbol. > Since P_SYMBOLs never have inline if expressions, this was in effect the > same as the dependencies of the menu item that the P_SYMBOL was attached > to. > > Neither of these situations accurately conveyed the dependencies of a > symbol--the first because it was actually the visibility, and the second > because it only showed the dependencies from a single definition. Hmm, OK. Commit bcdedcc1afd6 seemed to fix it as a side-effect, but you broke it by 1/4, then fixed it again by 3/4. Sample code: menu "bar" visible if BAR depends on BAZ config FOO bool "foo" endmenu Help of FOO: Before bcdedcc1afd6 ->Depends on: BAZ && BAR After bcdedcc1afd6 -> Depends on: BAZ After 1/4 -> Depends on: BAZ && BAR After 3/4 -> Depends on: BAZ I think "Depends on BAZ" is correct since BAR only affects the visibility of the prompt. In order to not break anything, maybe, does it make sense to re-order, like this? 2/4, 3/4, 1/4, 4/4 > Now that we print a "Depends on" line for every definition (regardless > of whether or not it has a prompt), we can do better: this patch > switches the "Depends on" line for prompts to show the real dependencies > of the corresponding menu item and additionally adds a "Visible if" line > that shows the visibility only if the visibility is different from the > dependencies (which it isn't for most prompts in Linux). > > Before: > > Symbol: THUMB2_KERNEL [=n] > Type : bool > Defined with prompt at arch/arm/Kconfig:1417 > Prompt: Compile the kernel in Thumb-2 mode > Depends on: (CPU_V7 [=y] || CPU_V7M [=n]) && !CPU_V6 [=n] && !CPU_V6K [=n] && !CPU_THUMBONLY [=n] > Location: > -> Kernel Features > Selects: ARM_UNWIND [=n] > > After: > > Symbol: THUMB2_KERNEL [=n] > Type : bool > Defined with prompt at arch/arm/Kconfig:1417 > Prompt: Compile the kernel in Thumb-2 mode > Depends on: (CPU_V7 [=y] || CPU_V7M [=n]) && !CPU_V6 [=n] && !CPU_V6K [=n] > Visible if: (CPU_V7 [=y] || CPU_V7M [=n]) && !CPU_V6 [=n] && !CPU_V6K [=n] && !CPU_THUMBONLY [=n] > Location: > -> Kernel Features > Selects: ARM_UNWIND [=n] > > Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> > --- > scripts/kconfig/expr.c | 3 +-- > scripts/kconfig/expr.h | 1 + > scripts/kconfig/menu.c | 12 +++++++++++- > 3 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > index 8284444cc3fa..849c574a28d5 100644 > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > @@ -13,7 +13,6 @@ > > #define DEBUG_EXPR 0 > > -static int expr_eq(struct expr *e1, struct expr *e2); > static struct expr *expr_eliminate_yn(struct expr *e); > > struct expr *expr_alloc_symbol(struct symbol *sym) > @@ -250,7 +249,7 @@ void expr_eliminate_eq(struct expr **ep1, struct expr **ep2) > * equals some operand in the other (operands do not need to appear in the same > * order), recursively. > */ > -static int expr_eq(struct expr *e1, struct expr *e2) > +int expr_eq(struct expr *e1, struct expr *e2) > { > int res, old_count; > > diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h > index 017843c9a4f4..d0f17bc9c4ef 100644 > --- a/scripts/kconfig/expr.h > +++ b/scripts/kconfig/expr.h > @@ -301,6 +301,7 @@ struct expr *expr_alloc_or(struct expr *e1, struct expr *e2); > struct expr *expr_copy(const struct expr *org); > void expr_free(struct expr *e); > void expr_eliminate_eq(struct expr **ep1, struct expr **ep2); > +int expr_eq(struct expr *e1, struct expr *e2); > tristate expr_calc_value(struct expr *e); > struct expr *expr_trans_bool(struct expr *e); > struct expr *expr_eliminate_dups(struct expr *e); > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c > index 59fead4b8823..4d0542875d70 100644 > --- a/scripts/kconfig/menu.c > +++ b/scripts/kconfig/menu.c > @@ -718,7 +718,17 @@ static void get_prompt_str(struct gstr *r, struct property *prop, > prop->menu->file->name, prop->menu->lineno); > str_printf(r, " Prompt: %s\n", prop->text); > > - get_dep_str(r, prop->visible.expr, " Depends on: "); > + get_dep_str(r, prop->menu->dep, " Depends on: "); > + /* Most prompts in Linux have visibility that exactly matches their > + * dependencies. For these, we print only the dependencies to improve > + * readability. However, prompts with inline "if" expressions and > + * prompts with a parent that has a "visible if" expression have > + * differing dependencies and visibility. In these rare cases, we > + * print both. */ > + if (!expr_eq(prop->menu->dep, prop->visible.expr)) { > + get_dep_str(r, prop->visible.expr, " Visible if: "); > + } > + The code looks correct to me. Just a nit: could you fix up the block comment style? /* * Blah, blah... * Blah, blah... */ > menu = prop->menu->parent; > for (i = 0; menu != &rootmenu && i < 8; menu = menu->parent) { > bool accessible = menu_is_visible(menu); > -- > 2.24.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/4] kconfig: fix nesting of symbol help text 2019-12-09 8:19 [PATCH 0/4] kconfig: rework symbol help text Thomas Hebb ` (2 preceding siblings ...) 2019-12-09 8:19 ` [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text Thomas Hebb @ 2019-12-09 8:19 ` Thomas Hebb 2019-12-16 4:57 ` [PATCH 0/4] kconfig: rework " Masahiro Yamada 4 siblings, 0 replies; 12+ messages in thread From: Thomas Hebb @ 2019-12-09 8:19 UTC (permalink / raw) To: linux-kernel, linux-kbuild; +Cc: Thomas Hebb, Masahiro Yamada When we generate the help text of a symbol (e.g. when a user presses '?' in menuconfig), we do two things: 1. We iterate through every prompt that belongs to that symbol, printing its text and its location in the menu tree. 2. We print symbol-wide information that's not linked to a particular prompt, such as what it selects/is selected by and what it implies/is implied by. Each prompt we print for 1 starts with a line that's not indented indicating where the prompt is defined, then continues with indented lines that describe properties of that particular definition. Once we get to 2, however, we print all the global data indented as well! Visually, this makes it look like the symbol-wide data is associated with the last prompt we happened to print rather than the symbol as a whole. Fix this by removing the indentation for symbol-wide information. Before: Symbol: CPU_FREQ [=n] Type : bool Defined with prompt at drivers/cpufreq/Kconfig:4 Prompt: CPU Frequency scaling Location: -> CPU Power Management -> CPU Frequency scaling Selects: SRCU [=n] Selected by [n]: - ARCH_SA1100 [=n] && <choice> After: Symbol: CPU_FREQ [=n] Type : bool Defined with prompt at drivers/cpufreq/Kconfig:4 Prompt: CPU Frequency scaling Location: -> CPU Power Management -> CPU Frequency scaling Selects: SRCU [=n] Selected by [n]: - ARCH_SA1100 [=n] && <choice> Signed-off-by: Thomas Hebb <tommyhebb@gmail.com> --- scripts/kconfig/menu.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index 4d0542875d70..25d836aa60fc 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -831,18 +831,18 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, } } - get_symbol_props_str(r, sym, P_SELECT, " Selects: "); + get_symbol_props_str(r, sym, P_SELECT, "Selects: "); if (sym->rev_dep.expr) { - expr_gstr_print_revdep(sym->rev_dep.expr, r, yes, " Selected by [y]:\n"); - expr_gstr_print_revdep(sym->rev_dep.expr, r, mod, " Selected by [m]:\n"); - expr_gstr_print_revdep(sym->rev_dep.expr, r, no, " Selected by [n]:\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, r, yes, "Selected by [y]:\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, r, mod, "Selected by [m]:\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, r, no, "Selected by [n]:\n"); } - get_symbol_props_str(r, sym, P_IMPLY, " Implies: "); + get_symbol_props_str(r, sym, P_IMPLY, "Implies: "); if (sym->implied.expr) { - expr_gstr_print_revdep(sym->implied.expr, r, yes, " Implied by [y]:\n"); - expr_gstr_print_revdep(sym->implied.expr, r, mod, " Implied by [m]:\n"); - expr_gstr_print_revdep(sym->implied.expr, r, no, " Implied by [n]:\n"); + expr_gstr_print_revdep(sym->implied.expr, r, yes, "Implied by [y]:\n"); + expr_gstr_print_revdep(sym->implied.expr, r, mod, "Implied by [m]:\n"); + expr_gstr_print_revdep(sym->implied.expr, r, no, "Implied by [n]:\n"); } str_append(r, "\n\n"); -- 2.24.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] kconfig: rework symbol help text 2019-12-09 8:19 [PATCH 0/4] kconfig: rework symbol help text Thomas Hebb ` (3 preceding siblings ...) 2019-12-09 8:19 ` [PATCH 4/4] kconfig: fix nesting of symbol " Thomas Hebb @ 2019-12-16 4:57 ` Masahiro Yamada 4 siblings, 0 replies; 12+ messages in thread From: Masahiro Yamada @ 2019-12-16 4:57 UTC (permalink / raw) To: Thomas Hebb; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list On Mon, Dec 9, 2019 at 5:19 PM Thomas Hebb <tommyhebb@gmail.com> wrote: > > This series fixes several issues with help text generated by Kconfig, > mainly affecting symbols that are defined in multiple places. Although > results of these patches are somewhat visible for the symbols in Linux, > what prompted me to write the series was working on U-Boot, which also > uses Kconfig and makes very heavy use of multiple definitions (e.g. for > overriding defaults). I have provided Linux examples where I could find > them, but the example for the biggest patch (the first one) is taken > from U-Boot because it was more illustrative than anything I could find > in Linux. Nice patch set. Thanks for sending it to kbuild ML first (then you or somebody else will backport it to U-Boot) BTW, talking about U-Boot, it abuses the multi-definition feature too much. This always causes broken dependency when U-Boot migrate CONFIG options to Kconfig. :-/ For my arm64 boards, I used CONFIG_POSITION_INDEPENDENT instead of coping with CONFIG_SYS_TEXT_BASE mess... > Thomas Hebb (4): > kconfig: list all definitions of a symbol in help text > kconfig: don't crash on NULL expressions in expr_eq() > kconfig: distinguish between dependencies and visibility in help text > kconfig: fix nesting of symbol help text > > scripts/kconfig/expr.c | 8 +++-- > scripts/kconfig/expr.h | 1 + > scripts/kconfig/menu.c | 75 ++++++++++++++++++++++++------------------ > 3 files changed, 50 insertions(+), 34 deletions(-) > > -- > 2.24.0 > -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-12-17 10:21 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-12-09 8:19 [PATCH 0/4] kconfig: rework symbol help text Thomas Hebb 2019-12-09 8:19 ` [PATCH 1/4] kconfig: list all definitions of a symbol in " Thomas Hebb 2019-12-09 8:19 ` Thomas Hebb 2019-12-09 8:19 ` Thomas Hebb 2019-12-16 4:49 ` Masahiro Yamada 2019-12-16 4:49 ` Masahiro Yamada 2019-12-09 8:19 ` [PATCH 2/4] kconfig: don't crash on NULL expressions in expr_eq() Thomas Hebb 2019-12-17 10:20 ` Masahiro Yamada 2019-12-09 8:19 ` [PATCH 3/4] kconfig: distinguish between dependencies and visibility in help text Thomas Hebb 2019-12-16 4:50 ` Masahiro Yamada 2019-12-09 8:19 ` [PATCH 4/4] kconfig: fix nesting of symbol " Thomas Hebb 2019-12-16 4:57 ` [PATCH 0/4] kconfig: rework " Masahiro Yamada
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.