On Fri, Jun 24, 2022 at 12:55 AM David Gow davidgow@google.com wrote:
def set_diff(self, other: 'Kconfig') -> Set[KconfigEntry]:return set(self._as_entries()) - set(other._as_entries())It took me a couple of goes to realise that this was looking at the difference between sets, not trying to set the difference. Maybe different_entries() or something like that'd be clearer, but I can't say it bothers me enough to be worth a new version.
Wdyt about `set_difference()`? Or maybe adding a verb: `get_set_diff()`, `compute_set_diff()`?
But we do want to make it clear it has set (asymmetric) difference semantics, see below.
Then again (as noted below), the direct set difference isn't exactly what we want, it's more the equivalent of is_subset_of(). The follow-up repeated-kunitconfig patch adds differing_options(), which is closer to what we'd want, I think: https://lore.kernel.org/linux-kselftest/20220624001247.3255978-1-dlatypov@go...
differing_options() does not have the right semantics. For this, we do explicitly want a set difference.
Context: This is the func used to print out these warnings: $ .../kunit.py run --kconfig_add=CONFIG_PCI=y ... Missing: CONFIG_PCI=y
That comes from invalid = self._kconfig.set_diff(validated_kconfig) Using differing_options() to get our version of the configs: invalid = (want for want, got in self._kconfig.differing_options(validated_kconfig)) we instead get an empty list.
The problem is that differing_options() only shows config options that are explicitly to different values.
There's probably a way I can name these functions better to make the difference more clear. Or perhaps we should move set_diff() out of this class and have kunit_kernel.py itself due the computation.
E.g. // make as_entries() "public", s/invalid/missing missing = set(self._kconfig.as_entries()) - set(validated_config.as_entries())
Thoughts?
Daniel