ACK/Cmnt: [SRU][Focal][PULL] Use new annotation model

Thu Sep 28 09:37:35 UTC 2023

On 27.09.23 15:55, Roxana Nicolescu wrote:
> BugLink: https://bugs.launchpad.net/bugs/2019000
> 
> SRU Justification:
> 
> [Impact]
> 
> * In Lunar, a new way to manage kernel configs was introduced, unifying the
> duplicated information defined in the annotation file + config chunks into
> an annotation-only-model.
> Because we support many types of releases, we need to have an unified way
> of dealing with config options to avoid different patches for different
> releases but for the same config change.
> 
> [Fix]
> 
> * Ported the commit that introduced the new annotation model infrasture
> from jammy:
> "UBUNTU: [Packaging] new annotations model infrastructure"
> 
> * And then ported the fixes that came after:
> "UBUNTU: [Packaging] kconfig/annotations.py: support older way of merging dicts"
> "UBUNTU: [Packaging] fix typo when calling the old config-check"
> "UBUNTU: [Packaging] fix typo in 4-checks.mk"
> "UBUNTU: [Packaging] config-check: Handle new annotations format 4"
> "UBUNTU: [Packaging] rules: Use old-kernelconfig for old configs"
> "UBUNTU: [Packaging] update annotations scripts"
> 
> * Changed the header format to 4 and added the proper arches and flavours
> 
> * Ran the sanitize-annotations tool
> 
> * Because in focal (at least) many config options were not documented in
> the annotation file, I imported the generated option in the annotation file
> before running the migrate script to avoid fixing the mess manually after.
> 
> * Ran 'cranky fdr migrateconfigs' that deleted the old config files
> 
> [Test Plan]
> 
> * Generated the configs before and after the migration with command
> `cranky fdr clean genconfigs`.
> They look identical.
> * Also did a cbd build and a boot test to be extra sure.

Looking at the files is complex, so this would be my primary handle for 
ACKing.

> 
> [Where problems could occur]
> 
> * Because config options are the same, I don't expect any regression in the
> main kernel.
> * But we may run into issues when derivatives are migrated, during the
> updateconfigs step. The plan is to migrate all derivatives in one SRU
> cycle, to avoid confusion. Extra documentation will be sent to help the
> transition.
> 
> [Other Info]
> 
> * In the old annotation file header, some arches did not correspond to the
> ones used in the actual policy (amd64 vs x86, powerpc vs ppce64el) and armhf
> was missing.
> * In debian/scripts/misc/kernelconfig i386 was not included as supported arch
> 
> ---------------------------------------------------------------
> The following changes since commit 0c7a14b8dd38ede66c7b69baea2bc55a9d916b47:
> 
>    net: Avoid address overwrite in kernel_connect (2023-09-22 09:46:02 +0200)
> 
> are available in the Git repository at:
> 
>    git://git.launchpad.net/~roxanan/+git/focal annotation-migration
> 
> for you to fetch changes up to f6e20df539778315b7d3fab2ad8f84218db054d7:
> 
>    UBUNTU: [Config] migrate all configs into annotations (2023-09-27 15:17:24 +0200)
> 
> ----------------------------------------------------------------
> Andrea Righi (3):
>        UBUNTU: [Packaging] new annotations model infrastructure
>        UBUNTU: [Packaging] kconfig/annotations.py: support older way of merging dicts
>        UBUNTU: [Packaging] fix typo when calling the old config-check
> 
> Andrei Gherzan (1):
>        UBUNTU: [Packaging] fix typo in 4-checks.mk
> 
> Juerg Haefliger (2):
>        UBUNTU: [Packaging] config-check: Handle new annotations format 4
>        UBUNTU: [Packaging] rules: Use old-kernelconfig for old configs
> 
> Roxana Nicolescu (4):
>        UBUNTU: [Config] sanitize annotations
>        UBUNTU: [Config] import generated configs into annotation file

Glancing over the changes here^ it seems like a lot of settings go away. 
That probably is because only the delta against a certain base config 
is/needs to be recorded. Might be helpful to put a reminding note about 
this into the commit message.

>        UBUNTU: [Packaging] kernelconfig: add i386 as supported arch
>        UBUNTU: [Config] migrate all configs into annotations

If I read that^ correctly it is more "remove all old config files" since 
the content was already imported in a previous step.

> 
> Stefan Bader (1):
>        UBUNTU: [Packaging] update annotations scripts

This^ is a bit surprising. I believe it was a cranky fix run. Maybe that 
could be added to the description. Like "Automatic import of 
fixes/updates rolled out via tooling" or so.

Apart from that minor nitpick...

Acked-by: Stefan Bader <stefan.bader at canonical.com>

> 
>   debian.master/config/README.rst                    |   185 +
>   debian.master/config/amd64/config.common.amd64     |   646 -
>   debian.master/config/amd64/config.flavour.generic  |    10 -
>   .../config/amd64/config.flavour.lowlatency         |    10 -
>   debian.master/config/annotations                   | 25650 +++++++++----------
>   debian.master/config/arm64/config.common.arm64     |   667 -
>   debian.master/config/arm64/config.flavour.generic  |     3 -
>   debian.master/config/armhf/config.common.armhf     |   640 -
>   debian.master/config/armhf/config.flavour.generic  |    23 -
>   .../config/armhf/config.flavour.generic-lpae       |    23 -
>   debian.master/config/config.common.ports           |     3 -
>   debian.master/config/config.common.ubuntu          | 11647 ---------
>   debian.master/config/i386/config.common.i386       |   639 -
>   debian.master/config/i386/config.flavour.generic   |     9 -
>   .../config/i386/config.flavour.lowlatency          |     9 -
>   debian.master/config/ppc64el/config.common.ppc64el |   638 -
>   .../config/ppc64el/config.flavour.generic          |     3 -
>   debian.master/config/s390x/config.common.s390x     |   575 -
>   debian.master/config/s390x/config.flavour.generic  |     3 -
>   debian.master/config/x32/config.common.x32         |    74 -
>   debian.master/config/x32/config.flavour.generic    |    74 -
>   debian/rules.d/1-maintainer.mk                     |    26 +-
>   debian/rules.d/2-binary-arch.mk                    |     9 +-
>   debian/rules.d/4-checks.mk                         |    12 +-
>   debian/scripts/config-check                        |     6 +-
>   debian/scripts/misc/annotations                    |   274 +
>   debian/scripts/misc/final-checks                   |    16 +-
>   debian/scripts/misc/kconfig/__init__.py            |     0
>   debian/scripts/misc/kconfig/annotations.py         |   431 +
>   debian/scripts/misc/kernelconfig                   |   327 +-
>   debian/scripts/misc/migrate-annotations            |    35 +
>   debian/scripts/misc/old-kernelconfig               |   207 +
>   debian/snapcraft.mk                                |     6 +-
>   33 files changed, 13908 insertions(+), 28972 deletions(-)
>   create mode 100644 debian.master/config/README.rst
>   delete mode 100644 debian.master/config/amd64/config.common.amd64
>   delete mode 100644 debian.master/config/amd64/config.flavour.generic
>   delete mode 100644 debian.master/config/amd64/config.flavour.lowlatency
>   delete mode 100644 debian.master/config/arm64/config.common.arm64
>   delete mode 100644 debian.master/config/arm64/config.flavour.generic
>   delete mode 100644 debian.master/config/armhf/config.common.armhf
>   delete mode 100644 debian.master/config/armhf/config.flavour.generic
>   delete mode 100644 debian.master/config/armhf/config.flavour.generic-lpae
>   delete mode 100644 debian.master/config/config.common.ports
>   delete mode 100644 debian.master/config/config.common.ubuntu
>   delete mode 100644 debian.master/config/i386/config.common.i386
>   delete mode 100644 debian.master/config/i386/config.flavour.generic
>   delete mode 100644 debian.master/config/i386/config.flavour.lowlatency
>   delete mode 100644 debian.master/config/ppc64el/config.common.ppc64el
>   delete mode 100644 debian.master/config/ppc64el/config.flavour.generic
>   delete mode 100644 debian.master/config/s390x/config.common.s390x
>   delete mode 100644 debian.master/config/s390x/config.flavour.generic
>   delete mode 100644 debian.master/config/x32/config.common.x32
>   delete mode 100644 debian.master/config/x32/config.flavour.generic
>   create mode 100755 debian/scripts/misc/annotations
>   create mode 100644 debian/scripts/misc/kconfig/__init__.py
>   create mode 100644 debian/scripts/misc/kconfig/annotations.py
>   create mode 100755 debian/scripts/misc/migrate-annotations
>   create mode 100644 debian/scripts/misc/old-kernelconfig
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0xE8675DEECBEECEA3.asc
Type: application/pgp-keys
Size: 44613 bytes
Desc: OpenPGP public key
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230928/f7d05ec3/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20230928/f7d05ec3/attachment-0001.sig>