Fwd: [PULL] [L/unstable] merge configs into annotations

[Andrea Righi]

On Mon, Dec 05, 2022 at 09:03:58AM +0900, Masahiro Yamada wrote:
> (+CC list)
> 
> ---------- Forwarded message ---------
> From: Masahiro Yamada <masahiro.yamada at canonical.com>
> Date: Mon, Dec 5, 2022 at 8:41 AM
> Subject: Re: [PULL] [L/unstable] merge configs into annotations
> To: Andrea Righi <andrea.righi at canonical.com>
> 
> 
> On Thu, Nov 17, 2022 at 3:09 AM Andrea Righi <andrea.righi at canonical.com> wrote:
> >
> > [Overview]
> >
> > Each Ubuntu kernel needs to maintain its own .config for each supported
> > architecture and each flavour.
> >
> > Every time a new patch is applied or a kernel is rebased on top of a new
> > one, we need to update the .config's accordingly (config options can be
> > added, removed and also renamed).
> >
> > So, we need to make sure that some critical config options are always
> > matching the desired value in order to have a functional kernel.
> >
> > [State of the art]
> >
> > At the moment configs are maintained as a set of Kconfig chunks (inside
> > debian.<kernel>/config/): a global one, plus per-arch / per-flavour
> > chunks.
> >
> > In addition to that, we need to maintain also a file called
> > 'annotations'; the purpose of this file is to make sure that some
> > critical config options are not silently removed or changed when the
> > real .config is re-generated (for example after a rebase or after
> > applying a new set of patches).
> >
> > The main problem with this approach is that, often, we have duplicate
> > information that is stored both in the Kconfig chunks *and* in the
> > annotations files and, at the same time, the whole .config's information
> > is distributed between Kconfig chunks and annotations, making it hard to
> > maintain, review and manage in general.
> >
> > [Proposed solution]
> >
> > The proposed solution is to store all the config information into the
> > "annotations" format and get rid of the config chunks (basically the
> > real .config's can be produced "compiling" annotations).
> >
> > [Implementation]
> >
> > To help the management of the annotations an helper script is provided
> > (debian/scripts/misc/annotations):
> > ```
> > usage: annotations [-h] [--version] [--file FILE] [--arch ARCH] [--flavour FLAVOUR] [--config CONFIG] (--query | --export | --import FILE | --check FILE)
> >
> > Manage Ubuntu kernel .config and annotations
> >
> > options:
> >   -h, --help            show this help message and exit
> >   --version, -v         show program's version number and exit
> >   --file FILE, -f FILE  Pass annotations or .config file to be parsed
> >   --arch ARCH, -a ARCH  Select architecture
> >   --flavour FLAVOUR, -l FLAVOUR
> >                         Select flavour (default is "generic")
> >   --config CONFIG, -c CONFIG
> >                         Select a specific config option
> >
> > Action:
> >   --query, -q           Query annotations
> >   --export, -e          Convert annotations to .config format
> >   --import FILE, -i FILE
> >                         Import a .config into annotations
> >   --check FILE, -k FILE
> >                         Validate kernel .config with annotations
> > ```
> 
> Since actions are mutually exclusive, I would make them
> a positional parameter, that is,
> 
> $  annotations query
> 
>  instead of
> 
> $ annotations --query
> 
> but it is up to you.

Good point. But some of them require a file as argument, others don't, I
wouldn't like to change the syntax too much at this point (potentially
breaking other scripts, since we already started using this stuff in
unstable). But it's definitely something to consider in the future to
make the tool look nicer and more usable.

> 
> 
> 
> 
> 
> 
> > This script allows to query config settings (per arch/flavour/config),
> > export them into the Kconfig format (generating the real .config files)
> > and check if the final .config matches the rules defined in the
> > annotations.
> >
> > Examples (annotations is defined as an alias to
> > debian/scripts/annotations):
> >
> >  - Show settings for `CONFIG_DEBUG_INFO_BTF` for master kernel across
> >    all the supported architectures and flavours:
> >
> > $ annotations --query --config CONFIG_DEBUG_INFO_BTF
> > {
> >     "policy": {
> >         "amd64": "y",
> >         "arm64": "y",
> >         "armhf": "n",
> >         "ppc64el": "y",
> >         "riscv64": "y",
> >         "s390x": "y"
> >     },
> >     "note": "'Needs newer pahole for armhf'"
> > }
> >
> >  - Dump kernel .config for arm64 and flavour generic-64k:
> >
> > $ annotations --arch arm64 --flavour generic-64k --export
> > CONFIG_DEBUG_FS=y
> > CONFIG_DEBUG_KERNEL=y
> > CONFIG_COMPAT=y
> > ...
> >
> >  - Update annotations file with a new kernel .config for amd64 flavour
> >    generic:
> >
> > $ annotations --arch amd64 --flavour generic --import build/.config
> >
> > Moreover, two additional kernelconfig commands are provided
> > (debian/rules targets):
> >  - listnewconfigs: allow to generate a list of new config options (e.g.,
> >    after a rebase) and store them in CONFIGS/new-<arch>-<flavour> for
> >    review
> >  - importconfigs: after new .config's are generated and reviewed (in
> >    CONFIGS/<arch>-<flavour>) we can use this command to automatically
> >    import all of them into the local annotations
> >
> > [Pros and Cons]
> >
> >  Pros:
> >   - avoid duplicate information in .config's and annotations
> >   - allow to easily define groups of config settings (for a specific
> >     environment or feature, such as annotations.clouds, annotations.ubuntu,
> >     annotations.snapd, etc.)
> 
> 
> debian.master/config/annotations is more than 14000 lines.
> 
> Each kernel source tree maintains 14000-line configs?
> 
> 
> This is beyond this patch set, but in my view,
> an even bigger issue is duplicated efforts among
> a few dozens of kernel trees.

That is exactly the point. This configs/annotations refactoring is the
first baby step toward a better way to maintain this stuff.

Next step would be to start defining multiple levels of annotations
(groups) so that different kernels can include and share the same
subsets of config options depending on their nature and required
features, for example we could have a main annotations called
'ubuntu-minimal', or 'ubuntu-clouds' or 'configs-required-by-snapd' and
so on.

Then individual kernel would be able to include this subsets (in a
multi-level hierarchy) to select the features that they need.

> 
> BTW, you introduced a new format, version 4, which
> perhaps might be a chance to consider more changes.
> 
> 
> Just some ideas.
> 
> 
> 
> 
> (1) Get rid of too-obvious CONFIG_ prefix
> 
> 
> 
> CONFIG_104_QUAD_8    policy<{'amd64': 'm'}>
> 
>      -->
> 
> 104_QUAD_8           policy<{'amd64': 'm'}>

I like this. I think I can easily apply this in the next
version/changes.

> 
> 
> 
> (2) wildcard
> 
> We spell out explicit values for all architectures.
> 
> 
> Is it better to support wildcards? '*' means the default.
> 
> 
>  policy<{'*': 'y'}>
> 
>      y for all architectures/flavours
> 
> 
>  policy<{'arm64-*': 'm', '*': 'y'}>
> 
>      m for all flavours of arm64,
>      y for others
> 
> 
>  policy<{'arm64-generic-64k': 'm', '*': 'y'}>
> 
>      m for arm64-generic-64 flavour,
>      y for others

I also like this. Now that arches and flavours must be defined in the
header of annotations we have all the required information to properly
expand '*'.

BTW, we still have flavours and arches re-defined in other places inside
debian/ and debian.<kernel>/, once all kernels are migrated to the new
annotations format (or at least all the devel kernels) I would like to
do some refactoring also in these scripts to get rid of these
re-definitions and always get flavours and supported architectures from
annotations.

> 
> 
> 
> (3) something different
> 
> 
>   The current format makes the parser easier to implement
>   because you can pass it to litaral_eval(),
>   at the cost of human readability.

I think once everything is migrated to this new format we can easily
convert the content of annotations to something better / more readable
without breaking anything else, because the only API to access and
manage this information is now just the 'annotations' script. So
basically we have the freedom to change the "backend" (annotations
format) to whatever we want if we change the "frontend" (annotations
script) accordingly.

-Andrea