| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABVgOSnNfp9kqVSyM-GdUuHu2CSysZS9yNMxb1C-jJNpwHeLuw@mail.gmail.com>
Date: Sat, 20 Nov 2021 08:40:24 +0800
From: David Gow <davidgow@...gle.com>
To: Daniel Latypov <dlatypov@...gle.com>
Cc: brendanhiggins@...gle.com, linux-kernel@...r.kernel.org,
kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org,
skhan@...uxfoundation.org
Subject: Re: [PATCH v2] kunit: tool: reconfigure when the used kunitconfig changes
On Sat, Nov 20, 2021 at 7:23 AM Daniel Latypov <dlatypov@...gle.com> wrote:
>
> Problem: currently, if you remove something from your kunitconfig,
> kunit.py will not regenerate the .config file.
> The same thing happens if you did --kunitconfig_add=CONFIG_KASAN=y [1]
> and then ran again without it. Your new run will still have KASAN.
>
> The reason is that kunit.py won't regenerate the .config file if it's a
> superset of the kunitconfig. This speeds it up a bit for iterating.
>
> This patch adds an additional check that forces kunit.py to regenerate
> the .config file if the current kunitconfig doesn't match the previous
> one.
>
> What this means:
> * deleting entries from .kunitconfig works as one would expect
> * dropping a --kunitconfig_add also triggers a rebuild
> * you can still edit .config directly to turn on new options
>
> We implement this by creating a `last_used_kunitconfig` file in the
> build directory (so .kunit, by default) after we generate the .config.
> When comparing the kconfigs, we compare python sets, so duplicates and
> permutations don't trip us up.
>
> The majority of this patch is adding unit tests for the existing logic
> and for the new case where `last_used_kunitconfig` differs.
>
> [1] https://lore.kernel.org/linux-kselftest/20211106013058.2621799-2-dlatypov@google.com/
>
> Signed-off-by: Daniel Latypov <dlatypov@...gle.com>
> ---
Thanks! No problems with this version, looks good to go!
Reviewed-by: David Gow <davidgow@...gle.com>
Cheers,
-- David
> Note: this patch is based on
> https://lore.kernel.org/linux-kselftest/20211106013058.2621799-1-dlatypov@google.com/
> This patch will work without it, but there'll be a false merge conflict.
>
> v1 -> v2:
> * always regenerate if last_used_kunitconfig doesn't exist
> * don't call os.remove() if last_used_kunitconfig doesn't exist
> * add in get_old_kunitconfig_path() to match get_kunitconfig_path()
> * s/_kconfig_changed/_kunitconfig_changed
> ---
> Documentation/dev-tools/kunit/start.rst | 8 ++---
> tools/testing/kunit/kunit_kernel.py | 40 ++++++++++++++++------
> tools/testing/kunit/kunit_tool_test.py | 45 +++++++++++++++++++++++++
> 3 files changed, 78 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/start.rst b/Documentation/dev-tools/kunit/start.rst
> index 1e00f9226f74..0a5e65540974 100644
> --- a/Documentation/dev-tools/kunit/start.rst
> +++ b/Documentation/dev-tools/kunit/start.rst
> @@ -50,10 +50,10 @@ It'll warn you if you haven't included the dependencies of the options you're
> using.
>
> .. note::
> - Note that removing something from the ``.kunitconfig`` will not trigger a
> - rebuild of the ``.config`` file: the configuration is only updated if the
> - ``.kunitconfig`` is not a subset of ``.config``. This means that you can use
> - other tools (such as make menuconfig) to adjust other config options.
> + If you change the ``.kunitconfig``, kunit.py will trigger a rebuild of the
> + ``.config`` file. But you can edit the ``.config`` file directly or with
> + tools like ``make menuconfig O=.kunit``. As long as its a superset of
> + ``.kunitconfig``, kunit.py won't overwrite your changes.
>
>
> Running the tests (KUnit Wrapper)
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index 350883672be0..12085e04a80c 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -21,6 +21,7 @@ import qemu_config
>
> KCONFIG_PATH = '.config'
> KUNITCONFIG_PATH = '.kunitconfig'
> +OLD_KUNITCONFIG_PATH = 'last_used_kunitconfig'
> DEFAULT_KUNITCONFIG_PATH = 'tools/testing/kunit/configs/default.config'
> BROKEN_ALLCONFIG_PATH = 'tools/testing/kunit/configs/broken_on_uml.config'
> OUTFILE_PATH = 'test.log'
> @@ -179,6 +180,9 @@ def get_kconfig_path(build_dir) -> str:
> def get_kunitconfig_path(build_dir) -> str:
> return get_file_path(build_dir, KUNITCONFIG_PATH)
>
> +def get_old_kunitconfig_path(build_dir) -> str:
> + return get_file_path(build_dir, OLD_KUNITCONFIG_PATH)
> +
> def get_outfile_path(build_dir) -> str:
> return get_file_path(build_dir, OUTFILE_PATH)
>
> @@ -289,24 +293,38 @@ class LinuxSourceTree(object):
> except ConfigError as e:
> logging.error(e)
> return False
> - return self.validate_config(build_dir)
> + if not self.validate_config(build_dir):
> + return False
> +
> + old_path = get_old_kunitconfig_path(build_dir)
> + if os.path.exists(old_path):
> + os.remove(old_path) # write_to_file appends to the file
> + self._kconfig.write_to_file(old_path)
> + return True
> +
> + def _kunitconfig_changed(self, build_dir: str) -> bool:
> + old_path = get_old_kunitconfig_path(build_dir)
> + if not os.path.exists(old_path):
> + return True
> +
> + old_kconfig = kunit_config.parse_file(old_path)
> + return old_kconfig.entries() != self._kconfig.entries()
>
> def build_reconfig(self, build_dir, make_options) -> bool:
> """Creates a new .config if it is not a subset of the .kunitconfig."""
> kconfig_path = get_kconfig_path(build_dir)
> - if os.path.exists(kconfig_path):
> - existing_kconfig = kunit_config.parse_file(kconfig_path)
> - self._ops.make_arch_qemuconfig(self._kconfig)
> - if not self._kconfig.is_subset_of(existing_kconfig):
> - print('Regenerating .config ...')
> - os.remove(kconfig_path)
> - return self.build_config(build_dir, make_options)
> - else:
> - return True
> - else:
> + if not os.path.exists(kconfig_path):
> print('Generating .config ...')
> return self.build_config(build_dir, make_options)
>
> + existing_kconfig = kunit_config.parse_file(kconfig_path)
> + 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 ...')
> + os.remove(kconfig_path)
> + return self.build_config(build_dir, make_options)
> +
> def build_kernel(self, alltests, jobs, build_dir, make_options) -> bool:
> try:
> if alltests:
> diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
> index 7e42a7c27987..572f133511aa 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -358,6 +358,51 @@ class LinuxSourceTreeTest(unittest.TestCase):
> with open(kunit_kernel.get_outfile_path(build_dir), 'rt') as outfile:
> self.assertEqual(outfile.read(), 'hi\nbye\n', msg='Missing some output')
>
> + def test_build_reconfig_no_config(self):
> + with tempfile.TemporaryDirectory('') as build_dir:
> + with open(kunit_kernel.get_kunitconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y')
> +
> + tree = kunit_kernel.LinuxSourceTree(build_dir)
> + mock_build_config = mock.patch.object(tree, 'build_config').start()
> +
> + # Should generate the .config
> + self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
> + mock_build_config.assert_called_once_with(build_dir, [])
> +
> + def test_build_reconfig_existing_config(self):
> + with tempfile.TemporaryDirectory('') as build_dir:
> + # Existing .config is a superset, should not touch it
> + with open(kunit_kernel.get_kunitconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y')
> + with open(kunit_kernel.get_old_kunitconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y')
> + with open(kunit_kernel.get_kconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
> +
> + tree = kunit_kernel.LinuxSourceTree(build_dir)
> + mock_build_config = mock.patch.object(tree, 'build_config').start()
> +
> + self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
> + self.assertEqual(mock_build_config.call_count, 0)
> +
> + def test_build_reconfig_remove_option(self):
> + with tempfile.TemporaryDirectory('') as build_dir:
> + # We removed CONFIG_KUNIT_TEST=y from our .kunitconfig...
> + with open(kunit_kernel.get_kunitconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y')
> + with open(kunit_kernel.get_old_kunitconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
> + with open(kunit_kernel.get_kconfig_path(build_dir), 'w') as f:
> + f.write('CONFIG_KUNIT=y\nCONFIG_KUNIT_TEST=y')
> +
> + tree = kunit_kernel.LinuxSourceTree(build_dir)
> + mock_build_config = mock.patch.object(tree, 'build_config').start()
> +
> + # ... so we should trigger a call to build_config()
> + self.assertTrue(tree.build_reconfig(build_dir, make_options=[]))
> + mock_build_config.assert_called_once_with(build_dir, [])
> +
> # TODO: add more test cases.
>
>
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>
Powered by blists - more mailing lists