On Tue, 4 Feb 2020 16:46:06 -0800 Brendan Higgins <brendanhiggins@xxxxxxxxxx> wrote: > Sorry for the delay. > > On Mon, Jan 27, 2020 at 10:03 PM SeongJae Park <sj38.park@xxxxxxxxx> wrote: > > > > On Mon, 27 Jan 2020 16:02:48 -0800 Brendan Higgins <brendanhiggins@xxxxxxxxxx> wrote: > > > > > On Sat, Jan 25, 2020 at 5:59 PM <sj38.park@xxxxxxxxx> wrote: > > > > > > > > From: SeongJae Park <sjpark@xxxxxxxxx> > > > > > > > > Deletions of configs in the '.kunitconfig' is not applied because kunit > > > > rebuilds '.config' only if the '.config' is not a subset of the > > > > '.kunitconfig'. To allow the deletions to applied, this commit modifies > > > > the '.config' rebuild condition to addtionally check the modified times > > > > of those files. > > > > > > The reason it only checks that .kunitconfig is a subset of .config is > > > because we don't want the .kunitconfig to remove options just because > > > it doesn't recognize them. > > > > > > It runs `make ARCH=um olddefconfig` on the .config that it generates > > > from the .kunitconfig, and most of the time that means you will get a > > > .config with lots of things in it that aren't in the .kunitconfig. > > > Consequently, nothing should ever be deleted from the .config just > > > because it was deleted in the .kunitconfig (unless, of course, you > > > change a =y to a =n or # ... is not set), so I don't see what this > > > change would do. > > > > > > Can you maybe provide an example? > > > > Sorry for my insufficient explanation. I added a kunit test > > (SYSCTL_KUNIT_TEST) to '.kunitconfig', ran the added test, and then removed it > > from the file. However, '.config' is not generated again due to the condition > > and therefore the test still runs. > > > > For more detail: > > > > $ ./tools/testing/kunit/kunit.py run --defconfig --build_dir ../kunit.out/ > > $ echo "CONFIG_SYSCTL_KUNIT_TEST=y" >> ../kunit.out/.kunitconfig > > $ ./tools/testing/kunit/kunit.py run --build_dir ../kunit.out/ > > $ sed -i '4d' ../kunit.out/.kunitconfig > > $ ./tools/testing/kunit/kunit.py run --build_dir ../kunit.out/ > > > > The 2nd line command adds sysctl kunit test and the 3rd line shows it runs the > > added test as expected. Because the default kunit config contains only 3 > > lines, The 4th line command removes the sysctl kunit test from the > > .kunitconfig. However, the 5th line still run the test. > > > > This patch is for such cases. Of course, this might make more false positives > > but I believe it would not be a big problem because .config generation takes no > > long time. If I missed something, please let me know. > > I think I understand. > > It is intentional - currently - that KUnit doesn't generate a new > .config with every invocation. The reason is basically to support > interaction with other methods of generating .configs. Consider that > you might want to use make menuconfig to turn something on. It is a > pretty handy interface if you work on vastly different parts of the > kernel. Or maybe you have a defconfig that you always use for some > platform, I think it is easier to run > > make foo_config; tools/testing/kunit/kunit.py run > > Then having to maintain both your defconfig and a .kunitconfig which > is a superset of the defconfig. > > Your change would make it so that you have to have a .kunitconfig for > every test environment that you care about, and you could not as > easily take advantage of menuconfig. Thank you for this kind answer. Now I understood the intention and agree with that. :) > > I think what we do now is a bit janky, and the use cases I mentioned > are not super well supported. So I am sympathetic to what you are > trying to do, maybe we could have a config option for it? > > I think Ted and Bjorn might have opinions on this; they had some > related opinions in the past. I'm ok with current state, but if related discussions continue and my opinion is required, I will join in. Thanks, SeongJae Park >
