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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
.../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
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 <[email protected]>
---
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
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 <[email protected]>
---
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 <[email protected]>
+#
+
+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)
+
[email protected](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
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 <[email protected]>
---
.../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
If recursive inclusion is detected, it should fail with error messages.
Test this.
Signed-off-by: Masahiro Yamada <[email protected]>
---
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
'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 <[email protected]>
---
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
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 <[email protected]>
---
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
These messages should be directed to stderr.
Signed-off-by: Masahiro Yamada <[email protected]>
---
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
"# 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 <[email protected]>
---
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
Recursive dependency should be detected and warned. Test this.
Signed-off-by: Masahiro Yamada <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
>
> 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
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
2018-02-06 18:34 GMT+09:00 Greg Kroah-Hartman <[email protected]>:
> 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 <[email protected]>
>> ---
>>
>> 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
On Tue, Feb 06, 2018 at 07:44:22PM +0900, Masahiro Yamada wrote:
> 2018-02-06 18:34 GMT+09:00 Greg Kroah-Hartman <[email protected]>:
> > 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 <[email protected]>
> >> ---
> >>
> >> 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
> 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
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> wrote:
> These messages should be directed to stderr.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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 <[email protected]>
Cheers,
Ulf
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
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 <[email protected]>
> ---
>
> 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 <[email protected]>
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
On Tue, Feb 6, 2018 at 10:38 AM, Greg Kroah-Hartman
<[email protected]> 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
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
Maybe this could be moved into the xfgets() function as well.
Cheers,
Ulf
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> .../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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
This indirectly tests that choices specified without a type get the
type of the first choice value specified with a type too.
Cheers,
Ulf
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> .../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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> wrote:
> Recursive dependency should be detected and warned. Test this.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> wrote:
> If recursive inclusion is detected, it should fail with error messages.
> Test this.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> 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 <[email protected]>
On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
<[email protected]> 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 <[email protected]>
> ---
>
> 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 <[email protected]>
> +#
> +
> +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)
> +
> [email protected](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 <[email protected]>
2018-02-08 5:24 GMT+09:00 Ulf Magnusson <[email protected]>:
> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
> <[email protected]> wrote:
>> These messages should be directed to stderr.
>>
>> Signed-off-by: Masahiro Yamada <[email protected]>
>> ---
>>
>> 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 <[email protected]>
>
> Cheers,
> Ulf
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
On Thu, Feb 8, 2018 at 2:49 AM, Masahiro Yamada
<[email protected]> wrote:
> 2018-02-08 5:24 GMT+09:00 Ulf Magnusson <[email protected]>:
>> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
>> <[email protected]> wrote:
>>> These messages should be directed to stderr.
>>>
>>> Signed-off-by: Masahiro Yamada <[email protected]>
>>> ---
>>>
>>> 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 <[email protected]>
>>
>> Cheers,
>> Ulf
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Best Regards
> Masahiro Yamada
Cheers,
Ulf
2018-02-08 7:55 GMT+09:00 Ulf Magnusson <[email protected]>:
> 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 <[email protected]>
>> ---
>
>> 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 <[email protected]>
>
> 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
On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
<[email protected]> wrote:
> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson <[email protected]>:
>> 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 <[email protected]>
>>> ---
>>
>>> 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 <[email protected]>
>>
>> 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
2018-02-08 8:34 GMT+09:00 Ulf Magnusson <[email protected]>:
> On Tue, Feb 6, 2018 at 1:34 AM, Masahiro Yamada
> <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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 <[email protected]>
>
> 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
On Thu, Feb 8, 2018 at 3:46 AM, Ulf Magnusson <[email protected]> wrote:
> On Thu, Feb 8, 2018 at 3:42 AM, Masahiro Yamada
> <[email protected]> wrote:
>> 2018-02-08 7:55 GMT+09:00 Ulf Magnusson <[email protected]>:
>>> 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 <[email protected]>
>>>> ---
>>>
>>>> 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 <[email protected]>
>>>
>>> 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
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