New subject: [PATCH v3 2/3] kunit: tool: make --kunitconfig repeatable, blindly concat

27 Jun 2022

Currently, you cannot ovewrwrite what's in your kunitconfig via
--kconfig_add.
Nor can you override something in a qemu_config via either means.
This patch makes it so we have this level of priority
* --kconfig_add
* kunitconfig file (the default or the one from --kunitconfig)
* qemu_config
The rationale for this order is that the more "dynamic" sources of
kconfig options should take priority.
--kconfig_add is obviously the most dynamic.
And for kunitconfig, users probably tweak the file manually or specify
--kunitconfig more often than they delve into qemu_config python files.
And internally, we convert the kconfigs from a python list into a set or
dict fairly often. We should just use a dict internally.
We exposed the set transform in the past since we didn't define __eq__,
so also take the chance to shore up the kunit_kconfig.Kconfig interface.
Example
=======
Let's consider the unrealistic example where someone would want to
disable CONFIG_KUNIT.
I.e. they run
$ ./tools/testing/kunit/kunit.py config --kconfig_add=CONFIG_KUNIT=n
Before
------
We'd write the following
...
# CONFIG_KUNIT is not set
CONFIG_KUNIT_ALL_TESTS=y
CONFIG_KUNIT_TEST=y
CONFIG_KUNIT=y
CONFIG_KUNIT_EXAMPLE_TEST=y
And we'd error out with
...
ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: # CONFIG_KUNIT is not set
After
-----
We'd write the following
...
# CONFIG_KUNIT is not set
CONFIG_KUNIT_TEST=y
CONFIG_KUNIT_ALL_TESTS=y
CONFIG_KUNIT_EXAMPLE_TEST=y
And we'd error out with
...
ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y, CONFIG_KUNIT_ALL_TESTS=y
Signed-off-by: Daniel Latypov dlatypov@google.com
Reviewed-by: David Gow davidgow@google.com
---
v1 -> v2: fix validate_config() func.
There was a bug found by David, see
https://lore.kernel.org/linux-kselftest/CAGS_qxpF338dvbB+6QW1n8_agddeS10+nkT...
v2 -> v3: remove `set_diff()` helper, merge into other kunitconfig
series
---
 tools/testing/kunit/kunit_config.py    | 45 ++++++++++++++------------
 tools/testing/kunit/kunit_kernel.py    | 20 ++++++------
 tools/testing/kunit/kunit_tool_test.py | 45 +++++++++++---------------
 3 files changed, 54 insertions(+), 56 deletions(-)

diff --git a/tools/testing/kunit/kunit_config.py b/tools/testing/kunit/kunit_config.py
index 75a8dc1683d4..898b2a35eb29 100644
--- a/tools/testing/kunit/kunit_config.py
+++ b/tools/testing/kunit/kunit_config.py
@@ -8,7 +8,7 @@
from dataclasses import dataclass
 import re
-from typing import List, Set
+from typing import Dict, Iterable, Set
CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$'
 CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$'
@@ -32,35 +32,42 @@ class Kconfig:
    """Represents defconfig or .config specified using the Kconfig language."""
def __init__(self) -> None:
-		self._entries = []  # type: List[KconfigEntry]
+		self._entries = {}  # type: Dict[str, str]
-	def entries(self) -> Set[KconfigEntry]:
-		return set(self._entries)
+	def __eq__(self, other) -> bool:
+		if not isinstance(other, self.__class__):
+			return False
+		return self._entries == other._entries
-	def add_entry(self, entry: KconfigEntry) -> None:
-		self._entries.append(entry)
+	def __repr__(self) -> str:
+		return ','.join(str(e) for e in self.as_entries())
+
+	def as_entries(self) -> Iterable[KconfigEntry]:
+		for name, value in self._entries.items():
+			yield KconfigEntry(name, value)
+
+	def add_entry(self, name: str, value: str) -> None:
+		self._entries[name] = value
def is_subset_of(self, other: 'Kconfig') -> bool:
-		other_dict = {e.name: e.value for e in other.entries()}
-		for a in self.entries():
-			b = other_dict.get(a.name)
+		for name, value in self._entries.items():
+			b = other._entries.get(name)
    		if b is None:
-				if a.value == 'n':
+				if value == 'n':
    				continue
    			return False
-			if a.value != b:
+			if value != b:
    			return False
    	return True
def merge_in_entries(self, other: 'Kconfig') -> None:
-		if other.is_subset_of(self):
-			return
-		self._entries = list(self.entries().union(other.entries()))
+		for name, value in other._entries.items():
+			self._entries[name] = value
def write_to_file(self, path: str) -> None:
    	with open(path, 'a+') as f:
-			for entry in self.entries():
-				f.write(str(entry) + '\n')
+			for e in self.as_entries():
+				f.write(str(e) + '\n')
def parse_file(path: str) -> Kconfig:
    with open(path, 'r') as f:
@@ -78,14 +85,12 @@ def parse_from_string(blob: str) -> Kconfig:
match = config_matcher.match(line)
    	if match:
-			entry = KconfigEntry(match.group(1), match.group(2))
-			kconfig.add_entry(entry)
+			kconfig.add_entry(match.group(1), match.group(2))
    		continue
empty_match = is_not_set_matcher.match(line)
    	if empty_match:
-			entry = KconfigEntry(empty_match.group(1), 'n')
-			kconfig.add_entry(entry)
+			kconfig.add_entry(empty_match.group(1), 'n')
    		continue
if line[0] == '#':
diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
index 3539efaf99ba..4115781185e1 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -53,8 +53,8 @@ class LinuxSourceTreeOperations:
    	except subprocess.CalledProcessError as e:
    		raise ConfigError(e.output.decode())
-	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
-		pass
+	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
+		return base_kunitconfig
def make_allyesconfig(self, build_dir: str, make_options) -> None:
    	raise ConfigError('Only the "um" arch is supported for alltests')
@@ -109,9 +109,10 @@ class LinuxSourceTreeOperationsQemu(LinuxSourceTreeOperations):
    	self._kernel_command_line = qemu_arch_params.kernel_command_line + ' kunit_shutdown=reboot'
    	self._extra_qemu_params = qemu_arch_params.extra_qemu_params
-	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> None:
+	def make_arch_qemuconfig(self, base_kunitconfig: kunit_config.Kconfig) -> kunit_config.Kconfig:
    	kconfig = kunit_config.parse_from_string(self._kconfig)
-		base_kunitconfig.merge_in_entries(kconfig)
+		kconfig.merge_in_entries(base_kunitconfig)
+		return kconfig
def start(self, params: List[str], build_dir: str) -> subprocess.Popen:
    	kernel_path = os.path.join(build_dir, self._kernel_path)
@@ -267,10 +268,10 @@ class LinuxSourceTree:
    	validated_kconfig = kunit_config.parse_file(kconfig_path)
    	if self._kconfig.is_subset_of(validated_kconfig):
    		return True
-		invalid = self._kconfig.entries() - validated_kconfig.entries()
+		missing = set(self._kconfig.as_entries()) - set(validated_kconfig.as_entries())
    	message = 'Not all Kconfig options selected in kunitconfig were in the generated .config.\n' \
    		  'This is probably due to unsatisfied dependencies.\n' \
-			  'Missing: ' + ', '.join([str(e) for e in invalid])
+			  'Missing: ' + ', '.join(str(e) for e in missing)
    	if self._arch == 'um':
    		message += '\nNote: many Kconfig options aren't available on UML. You can try running ' \
    			   'on a different architecture with something like "--arch=x86_64".'
@@ -282,7 +283,7 @@ class LinuxSourceTree:
    	if build_dir and not os.path.exists(build_dir):
    		os.mkdir(build_dir)
    	try:
-			self._ops.make_arch_qemuconfig(self._kconfig)
+			self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
    		self._kconfig.write_to_file(kconfig_path)
    		self._ops.make_olddefconfig(build_dir, make_options)
    	except ConfigError as e:
@@ -303,7 +304,7 @@ class LinuxSourceTree:
    		return True
old_kconfig = kunit_config.parse_file(old_path)
-		return old_kconfig.entries() != self._kconfig.entries()
+		return old_kconfig != self._kconfig
def build_reconfig(self, build_dir: str, make_options) -> bool:
    	"""Creates a new .config if it is not a subset of the .kunitconfig."""
@@ -313,7 +314,8 @@ class LinuxSourceTree:
    		return self.build_config(build_dir, make_options)
existing_kconfig = kunit_config.parse_file(kconfig_path)
-		self._ops.make_arch_qemuconfig(self._kconfig)
+		self._kconfig = self._ops.make_arch_qemuconfig(self._kconfig)
+
    	if self._kconfig.is_subset_of(existing_kconfig) and not self._kunitconfig_changed(build_dir):
    		return True
    	print('Regenerating .config ...')
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 25a2eb3bf114..0fbca18b6e65 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -45,7 +45,7 @@ class KconfigTest(unittest.TestCase):
    	self.assertTrue(kconfig0.is_subset_of(kconfig0))
kconfig1 = kunit_config.Kconfig()
-		kconfig1.add_entry(kunit_config.KconfigEntry('TEST', 'y'))
+		kconfig1.add_entry('TEST', 'y')
    	self.assertTrue(kconfig1.is_subset_of(kconfig1))
    	self.assertTrue(kconfig0.is_subset_of(kconfig1))
    	self.assertFalse(kconfig1.is_subset_of(kconfig0))
@@ -56,40 +56,28 @@ class KconfigTest(unittest.TestCase):
    	kconfig = kunit_config.parse_file(kconfig_path)
expected_kconfig = kunit_config.Kconfig()
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('UML', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MMU', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('EXAMPLE_TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MK8', 'n'))
-
-		self.assertEqual(kconfig.entries(), expected_kconfig.entries())
+		expected_kconfig.add_entry('UML', 'y')
+		expected_kconfig.add_entry('MMU', 'y')
+		expected_kconfig.add_entry('TEST', 'y')
+		expected_kconfig.add_entry('EXAMPLE_TEST', 'y')
+		expected_kconfig.add_entry('MK8', 'n')
+
+		self.assertEqual(kconfig, expected_kconfig)
def test_write_to_file(self):
    	kconfig_path = os.path.join(test_tmpdir, '.config')
expected_kconfig = kunit_config.Kconfig()
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('UML', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MMU', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('EXAMPLE_TEST', 'y'))
-		expected_kconfig.add_entry(
-			kunit_config.KconfigEntry('MK8', 'n'))
+		expected_kconfig.add_entry('UML', 'y')
+		expected_kconfig.add_entry('MMU', 'y')
+		expected_kconfig.add_entry('TEST', 'y')
+		expected_kconfig.add_entry('EXAMPLE_TEST', 'y')
+		expected_kconfig.add_entry('MK8', 'n')
expected_kconfig.write_to_file(kconfig_path)
actual_kconfig = kunit_config.parse_file(kconfig_path)
-
-		self.assertEqual(actual_kconfig.entries(),
-				 expected_kconfig.entries())
+		self.assertEqual(actual_kconfig, expected_kconfig)
class KUnitParserTest(unittest.TestCase):
@@ -381,8 +369,11 @@ class LinuxSourceTreeTest(unittest.TestCase):
    		kunit_kernel.LinuxSourceTree('', kunitconfig_path=dir)
def test_kconfig_add(self):
+		want_kconfig = kunit_config.Kconfig()
+		want_kconfig.add_entry('NOT_REAL', 'y')
+
    	tree = kunit_kernel.LinuxSourceTree('', kconfig_add=['CONFIG_NOT_REAL=y'])
-		self.assertIn(kunit_config.KconfigEntry('NOT_REAL', 'y'), tree._kconfig.entries())
+		self.assertTrue(want_kconfig.is_subset_of(tree._kconfig), msg=tree._kconfig)
def test_invalid_arch(self):
    	with self.assertRaisesRegex(kunit_kernel.ConfigError, 'not a valid arch, options are.*x86_64'):
base-commit: 274295c6e53f8b8b8dfa8b24a3fcb8a9d670c22c
-- 
2.37.0.rc0.161.g10f37bed90-goog


    

[PATCH v3 1/3] kunit: tool: refactor internal kconfig handling, allow overriding

Example

Before

After