[PATCH 00/14] Add Kconfig unit tests

[PATCH 00/14] Add Kconfig unit tests

[Masahiro Yamada]

I am applying various patches to Kconfig these days.

However, I fear regressions.  I have been thinking of unit-tests.

There are various cryptic parts in Kconfig and corner cases where
it is difficult to notice breakage.  If unit-tests cover those,
I will be able to apply changes more confidently.

So, here is the trial.

After fixing some problems, I will add a basic test framework.
This is based on pytest.  Also, this is written in Python 3.
Python 2 will return in 2020.  So, I believe new python tools should be
written in Python 3.

This is my Python 3 and pytest versions.

$ python3 --version
Python 3.5.2
$ python3 -m pytest --version
This is pytest version 3.4.0, imported from /home/masahiro/.local/lib/python3.5/site-packages/pytest.py

If I use old pytest version, some parts did not work as expected.
If this does not work for you, please consider using newer pytest.

I will brush up the code more and add more test cases to do a better job.
Before proceeding more, I'd like to get consensus for this approach.
If you have an idea for better implementation, comments are appreciated.

How to use?
-----------

Please make sure Python3 and pytest for Python3 is installed on your system.

Then, run "make testconfig"

The result looks like as follows:

masahiro@grover:~/workspace/linux-yamada$ make testconfig
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
python3 -B -m pytest ./scripts/kconfig/tests \
-o cache_dir=/home/masahiro/workspace/linux-yamada/scripts/kconfig/tests/.cache \

==================================================== test session starts =====================================================
platform linux -- Python 3.5.2, pytest-3.3.2, py-1.5.2, pluggy-0.6.0 -- /usr/bin/python3
cachedir: scripts/kconfig/tests/.cache
rootdir: /home/masahiro/workspace/linux-yamada/scripts/kconfig/tests, inifile: pytest.ini
collected 12 items

scripts/kconfig/tests/auto_submenu_creation/__init__.py::test PASSED                                                   [  8%]
scripts/kconfig/tests/choice/__init__.py::test_oldask0 PASSED                                                          [ 16%]
scripts/kconfig/tests/choice/__init__.py::test_oldask1 PASSED                                                          [ 25%]
scripts/kconfig/tests/choice/__init__.py::test_allyes PASSED                                                           [ 33%]
scripts/kconfig/tests/choice/__init__.py::test_allmod PASSED                                                           [ 41%]
scripts/kconfig/tests/choice/__init__.py::test_allno PASSED                                                            [ 50%]
scripts/kconfig/tests/choice/__init__.py::test_alldef PASSED                                                           [ 58%]
scripts/kconfig/tests/choice_value_with_m_dep/__init__.py::test PASSED                                                 [ 66%]
scripts/kconfig/tests/err_recursive_inc/__init__.py::test PASSED                                                       [ 75%]
scripts/kconfig/tests/new_choice_with_dep/__init__.py::test PASSED                                                     [ 83%]
scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py::test PASSED                                                   [ 91%]
scripts/kconfig/tests/warn_recursive_dep/__init__.py::test PASSED                                                      [100%]

================================================= 12 passed in 0.08 seconds ==================================================



Masahiro Yamada (14):
  kconfig: send error messages to stderr
  kconfig: do not write choice values when their dependency becomes n
  kconfig: show '?' prompt even if no help text is available
  kconfig: print additional new line for choice for redirection
  kconfig: remove 'config*' pattern from .gitignnore
  kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON
  kconfig: test: add framework for Kconfig unit-tests
  kconfig: test: add basic 'choice' tests
  kconfig: test: test automatic submenu creation
  kconfig: test: check if new symbols in choice are asked
  kconfig: test: check .config sanity for choice values with unmet dep
  kconfig: test: check visibility of tristate choice values in y choice
  kconfig: test: check if recursive dependencies are detected
  kconfig: test: check if recursive inclusion is detected

 Makefile                                           |   5 +-
 arch/ia64/Makefile                                 |   2 +-
 scripts/kconfig/.gitignore                         |   1 -
 scripts/kconfig/Makefile                           |   8 +
 scripts/kconfig/conf.c                             |  29 +--
 scripts/kconfig/symbol.c                           |   4 +-
 .../kconfig/tests/auto_submenu_creation/Kconfig    |  50 ++++
 .../tests/auto_submenu_creation/__init__.py        |  12 +
 .../tests/auto_submenu_creation/expected_stdout    |  10 +
 scripts/kconfig/tests/choice/Kconfig               |  54 +++++
 scripts/kconfig/tests/choice/__init__.py           |  37 +++
 .../kconfig/tests/choice/alldef_expected_config    |   5 +
 .../kconfig/tests/choice/allmod_expected_config    |   9 +
 scripts/kconfig/tests/choice/allno_expected_config |   5 +
 .../kconfig/tests/choice/allyes_expected_config    |   9 +
 .../kconfig/tests/choice/oldask0_expected_stdout   |  10 +
 scripts/kconfig/tests/choice/oldask1_config        |   2 +
 .../kconfig/tests/choice/oldask1_expected_stdout   |  15 ++
 .../kconfig/tests/choice_value_with_m_dep/Kconfig  |  20 ++
 .../tests/choice_value_with_m_dep/__init__.py      |  15 ++
 .../kconfig/tests/choice_value_with_m_dep/config   |   2 +
 .../tests/choice_value_with_m_dep/expected_config  |   3 +
 .../tests/choice_value_with_m_dep/expected_stdout  |   4 +
 scripts/kconfig/tests/conftest.py                  | 255 +++++++++++++++++++++
 scripts/kconfig/tests/err_recursive_inc/Kconfig    |   1 +
 .../kconfig/tests/err_recursive_inc/Kconfig.inc    |   1 +
 .../kconfig/tests/err_recursive_inc/__init__.py    |  10 +
 .../tests/err_recursive_inc/expected_stderr        |   4 +
 scripts/kconfig/tests/new_choice_with_dep/Kconfig  |  37 +++
 .../kconfig/tests/new_choice_with_dep/__init__.py  |  14 ++
 scripts/kconfig/tests/new_choice_with_dep/config   |   3 +
 .../tests/new_choice_with_dep/expected_stdout      |  10 +
 .../kconfig/tests/no_write_if_dep_unmet/Kconfig    |  14 ++
 .../tests/no_write_if_dep_unmet/__init__.py        |  17 ++
 scripts/kconfig/tests/no_write_if_dep_unmet/config |   1 +
 .../tests/no_write_if_dep_unmet/expected_config    |   5 +
 scripts/kconfig/tests/pytest.ini                   |   6 +
 scripts/kconfig/tests/warn_recursive_dep/Kconfig   |  62 +++++
 .../kconfig/tests/warn_recursive_dep/__init__.py   |  10 +
 .../tests/warn_recursive_dep/expected_stderr       |  33 +++
 scripts/kconfig/zconf.l                            |  27 ++-
 41 files changed, 789 insertions(+), 32 deletions(-)
 create mode 100644 scripts/kconfig/tests/auto_submenu_creation/Kconfig
 create mode 100644 scripts/kconfig/tests/auto_submenu_creation/__init__.py
 create mode 100644 scripts/kconfig/tests/auto_submenu_creation/expected_stdout
 create mode 100644 scripts/kconfig/tests/choice/Kconfig
 create mode 100644 scripts/kconfig/tests/choice/__init__.py
 create mode 100644 scripts/kconfig/tests/choice/alldef_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allmod_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allno_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allyes_expected_config
 create mode 100644 scripts/kconfig/tests/choice/oldask0_expected_stdout
 create mode 100644 scripts/kconfig/tests/choice/oldask1_config
 create mode 100644 scripts/kconfig/tests/choice/oldask1_expected_stdout
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
 create mode 100644 scripts/kconfig/tests/conftest.py
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
 create mode 100644 scripts/kconfig/tests/pytest.ini
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr

-- 
2.7.4

[PATCH 01/14] kconfig: send error messages to stderr

[Masahiro Yamada]

These messages should be directed to stderr.

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

 scripts/kconfig/conf.c  | 18 +++++++++++-------
 scripts/kconfig/zconf.l | 27 +++++++++++++++------------
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 307bc3f..90a76aa2 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -75,9 +75,11 @@ static void strip(char *str)
 static void check_stdin(void)
 {
 	if (!valid_stdin) {
-		printf(_("aborted!\n\n"));
-		printf(_("Console input/output is redirected. "));
-		printf(_("Run 'make oldconfig' to update configuration.\n\n"));
+		fprintf(stderr,
+			_("Aborted!\n"
+			  "Console input/output is redirected.\n"
+			  "Run 'make oldconfig' to update configuration.\n\n")
+			);
 		exit(1);
 	}
 }
@@ -565,7 +567,7 @@ int main(int ac, char **av)
 		}
 	}
 	if (ac == optind) {
-		printf(_("%s: Kconfig file missing\n"), av[0]);
+		fprintf(stderr, _("%s: Kconfig file missing\n"), av[0]);
 		conf_usage(progname);
 		exit(1);
 	}
@@ -590,9 +592,11 @@ int main(int ac, char **av)
 		if (!defconfig_file)
 			defconfig_file = conf_get_default_confname();
 		if (conf_read(defconfig_file)) {
-			printf(_("***\n"
-				"*** Can't find default configuration \"%s\"!\n"
-				"***\n"), defconfig_file);
+			fprintf(stderr,
+				_("***\n"
+				  "*** Can't find default configuration \"%s\"!\n"
+				  "***\n"),
+				defconfig_file);
 			exit(1);
 		}
 		break;
diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
index 07e074d..0ba4900 100644
--- a/scripts/kconfig/zconf.l
+++ b/scripts/kconfig/zconf.l
@@ -184,7 +184,9 @@ n	[A-Za-z0-9_-]
 			append_string(yytext, 1);
 	}
 	\n	{
-		printf("%s:%d:warning: multi-line strings not supported\n", zconf_curname(), zconf_lineno());
+		fprintf(stderr,
+			"%s:%d:warning: multi-line strings not supported\n",
+			zconf_curname(), zconf_lineno());
 		current_file->lineno++;
 		BEGIN(INITIAL);
 		return T_EOL;
@@ -294,7 +296,7 @@ void zconf_initscan(const char *name)
 {
 	yyin = zconf_fopen(name);
 	if (!yyin) {
-		printf("can't find file %s\n", name);
+		fprintf(stderr, "can't find file %s\n", name);
 		exit(1);
 	}
 
@@ -315,8 +317,8 @@ void zconf_nextfile(const char *name)
 	current_buf->state = YY_CURRENT_BUFFER;
 	yyin = zconf_fopen(file->name);
 	if (!yyin) {
-		printf("%s:%d: can't open file \"%s\"\n",
-		    zconf_curname(), zconf_lineno(), file->name);
+		fprintf(stderr, "%s:%d: can't open file \"%s\"\n",
+			zconf_curname(), zconf_lineno(), file->name);
 		exit(1);
 	}
 	yy_switch_to_buffer(yy_create_buffer(yyin, YY_BUF_SIZE));
@@ -325,20 +327,21 @@ void zconf_nextfile(const char *name)
 
 	for (iter = current_file->parent; iter; iter = iter->parent ) {
 		if (!strcmp(current_file->name,iter->name) ) {
-			printf("%s:%d: recursive inclusion detected. "
-			       "Inclusion path:\n  current file : '%s'\n",
-			       zconf_curname(), zconf_lineno(),
-			       zconf_curname());
+			fprintf(stderr,
+				"%s:%d: recursive inclusion detected. "
+				"Inclusion path:\n  current file : '%s'\n",
+				zconf_curname(), zconf_lineno(),
+				zconf_curname());
 			iter = current_file->parent;
 			while (iter && \
 			       strcmp(iter->name,current_file->name)) {
-				printf("  included from: '%s:%d'\n",
-				       iter->name, iter->lineno-1);
+				fprintf(stderr, "  included from: '%s:%d'\n",
+					iter->name, iter->lineno-1);
 				iter = iter->parent;
 			}
 			if (iter)
-				printf("  included from: '%s:%d'\n",
-				       iter->name, iter->lineno+1);
+				fprintf(stderr, "  included from: '%s:%d'\n",
+					iter->name, iter->lineno+1);
 			exit(1);
 		}
 	}
-- 
2.7.4

[PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Masahiro Yamada]

"# CONFIG_... is not set" for choice values are wrongly written into
the .config file if they are once visible, then become invisible later.

  Test case
  ---------

---------------------------(Kconfig)----------------------------
config A
	bool "A"

choice
	prompt "Choice ?"
	depends on A

config CHOICE_B
	bool "Choice B"

config CHOICE_C
	bool "Choice C"

endchoice
----------------------------------------------------------------

---------------------------(.config)----------------------------
CONFIG_A=y
----------------------------------------------------------------

With the Kconfig and .config above,

  $ make config
  scripts/kconfig/conf  --oldaskconfig Kconfig
  *
  * Linux Kernel Configuration
  *
  A (A) [Y/n] n
  #
  # configuration written to .config
  #
  $ cat .config
  #
  # Automatically generated file; DO NOT EDIT.
  # Linux Kernel Configuration
  #
  # CONFIG_A is not set
  # CONFIG_CHOICE_B is not set
  # CONFIG_CHOICE_C is not set

Here,

  # CONFIG_CHOICE_B is not set
  # CONFIG_CHOICE_C is not set

should not be written into the .config file because their dependency
"depends on A" is unmet.

Currently, there is no code that clears SYMBOL_WRITE of choice values.

Clear SYMBOL_WRITE for all symbols in sym_calc_value(), then set it
again after calculating visibility.

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

 scripts/kconfig/symbol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c9123ed..5d6f6b1 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -371,8 +371,7 @@ void sym_calc_value(struct symbol *sym)
 		sym->curr.tri = no;
 		return;
 	}
-	if (!sym_is_choice_value(sym))
-		sym->flags &= ~SYMBOL_WRITE;
+	sym->flags &= ~SYMBOL_WRITE;
 
 	sym_calc_visibility(sym);
 
@@ -385,6 +384,7 @@ void sym_calc_value(struct symbol *sym)
 		if (sym_is_choice_value(sym) && sym->visible == yes) {
 			prop = sym_get_choice_prop(sym);
 			newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
+			sym->flags |= SYMBOL_WRITE;
 		} else {
 			if (sym->visible != no) {
 				/* if the symbol is visible use the user value
-- 
2.7.4

[PATCH 03/14] kconfig: show '?' prompt even if no help text is available

[Masahiro Yamada]

'make config', 'make oldconfig', etc. always receive '?' as a valid
input and show useful information even if no help text is available.

------------------------>8------------------------
foo (FOO) [N/y] (NEW) ?

There is no help available for this option.
Symbol: FOO [=n]
Type  : bool
Prompt: foo
  Defined at Kconfig:1
------------------------>8------------------------

However, '?' is not shown in the prompt if its help text is missing.
Let's show '?' all the time so that the prompt and the behavior match.

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

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

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index 90a76aa2..d346642 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -201,9 +201,7 @@ static int conf_sym(struct menu *menu)
 			printf("/m");
 		if (oldval != yes && sym_tristate_within_range(sym, yes))
 			printf("/y");
-		if (menu_has_help(menu))
-			printf("/?");
-		printf("] ");
+		printf("/?] ");
 		if (!conf_askvalue(sym, sym_get_string_value(sym)))
 			return 0;
 		strip(line);
@@ -305,10 +303,7 @@ static int conf_choice(struct menu *menu)
 			printf("[1]: 1\n");
 			goto conf_childs;
 		}
-		printf("[1-%d", cnt);
-		if (menu_has_help(menu))
-			printf("?");
-		printf("]: ");
+		printf("[1-%d?]: ", cnt);
 		switch (input_mode) {
 		case oldconfig:
 		case silentoldconfig:
-- 
2.7.4

[PATCH 04/14] kconfig: print additional new line for choice for redirection

[Masahiro Yamada]

If stdout is redirected to a file, prompts look differently due to
missing new lines.

Currently, conf_askvalue() takes care of this by putting additional
new line, but conf_choice() does not.  Do likewise so that prompts
after 'choice' look properly.

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

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

diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
index d346642..6ce06c8 100644
--- a/scripts/kconfig/conf.c
+++ b/scripts/kconfig/conf.c
@@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
 		case oldaskconfig:
 			fflush(stdout);
 			xfgets(line, sizeof(line), stdin);
+			if (!tty_stdio)
+				printf("\n");
 			strip(line);
 			if (line[0] == '?') {
 				print_help(menu);
-- 
2.7.4

[PATCH 05/14] kconfig: remove 'config*' pattern from .gitignnore

[Masahiro Yamada]

I could not figure out why this pattern should be ignored.
Checking commit 1e65174a3378 ("Add some basic .gitignore files")
did not help.

Let's remove this pattern, then see if it is really needed.

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

 scripts/kconfig/.gitignore | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/kconfig/.gitignore b/scripts/kconfig/.gitignore
index 51f1c87..a76856e 100644
--- a/scripts/kconfig/.gitignore
+++ b/scripts/kconfig/.gitignore
@@ -1,7 +1,6 @@
 #
 # Generated files
 #
-config*
 *.lex.c
 *.tab.c
 *.tab.h
-- 
2.7.4

[PATCH 06/14] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON

[Masahiro Yamada]

The variable 'PYTHON' allows users to specify a proper executable
name in case the default 'python' does not work.  However, this does
not address the case where both Python 2 and Python 3 scripts are
used in one system.

PEP 394 (https://www.python.org/dev/peps/pep-0394/) provides a
convention for Python scripts portability.  Here is a quotation:

  In order to tolerate differences across platforms, all new code
  that needs to invoke the Python interpreter should not specify
  'python', but rather should specify either 'python2' or 'python3'.
  This distinction should be made in shebangs, when invoking from a
  shell script, when invoking via the system() call, or when invoking
  in any other context.

arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
it should be invoked by 'python2'.

It is legitimate to use 'python' for scripts compatible with both
Python 2 and Python 3, but this is rare (at least I do not see the
case in kernel tree).  You do not need to make efforts to write your
scripts in that way.  Anyway, Python 2 will retire in 2020.

This commit is needed for my new scripts written in Python 3.

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

 Makefile           | 5 +++--
 arch/ia64/Makefile | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 11aff0f..c4e935c 100644
--- a/Makefile
+++ b/Makefile
@@ -384,7 +384,8 @@ GENKSYMS	= scripts/genksyms/genksyms
 INSTALLKERNEL  := installkernel
 DEPMOD		= /sbin/depmod
 PERL		= perl
-PYTHON		= python
+PYTHON2		= python2
+PYTHON3		= python3
 CHECK		= sparse
 
 CHECKFLAGS     := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \
@@ -430,7 +431,7 @@ GCC_PLUGINS_CFLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
 export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBES
-export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE
+export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON2 PYTHON3 UTS_MACHINE
 export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
 
 export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS
diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile
index 2dd7f51..862a2ba 100644
--- a/arch/ia64/Makefile
+++ b/arch/ia64/Makefile
@@ -75,7 +75,7 @@ vmlinux.gz: vmlinux
 	$(Q)$(MAKE) $(build)=$(boot) $@
 
 unwcheck: vmlinux
-	-$(Q)READELF=$(READELF) $(PYTHON) $(srctree)/arch/ia64/scripts/unwcheck.py $<
+	-$(Q)READELF=$(READELF) $(PYTHON2) $(srctree)/arch/ia64/scripts/unwcheck.py $<
 
 archclean:
 	$(Q)$(MAKE) $(clean)=$(boot)
-- 
2.7.4

[PATCH 07/14] kconfig: test: add framework for Kconfig unit-tests

[Masahiro Yamada]

I admit various parts in Kconfig are cryptic and need refactoring,
but at the same time, I fear regressions.

There are several subtle corner cases where it is difficult to notice
breakage.  It is time to add unit-tests.

Here is a simple framework based on pytest.  The conftest.py provides
a fixture useful to run commands such as 'oldaskconfig' etc. and
to compare the resulted .config, stdout, stderr with expectations.

How to add test cases?
----------------------

For each test case, you should create a subdirectory under
scripts/kconfig/tests/ (so test cases are seperated from each other).
Every test case directory must contain the following files:

 - __init__.py: describes test functions
 - Kconfig: the top level Kconfig file for this test

To do a useful job, test cases generally need additional data like
input .config and information about expected results.

How to run tests?
-----------------

You need python3 and pytest.  Then, run "make testconfig".
O= option is supported.  If V=1 is given, details logs during tests
are displayed.

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

 scripts/kconfig/Makefile          |   8 ++
 scripts/kconfig/tests/conftest.py | 255 ++++++++++++++++++++++++++++++++++++++
 scripts/kconfig/tests/pytest.ini  |   6 +
 3 files changed, 269 insertions(+)
 create mode 100644 scripts/kconfig/tests/conftest.py
 create mode 100644 scripts/kconfig/tests/pytest.ini

diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index cb3ec53..c5d1d1a 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -135,6 +135,14 @@ PHONY += tinyconfig
 tinyconfig:
 	$(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
 
+# CHECK: -o cache_dir=<path> working?
+PHONY += testconfig
+testconfig: $(obj)/conf
+	$(PYTHON3) -B -m pytest $(srctree)/$(src)/tests \
+	-o cache_dir=$(abspath $(obj)/tests/.cache) \
+	$(if $(findstring 1,$(KBUILD_VERBOSE)),--capture=no)
+clean-dirs += tests/.cache
+
 # Help text used by make help
 help:
 	@echo  '  config	  - Update current config utilising a line-oriented program'
diff --git a/scripts/kconfig/tests/conftest.py b/scripts/kconfig/tests/conftest.py
new file mode 100644
index 0000000..f0f3237
--- /dev/null
+++ b/scripts/kconfig/tests/conftest.py
@@ -0,0 +1,255 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) 2018 Masahiro Yamada <yamada.masahiro@socionext.com>
+#
+
+import os
+import pytest
+import shutil
+import subprocess
+import tempfile
+
+conf_path = os.path.abspath(os.path.join('scripts', 'kconfig', 'conf'))
+
+class Conf:
+
+    def __init__(self, request):
+        """Create a new Conf object, which is a scripts/kconfig/conf
+        runner and result checker.
+
+        Arguments:
+        request - object to introspect the requesting test module
+        """
+
+        # the directory of the test being run
+        self.test_dir = os.path.dirname(str(request.fspath))
+
+    def __run_conf(self, mode, dot_config=None, out_file='.config',
+                   interactive=False, in_keys=None, extra_env={}):
+        """Run scripts/kconfig/conf
+
+        mode: input mode option (--oldaskconfig, --defconfig=<file> etc.)
+        dot_config: the .config file for input.
+        out_file: file name to contain the output config data.
+        interactive: flag to specify the interactive mode.
+        in_keys: key inputs for interactive modes.
+        extra_env: additional environment.
+        """
+
+        command = [conf_path, mode, 'Kconfig']
+
+        # Override 'srctree' environment to make the test as the top directory
+        extra_env['srctree'] = self.test_dir
+
+        # scripts/kconfig/conf is run in a temporary directory.
+        # This directory is automatically removed when done.
+        with tempfile.TemporaryDirectory() as temp_dir:
+
+            # if .config is given, copy it to the working directory
+            if dot_config:
+                shutil.copyfile(os.path.join(self.test_dir, dot_config),
+                                os.path.join(temp_dir, '.config'))
+
+            ps = subprocess.Popen(command,
+                                  stdin=subprocess.PIPE,
+                                  stdout=subprocess.PIPE,
+                                  stderr=subprocess.PIPE,
+                                  cwd=temp_dir,
+                                  env=dict(os.environ, **extra_env))
+
+            # If user key input is specified, feed it into stdin.
+            if in_keys:
+                ps.stdin.write(in_keys.encode('utf-8'))
+
+            while ps.poll() == None:
+                # For interactive modes such as 'make config', 'make oldconfig',
+                # send 'Enter' key until the program finishes.
+                if interactive:
+                    ps.stdin.write(b'\n')
+
+            self.retcode = ps.returncode
+            self.stdout = ps.stdout.read().decode()
+            self.stderr = ps.stderr.read().decode()
+
+            # Retrieve the resulted config data only when .config is supposed
+            # to exist.  If the command fails, the .config does not exist.
+            # 'make listnewconfig' does not produce .config in the first place.
+            if self.retcode == 0 and out_file:
+                with open(os.path.join(temp_dir, out_file)) as f:
+                    self.config = f.read()
+            else:
+                self.config = None
+
+        # Logging:
+        # Pytest captures the following information by default.  In failure
+        # of tests, the captured log will be displayed.  This will be useful to
+        # figure out what has happened.
+
+        print("command: {}\n".format(' '.join(command)))
+        print("retcode: {}\n".format(self.retcode))
+
+        if dot_config:
+            print("input .config:".format(dot_config))
+
+        print("stdout:")
+        print(self.stdout)
+        print("stderr:")
+        print(self.stderr)
+
+        if self.config is not None:
+            print("output of {}:".format(out_file))
+            print(self.config)
+
+        return self.retcode
+
+    def oldaskconfig(self, dot_config=None, in_keys=None):
+        """Run oldaskconfig (make config)
+
+        dot_config: the .config file for input (optional).
+        in_key: key inputs (optional).
+        """
+        return self.__run_conf('--oldaskconfig', dot_config=dot_config,
+                             interactive=True, in_keys=in_keys)
+
+    def oldconfig(self, dot_config=None, in_keys=None):
+        """Run oldconfig
+
+        dot_config: the .config file for input (optional).
+        in_key: key inputs (optional).
+        """
+        return self.__run_conf('--oldconfig', dot_config=dot_config,
+                             interactive=True, in_keys=in_keys)
+
+    def defconfig(self, defconfig):
+        """Run defconfig
+
+        defconfig: the defconfig file for input.
+        """
+        defconfig_path = os.path.join(self.test_dir, defconfig)
+        return self.__run_conf('--defconfig={}'.format(defconfig_path))
+
+    def olddefconfig(self, dot_config=None):
+        """Run olddefconfig
+
+        dot_config: the .config file for input (optional).
+        """
+        return self.__run_conf('--olddefconfig', dot_config=dot_config)
+
+    def __allconfig(self, foo, all_config):
+        """Run all*config
+
+        all_config: fragment config file for KCONFIG_ALLCONFIG (optional).
+        """
+        if all_config:
+            all_config_path = os.path.join(self.test_dir, all_config)
+            extra_env = {'KCONFIG_ALLCONFIG': all_config_path}
+        else:
+            extra_env = {}
+
+        return self.__run_conf('--all{}config'.format(foo), extra_env=extra_env)
+
+    def allyesconfig(self, all_config=None):
+        """Run allyesconfig
+        """
+        return self.__allconfig('yes', all_config)
+
+    def allmodconfig(self, all_config=None):
+        """Run allmodconfig
+        """
+        return self.__allconfig('mod', all_config)
+
+    def allnoconfig(self, all_config=None):
+        """Run allnoconfig
+        """
+        return self.__allconfig('no', all_config)
+
+    def alldefconfig(self, all_config=None):
+        """Run alldefconfig
+        """
+        return self.__allconfig('def', all_config)
+
+    def savedefconfig(self, dot_config):
+        """Run savedefconfig
+        """
+        return self.__run_conf('--savedefconfig', out_file='defconfig')
+
+    def listnewconfig(self, dot_config=None):
+        """Run listnewconfig
+        """
+        return self.__run_conf('--listnewconfig', dot_config=dot_config,
+                               out_file=None)
+
+    # checkers
+    def __read_and_compare(self, compare, expected):
+        """Compare the result with expectation.
+
+        Arguments:
+        compare: function to compare the result with expectation
+        expected: file that contains the expected data
+        """
+        with open(os.path.join(self.test_dir, expected)) as f:
+            expected_data = f.read()
+        print(expected_data)
+        return compare(self, expected_data)
+
+    def __contains(self, attr, expected):
+        print("{0} is expected to contain '{1}':".format(attr, expected))
+        return self.__read_and_compare(lambda s, e: getattr(s, attr).find(e) >= 0,
+                                       expected)
+
+    def __matches(self, attr, expected):
+        print("{0} is expected to match '{1}':".format(attr, expected))
+        return self.__read_and_compare(lambda s, e: getattr(s, attr) == e,
+                                       expected)
+
+    def config_contains(self, expected):
+        """Check if resulted configuration contains expected data.
+
+        Arguments:
+        expected: file that contains the expected data.
+        """
+        return self.__contains('config', expected)
+
+    def config_matches(self, expected):
+        """Check if resulted configuration exactly matches expected data.
+
+        Arguments:
+        expected: file that contains the expected data.
+        """
+        return self.__matches('config', expected)
+
+    def stdout_contains(self, expected):
+        """Check if resulted stdout contains expected data.
+
+        Arguments:
+        expected: file that contains the expected data.
+        """
+        return self.__contains('stdout', expected)
+
+    def stdout_matches(self, cmp_file):
+        """Check if resulted stdout exactly matches expected data.
+
+        Arguments:
+        expected: file that contains the expected data.
+        """
+        return self.__matches('stdout', expected)
+
+    def stderr_contains(self, expected):
+        """Check if resulted stderr contains expected data.
+
+        Arguments:
+        expected: file that contains the expected data.
+        """
+        return self.__contains('stderr', expected)
+
+    def stderr_matches(self, cmp_file):
+        """Check if resulted stderr exactly matches expected data.
+
+        Arguments:
+        expected: file that contains the expected data.
+        """
+        return self.__matches('stderr', expected)
+
+@pytest.fixture(scope="module")
+def conf(request):
+    return Conf(request)
diff --git a/scripts/kconfig/tests/pytest.ini b/scripts/kconfig/tests/pytest.ini
new file mode 100644
index 0000000..07b94e0
--- /dev/null
+++ b/scripts/kconfig/tests/pytest.ini
@@ -0,0 +1,6 @@
+[pytest]
+addopts = --verbose
+# Pytest requires that test files have unique names, because pytest imports
+# them as top-level modules.  It is silly to prefix or suffix a test file with
+# the directory name that contains it.  Use __init__.py for all test files.
+python_files = __init__.py
-- 
2.7.4

[PATCH 08/14] kconfig: test: add basic 'choice' tests

[Masahiro Yamada]

The calculation of 'choice' is a bit complicated part in Kconfig.

The behavior of 'y' choice is intuitive.  If choice values are tristate,
the choice can be 'm' where each value can be enabled independently.
Also, if a choice is marked as 'optional', the whole choice can be
invisible.

Test basic functionality of choice.

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

 scripts/kconfig/tests/choice/Kconfig               | 54 ++++++++++++++++++++++
 scripts/kconfig/tests/choice/__init__.py           | 37 +++++++++++++++
 .../kconfig/tests/choice/alldef_expected_config    |  5 ++
 .../kconfig/tests/choice/allmod_expected_config    |  9 ++++
 scripts/kconfig/tests/choice/allno_expected_config |  5 ++
 .../kconfig/tests/choice/allyes_expected_config    |  9 ++++
 .../kconfig/tests/choice/oldask0_expected_stdout   | 10 ++++
 scripts/kconfig/tests/choice/oldask1_config        |  2 +
 .../kconfig/tests/choice/oldask1_expected_stdout   | 15 ++++++
 9 files changed, 146 insertions(+)
 create mode 100644 scripts/kconfig/tests/choice/Kconfig
 create mode 100644 scripts/kconfig/tests/choice/__init__.py
 create mode 100644 scripts/kconfig/tests/choice/alldef_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allmod_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allno_expected_config
 create mode 100644 scripts/kconfig/tests/choice/allyes_expected_config
 create mode 100644 scripts/kconfig/tests/choice/oldask0_expected_stdout
 create mode 100644 scripts/kconfig/tests/choice/oldask1_config
 create mode 100644 scripts/kconfig/tests/choice/oldask1_expected_stdout

diff --git a/scripts/kconfig/tests/choice/Kconfig b/scripts/kconfig/tests/choice/Kconfig
new file mode 100644
index 0000000..cc60e9c
--- /dev/null
+++ b/scripts/kconfig/tests/choice/Kconfig
@@ -0,0 +1,54 @@
+config MODULES
+	bool "Enable loadable module support"
+	option modules
+	default y
+
+choice
+	prompt "boolean choice"
+	default BOOL_CHOICE1
+
+config BOOL_CHOICE0
+	bool "choice 0"
+
+config BOOL_CHOICE1
+	bool "choice 1"
+
+endchoice
+
+choice
+	prompt "optional boolean choice"
+	optional
+	default OPT_BOOL_CHOICE1
+
+config OPT_BOOL_CHOICE0
+	bool "choice 0"
+
+config OPT_BOOL_CHOICE1
+	bool "choice 1"
+
+endchoice
+
+choice
+	prompt "tristate choice"
+	default TRI_CHOICE1
+
+config TRI_CHOICE0
+	tristate "choice 0"
+
+config TRI_CHOICE1
+	tristate "choice 1"
+
+endchoice
+
+choice
+	prompt "optional tristate choice"
+	optional
+	default OPT_TRI_CHOICE1
+
+config OPT_TRI_CHOICE0
+	tristate "choice 0"
+
+config OPT_TRI_CHOICE1
+	tristate "choice 1"
+
+endchoice
diff --git a/scripts/kconfig/tests/choice/__init__.py b/scripts/kconfig/tests/choice/__init__.py
new file mode 100644
index 0000000..42022ac
--- /dev/null
+++ b/scripts/kconfig/tests/choice/__init__.py
@@ -0,0 +1,37 @@
+"""
+Basic choice tests
+==================
+
+The handling of 'choice' is a bit complicated part in Kconfig.
+
+The behavior of 'y' choice is intuitive.  If choice values are tristate,
+the choice can be 'm' where each value can be enabled independently.
+Also, if a choice is marked as 'optional', the whole choice can be
+invisible.
+
+Test basic functionality of choice.
+"""
+
+def test_oldask0(conf):
+    assert conf.oldaskconfig() == 0
+    assert conf.stdout_contains('oldask0_expected_stdout')
+
+def test_oldask1(conf):
+    assert conf.oldaskconfig('oldask1_config') == 0
+    assert conf.stdout_contains('oldask1_expected_stdout')
+
+def test_allyes(conf):
+    assert conf.allyesconfig() == 0
+    assert conf.config_contains('allyes_expected_config')
+
+def test_allmod(conf):
+    assert conf.allmodconfig() == 0
+    assert conf.config_contains('allmod_expected_config')
+
+def test_allno(conf):
+    assert conf.allnoconfig() == 0
+    assert conf.config_contains('allno_expected_config')
+
+def test_alldef(conf):
+    assert conf.alldefconfig() == 0
+    assert conf.config_contains('alldef_expected_config')
diff --git a/scripts/kconfig/tests/choice/alldef_expected_config b/scripts/kconfig/tests/choice/alldef_expected_config
new file mode 100644
index 0000000..7a754bf
--- /dev/null
+++ b/scripts/kconfig/tests/choice/alldef_expected_config
@@ -0,0 +1,5 @@
+CONFIG_MODULES=y
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_TRI_CHOICE0 is not set
+# CONFIG_TRI_CHOICE1 is not set
diff --git a/scripts/kconfig/tests/choice/allmod_expected_config b/scripts/kconfig/tests/choice/allmod_expected_config
new file mode 100644
index 0000000..f1f5dcd
--- /dev/null
+++ b/scripts/kconfig/tests/choice/allmod_expected_config
@@ -0,0 +1,9 @@
+CONFIG_MODULES=y
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_OPT_BOOL_CHOICE0 is not set
+CONFIG_OPT_BOOL_CHOICE1=y
+CONFIG_TRI_CHOICE0=m
+CONFIG_TRI_CHOICE1=m
+CONFIG_OPT_TRI_CHOICE0=m
+CONFIG_OPT_TRI_CHOICE1=m
diff --git a/scripts/kconfig/tests/choice/allno_expected_config b/scripts/kconfig/tests/choice/allno_expected_config
new file mode 100644
index 0000000..b88ee7a
--- /dev/null
+++ b/scripts/kconfig/tests/choice/allno_expected_config
@@ -0,0 +1,5 @@
+# CONFIG_MODULES is not set
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_TRI_CHOICE0 is not set
+CONFIG_TRI_CHOICE1=y
diff --git a/scripts/kconfig/tests/choice/allyes_expected_config b/scripts/kconfig/tests/choice/allyes_expected_config
new file mode 100644
index 0000000..e5a062a
--- /dev/null
+++ b/scripts/kconfig/tests/choice/allyes_expected_config
@@ -0,0 +1,9 @@
+CONFIG_MODULES=y
+# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE1=y
+# CONFIG_OPT_BOOL_CHOICE0 is not set
+CONFIG_OPT_BOOL_CHOICE1=y
+# CONFIG_TRI_CHOICE0 is not set
+CONFIG_TRI_CHOICE1=y
+# CONFIG_OPT_TRI_CHOICE0 is not set
+CONFIG_OPT_TRI_CHOICE1=y
diff --git a/scripts/kconfig/tests/choice/oldask0_expected_stdout b/scripts/kconfig/tests/choice/oldask0_expected_stdout
new file mode 100644
index 0000000..b251bba
--- /dev/null
+++ b/scripts/kconfig/tests/choice/oldask0_expected_stdout
@@ -0,0 +1,10 @@
+Enable loadable module support (MODULES) [Y/n/?] (NEW) 
+boolean choice
+  1. choice 0 (BOOL_CHOICE0) (NEW)
+> 2. choice 1 (BOOL_CHOICE1) (NEW)
+choice[1-2?]: 
+optional boolean choice [N/y/?] (NEW) 
+tristate choice [M/y/?] (NEW) 
+  choice 0 (TRI_CHOICE0) [N/m/?] (NEW) 
+  choice 1 (TRI_CHOICE1) [N/m/?] (NEW) 
+optional tristate choice [N/m/y/?] (NEW) 
diff --git a/scripts/kconfig/tests/choice/oldask1_config b/scripts/kconfig/tests/choice/oldask1_config
new file mode 100644
index 0000000..b67bfe3
--- /dev/null
+++ b/scripts/kconfig/tests/choice/oldask1_config
@@ -0,0 +1,2 @@
+# CONFIG_MODULES is not set
+CONFIG_OPT_BOOL_CHOICE0=y
diff --git a/scripts/kconfig/tests/choice/oldask1_expected_stdout b/scripts/kconfig/tests/choice/oldask1_expected_stdout
new file mode 100644
index 0000000..c2125e9b
--- /dev/null
+++ b/scripts/kconfig/tests/choice/oldask1_expected_stdout
@@ -0,0 +1,15 @@
+Enable loadable module support (MODULES) [N/y/?] 
+boolean choice
+  1. choice 0 (BOOL_CHOICE0) (NEW)
+> 2. choice 1 (BOOL_CHOICE1) (NEW)
+choice[1-2?]: 
+optional boolean choice [Y/n/?] (NEW) 
+optional boolean choice
+> 1. choice 0 (OPT_BOOL_CHOICE0)
+  2. choice 1 (OPT_BOOL_CHOICE1) (NEW)
+choice[1-2?]: 
+tristate choice
+  1. choice 0 (TRI_CHOICE0) (NEW)
+> 2. choice 1 (TRI_CHOICE1) (NEW)
+choice[1-2?]: 
+optional tristate choice [N/y/?] 
-- 
2.7.4

[PATCH 09/14] kconfig: test: test automatic submenu creation

[Masahiro Yamada]

If a symbols has dependency on the preceding symbol, the menu entry
should become the submenu of the preceding one, and displayed with
deeper indentation.

This is done by restructuring the menu tree in menu_finalize().
It is a bit complicated computation, so let's add a test case.

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

 .../kconfig/tests/auto_submenu_creation/Kconfig    | 50 ++++++++++++++++++++++
 .../tests/auto_submenu_creation/__init__.py        | 12 ++++++
 .../tests/auto_submenu_creation/expected_stdout    | 10 +++++
 3 files changed, 72 insertions(+)
 create mode 100644 scripts/kconfig/tests/auto_submenu_creation/Kconfig
 create mode 100644 scripts/kconfig/tests/auto_submenu_creation/__init__.py
 create mode 100644 scripts/kconfig/tests/auto_submenu_creation/expected_stdout

diff --git a/scripts/kconfig/tests/auto_submenu_creation/Kconfig b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
new file mode 100644
index 0000000..9e11502
--- /dev/null
+++ b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
@@ -0,0 +1,50 @@
+config A
+	bool "A"
+	default y
+
+config A0
+	bool "A0"
+	depends on A
+	default y
+	help
+	  This depends on A, so should be a submenu of A.
+
+config A0_0
+	bool "A1_0"
+	depends on A0
+	help
+	  Submenus are created recursively.
+	  This should be a submenu of A0.
+
+config A1
+	bool "A1"
+	depends on A
+	default y
+	help
+	  This should line up with A0.
+
+choice
+	prompt "choice"
+	depends on A1
+	help
+	  Choice should become a submenu as well.
+
+config A1_0
+	bool "A1_0"
+
+config A1_1
+	bool "A1_1"
+
+endchoice
+
+config B
+	bool "B"
+	help
+	  This is independent of A.
+
+config C
+	bool "C"
+	depends on A
+	help
+	  This depends on A, but not a consecutive item, so should not
+	  be a submenu.
diff --git a/scripts/kconfig/tests/auto_submenu_creation/__init__.py b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
new file mode 100644
index 0000000..dda866d
--- /dev/null
+++ b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
@@ -0,0 +1,12 @@
+"""
+Create submenu for symbols that depend on the preceding one
+===========================================================
+
+If a symbols has dependency on the preceding symbol, the menu entry
+should become the submenu of the preceding one, and displayed with
+deeper indentation.
+"""
+
+def test(conf):
+    assert conf.oldaskconfig() == 0
+    assert conf.stdout_contains('expected_stdout')
diff --git a/scripts/kconfig/tests/auto_submenu_creation/expected_stdout b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
new file mode 100644
index 0000000..bf5236f
--- /dev/null
+++ b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
@@ -0,0 +1,10 @@
+A (A) [Y/n/?] (NEW) 
+  A0 (A0) [Y/n/?] (NEW) 
+    A1_0 (A0_0) [N/y/?] (NEW) 
+  A1 (A1) [Y/n/?] (NEW) 
+    choice
+    > 1. A1_0 (A1_0) (NEW)
+      2. A1_1 (A1_1) (NEW)
+    choice[1-2?]: 
+B (B) [N/y/?] (NEW) 
+C (C) [N/y/?] (NEW) 
-- 
2.7.4

[PATCH 10/14] kconfig: test: check if new symbols in choice are asked

[Masahiro Yamada]

If new choice values are added with new dependency, and they become
visible during user configuration, oldconfig should recognize them
as (NEW), and ask the user for choice.

This issue was fixed by commit 5d09598d488f ("kconfig: fix new choices
being skipped upon config update").

This is a subtle corner case.  Add a test case to avoid breakage.

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

 scripts/kconfig/tests/new_choice_with_dep/Kconfig  | 37 ++++++++++++++++++++++
 .../kconfig/tests/new_choice_with_dep/__init__.py  | 14 ++++++++
 scripts/kconfig/tests/new_choice_with_dep/config   |  3 ++
 .../tests/new_choice_with_dep/expected_stdout      | 10 ++++++
 4 files changed, 64 insertions(+)
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
 create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout

diff --git a/scripts/kconfig/tests/new_choice_with_dep/Kconfig b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
new file mode 100644
index 0000000..53ef1b8
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
@@ -0,0 +1,37 @@
+config A
+	bool "A"
+	help
+	  This is a new symbol.
+
+choice
+	prompt "Choice ?"
+	depends on A
+	help
+	  "depends on A" has been newly added.
+
+config CHOICE_B
+	bool "Choice B"
+
+config CHOICE_C
+	bool "Choice C"
+	help
+	  This is a new symbol, so should be asked.
+
+endchoice
+
+choice
+	prompt "Choice2 ?"
+
+config CHOICE_D
+	bool "Choice D"
+
+config CHOICE_E
+	bool "Choice E"
+
+config CHOICE_F
+	bool "Choice F"
+	depends on A
+	help
+	  This is a new symbol, so should be asked.
+
+endchoice
diff --git a/scripts/kconfig/tests/new_choice_with_dep/__init__.py b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
new file mode 100644
index 0000000..4306ccf
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
@@ -0,0 +1,14 @@
+"""
+Ask new choice values when they become visible
+==============================================
+
+If new choice values are added with new dependency, and they become
+visible during user configuration, oldconfig should recognize them
+as (NEW), and ask the user for choice.
+
+Related Linux commit: 5d09598d488f081e3be23f885ed65cbbe2d073b5
+"""
+
+def test(conf):
+    assert conf.oldconfig('config', 'y') == 0
+    assert conf.stdout_contains('expected_stdout')
diff --git a/scripts/kconfig/tests/new_choice_with_dep/config b/scripts/kconfig/tests/new_choice_with_dep/config
new file mode 100644
index 0000000..47ef95d
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/config
@@ -0,0 +1,3 @@
+CONFIG_CHOICE_B=y
+# CONFIG_CHOICE_D is not set
+CONFIG_CHOICE_E=y
diff --git a/scripts/kconfig/tests/new_choice_with_dep/expected_stdout b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
new file mode 100644
index 0000000..358d5cf
--- /dev/null
+++ b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
@@ -0,0 +1,10 @@
+A (A) [N/y/?] (NEW) 
+  Choice ?
+  > 1. Choice B (CHOICE_B)
+    2. Choice C (CHOICE_C) (NEW)
+  choice[1-2?]: 
+Choice2 ?
+  1. Choice D (CHOICE_D)
+> 2. Choice E (CHOICE_E)
+  3. Choice F (CHOICE_F) (NEW)
+choice[1-3?]: 
-- 
2.7.4

[PATCH 11/14] kconfig: test: check .config sanity for choice values with unmet dep

[Masahiro Yamada]

I fixed a problem where "# CONFIG_..." for choice values are wrongly
written into the .config file when they are once visible, then become
invisible later.

Add a test for this subtle case.

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

 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig     | 14 ++++++++++++++
 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py | 17 +++++++++++++++++
 scripts/kconfig/tests/no_write_if_dep_unmet/config      |  1 +
 .../kconfig/tests/no_write_if_dep_unmet/expected_config |  5 +++++
 4 files changed, 37 insertions(+)
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
 create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/expected_config

diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
new file mode 100644
index 0000000..c00b8fe
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
@@ -0,0 +1,14 @@
+config A
+	bool "A"
+
+choice
+	prompt "Choice ?"
+	depends on A
+
+config CHOICE_B
+	bool "Choice B"
+
+config CHOICE_C
+	bool "Choice C"
+
+endchoice
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
new file mode 100644
index 0000000..d096622
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
@@ -0,0 +1,17 @@
+"""
+Do not write choice values to .config if the dependency is unmet
+================================================================
+
+"# CONFIG_... is not set" should not be written into the .config file
+for symbols with unmet dependency.
+
+This was not working correctly for choice values because choice needs
+a bit different symbol computation.
+
+This checks that no unneeded "# COFIG_... is not set" is contained in
+the .config file.
+"""
+
+def test(conf):
+    assert conf.oldaskconfig('config', 'n') == 0
+    assert conf.config_matches('expected_config')
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/config b/scripts/kconfig/tests/no_write_if_dep_unmet/config
new file mode 100644
index 0000000..abd280e
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/config
@@ -0,0 +1 @@
+CONFIG_A=y
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
new file mode 100644
index 0000000..0d15e41
--- /dev/null
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
@@ -0,0 +1,5 @@
+#
+# Automatically generated file; DO NOT EDIT.
+# Linux Kernel Configuration
+#
+# CONFIG_A is not set
-- 
2.7.4

[PATCH 12/14] kconfig: test: check visibility of tristate choice values in y choice

[Masahiro Yamada]

If tristate choice values depend on symbols set to 'm', they should be
hidden when the choice containing them is changed from 'm' to 'y'
(i.e. exclusive choice).

This issue was fixed by commit fa64e5f6a35e ("kconfig/symbol.c: handle
choice_values that depend on 'm' symbols").

Add a test case to avoid regression.

For the input in this unit test, there is a room for argument if
"# CONFIG_CHOICE1 is not set" should be written to the .config file.

After commit fa64e5f6a35e, this line was written to the .config file.

Then, it is not written after my fix "kconfig: do not write choice
values when their dependency becomes n".

In this test, "# CONFIG_CHOICE1 is not set" is don't care.

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

 .../kconfig/tests/choice_value_with_m_dep/Kconfig    | 20 ++++++++++++++++++++
 .../tests/choice_value_with_m_dep/__init__.py        | 15 +++++++++++++++
 scripts/kconfig/tests/choice_value_with_m_dep/config |  2 ++
 .../tests/choice_value_with_m_dep/expected_config    |  3 +++
 .../tests/choice_value_with_m_dep/expected_stdout    |  4 ++++
 5 files changed, 44 insertions(+)
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_config
 create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout

diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
new file mode 100644
index 0000000..dbc49e2
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
@@ -0,0 +1,20 @@
+config MODULES
+	bool
+	default y
+	option modules
+
+config DEP
+	tristate
+	default m
+
+choice
+	prompt "Tristate Choice"
+
+config CHOICE0
+	tristate "Choice 0"
+
+config CHOICE1
+	tristate "Choice 1"
+	depends on DEP
+
+endchoice
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
new file mode 100644
index 0000000..ec71777
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
@@ -0,0 +1,15 @@
+"""
+Hide tristate choice values with mod dependency in y choice
+===========================================================
+
+If tristate choice values depend on symbols set to 'm', they should be
+hidden when the choice containing them is changed from 'm' to 'y'
+(i.e. exclusive choice).
+
+Related Linux commit: fa64e5f6a35efd5e77d639125d973077ca506074
+"""
+
+def test(conf):
+    assert conf.oldaskconfig('config', 'y') == 0
+    assert conf.config_contains('expected_config')
+    assert conf.stdout_contains('expected_stdout')
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/config b/scripts/kconfig/tests/choice_value_with_m_dep/config
new file mode 100644
index 0000000..3a126b7
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/config
@@ -0,0 +1,2 @@
+CONFIG_CHOICE0=m
+CONFIG_CHOICE1=m
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_config b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
new file mode 100644
index 0000000..4d07b44
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
@@ -0,0 +1,3 @@
+CONFIG_MODULES=y
+CONFIG_DEP=m
+CONFIG_CHOICE0=y
diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
new file mode 100644
index 0000000..5eac85d
--- /dev/null
+++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
@@ -0,0 +1,4 @@
+Tristate Choice [M/y/?] 
+Tristate Choice
+> 1. Choice 0 (CHOICE0)
+choice[1]: 1
-- 
2.7.4

[PATCH 13/14] kconfig: test: check if recursive dependencies are detected

[Masahiro Yamada]

Recursive dependency should be detected and warned.  Test this.

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

 scripts/kconfig/tests/warn_recursive_dep/Kconfig   | 62 ++++++++++++++++++++++
 .../kconfig/tests/warn_recursive_dep/__init__.py   | 10 ++++
 .../tests/warn_recursive_dep/expected_stderr       | 33 ++++++++++++
 3 files changed, 105 insertions(+)
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
 create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr

diff --git a/scripts/kconfig/tests/warn_recursive_dep/Kconfig b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
new file mode 100644
index 0000000..9ab2474
--- /dev/null
+++ b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
@@ -0,0 +1,62 @@
+# depends on itself
+
+config A
+	bool "A"
+	depends on A
+
+# select itself
+
+config B
+	bool
+	select B
+
+# depends on each other
+
+config C1
+	bool "C1
+	depends on C2
+
+config C2
+	bool "C2"
+	depends on C1
+
+# depends on and select
+
+config D1
+	bool "D1"
+	depends on D2
+	select D2
+
+config D2
+	bool
+
+# depends on and imply
+# This is not recursive dependency
+
+config E1
+	bool "E1"
+	depends on E2
+	imply E2
+
+config E2
+	bool "E2"
+
+# property
+
+config F1
+	bool "F1"
+	default F2
+
+config F2
+	bool "F2"
+	depends on F1
+
+# menu
+
+menu "menu depending on its content"
+	depends on G
+
+config G
+	bool "G"
+
+endmenu
diff --git a/scripts/kconfig/tests/warn_recursive_dep/__init__.py b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
new file mode 100644
index 0000000..cdf2c13
--- /dev/null
+++ b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
@@ -0,0 +1,10 @@
+"""
+Warn recursive inclusion
+========================
+
+Recursive dependency should be warned.
+"""
+
+def test(conf):
+    assert conf.oldaskconfig() == 0
+    assert conf.stderr_contains('expected_stderr')
diff --git a/scripts/kconfig/tests/warn_recursive_dep/expected_stderr b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
new file mode 100644
index 0000000..ea47cae
--- /dev/null
+++ b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
@@ -0,0 +1,33 @@
+Kconfig:16:warning: multi-line strings not supported
+Kconfig:9:error: recursive dependency detected!
+Kconfig:9:	symbol B is selected by B
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:3:error: recursive dependency detected!
+Kconfig:3:	symbol A depends on A
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:15:error: recursive dependency detected!
+Kconfig:15:	symbol C1 depends on C2
+Kconfig:19:	symbol C2 depends on C1
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:30:error: recursive dependency detected!
+Kconfig:30:	symbol D2 is selected by D1
+Kconfig:25:	symbol D1 depends on D2
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:59:error: recursive dependency detected!
+Kconfig:59:	symbol G depends on G
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
+
+Kconfig:50:error: recursive dependency detected!
+Kconfig:50:	symbol F2 depends on F1
+Kconfig:48:	symbol F1 default value contains F2
+For a resolution refer to Documentation/kbuild/kconfig-language.txt
+subsection "Kconfig recursive dependency limitations"
-- 
2.7.4

[PATCH 14/14] kconfig: test: check if recursive inclusion is detected

[Masahiro Yamada]

If recursive inclusion is detected, it should fail with error messages.
Test this.

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

 scripts/kconfig/tests/err_recursive_inc/Kconfig         |  1 +
 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc     |  1 +
 scripts/kconfig/tests/err_recursive_inc/__init__.py     | 10 ++++++++++
 scripts/kconfig/tests/err_recursive_inc/expected_stderr |  4 ++++
 4 files changed, 16 insertions(+)
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
 create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr

diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig b/scripts/kconfig/tests/err_recursive_inc/Kconfig
new file mode 100644
index 0000000..3ce7a3f
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig
@@ -0,0 +1 @@
+source "Kconfig.inc"
diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
new file mode 100644
index 0000000..1fab1c1
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
@@ -0,0 +1 @@
+source "Kconfig"
diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py b/scripts/kconfig/tests/err_recursive_inc/__init__.py
new file mode 100644
index 0000000..1dae64f
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py
@@ -0,0 +1,10 @@
+"""
+Detect recursive inclusion error
+================================
+
+If recursive inclusion is detected, it should fail with error messages.
+"""
+
+def test(conf):
+    assert conf.oldaskconfig() != 0
+    assert conf.stderr_contains('expected_stderr')
diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
new file mode 100644
index 0000000..b256c91
--- /dev/null
+++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
@@ -0,0 +1,4 @@
+Kconfig:1: recursive inclusion detected. Inclusion path:
+  current file : 'Kconfig'
+  included from: 'Kconfig.inc:1'
+  included from: 'Kconfig:3'
-- 
2.7.4

Re: [PATCH 06/14] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON

[Greg Kroah-Hartman]

On Tue, Feb 06, 2018 at 09:34:46AM +0900, Masahiro Yamada wrote:
> The variable 'PYTHON' allows users to specify a proper executable
> name in case the default 'python' does not work.  However, this does
> not address the case where both Python 2 and Python 3 scripts are
> used in one system.
> 
> PEP 394 (https://www.python.org/dev/peps/pep-0394/) provides a
> convention for Python scripts portability.  Here is a quotation:
> 
>   In order to tolerate differences across platforms, all new code
>   that needs to invoke the Python interpreter should not specify
>   'python', but rather should specify either 'python2' or 'python3'.
>   This distinction should be made in shebangs, when invoking from a
>   shell script, when invoking via the system() call, or when invoking
>   in any other context.
> 
> arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
> it should be invoked by 'python2'.
> 
> It is legitimate to use 'python' for scripts compatible with both
> Python 2 and Python 3, but this is rare (at least I do not see the
> case in kernel tree).  You do not need to make efforts to write your
> scripts in that way.  Anyway, Python 2 will retire in 2020.
> 
> This commit is needed for my new scripts written in Python 3.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  Makefile           | 5 +++--
>  arch/ia64/Makefile | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 11aff0f..c4e935c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -384,7 +384,8 @@ GENKSYMS	= scripts/genksyms/genksyms
>  INSTALLKERNEL  := installkernel
>  DEPMOD		= /sbin/depmod
>  PERL		= perl
> -PYTHON		= python
> +PYTHON2		= python2
> +PYTHON3		= python3

Is this going to break any systems that were previous setting PYTHON?

I like this change, and feel it is the correct thing to do, but having a
"fallback" might be needed here.

Could you do what the perf makefile does and do something like:
	override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON2))
or is it really not an issue as only ia64 seems to care about this?

thanks,

greg k-h

Re: [PATCH 00/14] Add Kconfig unit tests

[Greg Kroah-Hartman]

On Tue, Feb 06, 2018 at 09:34:40AM +0900, Masahiro Yamada wrote:
> I am applying various patches to Kconfig these days.
> 
> However, I fear regressions.  I have been thinking of unit-tests.
> 
> There are various cryptic parts in Kconfig and corner cases where
> it is difficult to notice breakage.  If unit-tests cover those,
> I will be able to apply changes more confidently.
> 
> So, here is the trial.
> 
> After fixing some problems, I will add a basic test framework.
> This is based on pytest.  Also, this is written in Python 3.
> Python 2 will return in 2020.  So, I believe new python tools should be
> written in Python 3.
> 
> This is my Python 3 and pytest versions.
> 
> $ python3 --version
> Python 3.5.2
> $ python3 -m pytest --version
> This is pytest version 3.4.0, imported from /home/masahiro/.local/lib/python3.5/site-packages/pytest.py
> 
> If I use old pytest version, some parts did not work as expected.
> If this does not work for you, please consider using newer pytest.
> 
> I will brush up the code more and add more test cases to do a better job.
> Before proceeding more, I'd like to get consensus for this approach.
> If you have an idea for better implementation, comments are appreciated.

Personally I think this is great stuff.  I too have never wanted to
touch Kconfig stuff due to the complexity, and having unit tests like
this is a great idea to help ensure that things do not break.

Your first 5 patches should be queued up for the next merge window, no
problem (see my comments on the 6th).  As for the rest, I don't have any
objection to them, and using python3 over python2 is a good idea.  And
anyone who wants to do Kconfig work can easily install the needed
packages, it's not required by any "normal" kernel developer.

Anyway, nice job, it's great to see this happening, no objection from me
at all!

greg k-h

Re: [PATCH 06/14] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON

[Masahiro Yamada]

2018-02-06 18:34 GMT+09:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Tue, Feb 06, 2018 at 09:34:46AM +0900, Masahiro Yamada wrote:
>> The variable 'PYTHON' allows users to specify a proper executable
>> name in case the default 'python' does not work.  However, this does
>> not address the case where both Python 2 and Python 3 scripts are
>> used in one system.
>>
>> PEP 394 (https://www.python.org/dev/peps/pep-0394/) provides a
>> convention for Python scripts portability.  Here is a quotation:
>>
>>   In order to tolerate differences across platforms, all new code
>>   that needs to invoke the Python interpreter should not specify
>>   'python', but rather should specify either 'python2' or 'python3'.
>>   This distinction should be made in shebangs, when invoking from a
>>   shell script, when invoking via the system() call, or when invoking
>>   in any other context.
>>
>> arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
>> it should be invoked by 'python2'.
>>
>> It is legitimate to use 'python' for scripts compatible with both
>> Python 2 and Python 3, but this is rare (at least I do not see the
>> case in kernel tree).  You do not need to make efforts to write your
>> scripts in that way.  Anyway, Python 2 will retire in 2020.
>>
>> This commit is needed for my new scripts written in Python 3.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  Makefile           | 5 +++--
>>  arch/ia64/Makefile | 2 +-
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 11aff0f..c4e935c 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -384,7 +384,8 @@ GENKSYMS  = scripts/genksyms/genksyms
>>  INSTALLKERNEL  := installkernel
>>  DEPMOD               = /sbin/depmod
>>  PERL         = perl
>> -PYTHON               = python
>> +PYTHON2              = python2
>> +PYTHON3              = python3
>
> Is this going to break any systems that were previous setting PYTHON?
>
> I like this change, and feel it is the correct thing to do, but having a
> "fallback" might be needed here.
>
> Could you do what the perf makefile does and do something like:
>         override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON2))
> or is it really not an issue as only ia64 seems to care about this?
>

As far as I see, ia64 is the only instance that has used this ever.

(the perf Makefile defines PYTHON by itself, so should not be a problem.)


If people expect the backward-compatibility for this, I can do like follows:

# backward compatibility for 'PYTHON'
PYTHON2  := $(if $(PYTHON), $(PYTHON), python2)



Another (unlikely) possible breakage is
'python2' may not be installed on users' system.

I believe this is rare, but if needed, I could do like follows
at the cost of ugliness.


PYTHON2 := $(if $(PYTHON), $(PYTHON), \
        $(shell python2 --version 2>/dev/null && echo python2 || echo python))



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 06/14] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON

[Greg Kroah-Hartman]

On Tue, Feb 06, 2018 at 07:44:22PM +0900, Masahiro Yamada wrote:
> 2018-02-06 18:34 GMT+09:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> > On Tue, Feb 06, 2018 at 09:34:46AM +0900, Masahiro Yamada wrote:
> >> The variable 'PYTHON' allows users to specify a proper executable
> >> name in case the default 'python' does not work.  However, this does
> >> not address the case where both Python 2 and Python 3 scripts are
> >> used in one system.
> >>
> >> PEP 394 (https://www.python.org/dev/peps/pep-0394/) provides a
> >> convention for Python scripts portability.  Here is a quotation:
> >>
> >>   In order to tolerate differences across platforms, all new code
> >>   that needs to invoke the Python interpreter should not specify
> >>   'python', but rather should specify either 'python2' or 'python3'.
> >>   This distinction should be made in shebangs, when invoking from a
> >>   shell script, when invoking via the system() call, or when invoking
> >>   in any other context.
> >>
> >> arch/ia64/scripts/unwcheck.py is apparently written in Python 2, so
> >> it should be invoked by 'python2'.
> >>
> >> It is legitimate to use 'python' for scripts compatible with both
> >> Python 2 and Python 3, but this is rare (at least I do not see the
> >> case in kernel tree).  You do not need to make efforts to write your
> >> scripts in that way.  Anyway, Python 2 will retire in 2020.
> >>
> >> This commit is needed for my new scripts written in Python 3.
> >>
> >> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> ---
> >>
> >>  Makefile           | 5 +++--
> >>  arch/ia64/Makefile | 2 +-
> >>  2 files changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 11aff0f..c4e935c 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -384,7 +384,8 @@ GENKSYMS  = scripts/genksyms/genksyms
> >>  INSTALLKERNEL  := installkernel
> >>  DEPMOD               = /sbin/depmod
> >>  PERL         = perl
> >> -PYTHON               = python
> >> +PYTHON2              = python2
> >> +PYTHON3              = python3
> >
> > Is this going to break any systems that were previous setting PYTHON?
> >
> > I like this change, and feel it is the correct thing to do, but having a
> > "fallback" might be needed here.
> >
> > Could you do what the perf makefile does and do something like:
> >         override PYTHON := $(call get-executable-or-default,PYTHON,$(PYTHON2))
> > or is it really not an issue as only ia64 seems to care about this?
> >
> 
> As far as I see, ia64 is the only instance that has used this ever.
> 
> (the perf Makefile defines PYTHON by itself, so should not be a problem.)
> 
> 
> If people expect the backward-compatibility for this, I can do like follows:
> 
> # backward compatibility for 'PYTHON'
> PYTHON2  := $(if $(PYTHON), $(PYTHON), python2)

That's true, and who knows if python3 is running on ia64 :)

> Another (unlikely) possible breakage is
> 'python2' may not be installed on users' system.
> 
> I believe this is rare, but if needed, I could do like follows
> at the cost of ugliness.
> 
> 
> PYTHON2 := $(if $(PYTHON), $(PYTHON), \
>         $(shell python2 --version 2>/dev/null && echo python2 || echo python))

Ick, I guess that might work.  Try getting your patch set into the 0-day
system to see how it handles things.  I know it builds for ia64.

But this is a real tiny question overall, and should not derail this
patchset at all :)

thanks,

greg k-h

RE: [PATCH 06/14] kbuild: define PYTHON2 and PYTHON3 variables instead of PYTHON

[Luck, Tony]

> That's true, and who knows if python3 is running on ia64 :)

$ python3
-bash: python3: command not found

So I don't have it installed on my ia64 test/build machine.

-Tony

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> These messages should be directed to stderr.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c  | 18 +++++++++++-------
>  scripts/kconfig/zconf.l | 27 +++++++++++++++------------
>  2 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 307bc3f..90a76aa2 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -75,9 +75,11 @@ static void strip(char *str)
>  static void check_stdin(void)
>  {
>         if (!valid_stdin) {
> -               printf(_("aborted!\n\n"));
> -               printf(_("Console input/output is redirected. "));
> -               printf(_("Run 'make oldconfig' to update configuration.\n\n"));
> +               fprintf(stderr,
> +                       _("Aborted!\n"
> +                         "Console input/output is redirected.\n"
> +                         "Run 'make oldconfig' to update configuration.\n\n")
> +                       );

This could use fputs() too, moving the stderr to the last argument.

I think the _() thingies around the strings are for gettext
(https://www.gnu.org/software/gettext/manual/html_node/Mark-Keywords.html).
This would break it if there are existing translations, since the
msgids change.

More practically, I doubt anyone is translating these tools. IMO we
should remove the gettext stuff unless we find traces of translations.

>                 exit(1);
>         }
>  }
> @@ -565,7 +567,7 @@ int main(int ac, char **av)
>                 }
>         }
>         if (ac == optind) {
> -               printf(_("%s: Kconfig file missing\n"), av[0]);
> +               fprintf(stderr, _("%s: Kconfig file missing\n"), av[0]);
>                 conf_usage(progname);
>                 exit(1);
>         }
> @@ -590,9 +592,11 @@ int main(int ac, char **av)
>                 if (!defconfig_file)
>                         defconfig_file = conf_get_default_confname();
>                 if (conf_read(defconfig_file)) {
> -                       printf(_("***\n"
> -                               "*** Can't find default configuration \"%s\"!\n"
> -                               "***\n"), defconfig_file);
> +                       fprintf(stderr,
> +                               _("***\n"
> +                                 "*** Can't find default configuration \"%s\"!\n"
> +                                 "***\n"),
> +                               defconfig_file);
>                         exit(1);
>                 }
>                 break;
> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
> index 07e074d..0ba4900 100644
> --- a/scripts/kconfig/zconf.l
> +++ b/scripts/kconfig/zconf.l
> @@ -184,7 +184,9 @@ n   [A-Za-z0-9_-]
>                         append_string(yytext, 1);
>         }
>         \n      {
> -               printf("%s:%d:warning: multi-line strings not supported\n", zconf_curname(), zconf_lineno());
> +               fprintf(stderr,
> +                       "%s:%d:warning: multi-line strings not supported\n",
> +                       zconf_curname(), zconf_lineno());

Whether stuff is translated seems inconsistent too.

>                 current_file->lineno++;
>                 BEGIN(INITIAL);
>                 return T_EOL;
> @@ -294,7 +296,7 @@ void zconf_initscan(const char *name)
>  {
>         yyin = zconf_fopen(name);
>         if (!yyin) {
> -               printf("can't find file %s\n", name);
> +               fprintf(stderr, "can't find file %s\n", name);
>                 exit(1);
>         }
>
> @@ -315,8 +317,8 @@ void zconf_nextfile(const char *name)
>         current_buf->state = YY_CURRENT_BUFFER;
>         yyin = zconf_fopen(file->name);
>         if (!yyin) {
> -               printf("%s:%d: can't open file \"%s\"\n",
> -                   zconf_curname(), zconf_lineno(), file->name);
> +               fprintf(stderr, "%s:%d: can't open file \"%s\"\n",
> +                       zconf_curname(), zconf_lineno(), file->name);
>                 exit(1);
>         }
>         yy_switch_to_buffer(yy_create_buffer(yyin, YY_BUF_SIZE));
> @@ -325,20 +327,21 @@ void zconf_nextfile(const char *name)
>
>         for (iter = current_file->parent; iter; iter = iter->parent ) {
>                 if (!strcmp(current_file->name,iter->name) ) {
> -                       printf("%s:%d: recursive inclusion detected. "
> -                              "Inclusion path:\n  current file : '%s'\n",
> -                              zconf_curname(), zconf_lineno(),
> -                              zconf_curname());
> +                       fprintf(stderr,
> +                               "%s:%d: recursive inclusion detected. "
> +                               "Inclusion path:\n  current file : '%s'\n",
> +                               zconf_curname(), zconf_lineno(),
> +                               zconf_curname());
>                         iter = current_file->parent;
>                         while (iter && \
>                                strcmp(iter->name,current_file->name)) {
> -                               printf("  included from: '%s:%d'\n",
> -                                      iter->name, iter->lineno-1);
> +                               fprintf(stderr, "  included from: '%s:%d'\n",
> +                                       iter->name, iter->lineno-1);
>                                 iter = iter->parent;
>                         }
>                         if (iter)
> -                               printf("  included from: '%s:%d'\n",
> -                                      iter->name, iter->lineno+1);
> +                               fprintf(stderr, "  included from: '%s:%d'\n",
> +                                       iter->name, iter->lineno+1);
>                         exit(1);
>                 }
>         }
> --
> 2.7.4
>

The unrelated gettext stuff aside:

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

Cheers,
Ulf

Re: [PATCH 03/14] kconfig: show '?' prompt even if no help text is available

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 'make config', 'make oldconfig', etc. always receive '?' as a valid
> input and show useful information even if no help text is available.
>
> ------------------------>8------------------------
> foo (FOO) [N/y] (NEW) ?
>
> There is no help available for this option.
> Symbol: FOO [=n]
> Type  : bool
> Prompt: foo
>   Defined at Kconfig:1
> ------------------------>8------------------------
>
> However, '?' is not shown in the prompt if its help text is missing.
> Let's show '?' all the time so that the prompt and the behavior match.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 90a76aa2..d346642 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -201,9 +201,7 @@ static int conf_sym(struct menu *menu)
>                         printf("/m");
>                 if (oldval != yes && sym_tristate_within_range(sym, yes))
>                         printf("/y");
> -               if (menu_has_help(menu))
> -                       printf("/?");
> -               printf("] ");
> +               printf("/?] ");
>                 if (!conf_askvalue(sym, sym_get_string_value(sym)))
>                         return 0;
>                 strip(line);
> @@ -305,10 +303,7 @@ static int conf_choice(struct menu *menu)
>                         printf("[1]: 1\n");
>                         goto conf_childs;
>                 }
> -               printf("[1-%d", cnt);
> -               if (menu_has_help(menu))
> -                       printf("?");
> -               printf("]: ");
> +               printf("[1-%d?]: ", cnt);
>                 switch (input_mode) {
>                 case oldconfig:
>                 case silentoldconfig:
> --
> 2.7.4
>

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

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Tue, Feb 06, 2018 at 09:34:42AM +0900, Masahiro Yamada wrote:
> "# CONFIG_... is not set" for choice values are wrongly written into
> the .config file if they are once visible, then become invisible later.
> 
>   Test case
>   ---------
> 
> ---------------------------(Kconfig)----------------------------
> config A
> 	bool "A"
> 
> choice
> 	prompt "Choice ?"
> 	depends on A
> 
> config CHOICE_B
> 	bool "Choice B"
> 
> config CHOICE_C
> 	bool "Choice C"
> 
> endchoice
> ----------------------------------------------------------------
> 
> ---------------------------(.config)----------------------------
> CONFIG_A=y
> ----------------------------------------------------------------
> 
> With the Kconfig and .config above,
> 
>   $ make config
>   scripts/kconfig/conf  --oldaskconfig Kconfig
>   *
>   * Linux Kernel Configuration
>   *
>   A (A) [Y/n] n
>   #
>   # configuration written to .config
>   #
>   $ cat .config
>   #
>   # Automatically generated file; DO NOT EDIT.
>   # Linux Kernel Configuration
>   #
>   # CONFIG_A is not set
>   # CONFIG_CHOICE_B is not set
>   # CONFIG_CHOICE_C is not set
> 
> Here,
> 
>   # CONFIG_CHOICE_B is not set
>   # CONFIG_CHOICE_C is not set
> 
> should not be written into the .config file because their dependency
> "depends on A" is unmet.
> 
> Currently, there is no code that clears SYMBOL_WRITE of choice values.
> 
> Clear SYMBOL_WRITE for all symbols in sym_calc_value(), then set it
> again after calculating visibility.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  scripts/kconfig/symbol.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index c9123ed..5d6f6b1 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -371,8 +371,7 @@ void sym_calc_value(struct symbol *sym)
>  		sym->curr.tri = no;
>  		return;
>  	}
> -	if (!sym_is_choice_value(sym))
> -		sym->flags &= ~SYMBOL_WRITE;
> +	sym->flags &= ~SYMBOL_WRITE;
>  
>  	sym_calc_visibility(sym);
>  
> @@ -385,6 +384,7 @@ void sym_calc_value(struct symbol *sym)
>  		if (sym_is_choice_value(sym) && sym->visible == yes) {
>  			prop = sym_get_choice_prop(sym);
>  			newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
> +			sym->flags |= SYMBOL_WRITE;
>  		} else {
>  			if (sym->visible != no) {
>  				/* if the symbol is visible use the user value
> -- 
> 2.7.4
> 

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

There's a possible simplification here:

All defined symbols, regardless of type, and regardless of whether
they're choice value symbols or not, always get written out if they have
non-n visibility. Therefore, the sym->visible != no check for
SYMBOL_WRITE can be moved to before the symbol type check, which gets
rid of two SYMBOL_WRITE assignments and makes it clear that the logic is
the same for all paths.

This is safe for symbols defined without a type (S_UNKNOWN) too, because
conf_write() skips those (plus they already generate a warning).

This matches how I do it in the tri_value() function in Kconfiglib:
https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2574.
SYMBOL_WRITE corresponds to _write_to_conf.

I've included a patch below. I tested it with the Kconfiglib test suite,
which verifies that the C implementation still generates the same
.config file for all defconfig files as well as for
all{no,yes,def}config, for all ARCHes.

(The Kconfiglib test suite runs scripts/kconfig/conf and compares its
output against it, which means it doubles as a regression test for the C
tools.)


diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c9123ed2b791..13f7fdfe328d 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -371,11 +371,13 @@ void sym_calc_value(struct symbol *sym)
 		sym->curr.tri = no;
 		return;
 	}
-	if (!sym_is_choice_value(sym))
-		sym->flags &= ~SYMBOL_WRITE;
+	sym->flags &= ~SYMBOL_WRITE;
 
 	sym_calc_visibility(sym);
 
+	if (sym->visible != no)
+		sym->flags |= SYMBOL_WRITE;
+
 	/* set default if recursively called */
 	sym->curr = newval;
 
@@ -390,7 +392,6 @@ void sym_calc_value(struct symbol *sym)
 				/* if the symbol is visible use the user value
 				 * if available, otherwise try the default value
 				 */
-				sym->flags |= SYMBOL_WRITE;
 				if (sym_has_value(sym)) {
 					newval.tri = EXPR_AND(sym->def[S_DEF_USER].tri,
 							      sym->visible);
@@ -433,12 +434,9 @@ void sym_calc_value(struct symbol *sym)
 	case S_STRING:
 	case S_HEX:
 	case S_INT:
-		if (sym->visible != no) {
-			sym->flags |= SYMBOL_WRITE;
-			if (sym_has_value(sym)) {
-				newval.val = sym->def[S_DEF_USER].val;
-				break;
-			}
+		if (sym->visible != no && sym_has_value(sym)) {
+			newval.val = sym->def[S_DEF_USER].val;
+			break;
 		}
 		prop = sym_get_default_prop(sym);
 		if (prop) {
-- 
2.14.1

Re: [PATCH 00/14] Add Kconfig unit tests

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 10:38 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Feb 06, 2018 at 09:34:40AM +0900, Masahiro Yamada wrote:
>> I am applying various patches to Kconfig these days.
>>
>> However, I fear regressions.  I have been thinking of unit-tests.
>>
>> There are various cryptic parts in Kconfig and corner cases where
>> it is difficult to notice breakage.  If unit-tests cover those,
>> I will be able to apply changes more confidently.
>>
>> So, here is the trial.
>>
>> After fixing some problems, I will add a basic test framework.
>> This is based on pytest.  Also, this is written in Python 3.
>> Python 2 will return in 2020.  So, I believe new python tools should be
>> written in Python 3.
>>
>> This is my Python 3 and pytest versions.
>>
>> $ python3 --version
>> Python 3.5.2
>> $ python3 -m pytest --version
>> This is pytest version 3.4.0, imported from /home/masahiro/.local/lib/python3.5/site-packages/pytest.py
>>
>> If I use old pytest version, some parts did not work as expected.
>> If this does not work for you, please consider using newer pytest.
>>
>> I will brush up the code more and add more test cases to do a better job.
>> Before proceeding more, I'd like to get consensus for this approach.
>> If you have an idea for better implementation, comments are appreciated.
>
> Personally I think this is great stuff.  I too have never wanted to
> touch Kconfig stuff due to the complexity, and having unit tests like
> this is a great idea to help ensure that things do not break.
>
> Your first 5 patches should be queued up for the next merge window, no
> problem (see my comments on the 6th).  As for the rest, I don't have any
> objection to them, and using python3 over python2 is a good idea.  And
> anyone who wants to do Kconfig work can easily install the needed
> packages, it's not required by any "normal" kernel developer.
>
> Anyway, nice job, it's great to see this happening, no objection from me
> at all!
>
> greg k-h

Yeah, breaking Kconfig is a sure way to feel the wrath.

The only reason I feel somewhat confident modifying Kconfig is that
the Kconfiglib test suite happens to work as a regression test for the
C implementation as well. It compares the .config files produced by
the two implementations for all defconfig files and for
all{no,yes,def}config, for all ARCHes, meaning any changes to the
output of the C tools get flagged as well (with a diff).

Having some "native" tests is great as well. I'm a big fan of
automatic testing. :)

In case you want to run the Kconfiglib test suite at any point, here's
how to do it (in the kernel root):

        $ git clone git://github.com/ulfalizer/Kconfiglib.git
        $ git am Kconfiglib/makefile.patch
        $ python Kconfiglib/testsuite.py speedy

Cheers,
Ulf

Re: [PATCH 04/14] kconfig: print additional new line for choice for redirection

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> If stdout is redirected to a file, prompts look differently due to
> missing new lines.
>
> Currently, conf_askvalue() takes care of this by putting additional
> new line, but conf_choice() does not.  Do likewise so that prompts
> after 'choice' look properly.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/conf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index d346642..6ce06c8 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
>                 case oldaskconfig:
>                         fflush(stdout);
>                         xfgets(line, sizeof(line), stdin);
> +                       if (!tty_stdio)
> +                               printf("\n");
>                         strip(line);
>                         if (line[0] == '?') {
>                                 print_help(menu);
> --
> 2.7.4
>

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

Maybe this could be moved into the xfgets() function as well.

Cheers,
Ulf

Re: [PATCH 05/14] kconfig: remove 'config*' pattern from .gitignnore

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> I could not figure out why this pattern should be ignored.
> Checking commit 1e65174a3378 ("Add some basic .gitignore files")
> did not help.
>
> Let's remove this pattern, then see if it is really needed.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/.gitignore | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/scripts/kconfig/.gitignore b/scripts/kconfig/.gitignore
> index 51f1c87..a76856e 100644
> --- a/scripts/kconfig/.gitignore
> +++ b/scripts/kconfig/.gitignore
> @@ -1,7 +1,6 @@
>  #
>  # Generated files
>  #
> -config*
>  *.lex.c
>  *.tab.c
>  *.tab.h
> --
> 2.7.4
>

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

Re: [PATCH 08/14] kconfig: test: add basic 'choice' tests

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> The calculation of 'choice' is a bit complicated part in Kconfig.
>
> The behavior of 'y' choice is intuitive.  If choice values are tristate,
> the choice can be 'm' where each value can be enabled independently.
> Also, if a choice is marked as 'optional', the whole choice can be
> invisible.
>
> Test basic functionality of choice.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/tests/choice/Kconfig               | 54 ++++++++++++++++++++++
>  scripts/kconfig/tests/choice/__init__.py           | 37 +++++++++++++++
>  .../kconfig/tests/choice/alldef_expected_config    |  5 ++
>  .../kconfig/tests/choice/allmod_expected_config    |  9 ++++
>  scripts/kconfig/tests/choice/allno_expected_config |  5 ++
>  .../kconfig/tests/choice/allyes_expected_config    |  9 ++++
>  .../kconfig/tests/choice/oldask0_expected_stdout   | 10 ++++
>  scripts/kconfig/tests/choice/oldask1_config        |  2 +
>  .../kconfig/tests/choice/oldask1_expected_stdout   | 15 ++++++
>  9 files changed, 146 insertions(+)
>  create mode 100644 scripts/kconfig/tests/choice/Kconfig
>  create mode 100644 scripts/kconfig/tests/choice/__init__.py
>  create mode 100644 scripts/kconfig/tests/choice/alldef_expected_config
>  create mode 100644 scripts/kconfig/tests/choice/allmod_expected_config
>  create mode 100644 scripts/kconfig/tests/choice/allno_expected_config
>  create mode 100644 scripts/kconfig/tests/choice/allyes_expected_config
>  create mode 100644 scripts/kconfig/tests/choice/oldask0_expected_stdout
>  create mode 100644 scripts/kconfig/tests/choice/oldask1_config
>  create mode 100644 scripts/kconfig/tests/choice/oldask1_expected_stdout
>
> diff --git a/scripts/kconfig/tests/choice/Kconfig b/scripts/kconfig/tests/choice/Kconfig
> new file mode 100644
> index 0000000..cc60e9c
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/Kconfig
> @@ -0,0 +1,54 @@
> +config MODULES
> +       bool "Enable loadable module support"
> +       option modules
> +       default y
> +
> +choice
> +       prompt "boolean choice"
> +       default BOOL_CHOICE1
> +
> +config BOOL_CHOICE0
> +       bool "choice 0"
> +
> +config BOOL_CHOICE1
> +       bool "choice 1"
> +
> +endchoice
> +
> +choice
> +       prompt "optional boolean choice"
> +       optional
> +       default OPT_BOOL_CHOICE1
> +
> +config OPT_BOOL_CHOICE0
> +       bool "choice 0"
> +
> +config OPT_BOOL_CHOICE1
> +       bool "choice 1"
> +
> +endchoice
> +
> +choice
> +       prompt "tristate choice"
> +       default TRI_CHOICE1
> +
> +config TRI_CHOICE0
> +       tristate "choice 0"
> +
> +config TRI_CHOICE1
> +       tristate "choice 1"
> +
> +endchoice
> +
> +choice
> +       prompt "optional tristate choice"
> +       optional
> +       default OPT_TRI_CHOICE1
> +
> +config OPT_TRI_CHOICE0
> +       tristate "choice 0"
> +
> +config OPT_TRI_CHOICE1
> +       tristate "choice 1"
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/choice/__init__.py b/scripts/kconfig/tests/choice/__init__.py
> new file mode 100644
> index 0000000..42022ac
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/__init__.py
> @@ -0,0 +1,37 @@
> +"""
> +Basic choice tests
> +==================
> +
> +The handling of 'choice' is a bit complicated part in Kconfig.
> +
> +The behavior of 'y' choice is intuitive.  If choice values are tristate,
> +the choice can be 'm' where each value can be enabled independently.
> +Also, if a choice is marked as 'optional', the whole choice can be
> +invisible.
> +
> +Test basic functionality of choice.
> +"""
> +
> +def test_oldask0(conf):
> +    assert conf.oldaskconfig() == 0
> +    assert conf.stdout_contains('oldask0_expected_stdout')
> +
> +def test_oldask1(conf):
> +    assert conf.oldaskconfig('oldask1_config') == 0
> +    assert conf.stdout_contains('oldask1_expected_stdout')
> +
> +def test_allyes(conf):
> +    assert conf.allyesconfig() == 0
> +    assert conf.config_contains('allyes_expected_config')
> +
> +def test_allmod(conf):
> +    assert conf.allmodconfig() == 0
> +    assert conf.config_contains('allmod_expected_config')
> +
> +def test_allno(conf):
> +    assert conf.allnoconfig() == 0
> +    assert conf.config_contains('allno_expected_config')
> +
> +def test_alldef(conf):
> +    assert conf.alldefconfig() == 0
> +    assert conf.config_contains('alldef_expected_config')
> diff --git a/scripts/kconfig/tests/choice/alldef_expected_config b/scripts/kconfig/tests/choice/alldef_expected_config
> new file mode 100644
> index 0000000..7a754bf
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/alldef_expected_config
> @@ -0,0 +1,5 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +# CONFIG_TRI_CHOICE1 is not set
> diff --git a/scripts/kconfig/tests/choice/allmod_expected_config b/scripts/kconfig/tests/choice/allmod_expected_config
> new file mode 100644
> index 0000000..f1f5dcd
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allmod_expected_config
> @@ -0,0 +1,9 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE1=y
> +CONFIG_TRI_CHOICE0=m
> +CONFIG_TRI_CHOICE1=m
> +CONFIG_OPT_TRI_CHOICE0=m
> +CONFIG_OPT_TRI_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice/allno_expected_config b/scripts/kconfig/tests/choice/allno_expected_config
> new file mode 100644
> index 0000000..b88ee7a
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allno_expected_config
> @@ -0,0 +1,5 @@
> +# CONFIG_MODULES is not set
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/allyes_expected_config b/scripts/kconfig/tests/choice/allyes_expected_config
> new file mode 100644
> index 0000000..e5a062a
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/allyes_expected_config
> @@ -0,0 +1,9 @@
> +CONFIG_MODULES=y
> +# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE1=y
> +# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE1=y
> +# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE1=y
> +# CONFIG_OPT_TRI_CHOICE0 is not set
> +CONFIG_OPT_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/oldask0_expected_stdout b/scripts/kconfig/tests/choice/oldask0_expected_stdout
> new file mode 100644
> index 0000000..b251bba
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask0_expected_stdout
> @@ -0,0 +1,10 @@
> +Enable loadable module support (MODULES) [Y/n/?] (NEW)
> +boolean choice
> +  1. choice 0 (BOOL_CHOICE0) (NEW)
> +> 2. choice 1 (BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional boolean choice [N/y/?] (NEW)
> +tristate choice [M/y/?] (NEW)
> +  choice 0 (TRI_CHOICE0) [N/m/?] (NEW)
> +  choice 1 (TRI_CHOICE1) [N/m/?] (NEW)
> +optional tristate choice [N/m/y/?] (NEW)
> diff --git a/scripts/kconfig/tests/choice/oldask1_config b/scripts/kconfig/tests/choice/oldask1_config
> new file mode 100644
> index 0000000..b67bfe3
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask1_config
> @@ -0,0 +1,2 @@
> +# CONFIG_MODULES is not set
> +CONFIG_OPT_BOOL_CHOICE0=y
> diff --git a/scripts/kconfig/tests/choice/oldask1_expected_stdout b/scripts/kconfig/tests/choice/oldask1_expected_stdout
> new file mode 100644
> index 0000000..c2125e9b
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice/oldask1_expected_stdout
> @@ -0,0 +1,15 @@
> +Enable loadable module support (MODULES) [N/y/?]
> +boolean choice
> +  1. choice 0 (BOOL_CHOICE0) (NEW)
> +> 2. choice 1 (BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional boolean choice [Y/n/?] (NEW)
> +optional boolean choice
> +> 1. choice 0 (OPT_BOOL_CHOICE0)
> +  2. choice 1 (OPT_BOOL_CHOICE1) (NEW)
> +choice[1-2?]:
> +tristate choice
> +  1. choice 0 (TRI_CHOICE0) (NEW)
> +> 2. choice 1 (TRI_CHOICE1) (NEW)
> +choice[1-2?]:
> +optional tristate choice [N/y/?]
> --
> 2.7.4
>

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

Re: [PATCH 09/14] kconfig: test: test automatic submenu creation

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> If a symbols has dependency on the preceding symbol, the menu entry
> should become the submenu of the preceding one, and displayed with
> deeper indentation.
>
> This is done by restructuring the menu tree in menu_finalize().
> It is a bit complicated computation, so let's add a test case.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  .../kconfig/tests/auto_submenu_creation/Kconfig    | 50 ++++++++++++++++++++++
>  .../tests/auto_submenu_creation/__init__.py        | 12 ++++++
>  .../tests/auto_submenu_creation/expected_stdout    | 10 +++++
>  3 files changed, 72 insertions(+)
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/Kconfig
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/__init__.py
>  create mode 100644 scripts/kconfig/tests/auto_submenu_creation/expected_stdout
>
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/Kconfig b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
> new file mode 100644
> index 0000000..9e11502
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/Kconfig
> @@ -0,0 +1,50 @@
> +config A
> +       bool "A"
> +       default y
> +
> +config A0
> +       bool "A0"
> +       depends on A
> +       default y
> +       help
> +         This depends on A, so should be a submenu of A.
> +
> +config A0_0
> +       bool "A1_0"
> +       depends on A0
> +       help
> +         Submenus are created recursively.
> +         This should be a submenu of A0.
> +
> +config A1
> +       bool "A1"
> +       depends on A
> +       default y
> +       help
> +         This should line up with A0.
> +
> +choice
> +       prompt "choice"
> +       depends on A1
> +       help
> +         Choice should become a submenu as well.
> +
> +config A1_0
> +       bool "A1_0"
> +
> +config A1_1
> +       bool "A1_1"
> +
> +endchoice
> +
> +config B
> +       bool "B"
> +       help
> +         This is independent of A.
> +
> +config C
> +       bool "C"
> +       depends on A
> +       help
> +         This depends on A, but not a consecutive item, so should not
> +         be a submenu.
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/__init__.py b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
> new file mode 100644
> index 0000000..dda866d
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/__init__.py
> @@ -0,0 +1,12 @@
> +"""
> +Create submenu for symbols that depend on the preceding one
> +===========================================================
> +
> +If a symbols has dependency on the preceding symbol, the menu entry
> +should become the submenu of the preceding one, and displayed with
> +deeper indentation.
> +"""
> +
> +def test(conf):
> +    assert conf.oldaskconfig() == 0
> +    assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/auto_submenu_creation/expected_stdout b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
> new file mode 100644
> index 0000000..bf5236f
> --- /dev/null
> +++ b/scripts/kconfig/tests/auto_submenu_creation/expected_stdout
> @@ -0,0 +1,10 @@
> +A (A) [Y/n/?] (NEW)
> +  A0 (A0) [Y/n/?] (NEW)
> +    A1_0 (A0_0) [N/y/?] (NEW)
> +  A1 (A1) [Y/n/?] (NEW)
> +    choice
> +    > 1. A1_0 (A1_0) (NEW)
> +      2. A1_1 (A1_1) (NEW)
> +    choice[1-2?]:
> +B (B) [N/y/?] (NEW)
> +C (C) [N/y/?] (NEW)
> --
> 2.7.4
>

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

Re: [PATCH 10/14] kconfig: test: check if new symbols in choice are asked

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> If new choice values are added with new dependency, and they become
> visible during user configuration, oldconfig should recognize them
> as (NEW), and ask the user for choice.
>
> This issue was fixed by commit 5d09598d488f ("kconfig: fix new choices
> being skipped upon config update").
>
> This is a subtle corner case.  Add a test case to avoid breakage.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/tests/new_choice_with_dep/Kconfig  | 37 ++++++++++++++++++++++
>  .../kconfig/tests/new_choice_with_dep/__init__.py  | 14 ++++++++
>  scripts/kconfig/tests/new_choice_with_dep/config   |  3 ++
>  .../tests/new_choice_with_dep/expected_stdout      | 10 ++++++
>  4 files changed, 64 insertions(+)
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config
>  create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout
>
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/Kconfig b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
> new file mode 100644
> index 0000000..53ef1b8
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/Kconfig
> @@ -0,0 +1,37 @@
> +config A
> +       bool "A"
> +       help
> +         This is a new symbol.
> +
> +choice
> +       prompt "Choice ?"
> +       depends on A
> +       help
> +         "depends on A" has been newly added.
> +
> +config CHOICE_B
> +       bool "Choice B"
> +
> +config CHOICE_C
> +       bool "Choice C"
> +       help
> +         This is a new symbol, so should be asked.
> +
> +endchoice
> +
> +choice
> +       prompt "Choice2 ?"
> +
> +config CHOICE_D
> +       bool "Choice D"
> +
> +config CHOICE_E
> +       bool "Choice E"
> +
> +config CHOICE_F
> +       bool "Choice F"
> +       depends on A
> +       help
> +         This is a new symbol, so should be asked.
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/__init__.py b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
> new file mode 100644
> index 0000000..4306ccf
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/__init__.py
> @@ -0,0 +1,14 @@
> +"""
> +Ask new choice values when they become visible
> +==============================================
> +
> +If new choice values are added with new dependency, and they become
> +visible during user configuration, oldconfig should recognize them
> +as (NEW), and ask the user for choice.
> +
> +Related Linux commit: 5d09598d488f081e3be23f885ed65cbbe2d073b5
> +"""
> +
> +def test(conf):
> +    assert conf.oldconfig('config', 'y') == 0
> +    assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/config b/scripts/kconfig/tests/new_choice_with_dep/config
> new file mode 100644
> index 0000000..47ef95d
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/config
> @@ -0,0 +1,3 @@
> +CONFIG_CHOICE_B=y
> +# CONFIG_CHOICE_D is not set
> +CONFIG_CHOICE_E=y
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/expected_stdout b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
> new file mode 100644
> index 0000000..358d5cf
> --- /dev/null
> +++ b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout
> @@ -0,0 +1,10 @@
> +A (A) [N/y/?] (NEW)
> +  Choice ?
> +  > 1. Choice B (CHOICE_B)
> +    2. Choice C (CHOICE_C) (NEW)
> +  choice[1-2?]:
> +Choice2 ?
> +  1. Choice D (CHOICE_D)
> +> 2. Choice E (CHOICE_E)
> +  3. Choice F (CHOICE_F) (NEW)
> +choice[1-3?]:
> --
> 2.7.4
>

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

Re: [PATCH 11/14] kconfig: test: check .config sanity for choice values with unmet dep

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> I fixed a problem where "# CONFIG_..." for choice values are wrongly
> written into the .config file when they are once visible, then become
> invisible later.
>
> Add a test for this subtle case.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig     | 14 ++++++++++++++
>  scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py | 17 +++++++++++++++++
>  scripts/kconfig/tests/no_write_if_dep_unmet/config      |  1 +
>  .../kconfig/tests/no_write_if_dep_unmet/expected_config |  5 +++++
>  4 files changed, 37 insertions(+)
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config
>  create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
>
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
> new file mode 100644
> index 0000000..c00b8fe
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig
> @@ -0,0 +1,14 @@
> +config A
> +       bool "A"
> +
> +choice
> +       prompt "Choice ?"
> +       depends on A
> +
> +config CHOICE_B
> +       bool "Choice B"
> +
> +config CHOICE_C
> +       bool "Choice C"
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> new file mode 100644
> index 0000000..d096622
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> @@ -0,0 +1,17 @@
> +"""
> +Do not write choice values to .config if the dependency is unmet
> +================================================================
> +
> +"# CONFIG_... is not set" should not be written into the .config file
> +for symbols with unmet dependency.
> +
> +This was not working correctly for choice values because choice needs
> +a bit different symbol computation.
> +
> +This checks that no unneeded "# COFIG_... is not set" is contained in
> +the .config file.
> +"""
> +
> +def test(conf):
> +    assert conf.oldaskconfig('config', 'n') == 0
> +    assert conf.config_matches('expected_config')
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/config b/scripts/kconfig/tests/no_write_if_dep_unmet/config
> new file mode 100644
> index 0000000..abd280e
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/config
> @@ -0,0 +1 @@
> +CONFIG_A=y
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> new file mode 100644
> index 0000000..0d15e41
> --- /dev/null
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> @@ -0,0 +1,5 @@
> +#
> +# Automatically generated file; DO NOT EDIT.
> +# Linux Kernel Configuration
> +#
> +# CONFIG_A is not set
> --
> 2.7.4
>

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

This indirectly tests that choices specified without a type get the
type of the first choice value specified with a type too.

Cheers,
Ulf

Re: [PATCH 12/14] kconfig: test: check visibility of tristate choice values in y choice

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> If tristate choice values depend on symbols set to 'm', they should be
> hidden when the choice containing them is changed from 'm' to 'y'
> (i.e. exclusive choice).
>
> This issue was fixed by commit fa64e5f6a35e ("kconfig/symbol.c: handle
> choice_values that depend on 'm' symbols").
>
> Add a test case to avoid regression.
>
> For the input in this unit test, there is a room for argument if
> "# CONFIG_CHOICE1 is not set" should be written to the .config file.
>
> After commit fa64e5f6a35e, this line was written to the .config file.
>
> Then, it is not written after my fix "kconfig: do not write choice
> values when their dependency becomes n".
>
> In this test, "# CONFIG_CHOICE1 is not set" is don't care.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  .../kconfig/tests/choice_value_with_m_dep/Kconfig    | 20 ++++++++++++++++++++
>  .../tests/choice_value_with_m_dep/__init__.py        | 15 +++++++++++++++
>  scripts/kconfig/tests/choice_value_with_m_dep/config |  2 ++
>  .../tests/choice_value_with_m_dep/expected_config    |  3 +++
>  .../tests/choice_value_with_m_dep/expected_stdout    |  4 ++++
>  5 files changed, 44 insertions(+)
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_config
>  create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
>
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
> new file mode 100644
> index 0000000..dbc49e2
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig
> @@ -0,0 +1,20 @@
> +config MODULES
> +       bool
> +       default y
> +       option modules
> +
> +config DEP
> +       tristate
> +       default m
> +
> +choice
> +       prompt "Tristate Choice"
> +
> +config CHOICE0
> +       tristate "Choice 0"
> +
> +config CHOICE1
> +       tristate "Choice 1"
> +       depends on DEP
> +
> +endchoice
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
> new file mode 100644
> index 0000000..ec71777
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
> @@ -0,0 +1,15 @@
> +"""
> +Hide tristate choice values with mod dependency in y choice
> +===========================================================
> +
> +If tristate choice values depend on symbols set to 'm', they should be
> +hidden when the choice containing them is changed from 'm' to 'y'
> +(i.e. exclusive choice).
> +
> +Related Linux commit: fa64e5f6a35efd5e77d639125d973077ca506074
> +"""
> +
> +def test(conf):
> +    assert conf.oldaskconfig('config', 'y') == 0
> +    assert conf.config_contains('expected_config')
> +    assert conf.stdout_contains('expected_stdout')
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/config b/scripts/kconfig/tests/choice_value_with_m_dep/config
> new file mode 100644
> index 0000000..3a126b7
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/config
> @@ -0,0 +1,2 @@
> +CONFIG_CHOICE0=m
> +CONFIG_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_config b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
> new file mode 100644
> index 0000000..4d07b44
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config
> @@ -0,0 +1,3 @@
> +CONFIG_MODULES=y
> +CONFIG_DEP=m
> +CONFIG_CHOICE0=y
> diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
> new file mode 100644
> index 0000000..5eac85d
> --- /dev/null
> +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout
> @@ -0,0 +1,4 @@
> +Tristate Choice [M/y/?]
> +Tristate Choice
> +> 1. Choice 0 (CHOICE0)
> +choice[1]: 1
> --
> 2.7.4
>

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

Re: [PATCH 13/14] kconfig: test: check if recursive dependencies are detected

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> Recursive dependency should be detected and warned.  Test this.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/tests/warn_recursive_dep/Kconfig   | 62 ++++++++++++++++++++++
>  .../kconfig/tests/warn_recursive_dep/__init__.py   | 10 ++++
>  .../tests/warn_recursive_dep/expected_stderr       | 33 ++++++++++++
>  3 files changed, 105 insertions(+)
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py
>  create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr
>
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/Kconfig b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
> new file mode 100644
> index 0000000..9ab2474
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/Kconfig
> @@ -0,0 +1,62 @@
> +# depends on itself
> +
> +config A
> +       bool "A"
> +       depends on A
> +
> +# select itself
> +
> +config B
> +       bool
> +       select B
> +
> +# depends on each other
> +
> +config C1
> +       bool "C1

Missing end quote here. Noticed because the output contains
"Kconfig:16:warning: multi-line strings not supported". :)

> +       depends on C2
> +
> +config C2
> +       bool "C2"
> +       depends on C1
> +
> +# depends on and select
> +
> +config D1
> +       bool "D1"
> +       depends on D2
> +       select D2
> +
> +config D2
> +       bool
> +
> +# depends on and imply
> +# This is not recursive dependency
> +
> +config E1
> +       bool "E1"
> +       depends on E2
> +       imply E2
> +
> +config E2
> +       bool "E2"
> +
> +# property
> +
> +config F1
> +       bool "F1"
> +       default F2
> +
> +config F2
> +       bool "F2"
> +       depends on F1
> +
> +# menu
> +
> +menu "menu depending on its content"
> +       depends on G
> +
> +config G
> +       bool "G"
> +
> +endmenu
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/__init__.py b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
> new file mode 100644
> index 0000000..cdf2c13
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Warn recursive inclusion
> +========================
> +
> +Recursive dependency should be warned.
> +"""
> +
> +def test(conf):
> +    assert conf.oldaskconfig() == 0
> +    assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/warn_recursive_dep/expected_stderr b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
> new file mode 100644
> index 0000000..ea47cae
> --- /dev/null
> +++ b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr
> @@ -0,0 +1,33 @@
> +Kconfig:16:warning: multi-line strings not supported
> +Kconfig:9:error: recursive dependency detected!
> +Kconfig:9:     symbol B is selected by B
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:3:error: recursive dependency detected!
> +Kconfig:3:     symbol A depends on A
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:15:error: recursive dependency detected!
> +Kconfig:15:    symbol C1 depends on C2
> +Kconfig:19:    symbol C2 depends on C1
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:30:error: recursive dependency detected!
> +Kconfig:30:    symbol D2 is selected by D1
> +Kconfig:25:    symbol D1 depends on D2
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:59:error: recursive dependency detected!
> +Kconfig:59:    symbol G depends on G
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> +
> +Kconfig:50:error: recursive dependency detected!
> +Kconfig:50:    symbol F2 depends on F1
> +Kconfig:48:    symbol F1 default value contains F2
> +For a resolution refer to Documentation/kbuild/kconfig-language.txt
> +subsection "Kconfig recursive dependency limitations"
> --
> 2.7.4
>

Besides the missing end quote:

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

Re: [PATCH 14/14] kconfig: test: check if recursive inclusion is detected

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> If recursive inclusion is detected, it should fail with error messages.
> Test this.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/tests/err_recursive_inc/Kconfig         |  1 +
>  scripts/kconfig/tests/err_recursive_inc/Kconfig.inc     |  1 +
>  scripts/kconfig/tests/err_recursive_inc/__init__.py     | 10 ++++++++++
>  scripts/kconfig/tests/err_recursive_inc/expected_stderr |  4 ++++
>  4 files changed, 16 insertions(+)
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py
>  create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr
>
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> new file mode 100644
> index 0000000..3ce7a3f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig
> @@ -0,0 +1 @@
> +source "Kconfig.inc"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> new file mode 100644
> index 0000000..1fab1c1
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc
> @@ -0,0 +1 @@
> +source "Kconfig"
> diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> new file mode 100644
> index 0000000..1dae64f
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py
> @@ -0,0 +1,10 @@
> +"""
> +Detect recursive inclusion error
> +================================
> +
> +If recursive inclusion is detected, it should fail with error messages.
> +"""
> +
> +def test(conf):
> +    assert conf.oldaskconfig() != 0
> +    assert conf.stderr_contains('expected_stderr')
> diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> new file mode 100644
> index 0000000..b256c91
> --- /dev/null
> +++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr
> @@ -0,0 +1,4 @@
> +Kconfig:1: recursive inclusion detected. Inclusion path:
> +  current file : 'Kconfig'
> +  included from: 'Kconfig.inc:1'
> +  included from: 'Kconfig:3'
> --
> 2.7.4
>

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

Re: [PATCH 07/14] kconfig: test: add framework for Kconfig unit-tests

[Ulf Magnusson]

On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> I admit various parts in Kconfig are cryptic and need refactoring,
> but at the same time, I fear regressions.
>
> There are several subtle corner cases where it is difficult to notice
> breakage.  It is time to add unit-tests.
>
> Here is a simple framework based on pytest.  The conftest.py provides
> a fixture useful to run commands such as 'oldaskconfig' etc. and
> to compare the resulted .config, stdout, stderr with expectations.
>
> How to add test cases?
> ----------------------
>
> For each test case, you should create a subdirectory under
> scripts/kconfig/tests/ (so test cases are seperated from each other).
> Every test case directory must contain the following files:
>
>  - __init__.py: describes test functions
>  - Kconfig: the top level Kconfig file for this test
>
> To do a useful job, test cases generally need additional data like
> input .config and information about expected results.
>
> How to run tests?
> -----------------
>
> You need python3 and pytest.  Then, run "make testconfig".
> O= option is supported.  If V=1 is given, details logs during tests
> are displayed.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  scripts/kconfig/Makefile          |   8 ++
>  scripts/kconfig/tests/conftest.py | 255 ++++++++++++++++++++++++++++++++++++++
>  scripts/kconfig/tests/pytest.ini  |   6 +
>  3 files changed, 269 insertions(+)
>  create mode 100644 scripts/kconfig/tests/conftest.py
>  create mode 100644 scripts/kconfig/tests/pytest.ini
>
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index cb3ec53..c5d1d1a 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -135,6 +135,14 @@ PHONY += tinyconfig
>  tinyconfig:
>         $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config
>
> +# CHECK: -o cache_dir=<path> working?
> +PHONY += testconfig
> +testconfig: $(obj)/conf
> +       $(PYTHON3) -B -m pytest $(srctree)/$(src)/tests \
> +       -o cache_dir=$(abspath $(obj)/tests/.cache) \
> +       $(if $(findstring 1,$(KBUILD_VERBOSE)),--capture=no)
> +clean-dirs += tests/.cache
> +
>  # Help text used by make help
>  help:
>         @echo  '  config          - Update current config utilising a line-oriented program'
> diff --git a/scripts/kconfig/tests/conftest.py b/scripts/kconfig/tests/conftest.py
> new file mode 100644
> index 0000000..f0f3237
> --- /dev/null
> +++ b/scripts/kconfig/tests/conftest.py
> @@ -0,0 +1,255 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Copyright (C) 2018 Masahiro Yamada <yamada.masahiro@socionext.com>
> +#
> +
> +import os
> +import pytest
> +import shutil
> +import subprocess
> +import tempfile
> +
> +conf_path = os.path.abspath(os.path.join('scripts', 'kconfig', 'conf'))
> +
> +class Conf:
> +
> +    def __init__(self, request):
> +        """Create a new Conf object, which is a scripts/kconfig/conf
> +        runner and result checker.
> +
> +        Arguments:
> +        request - object to introspect the requesting test module
> +        """
> +
> +        # the directory of the test being run
> +        self.test_dir = os.path.dirname(str(request.fspath))
> +
> +    def __run_conf(self, mode, dot_config=None, out_file='.config',
> +                   interactive=False, in_keys=None, extra_env={}):
> +        """Run scripts/kconfig/conf
> +
> +        mode: input mode option (--oldaskconfig, --defconfig=<file> etc.)
> +        dot_config: the .config file for input.
> +        out_file: file name to contain the output config data.
> +        interactive: flag to specify the interactive mode.
> +        in_keys: key inputs for interactive modes.
> +        extra_env: additional environment.
> +        """
> +
> +        command = [conf_path, mode, 'Kconfig']
> +
> +        # Override 'srctree' environment to make the test as the top directory
> +        extra_env['srctree'] = self.test_dir
> +
> +        # scripts/kconfig/conf is run in a temporary directory.
> +        # This directory is automatically removed when done.
> +        with tempfile.TemporaryDirectory() as temp_dir:
> +
> +            # if .config is given, copy it to the working directory
> +            if dot_config:
> +                shutil.copyfile(os.path.join(self.test_dir, dot_config),
> +                                os.path.join(temp_dir, '.config'))
> +
> +            ps = subprocess.Popen(command,
> +                                  stdin=subprocess.PIPE,
> +                                  stdout=subprocess.PIPE,
> +                                  stderr=subprocess.PIPE,
> +                                  cwd=temp_dir,
> +                                  env=dict(os.environ, **extra_env))
> +
> +            # If user key input is specified, feed it into stdin.
> +            if in_keys:
> +                ps.stdin.write(in_keys.encode('utf-8'))
> +
> +            while ps.poll() == None:
> +                # For interactive modes such as 'make config', 'make oldconfig',
> +                # send 'Enter' key until the program finishes.
> +                if interactive:
> +                    ps.stdin.write(b'\n')
> +
> +            self.retcode = ps.returncode
> +            self.stdout = ps.stdout.read().decode()
> +            self.stderr = ps.stderr.read().decode()
> +
> +            # Retrieve the resulted config data only when .config is supposed
> +            # to exist.  If the command fails, the .config does not exist.
> +            # 'make listnewconfig' does not produce .config in the first place.
> +            if self.retcode == 0 and out_file:
> +                with open(os.path.join(temp_dir, out_file)) as f:
> +                    self.config = f.read()
> +            else:
> +                self.config = None
> +
> +        # Logging:
> +        # Pytest captures the following information by default.  In failure
> +        # of tests, the captured log will be displayed.  This will be useful to
> +        # figure out what has happened.
> +
> +        print("command: {}\n".format(' '.join(command)))
> +        print("retcode: {}\n".format(self.retcode))
> +
> +        if dot_config:
> +            print("input .config:".format(dot_config))
> +
> +        print("stdout:")
> +        print(self.stdout)
> +        print("stderr:")
> +        print(self.stderr)
> +
> +        if self.config is not None:
> +            print("output of {}:".format(out_file))
> +            print(self.config)
> +
> +        return self.retcode
> +
> +    def oldaskconfig(self, dot_config=None, in_keys=None):
> +        """Run oldaskconfig (make config)
> +
> +        dot_config: the .config file for input (optional).
> +        in_key: key inputs (optional).
> +        """
> +        return self.__run_conf('--oldaskconfig', dot_config=dot_config,
> +                             interactive=True, in_keys=in_keys)
> +
> +    def oldconfig(self, dot_config=None, in_keys=None):
> +        """Run oldconfig
> +
> +        dot_config: the .config file for input (optional).
> +        in_key: key inputs (optional).
> +        """
> +        return self.__run_conf('--oldconfig', dot_config=dot_config,
> +                             interactive=True, in_keys=in_keys)
> +
> +    def defconfig(self, defconfig):
> +        """Run defconfig
> +
> +        defconfig: the defconfig file for input.
> +        """
> +        defconfig_path = os.path.join(self.test_dir, defconfig)
> +        return self.__run_conf('--defconfig={}'.format(defconfig_path))
> +
> +    def olddefconfig(self, dot_config=None):
> +        """Run olddefconfig
> +
> +        dot_config: the .config file for input (optional).
> +        """
> +        return self.__run_conf('--olddefconfig', dot_config=dot_config)
> +
> +    def __allconfig(self, foo, all_config):
> +        """Run all*config
> +
> +        all_config: fragment config file for KCONFIG_ALLCONFIG (optional).
> +        """
> +        if all_config:
> +            all_config_path = os.path.join(self.test_dir, all_config)
> +            extra_env = {'KCONFIG_ALLCONFIG': all_config_path}
> +        else:
> +            extra_env = {}
> +
> +        return self.__run_conf('--all{}config'.format(foo), extra_env=extra_env)
> +
> +    def allyesconfig(self, all_config=None):
> +        """Run allyesconfig
> +        """
> +        return self.__allconfig('yes', all_config)
> +
> +    def allmodconfig(self, all_config=None):
> +        """Run allmodconfig
> +        """
> +        return self.__allconfig('mod', all_config)
> +
> +    def allnoconfig(self, all_config=None):
> +        """Run allnoconfig
> +        """
> +        return self.__allconfig('no', all_config)
> +
> +    def alldefconfig(self, all_config=None):
> +        """Run alldefconfig
> +        """
> +        return self.__allconfig('def', all_config)
> +
> +    def savedefconfig(self, dot_config):
> +        """Run savedefconfig
> +        """
> +        return self.__run_conf('--savedefconfig', out_file='defconfig')
> +
> +    def listnewconfig(self, dot_config=None):
> +        """Run listnewconfig
> +        """
> +        return self.__run_conf('--listnewconfig', dot_config=dot_config,
> +                               out_file=None)
> +
> +    # checkers
> +    def __read_and_compare(self, compare, expected):
> +        """Compare the result with expectation.
> +
> +        Arguments:
> +        compare: function to compare the result with expectation
> +        expected: file that contains the expected data
> +        """
> +        with open(os.path.join(self.test_dir, expected)) as f:
> +            expected_data = f.read()
> +        print(expected_data)
> +        return compare(self, expected_data)
> +
> +    def __contains(self, attr, expected):
> +        print("{0} is expected to contain '{1}':".format(attr, expected))
> +        return self.__read_and_compare(lambda s, e: getattr(s, attr).find(e) >= 0,
> +                                       expected)
> +
> +    def __matches(self, attr, expected):
> +        print("{0} is expected to match '{1}':".format(attr, expected))
> +        return self.__read_and_compare(lambda s, e: getattr(s, attr) == e,
> +                                       expected)
> +
> +    def config_contains(self, expected):
> +        """Check if resulted configuration contains expected data.
> +
> +        Arguments:
> +        expected: file that contains the expected data.
> +        """
> +        return self.__contains('config', expected)
> +
> +    def config_matches(self, expected):
> +        """Check if resulted configuration exactly matches expected data.
> +
> +        Arguments:
> +        expected: file that contains the expected data.
> +        """
> +        return self.__matches('config', expected)
> +
> +    def stdout_contains(self, expected):
> +        """Check if resulted stdout contains expected data.
> +
> +        Arguments:
> +        expected: file that contains the expected data.
> +        """
> +        return self.__contains('stdout', expected)
> +
> +    def stdout_matches(self, cmp_file):
> +        """Check if resulted stdout exactly matches expected data.
> +
> +        Arguments:
> +        expected: file that contains the expected data.
> +        """
> +        return self.__matches('stdout', expected)
> +
> +    def stderr_contains(self, expected):
> +        """Check if resulted stderr contains expected data.
> +
> +        Arguments:
> +        expected: file that contains the expected data.
> +        """
> +        return self.__contains('stderr', expected)
> +
> +    def stderr_matches(self, cmp_file):
> +        """Check if resulted stderr exactly matches expected data.
> +
> +        Arguments:
> +        expected: file that contains the expected data.
> +        """
> +        return self.__matches('stderr', expected)
> +
> +@pytest.fixture(scope="module")
> +def conf(request):
> +    return Conf(request)
> diff --git a/scripts/kconfig/tests/pytest.ini b/scripts/kconfig/tests/pytest.ini
> new file mode 100644
> index 0000000..07b94e0
> --- /dev/null
> +++ b/scripts/kconfig/tests/pytest.ini
> @@ -0,0 +1,6 @@
> +[pytest]
> +addopts = --verbose
> +# Pytest requires that test files have unique names, because pytest imports
> +# them as top-level modules.  It is silly to prefix or suffix a test file with
> +# the directory name that contains it.  Use __init__.py for all test files.
> +python_files = __init__.py
> --
> 2.7.4
>

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

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Masahiro Yamada]

2018-02-08 5:24 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> These messages should be directed to stderr.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  scripts/kconfig/conf.c  | 18 +++++++++++-------
>>  scripts/kconfig/zconf.l | 27 +++++++++++++++------------
>>  2 files changed, 26 insertions(+), 19 deletions(-)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index 307bc3f..90a76aa2 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -75,9 +75,11 @@ static void strip(char *str)
>>  static void check_stdin(void)
>>  {
>>         if (!valid_stdin) {
>> -               printf(_("aborted!\n\n"));
>> -               printf(_("Console input/output is redirected. "));
>> -               printf(_("Run 'make oldconfig' to update configuration.\n\n"));
>> +               fprintf(stderr,
>> +                       _("Aborted!\n"
>> +                         "Console input/output is redirected.\n"
>> +                         "Run 'make oldconfig' to update configuration.\n\n")
>> +                       );
>
> This could use fputs() too, moving the stderr to the last argument.


In general, I am not keen on replacement of (f)printf with (f)puts.

If '%' does not appear in the format literal, compiler will automatically
replace the function call with a faster one.

My compiler replaced both fprintf(stderr, "blah\n") and fputs("blah\n", stderr)
with fwrite.

At this moment, right, the compiler cannot optimize it
because it never knows the result of _(...).

If we rip off the gettext things, it will be optimized.



> I think the _() thingies around the strings are for gettext
> (https://www.gnu.org/software/gettext/manual/html_node/Mark-Keywords.html).
> This would break it if there are existing translations, since the
> msgids change.

Right.  I will keep  inside _(...) as-is just in case
to not break gettext.




> More practically, I doubt anyone is translating these tools. IMO we
> should remove the gettext stuff unless we find traces of translations.

I agree.  Probably, it should not hurt to eliminate gettext,
but out of scope of this series.


>>                 exit(1);
>>         }
>>  }
>> @@ -565,7 +567,7 @@ int main(int ac, char **av)
>>                 }
>>         }
>>         if (ac == optind) {
>> -               printf(_("%s: Kconfig file missing\n"), av[0]);
>> +               fprintf(stderr, _("%s: Kconfig file missing\n"), av[0]);
>>                 conf_usage(progname);
>>                 exit(1);
>>         }
>> @@ -590,9 +592,11 @@ int main(int ac, char **av)
>>                 if (!defconfig_file)
>>                         defconfig_file = conf_get_default_confname();
>>                 if (conf_read(defconfig_file)) {
>> -                       printf(_("***\n"
>> -                               "*** Can't find default configuration \"%s\"!\n"
>> -                               "***\n"), defconfig_file);
>> +                       fprintf(stderr,
>> +                               _("***\n"
>> +                                 "*** Can't find default configuration \"%s\"!\n"
>> +                                 "***\n"),
>> +                               defconfig_file);
>>                         exit(1);
>>                 }
>>                 break;
>> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
>> index 07e074d..0ba4900 100644
>> --- a/scripts/kconfig/zconf.l
>> +++ b/scripts/kconfig/zconf.l
>> @@ -184,7 +184,9 @@ n   [A-Za-z0-9_-]
>>                         append_string(yytext, 1);
>>         }
>>         \n      {
>> -               printf("%s:%d:warning: multi-line strings not supported\n", zconf_curname(), zconf_lineno());
>> +               fprintf(stderr,
>> +                       "%s:%d:warning: multi-line strings not supported\n",
>> +                       zconf_curname(), zconf_lineno());
>
> Whether stuff is translated seems inconsistent too.


Right.  Many people change the same tool, it is difficult to
keep the consistency.




>>                 current_file->lineno++;
>>                 BEGIN(INITIAL);
>>                 return T_EOL;
>> @@ -294,7 +296,7 @@ void zconf_initscan(const char *name)
>>  {
>>         yyin = zconf_fopen(name);
>>         if (!yyin) {
>> -               printf("can't find file %s\n", name);
>> +               fprintf(stderr, "can't find file %s\n", name);
>>                 exit(1);
>>         }
>>
>> @@ -315,8 +317,8 @@ void zconf_nextfile(const char *name)
>>         current_buf->state = YY_CURRENT_BUFFER;
>>         yyin = zconf_fopen(file->name);
>>         if (!yyin) {
>> -               printf("%s:%d: can't open file \"%s\"\n",
>> -                   zconf_curname(), zconf_lineno(), file->name);
>> +               fprintf(stderr, "%s:%d: can't open file \"%s\"\n",
>> +                       zconf_curname(), zconf_lineno(), file->name);
>>                 exit(1);
>>         }
>>         yy_switch_to_buffer(yy_create_buffer(yyin, YY_BUF_SIZE));
>> @@ -325,20 +327,21 @@ void zconf_nextfile(const char *name)
>>
>>         for (iter = current_file->parent; iter; iter = iter->parent ) {
>>                 if (!strcmp(current_file->name,iter->name) ) {
>> -                       printf("%s:%d: recursive inclusion detected. "
>> -                              "Inclusion path:\n  current file : '%s'\n",
>> -                              zconf_curname(), zconf_lineno(),
>> -                              zconf_curname());
>> +                       fprintf(stderr,
>> +                               "%s:%d: recursive inclusion detected. "
>> +                               "Inclusion path:\n  current file : '%s'\n",
>> +                               zconf_curname(), zconf_lineno(),
>> +                               zconf_curname());
>>                         iter = current_file->parent;
>>                         while (iter && \
>>                                strcmp(iter->name,current_file->name)) {
>> -                               printf("  included from: '%s:%d'\n",
>> -                                      iter->name, iter->lineno-1);
>> +                               fprintf(stderr, "  included from: '%s:%d'\n",
>> +                                       iter->name, iter->lineno-1);
>>                                 iter = iter->parent;
>>                         }
>>                         if (iter)
>> -                               printf("  included from: '%s:%d'\n",
>> -                                      iter->name, iter->lineno+1);
>> +                               fprintf(stderr, "  included from: '%s:%d'\n",
>> +                                       iter->name, iter->lineno+1);
>>                         exit(1);
>>                 }
>>         }
>> --
>> 2.7.4
>>
>
> The unrelated gettext stuff aside:
>
> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>
> Cheers,
> Ulf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 01/14] kconfig: send error messages to stderr

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 2:49 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2018-02-08 5:24 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>>> These messages should be directed to stderr.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  scripts/kconfig/conf.c  | 18 +++++++++++-------
>>>  scripts/kconfig/zconf.l | 27 +++++++++++++++------------
>>>  2 files changed, 26 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>>> index 307bc3f..90a76aa2 100644
>>> --- a/scripts/kconfig/conf.c
>>> +++ b/scripts/kconfig/conf.c
>>> @@ -75,9 +75,11 @@ static void strip(char *str)
>>>  static void check_stdin(void)
>>>  {
>>>         if (!valid_stdin) {
>>> -               printf(_("aborted!\n\n"));
>>> -               printf(_("Console input/output is redirected. "));
>>> -               printf(_("Run 'make oldconfig' to update configuration.\n\n"));
>>> +               fprintf(stderr,
>>> +                       _("Aborted!\n"
>>> +                         "Console input/output is redirected.\n"
>>> +                         "Run 'make oldconfig' to update configuration.\n\n")
>>> +                       );
>>
>> This could use fputs() too, moving the stderr to the last argument.
>
>
> In general, I am not keen on replacement of (f)printf with (f)puts.
>
> If '%' does not appear in the format literal, compiler will automatically
> replace the function call with a faster one.
>
> My compiler replaced both fprintf(stderr, "blah\n") and fputs("blah\n", stderr)
> with fwrite.
>
> At this moment, right, the compiler cannot optimize it
> because it never knows the result of _(...).
>
> If we rip off the gettext things, it will be optimized.

No problem with me. Just some personal OCD. :)

>
>
>
>> I think the _() thingies around the strings are for gettext
>> (https://www.gnu.org/software/gettext/manual/html_node/Mark-Keywords.html).
>> This would break it if there are existing translations, since the
>> msgids change.
>
> Right.  I will keep  inside _(...) as-is just in case
> to not break gettext.
>
>
>
>
>> More practically, I doubt anyone is translating these tools. IMO we
>> should remove the gettext stuff unless we find traces of translations.
>
> I agree.  Probably, it should not hurt to eliminate gettext,
> but out of scope of this series.

Yeah, out of scope.

>
>
>>>                 exit(1);
>>>         }
>>>  }
>>> @@ -565,7 +567,7 @@ int main(int ac, char **av)
>>>                 }
>>>         }
>>>         if (ac == optind) {
>>> -               printf(_("%s: Kconfig file missing\n"), av[0]);
>>> +               fprintf(stderr, _("%s: Kconfig file missing\n"), av[0]);
>>>                 conf_usage(progname);
>>>                 exit(1);
>>>         }
>>> @@ -590,9 +592,11 @@ int main(int ac, char **av)
>>>                 if (!defconfig_file)
>>>                         defconfig_file = conf_get_default_confname();
>>>                 if (conf_read(defconfig_file)) {
>>> -                       printf(_("***\n"
>>> -                               "*** Can't find default configuration \"%s\"!\n"
>>> -                               "***\n"), defconfig_file);
>>> +                       fprintf(stderr,
>>> +                               _("***\n"
>>> +                                 "*** Can't find default configuration \"%s\"!\n"
>>> +                                 "***\n"),
>>> +                               defconfig_file);
>>>                         exit(1);
>>>                 }
>>>                 break;
>>> diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l
>>> index 07e074d..0ba4900 100644
>>> --- a/scripts/kconfig/zconf.l
>>> +++ b/scripts/kconfig/zconf.l
>>> @@ -184,7 +184,9 @@ n   [A-Za-z0-9_-]
>>>                         append_string(yytext, 1);
>>>         }
>>>         \n      {
>>> -               printf("%s:%d:warning: multi-line strings not supported\n", zconf_curname(), zconf_lineno());
>>> +               fprintf(stderr,
>>> +                       "%s:%d:warning: multi-line strings not supported\n",
>>> +                       zconf_curname(), zconf_lineno());
>>
>> Whether stuff is translated seems inconsistent too.
>
>
> Right.  Many people change the same tool, it is difficult to
> keep the consistency.
>
>
>
>
>>>                 current_file->lineno++;
>>>                 BEGIN(INITIAL);
>>>                 return T_EOL;
>>> @@ -294,7 +296,7 @@ void zconf_initscan(const char *name)
>>>  {
>>>         yyin = zconf_fopen(name);
>>>         if (!yyin) {
>>> -               printf("can't find file %s\n", name);
>>> +               fprintf(stderr, "can't find file %s\n", name);
>>>                 exit(1);
>>>         }
>>>
>>> @@ -315,8 +317,8 @@ void zconf_nextfile(const char *name)
>>>         current_buf->state = YY_CURRENT_BUFFER;
>>>         yyin = zconf_fopen(file->name);
>>>         if (!yyin) {
>>> -               printf("%s:%d: can't open file \"%s\"\n",
>>> -                   zconf_curname(), zconf_lineno(), file->name);
>>> +               fprintf(stderr, "%s:%d: can't open file \"%s\"\n",
>>> +                       zconf_curname(), zconf_lineno(), file->name);
>>>                 exit(1);
>>>         }
>>>         yy_switch_to_buffer(yy_create_buffer(yyin, YY_BUF_SIZE));
>>> @@ -325,20 +327,21 @@ void zconf_nextfile(const char *name)
>>>
>>>         for (iter = current_file->parent; iter; iter = iter->parent ) {
>>>                 if (!strcmp(current_file->name,iter->name) ) {
>>> -                       printf("%s:%d: recursive inclusion detected. "
>>> -                              "Inclusion path:\n  current file : '%s'\n",
>>> -                              zconf_curname(), zconf_lineno(),
>>> -                              zconf_curname());
>>> +                       fprintf(stderr,
>>> +                               "%s:%d: recursive inclusion detected. "
>>> +                               "Inclusion path:\n  current file : '%s'\n",
>>> +                               zconf_curname(), zconf_lineno(),
>>> +                               zconf_curname());
>>>                         iter = current_file->parent;
>>>                         while (iter && \
>>>                                strcmp(iter->name,current_file->name)) {
>>> -                               printf("  included from: '%s:%d'\n",
>>> -                                      iter->name, iter->lineno-1);
>>> +                               fprintf(stderr, "  included from: '%s:%d'\n",
>>> +                                       iter->name, iter->lineno-1);
>>>                                 iter = iter->parent;
>>>                         }
>>>                         if (iter)
>>> -                               printf("  included from: '%s:%d'\n",
>>> -                                      iter->name, iter->lineno+1);
>>> +                               fprintf(stderr, "  included from: '%s:%d'\n",
>>> +                                       iter->name, iter->lineno+1);
>>>                         exit(1);
>>>                 }
>>>         }
>>> --
>>> 2.7.4
>>>
>>
>> The unrelated gettext stuff aside:
>>
>> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>>
>> Cheers,
>> Ulf
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Best Regards
> Masahiro Yamada

Cheers,
Ulf

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Masahiro Yamada]

2018-02-08 7:55 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Tue, Feb 06, 2018 at 09:34:42AM +0900, Masahiro Yamada wrote:
>> "# CONFIG_... is not set" for choice values are wrongly written into
>> the .config file if they are once visible, then become invisible later.
>>
>>   Test case
>>   ---------
>>
>> ---------------------------(Kconfig)----------------------------
>> config A
>>       bool "A"
>>
>> choice
>>       prompt "Choice ?"
>>       depends on A
>>
>> config CHOICE_B
>>       bool "Choice B"
>>
>> config CHOICE_C
>>       bool "Choice C"
>>
>> endchoice
>> ----------------------------------------------------------------
>>
>> ---------------------------(.config)----------------------------
>> CONFIG_A=y
>> ----------------------------------------------------------------
>>
>> With the Kconfig and .config above,
>>
>>   $ make config
>>   scripts/kconfig/conf  --oldaskconfig Kconfig
>>   *
>>   * Linux Kernel Configuration
>>   *
>>   A (A) [Y/n] n
>>   #
>>   # configuration written to .config
>>   #
>>   $ cat .config
>>   #
>>   # Automatically generated file; DO NOT EDIT.
>>   # Linux Kernel Configuration
>>   #
>>   # CONFIG_A is not set
>>   # CONFIG_CHOICE_B is not set
>>   # CONFIG_CHOICE_C is not set
>>
>> Here,
>>
>>   # CONFIG_CHOICE_B is not set
>>   # CONFIG_CHOICE_C is not set
>>
>> should not be written into the .config file because their dependency
>> "depends on A" is unmet.
>>
>> Currently, there is no code that clears SYMBOL_WRITE of choice values.
>>
>> Clear SYMBOL_WRITE for all symbols in sym_calc_value(), then set it
>> again after calculating visibility.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>
>>  scripts/kconfig/symbol.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>> index c9123ed..5d6f6b1 100644
>> --- a/scripts/kconfig/symbol.c
>> +++ b/scripts/kconfig/symbol.c
>> @@ -371,8 +371,7 @@ void sym_calc_value(struct symbol *sym)
>>               sym->curr.tri = no;
>>               return;
>>       }
>> -     if (!sym_is_choice_value(sym))
>> -             sym->flags &= ~SYMBOL_WRITE;
>> +     sym->flags &= ~SYMBOL_WRITE;
>>
>>       sym_calc_visibility(sym);
>>
>> @@ -385,6 +384,7 @@ void sym_calc_value(struct symbol *sym)
>>               if (sym_is_choice_value(sym) && sym->visible == yes) {
>>                       prop = sym_get_choice_prop(sym);
>>                       newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
>> +                     sym->flags |= SYMBOL_WRITE;
>>               } else {
>>                       if (sym->visible != no) {
>>                               /* if the symbol is visible use the user value
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>
> There's a possible simplification here:
>
> All defined symbols, regardless of type, and regardless of whether
> they're choice value symbols or not, always get written out if they have
> non-n visibility. Therefore, the sym->visible != no check for
> SYMBOL_WRITE can be moved to before the symbol type check, which gets
> rid of two SYMBOL_WRITE assignments and makes it clear that the logic is
> the same for all paths.
>
> This is safe for symbols defined without a type (S_UNKNOWN) too, because
> conf_write() skips those (plus they already generate a warning).
>
> This matches how I do it in the tri_value() function in Kconfiglib:
> https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2574.
> SYMBOL_WRITE corresponds to _write_to_conf.
>
> I've included a patch below. I tested it with the Kconfiglib test suite,
> which verifies that the C implementation still generates the same
> .config file for all defconfig files as well as for
> all{no,yes,def}config, for all ARCHes.
>
> (The Kconfiglib test suite runs scripts/kconfig/conf and compares its
> output against it, which means it doubles as a regression test for the C
> tools.)

Thank you for this.  This is simpler, and please let me take it.

I confirmed the same results were produced.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>> On Tue, Feb 06, 2018 at 09:34:42AM +0900, Masahiro Yamada wrote:
>>> "# CONFIG_... is not set" for choice values are wrongly written into
>>> the .config file if they are once visible, then become invisible later.
>>>
>>>   Test case
>>>   ---------
>>>
>>> ---------------------------(Kconfig)----------------------------
>>> config A
>>>       bool "A"
>>>
>>> choice
>>>       prompt "Choice ?"
>>>       depends on A
>>>
>>> config CHOICE_B
>>>       bool "Choice B"
>>>
>>> config CHOICE_C
>>>       bool "Choice C"
>>>
>>> endchoice
>>> ----------------------------------------------------------------
>>>
>>> ---------------------------(.config)----------------------------
>>> CONFIG_A=y
>>> ----------------------------------------------------------------
>>>
>>> With the Kconfig and .config above,
>>>
>>>   $ make config
>>>   scripts/kconfig/conf  --oldaskconfig Kconfig
>>>   *
>>>   * Linux Kernel Configuration
>>>   *
>>>   A (A) [Y/n] n
>>>   #
>>>   # configuration written to .config
>>>   #
>>>   $ cat .config
>>>   #
>>>   # Automatically generated file; DO NOT EDIT.
>>>   # Linux Kernel Configuration
>>>   #
>>>   # CONFIG_A is not set
>>>   # CONFIG_CHOICE_B is not set
>>>   # CONFIG_CHOICE_C is not set
>>>
>>> Here,
>>>
>>>   # CONFIG_CHOICE_B is not set
>>>   # CONFIG_CHOICE_C is not set
>>>
>>> should not be written into the .config file because their dependency
>>> "depends on A" is unmet.
>>>
>>> Currently, there is no code that clears SYMBOL_WRITE of choice values.
>>>
>>> Clear SYMBOL_WRITE for all symbols in sym_calc_value(), then set it
>>> again after calculating visibility.
>>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>
>>>  scripts/kconfig/symbol.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>> index c9123ed..5d6f6b1 100644
>>> --- a/scripts/kconfig/symbol.c
>>> +++ b/scripts/kconfig/symbol.c
>>> @@ -371,8 +371,7 @@ void sym_calc_value(struct symbol *sym)
>>>               sym->curr.tri = no;
>>>               return;
>>>       }
>>> -     if (!sym_is_choice_value(sym))
>>> -             sym->flags &= ~SYMBOL_WRITE;
>>> +     sym->flags &= ~SYMBOL_WRITE;
>>>
>>>       sym_calc_visibility(sym);
>>>
>>> @@ -385,6 +384,7 @@ void sym_calc_value(struct symbol *sym)
>>>               if (sym_is_choice_value(sym) && sym->visible == yes) {
>>>                       prop = sym_get_choice_prop(sym);
>>>                       newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
>>> +                     sym->flags |= SYMBOL_WRITE;
>>>               } else {
>>>                       if (sym->visible != no) {
>>>                               /* if the symbol is visible use the user value
>>> --
>>> 2.7.4
>>>
>>
>> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>>
>> There's a possible simplification here:
>>
>> All defined symbols, regardless of type, and regardless of whether
>> they're choice value symbols or not, always get written out if they have
>> non-n visibility. Therefore, the sym->visible != no check for
>> SYMBOL_WRITE can be moved to before the symbol type check, which gets
>> rid of two SYMBOL_WRITE assignments and makes it clear that the logic is
>> the same for all paths.
>>
>> This is safe for symbols defined without a type (S_UNKNOWN) too, because
>> conf_write() skips those (plus they already generate a warning).
>>
>> This matches how I do it in the tri_value() function in Kconfiglib:
>> https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2574.
>> SYMBOL_WRITE corresponds to _write_to_conf.
>>
>> I've included a patch below. I tested it with the Kconfiglib test suite,
>> which verifies that the C implementation still generates the same
>> .config file for all defconfig files as well as for
>> all{no,yes,def}config, for all ARCHes.
>>
>> (The Kconfiglib test suite runs scripts/kconfig/conf and compares its
>> output against it, which means it doubles as a regression test for the C
>> tools.)
>
> Thank you for this.  This is simpler, and please let me take it.
>

Could just mod the existing patch. Saves the hassle of creating a new one. :)

> I confirmed the same results were produced.
>
> --
> Best Regards
> Masahiro Yamada

Cheers,
Ulf

Re: [PATCH 04/14] kconfig: print additional new line for choice for redirection

[Masahiro Yamada]

2018-02-08 8:34 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> If stdout is redirected to a file, prompts look differently due to
>> missing new lines.
>>
>> Currently, conf_askvalue() takes care of this by putting additional
>> new line, but conf_choice() does not.  Do likewise so that prompts
>> after 'choice' look properly.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  scripts/kconfig/conf.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
>> index d346642..6ce06c8 100644
>> --- a/scripts/kconfig/conf.c
>> +++ b/scripts/kconfig/conf.c
>> @@ -317,6 +317,8 @@ static int conf_choice(struct menu *menu)
>>                 case oldaskconfig:
>>                         fflush(stdout);
>>                         xfgets(line, sizeof(line), stdin);
>> +                       if (!tty_stdio)
>> +                               printf("\n");
>>                         strip(line);
>>                         if (line[0] == '?') {
>>                                 print_help(menu);
>> --
>> 2.7.4
>>
>
> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>
> Maybe this could be moved into the xfgets() function as well.
>

Thanks for your comment!
Yes, it is better to move this into xfgets().

I improve this a bit more so that the redirected stdout
contains not only '\n' but also input keys.

https://patchwork.kernel.org/patch/10206611/


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 02/14] kconfig: do not write choice values when their dependency becomes n

[Ulf Magnusson]

On Thu, Feb 8, 2018 at 3:46 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote:
> On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
>> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
>>> On Tue, Feb 06, 2018 at 09:34:42AM +0900, Masahiro Yamada wrote:
>>>> "# CONFIG_... is not set" for choice values are wrongly written into
>>>> the .config file if they are once visible, then become invisible later.
>>>>
>>>>   Test case
>>>>   ---------
>>>>
>>>> ---------------------------(Kconfig)----------------------------
>>>> config A
>>>>       bool "A"
>>>>
>>>> choice
>>>>       prompt "Choice ?"
>>>>       depends on A
>>>>
>>>> config CHOICE_B
>>>>       bool "Choice B"
>>>>
>>>> config CHOICE_C
>>>>       bool "Choice C"
>>>>
>>>> endchoice
>>>> ----------------------------------------------------------------
>>>>
>>>> ---------------------------(.config)----------------------------
>>>> CONFIG_A=y
>>>> ----------------------------------------------------------------
>>>>
>>>> With the Kconfig and .config above,
>>>>
>>>>   $ make config
>>>>   scripts/kconfig/conf  --oldaskconfig Kconfig
>>>>   *
>>>>   * Linux Kernel Configuration
>>>>   *
>>>>   A (A) [Y/n] n
>>>>   #
>>>>   # configuration written to .config
>>>>   #
>>>>   $ cat .config
>>>>   #
>>>>   # Automatically generated file; DO NOT EDIT.
>>>>   # Linux Kernel Configuration
>>>>   #
>>>>   # CONFIG_A is not set
>>>>   # CONFIG_CHOICE_B is not set
>>>>   # CONFIG_CHOICE_C is not set
>>>>
>>>> Here,
>>>>
>>>>   # CONFIG_CHOICE_B is not set
>>>>   # CONFIG_CHOICE_C is not set
>>>>
>>>> should not be written into the .config file because their dependency
>>>> "depends on A" is unmet.
>>>>
>>>> Currently, there is no code that clears SYMBOL_WRITE of choice values.
>>>>
>>>> Clear SYMBOL_WRITE for all symbols in sym_calc_value(), then set it
>>>> again after calculating visibility.
>>>>
>>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>>> ---
>>>
>>>>  scripts/kconfig/symbol.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
>>>> index c9123ed..5d6f6b1 100644
>>>> --- a/scripts/kconfig/symbol.c
>>>> +++ b/scripts/kconfig/symbol.c
>>>> @@ -371,8 +371,7 @@ void sym_calc_value(struct symbol *sym)
>>>>               sym->curr.tri = no;
>>>>               return;
>>>>       }
>>>> -     if (!sym_is_choice_value(sym))
>>>> -             sym->flags &= ~SYMBOL_WRITE;
>>>> +     sym->flags &= ~SYMBOL_WRITE;
>>>>
>>>>       sym_calc_visibility(sym);
>>>>
>>>> @@ -385,6 +384,7 @@ void sym_calc_value(struct symbol *sym)
>>>>               if (sym_is_choice_value(sym) && sym->visible == yes) {
>>>>                       prop = sym_get_choice_prop(sym);
>>>>                       newval.tri = (prop_get_symbol(prop)->curr.val == sym) ? yes : no;
>>>> +                     sym->flags |= SYMBOL_WRITE;
>>>>               } else {
>>>>                       if (sym->visible != no) {
>>>>                               /* if the symbol is visible use the user value
>>>> --
>>>> 2.7.4
>>>>
>>>
>>> Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com>
>>>
>>> There's a possible simplification here:
>>>
>>> All defined symbols, regardless of type, and regardless of whether
>>> they're choice value symbols or not, always get written out if they have
>>> non-n visibility. Therefore, the sym->visible != no check for
>>> SYMBOL_WRITE can be moved to before the symbol type check, which gets
>>> rid of two SYMBOL_WRITE assignments and makes it clear that the logic is
>>> the same for all paths.
>>>
>>> This is safe for symbols defined without a type (S_UNKNOWN) too, because
>>> conf_write() skips those (plus they already generate a warning).
>>>
>>> This matches how I do it in the tri_value() function in Kconfiglib:
>>> https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L2574.
>>> SYMBOL_WRITE corresponds to _write_to_conf.
>>>
>>> I've included a patch below. I tested it with the Kconfiglib test suite,
>>> which verifies that the C implementation still generates the same
>>> .config file for all defconfig files as well as for
>>> all{no,yes,def}config, for all ARCHes.
>>>
>>> (The Kconfiglib test suite runs scripts/kconfig/conf and compares its
>>> output against it, which means it doubles as a regression test for the C
>>> tools.)
>>
>> Thank you for this.  This is simpler, and please let me take it.
>>
>
> Could just mod the existing patch. Saves the hassle of creating a new one. :)
>
>> I confirmed the same results were produced.
>>
>> --
>> Best Regards
>> Masahiro Yamada
>
> Cheers,
> Ulf

I might've misunderstood.

Should I prepare a separate patch that adds the simplification,
assuming your patch? I'm fine with whatever approach.

Could always say "includes a simplification suggested by..." if you go
with a single patch and want to give credit (which isn't required).

Cheers,
Ulf

Re: [PATCH 00/14] Add Kconfig unit tests

[Sam Ravnborg]

Hi Masahiro.

On Tue, Feb 06, 2018 at 09:34:40AM +0900, Masahiro Yamada wrote:
> I am applying various patches to Kconfig these days.
> 
> However, I fear regressions.  I have been thinking of unit-tests.
> 
> There are various cryptic parts in Kconfig and corner cases where
> it is difficult to notice breakage.  If unit-tests cover those,
> I will be able to apply changes more confidently.
> 
> So, here is the trial.
...

Great to see this done.
And +1 for basing this on a standard framework rather than inventing your own.

> If you have an idea for better implementation, comments are appreciated.
Nope, please continue with pytest.

	Sam