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

Fri Sep 29 06:39:40 UTC 2023

On 28/09/2023 11:37, Stefan Bader wrote:
> 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.
Diff here does not look right indeed, but these configs are actually 
moved, nothing goes away. Otherwise the generated configs won't match 
the previous ones. I'll update 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.
Correct.
>
>>
>> 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.
Indeed, it was picked by cranky fix. I'll update the commit message. 
I'll also modify the cranky fix script to take into account 20.04 versions.
>
> 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
Hi Stefan,

Thanks for review. I left some comments inline. I will improve the 
commit messages when I apply these.

Roxana