diff mbox series

[6/6] kconfig: hide irrelevant menu for oldconfig

Message ID 1515567374-12722-6-git-send-email-yamada.masahiro@socionext.com
State Superseded
Headers show
Series [1/6] kconfig: do not call check_conf() for olddefconfig | expand

Commit Message

Masahiro Yamada Jan. 10, 2018, 6:56 a.m. UTC
Historically, "make oldconfig" has changed its behavior several times,
quieter or louder.  (I attached the history below.)  Currently, it is
not as quiet as it should be.  This commit addresses it.

  Test Case
  ---------

---------------------------(Kconfig)----------------------------
menu "menu"

config FOO
        bool "foo"

menu "sub menu"

config BAR
        bool "bar"

endmenu

endmenu

menu "sibling menu"

config BAZ
        bool "baz"

endmenu
----------------------------------------------------------------

---------------------------(.config)----------------------------
CONFIG_BAR=y
CONFIG_BAZ=y
----------------------------------------------------------------

With the Kconfig and .config above, "make silentoldconfig" and
"make oldconfig" work differently as follows:

  $ make silentoldconfig
  scripts/kconfig/conf  --silentoldconfig Kconfig
  *
  * Restart config...
  *
  *
  * menu
  *
  foo (FOO) [N/y] (NEW) y
  #
  # configuration written to .config
  #

  $ make oldconfig
  scripts/kconfig/conf  --oldconfig Kconfig
  *
  * Restart config...
  *
  *
  * menu
  *
  foo (FOO) [N/y] (NEW) y
  *
  * sub menu
  *
  bar (BAR) [Y/n] y
  #
  # configuration written to .config
  #

Both hide "sibling node" since it is irrelevant.  The difference is
that silentoldconfig hides "sub menu" whereas oldconfig does not.
The behavior of silentoldconfig is preferred since the "sub menu"
does not contain any new symbol.

The root cause is in conf().  There are three input modes that can
call conf(); oldaskconfig, oldconfig, and silentoldconfig.

Everytime conf() encounters a menu entry, it calls check_conf() to
check if it contains new symbols.  If no new symbol is found, the
menu is just skipped.

Currently, this happens only when input_mode == silentoldconfig.
The oldaskconfig enters into the check_conf() loop as silentoldconfig,
so oldaskconfig works likewise for the second loop or later, but it
never happens for oldconfig.  So, irrelevant sub-menus are shown for
oldconfig.

Change the test condition to "input_mode != oldaskconfig".  This is
false only for the first loop of oldaskconfig; it must ask the user
all symbols, so no need to call check_conf().

  History of oldconfig
  --------------------

[0] Originally, "make oldconfig" was as loud as "make config"  (It
    showed the entire .config file)

[1] Commit cd9140e1e73a ("kconfig: make oldconfig is now less chatty")
    made oldconfig quieter, but it was still less quieter than
    silentoldconfig.  (oldconfig did not hide sub-menus)

[2] Commit 204c96f60904 ("kconfig: fix silentoldconfig") changed
    the input_mode of oldconfig to "ask_silent" from "ask_new".
    So, oldconfig really became as quiet as silentoldconfig.
    (oldconfig hided irrelevant sub-menus)

[3] Commit 4062f1a4c030 ("kconfig: use long options in conf") made
    oldconfig as loud as [0] due to misconversion.

[4] Commit 14828349719a ("kconfig: fix make oldconfig") addressed
    the misconversion of [3], but it made oldconfig quieter only to
    the same level as [1], not [2].

This commit is restoring the behavior of [2].

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 scripts/kconfig/conf.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
2.7.4

Comments

Ulf Magnusson Jan. 12, 2018, 4:23 p.m. UTC | #1
On Wed, Jan 10, 2018 at 7:56 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Historically, "make oldconfig" has changed its behavior several times,

> quieter or louder.  (I attached the history below.)  Currently, it is

> not as quiet as it should be.  This commit addresses it.

>

>   Test Case

>   ---------

>

> ---------------------------(Kconfig)----------------------------

> menu "menu"

>

> config FOO

>         bool "foo"

>

> menu "sub menu"

>

> config BAR

>         bool "bar"

>

> endmenu

>

> endmenu

>

> menu "sibling menu"

>

> config BAZ

>         bool "baz"

>

> endmenu

> ----------------------------------------------------------------

>

> ---------------------------(.config)----------------------------

> CONFIG_BAR=y

> CONFIG_BAZ=y

> ----------------------------------------------------------------

>

> With the Kconfig and .config above, "make silentoldconfig" and

> "make oldconfig" work differently as follows:

>

>   $ make silentoldconfig

>   scripts/kconfig/conf  --silentoldconfig Kconfig

>   *

>   * Restart config...

>   *

>   *

>   * menu

>   *

>   foo (FOO) [N/y] (NEW) y

>   #

>   # configuration written to .config

>   #

>

>   $ make oldconfig

>   scripts/kconfig/conf  --oldconfig Kconfig

>   *

>   * Restart config...

>   *

>   *

>   * menu

>   *

>   foo (FOO) [N/y] (NEW) y

>   *

>   * sub menu

>   *

>   bar (BAR) [Y/n] y

>   #

>   # configuration written to .config

>   #

>

> Both hide "sibling node" since it is irrelevant.  The difference is

> that silentoldconfig hides "sub menu" whereas oldconfig does not.

> The behavior of silentoldconfig is preferred since the "sub menu"

> does not contain any new symbol.

>

> The root cause is in conf().  There are three input modes that can

> call conf(); oldaskconfig, oldconfig, and silentoldconfig.

>

> Everytime conf() encounters a menu entry, it calls check_conf() to

> check if it contains new symbols.  If no new symbol is found, the

> menu is just skipped.

>

> Currently, this happens only when input_mode == silentoldconfig.

> The oldaskconfig enters into the check_conf() loop as silentoldconfig,

> so oldaskconfig works likewise for the second loop or later, but it

> never happens for oldconfig.  So, irrelevant sub-menus are shown for

> oldconfig.

>

> Change the test condition to "input_mode != oldaskconfig".  This is

> false only for the first loop of oldaskconfig; it must ask the user

> all symbols, so no need to call check_conf().

>

>   History of oldconfig

>   --------------------

>

> [0] Originally, "make oldconfig" was as loud as "make config"  (It

>     showed the entire .config file)

>

> [1] Commit cd9140e1e73a ("kconfig: make oldconfig is now less chatty")

>     made oldconfig quieter, but it was still less quieter than

>     silentoldconfig.  (oldconfig did not hide sub-menus)

>

> [2] Commit 204c96f60904 ("kconfig: fix silentoldconfig") changed

>     the input_mode of oldconfig to "ask_silent" from "ask_new".

>     So, oldconfig really became as quiet as silentoldconfig.

>     (oldconfig hided irrelevant sub-menus)

>

> [3] Commit 4062f1a4c030 ("kconfig: use long options in conf") made

>     oldconfig as loud as [0] due to misconversion.

>

> [4] Commit 14828349719a ("kconfig: fix make oldconfig") addressed

>     the misconversion of [3], but it made oldconfig quieter only to

>     the same level as [1], not [2].

>

> This commit is restoring the behavior of [2].

>

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

>

>  scripts/kconfig/conf.c | 5 ++---

>  1 file changed, 2 insertions(+), 3 deletions(-)

>

> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c

> index 1d2ed3e..8b9cdf4 100644

> --- a/scripts/kconfig/conf.c

> +++ b/scripts/kconfig/conf.c

> @@ -368,8 +368,7 @@ static void conf(struct menu *menu)

>

>                 switch (prop->type) {

>                 case P_MENU:

> -                       if (input_mode == silentoldconfig &&

> -                           rootEntry != menu) {

> +                       if (input_mode != oldaskconfig && rootEntry != menu) {

>                                 check_conf(menu);

>                                 return;

>                         }

> @@ -660,7 +659,7 @@ int main(int ac, char **av)

>         case oldaskconfig:

>                 rootEntry = &rootmenu;

>                 conf(&rootmenu);

> -               input_mode = silentoldconfig;

> +               input_mode = oldconfig;

>                 /* fall through */

>         case oldconfig:

>         case listnewconfig:

> --

> 2.7.4

>


Looks good to me.

Maybe a comment along the following lines would be nice for
check_conf() as well:

/*
 * Recursively process modifiable symbols (and choices) in 'menu' that don't
 * have a user value
 */

check_conf() could be renamed to something like
process_new_modifiable_syms() as well.

The check_conf() call in conf() could have this above it as well:

/*
 * Except in oldaskconfig mode, we're only interested in new modifiable symbols
 */

Acked-by: Ulf Magnusson <ulfalizer@gmail.com>


Cheers,
Ulf
diff mbox series

Patch

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 1d2ed3e..8b9cdf4 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -368,8 +368,7 @@  static void conf(struct menu *menu)
 
 		switch (prop->type) {
 		case P_MENU:
-			if (input_mode == silentoldconfig &&
-			    rootEntry != menu) {
+			if (input_mode != oldaskconfig && rootEntry != menu) {
 				check_conf(menu);
 				return;
 			}
@@ -660,7 +659,7 @@  int main(int ac, char **av)
 	case oldaskconfig:
 		rootEntry = &rootmenu;
 		conf(&rootmenu);
-		input_mode = silentoldconfig;
+		input_mode = oldconfig;
 		/* fall through */
 	case oldconfig:
 	case listnewconfig: