On Mon, Dec 9, 2019 at 5:19 PM Thomas Hebb <tommyhebb@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
