On Sat, Jun 25, 2022 at 4:24 AM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > > On Fri, Jun 24, 2022 at 12:55 AM David Gow <davidgow@xxxxxxxxxx> 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. > I like adding a verb, personally. > > > > 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@xxxxxxxxxx/ > > 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. Ah, yep: my mistake. I was mixing up the (!b && value=n) check in is_subset_of with the (b && value != b) in differing options. Whoops. But, thinking about it, the special handling of "=n" in set A not needing an equivalent in set B should fall out from the set difference anyway. So no problem here at all. > There's probably a way I can name these functions better to make the > difference more clear. How about "conflicting_options" versus "missing_options"? > 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? Fair enough point: > > Daniel
