Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20200126015924.4198-1-sj38.park@gmail.com>
In-Reply-To: <20200126015924.4198-1-sj38.park@gmail.com>
From:   Brendan Higgins <brendanhiggins@google.com>
Date:   Mon, 27 Jan 2020 16:02:48 -0800
Message-ID: <CAFd5g46v-RyNMP7GROn4bUEAATOPZ=w5AyO+tvuTG25aqt6oAg@mail.gmail.com>
Subject: Re: [PATCH] kunit/kunit_kernel: Rebuild .config if .kunitconfig is modified
To:     SeongJae Park <sj38.park@gmail.com>
Cc:     "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        KUnit Development <kunit-dev@googlegroups.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        SeongJae Park <sjpark@amazon.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Jan 25, 2020 at 5:59 PM <sj38.park@gmail.com> wrote:
>
> From: SeongJae Park <sjpark@amazon.de>
>
> 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?

> Signed-off-by: SeongJae Park <sjpark@amazon.de>
> ---
>  tools/testing/kunit/kunit_kernel.py | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py
> index cc5d844ecca1..a3a5d6c7e66d 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -111,17 +111,22 @@ class LinuxSourceTree(object):
>                 return True
>
>         def build_reconfig(self, build_dir):
> -               """Creates a new .config if it is not a subset of the .kunitconfig."""
> +               """Creates a new .config if it is not a subset of, or older than the .kunitconfig."""
>                 kconfig_path = get_kconfig_path(build_dir)
>                 if os.path.exists(kconfig_path):
>                         existing_kconfig = kunit_config.Kconfig()
>                         existing_kconfig.read_from_file(kconfig_path)
> -                       if not self._kconfig.is_subset_of(existing_kconfig):
> -                               print('Regenerating .config ...')
> -                               os.remove(kconfig_path)
> -                               return self.build_config(build_dir)
> -                       else:
> +                       subset = self._kconfig.is_subset_of(existing_kconfig)
> +
> +                       kunitconfig_mtime = os.path.getmtime(kunitconfig_path)
> +                       kconfig_mtime = os.path.getmtime(kconfig_path)
> +                       older = kconfig_mtime < kunitconfig_mtime
> +
> +                       if subset and not older:
>                                 return True
> +                       print('Regenerating .config ...')
> +                       os.remove(kconfig_path)
> +                       return self.build_config(build_dir)
>                 else:
>                         print('Generating .config ...')
>                         return self.build_config(build_dir)
> --
> 2.17.1
>