Re: [PATCH 2/2] centralize multistrap configuration generation

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2018-02-15 09:16, Claudius Heine wrote:
> Hi Jan,
> 
> On 02/15/2018 08:28 AM, Jan Kiszka wrote:
>> On 2018-02-14 14:15, [ext] claudius.heine.ext@siemens.com wrote:
>>> From: Claudius Heine <ch@denx.de>
>>>
>>> Previous there was redundant code between the buildchroot and
>>> isar-image-base recipe.
>>>
>>> This patch moves the generation of the multistrap configuration to a
>>> central python script that can be parametrized by both recipes.
>>>
>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>> ---
>>>   .../recipes-core/images/files/multistrap.conf.in   | 38 ----------
>>>   meta-isar/recipes-core/images/isar-image-base.bb   | 33 ++++-----
>>>   meta/classes/base.bbclass                          |  2 +-
>>>   meta/lib/isar/__init__.py                          |  8 +++
>>>   meta/lib/isar/multistrap.py                        | 80
>>> ++++++++++++++++++++++
>>>   meta/recipes-devtools/buildchroot/buildchroot.bb   | 28 +++-----
>>>   .../buildchroot/files/multistrap.conf.in           | 37 ----------
>>>   7 files changed, 111 insertions(+), 115 deletions(-)
>>>   delete mode 100644
>>> meta-isar/recipes-core/images/files/multistrap.conf.in
>>>   create mode 100644 meta/lib/isar/__init__.py
>>>   create mode 100644 meta/lib/isar/multistrap.py
>>>   delete mode 100644
>>> meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>>>
>>> diff --git a/meta-isar/recipes-core/images/files/multistrap.conf.in
>>> b/meta-isar/recipes-core/images/files/multistrap.conf.in
>>> deleted file mode 100644
>>> index 432b6af..0000000
>>> --- a/meta-isar/recipes-core/images/files/multistrap.conf.in
>>> +++ /dev/null
>>> @@ -1,38 +0,0 @@
>>> -# This software is a part of ISAR.
>>> -# Copyright (C) 2015-2017 ilbers GmbH
>>> -
>>> -[General]
>>> -noauth=true
>>> -unpack=true
>>> -ignorenativearch=true
>>> -bootstrap=##DISTRO_MULTICONF_BOOTSTRAP## Isar
>>> -aptsources=##DISTRO_MULTICONF_APTSOURCES##
>>> -configscript=##CONFIG_SCRIPT##
>>> -setupscript=##SETUP_SCRIPT##
>>> -hookdir=##DIR_HOOKS##
>>> -
>>> -[base]
>>> -source=##DISTRO_APT_SOURCE##
>>> -suite=##DISTRO_SUITE##
>>> -components=##DISTRO_COMPONENTS##
>>> -packages=##IMAGE_PREINSTALL##
>>> -omitdebsrc=true
>>> -
>>> -[updates]
>>> -source=##DISTRO_APT_SOURCE##
>>> -suite=##DISTRO_SUITE##-updates
>>> -components=##DISTRO_COMPONENTS##
>>> -omitdebsrc=true
>>> -
>>> -[security]
>>> -source=##DISTRO_APT_SOURCE_SEC##
>>> -suite=##DISTRO_SUITE##/updates
>>> -components=##DISTRO_COMPONENTS##
>>> -omitdebsrc=true
>>> -
>>> -[Isar]
>>> -packages=##IMAGE_INSTALL##
>>> -source=##DEPLOY_DIR_APT##
>>> -suite=##ISAR_DISTRO_SUITE##
>>> -components=main
>>> -omitdebsrc=true
>>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb
>>> b/meta-isar/recipes-core/images/isar-image-base.bb
>>> index 8ddbabb..9913977 100644
>>> --- a/meta-isar/recipes-core/images/isar-image-base.bb
>>> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
>>> @@ -9,8 +9,7 @@ LICENSE = "gpl-2.0"
>>>   LIC_FILES_CHKSUM =
>>> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260aa954499f7abaabaa882bbe"
>>>
>>>     FILESPATH =. "${LAYERDIR_isar}/recipes-core/images/files:"
>>> -SRC_URI = "file://multistrap.conf.in \
>>> -       file://${DISTRO_CONFIG_SCRIPT} \
>>> +SRC_URI = "file://${DISTRO_CONFIG_SCRIPT} \
>>>          file://setup.sh \
>>>          file://download_dev-random"
>>>   @@ -28,6 +27,14 @@ WORKDIR =
>>> "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
>>>     do_rootfs[dirs] = "${WORKDIR}/hooks_multistrap"
>>>   +MULTISTRAP_CONFIG_SCRIPT = "${DISTRO_CONFIG_SCRIPT}"
>>> +MULTISTRAP_APTSOURCES = "${DISTRO_MULTICONF_APTSOURCES}"
>>> +MULTISTRAP_BOOTSTRAP = "${DISTRO_MULTICONF_BOOTSTRAP} isar"
>>> +MULTISTRAP_PREINSTALL = "${IMAGE_PREINSTALL}"
>>> +MULTISTRAP_ISAR_SOURCE = "copy:///${DEPLOY_DIR_APT}/${DISTRO}"
>>> +MULTISTRAP_ISAR_INSTALL = "${IMAGE_INSTALL}"
>>> +MULTISTRAP_ISAR_SUITE = "${DEBDISTRONAME}"
>>> +
>>>   do_rootfs() {
>>>       E="${@ bb.utils.export_proxies(d)}"
>>>   @@ -35,25 +42,8 @@ do_rootfs() {
>>>       chmod +x "${WORKDIR}/setup.sh"
>>>       install -m 755 "${WORKDIR}/download_dev-random"
>>> "${WORKDIR}/hooks_multistrap/"
>>>   -    # Multistrap accepts only relative path in configuration
>>> files, so get it:
>>> -    cd ${TOPDIR}
>>> -    WORKDIR_REL=${@ os.path.relpath(d.getVar("WORKDIR", True))}
>>> -
>>> -    # Adjust multistrap config
>>> -    sed -e 's|##IMAGE_PREINSTALL##|${IMAGE_PREINSTALL}|g' \
>>> -        -e
>>> 's|##DISTRO_MULTICONF_BOOTSTRAP##|${DISTRO_MULTICONF_BOOTSTRAP}|g' \
>>> -        -e
>>> 's|##DISTRO_MULTICONF_APTSOURCES##|${DISTRO_MULTICONF_APTSOURCES}|g' \
>>> -        -e 's|##DISTRO_APT_SOURCE##|${DISTRO_APT_SOURCE}|g' \
>>> -        -e 's|##DISTRO_APT_SOURCE_SEC##|${DISTRO_APT_SOURCE_SEC}|g' \
>>> -        -e 's|##DISTRO_SUITE##|${DISTRO_SUITE}|g' \
>>> -        -e 's|##DISTRO_COMPONENTS##|${DISTRO_COMPONENTS}|g' \
>>> -        -e
>>> 's|##CONFIG_SCRIPT##|./'"$WORKDIR_REL"'/${DISTRO_CONFIG_SCRIPT}|g' \
>>> -        -e 's|##SETUP_SCRIPT##|./'"$WORKDIR_REL"'/setup.sh|g' \
>>> -        -e 's|##DIR_HOOKS##|./'"$WORKDIR_REL"'/hooks_multistrap|g' \
>>> -        -e 's|##IMAGE_INSTALL##|${IMAGE_INSTALL}|g' \
>>> -        -e
>>> 's|##DEPLOY_DIR_APT##|copy:///${DEPLOY_DIR_APT}/${DISTRO}|g' \
>>> -        -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
>>> -           "${WORKDIR}/multistrap.conf.in" >
>>> "${WORKDIR}/multistrap.conf"
>>> +    echo "${@ isar.multistrap.generate_multistrap_config(d) }" > \
>>> +         "${WORKDIR}/multistrap.conf"
>>>         # Do not use bitbake flag [dirs] here because this folder
>>> should have
>>>       # specific ownership.
>>> @@ -62,6 +52,7 @@ do_rootfs() {
>>>         # Create root filesystem. We must use sudo -E here to
>>> preserve the environment
>>>       # because of proxy settings
>>> +    cd "${TOPDIR}"
>>>       sudo -E multistrap -a ${DISTRO_ARCH} -d "${IMAGE_ROOTFS}" -f
>>> "${WORKDIR}/multistrap.conf"
>>>         # Configure root filesystem
>>> diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
>>> index 3334789..2a45a1e 100644
>>> --- a/meta/classes/base.bbclass
>>> +++ b/meta/classes/base.bbclass
>>> @@ -38,7 +38,7 @@ bbfatal() {
>>>   }
>>>     # Derived from openembedded-core/meta/classes/base.bbclass
>>> -ISAR_IMPORTS += "os sys time"
>>> +ISAR_IMPORTS += "os sys time isar"
>>>     def isar_imports(d):
>>>       import sys
>>> diff --git a/meta/lib/isar/__init__.py b/meta/lib/isar/__init__.py
>>> new file mode 100644
>>> index 0000000..e6aeeaa
>>> --- /dev/null
>>> +++ b/meta/lib/isar/__init__.py
>>> @@ -0,0 +1,8 @@
>>> +# Custom kernel recipe include
>>> +#
>>> +# This software is a part of ISAR.
>>> +# Copyright (c) Siemens AG, 2018
>>> +#
>>> +# SPDX-License-Identifier: MIT
>>> +
>>> +from . import multistrap
>>> diff --git a/meta/lib/isar/multistrap.py b/meta/lib/isar/multistrap.py
>>> new file mode 100644
>>> index 0000000..7022f91
>>> --- /dev/null
>>> +++ b/meta/lib/isar/multistrap.py
>>> @@ -0,0 +1,80 @@
>>> +# Custom kernel recipe include
>>> +#
>>> +# This software is a part of ISAR.
>>> +# Copyright (c) Siemens AG, 2018
>>> +#
>>> +# SPDX-License-Identifier: MIT
>>> +
>>> +def generate_multistrap_config(d):
>>> +    import configparser as cp
>>> +    import io
>>> +
>>> +    workdir = d.getVar("WORKDIR", True) or ""
>>> +    topdir = d.getVar("TOPDIR", True) or ""
>>> +    workdir_rel = os.path.relpath(workdir, start=topdir)
>>> +
>>> +    msconf = cp.RawConfigParser()
>>> +
>>> +    # Set general multistrap configuration:
>>> +    configscript = d.getVar("MULTISTRAP_CONFIG_SCRIPT", True) or ""
>>> +    setupscript = d.getVar("MULTISTRAP_SETUP_SCRIPT", True) or
>>> "setup.sh"
>>> +    hookdir = d.getVar("MULTISTRAP_HOOK_DIR", True) or
>>> "hooks_multistrap"
>>> +    aptsources = (d.getVar("MULTISTRAP_APTSOURCES", True) or
>>> "").split()
>>> +    bootstrap = (d.getVar("MULTISTRAP_BOOTSTRAP", True) or "").split()
>>> +
>>> +    msconf.add_section("General")
>>> +    msconf.set("General", "noauth", "true")
>>> +    msconf.set("General", "unpack", "true")
>>> +    msconf.set("General", "ignorenativearch", "true")
>>> +    msconf.set("General", "configscript", "./" +
>>> +               os.path.join(workdir_rel, configscript))
>>> +    msconf.set("General", "setupscript", "./" +
>>> +               os.path.join(workdir_rel, setupscript))
>>> +    msconf.set("General", "hookdir", "./" +
>>> +               os.path.join(workdir_rel, hookdir))
>>> +    msconf.set("General", "aptsources", " ".join(aptsources))
>>> +    msconf.set("General", "bootstrap", " ".join(bootstrap))
>>> +
>>> +    # Setup upstream repositories in multistrap configuration:
>>> +    apt_source = d.getVar("DISTRO_APT_SOURCE", True) or ""
>>> +    apt_source_sec = d.getVar("DISTRO_APT_SOURCE_SEC", True) or ""
>>> +    apt_suite = d.getVar("DISTRO_SUITE", True) or ""
>>> +    apt_components = (d.getVar("DISTRO_COMPONENTS", True) or
>>> "").split()
>>> +    deb_packages = (d.getVar("MULTISTRAP_PREINSTALL", True) or
>>> "").split()
>>> +    isar_source = d.getVar("MULTISTRAP_ISAR_SOURCE", True) or ""
>>> +    isar_packages = (d.getVar("MULTISTRAP_ISAR_INSTALL", True) or
>>> "").split()
>>> +    isar_suite = d.getVar("MULTISTRAP_ISAR_SUITE", True) or "isar"
>>> +
>>> +    msconf.add_section("base")
>>> +    msconf.set("base", "source", apt_source)
>>> +    msconf.set("base", "suite", apt_suite)
>>> +    msconf.set("base", "components", " ".join(apt_components))
>>> +    msconf.set("base", "omitdebsrc", "true")
>>> +    msconf.set("base", "packages", " ".join(deb_packages))
>>> +
>>> +    msconf.add_section("updates")
>>> +    msconf.set("updates", "source", apt_source)
>>> +    msconf.set("updates", "suite", apt_suite + "-updates")
>>> +    msconf.set("updates", "components", " ".join(apt_components))
>>> +    msconf.set("updates", "omitdebsrc", "true")
>>> +
>>> +    msconf.add_section("security")
>>> +    msconf.set("security", "source", apt_source_sec)
>>> +    msconf.set("security", "suite", apt_suite + "/updates")
>>> +    msconf.set("security", "components", " ".join(apt_components))
>>> +    msconf.set("security", "omitdebsrc", "true")
>>> +
>>> +    msconf.add_section("isar")
>>> +    msconf.set("isar", "source", isar_source)
>>> +    msconf.set("isar", "suite", isar_suite)
>>> +    msconf.set("isar", "components", "main")
>>> +    msconf.set("isar", "omitdebsrc", "true")
>>> +    if isar_packages :
>>> +        msconf.set("isar", "packages",
>>> +                   " ".join(isar_packages))
>>> +
>>> +    ret = ""
>>> +    with io.StringIO() as out:
>>> +        msconf.write(out)
>>> +        ret = out.getvalue()
>>> +    return ret
>>> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb
>>> b/meta/recipes-devtools/buildchroot/buildchroot.bb
>>> index 570c0ad..59b4e7e 100644
>>> --- a/meta/recipes-devtools/buildchroot/buildchroot.bb
>>> +++ b/meta/recipes-devtools/buildchroot/buildchroot.bb
>>> @@ -9,8 +9,7 @@ LICENSE = "gpl-2.0"
>>>   LIC_FILES_CHKSUM =
>>> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260aa954499f7abaabaa882bbe"
>>>
>>>     FILESPATH =. "${LAYERDIR_core}/recipes-devtools/buildchroot/files:"
>>> -SRC_URI = "file://multistrap.conf.in \
>>> -           file://configscript.sh \
>>> +SRC_URI = "file://configscript.sh \
>>>              file://setup.sh \
>>>              file://build.sh"
>>>   PV = "1.0"
>>> @@ -37,32 +36,25 @@ do_build[dirs] = "${BUILDCHROOT_DIR}/isar-apt \
>>>                     ${BUILDCHROOT_DIR}/sys"
>>>   do_build[depends] = "isar-apt:do_cache_config"
>>>   +MULTISTRAP_CONFIG_SCRIPT = "configscript.sh"
>>> +MULTISTRAP_APTSOURCES = "isar ${DISTRO_MULTICONF_APTSOURCES}"
>>> +MULTISTRAP_BOOTSTRAP = "${DISTRO_MULTICONF_BOOTSTRAP}"
>>> +MULTISTRAP_PREINSTALL = "${BUILDCHROOT_PREINSTALL}"
>>> +MULTISTRAP_ISAR_SOURCE = "file:///isar-apt"
>>> +
>>>   do_build() {
>>>       E="${@ bb.utils.export_proxies(d)}"
>>>         chmod +x "${WORKDIR}/setup.sh"
>>>       chmod +x "${WORKDIR}/configscript.sh"
>>>   -    # Multistrap accepts only relative path in configuration
>>> files, so get it:
>>> -    cd ${TOPDIR}
>>> -    WORKDIR_REL=${@ os.path.relpath(d.getVar("WORKDIR", True))}
>>> -
>>> -    # Adjust multistrap config
>>> -    sed -e 's|##BUILDCHROOT_PREINSTALL##|${BUILDCHROOT_PREINSTALL}|g' \
>>> -        -e
>>> 's|##DISTRO_MULTICONF_BOOTSTRAP##|${DISTRO_MULTICONF_BOOTSTRAP}|g' \
>>> -        -e
>>> 's|##DISTRO_MULTICONF_APTSOURCES##|${DISTRO_MULTICONF_APTSOURCES}|g' \
>>> -        -e 's|##DISTRO_APT_SOURCE##|${DISTRO_APT_SOURCE}|g' \
>>> -        -e 's|##DISTRO_APT_SOURCE_SEC##|${DISTRO_APT_SOURCE_SEC}|g' \
>>> -        -e 's|##DISTRO_SUITE##|${DISTRO_SUITE}|g' \
>>> -        -e 's|##DISTRO_COMPONENTS##|${DISTRO_COMPONENTS}|g' \
>>> -        -e 's|##CONFIG_SCRIPT##|./'"$WORKDIR_REL"'/configscript.sh|g' \
>>> -        -e 's|##SETUP_SCRIPT##|./'"$WORKDIR_REL"'/setup.sh|g' \
>>> -        -e 's|##DIR_HOOKS##|./'"$WORKDIR_REL"'/hooks_multistrap|g' \
>>> -           "${WORKDIR}/multistrap.conf.in" >
>>> "${WORKDIR}/multistrap.conf"
>>> +    echo "${@ isar.multistrap.generate_multistrap_config(d) }" > \
>>> +         "${WORKDIR}/multistrap.conf"
>>>         do_setup_mounts
>>>         # Create root filesystem
>>> +    cd "${TOPDIR}"
>>>       sudo -E multistrap -a ${DISTRO_ARCH} -d "${BUILDCHROOT_DIR}" -f
>>> "${WORKDIR}/multistrap.conf"
>>>         # Install package builder script
>>> diff --git
>>> a/meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>>> b/meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>>> deleted file mode 100644
>>> index 480a4b8..0000000
>>> --- a/meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>>> +++ /dev/null
>>> @@ -1,37 +0,0 @@
>>> -# This software is a part of ISAR.
>>> -# Copyright (C) 2015-2017 ilbers GmbH
>>> -
>>> -[General]
>>> -noauth=true
>>> -unpack=true
>>> -ignorenativearch=true
>>> -bootstrap=##DISTRO_MULTICONF_BOOTSTRAP##
>>> -aptsources=isar-apt ##DISTRO_MULTICONF_APTSOURCES##
>>> -configscript=##CONFIG_SCRIPT##
>>> -setupscript=##SETUP_SCRIPT##
>>> -hookdir=##DIR_HOOKS##
>>> -
>>> -[base]
>>> -source=##DISTRO_APT_SOURCE##
>>> -suite=##DISTRO_SUITE##
>>> -components=##DISTRO_COMPONENTS##
>>> -packages=##BUILDCHROOT_PREINSTALL##
>>> -omitdebsrc=true
>>> -
>>> -[updates]
>>> -source=##DISTRO_APT_SOURCE##
>>> -suite=##DISTRO_SUITE##-updates
>>> -components=##DISTRO_COMPONENTS##
>>> -omitdebsrc=true
>>> -
>>> -[security]
>>> -source=##DISTRO_APT_SOURCE_SEC##
>>> -suite=##DISTRO_SUITE##/updates
>>> -components=##DISTRO_COMPONENTS##
>>> -omitdebsrc=true
>>> -
>>> -[isar-apt]
>>> -source=file:///isar-apt
>>> -suite=isar
>>> -components=main
>>> -omitdebsrc=true
>>>
>>
>> This requires rebasing. I quickly tried, but the result broke the
>> package build (didn't analyze any details).
> 
> I rebased this patch to next before sending it to the mailinglist, but
> there might have been some other patches applied in the between.
> 
> If you put this patch on top of:
> af265bc0731d242afc5bc0ac9e8ebc6ffe6223f6
> It should work. (It did at least for me.)
> 

Next moved on, so this required rebasing, maybe more. But let's first of
all decide about the strategy before investing further into multistrap.

>> While this moves us in the right direction, it still leaves some
>> duplications behind (setup.sh) - and the big question if we should
>> invest any further in this horrible multistrap mess.
>>
>> I've just enabled arm64, and it took a while to figure out that I was
>> missing some lines in setup.sh. In contrast, qemu-debootstrap just
>> worked (I used it to debug the multistrap output).
>>
>> So, before we move on here: What all does multistrap currently give us,
>> compared to standard debootstrap? I only see "multiple repositories" so
>> far. But as Claudius is going to massage that part heavily anyway, I
>> really wonder if it's worth building on the multistrap indirection any
>> further.
>>
>> How about something like this?
>>
>> - [qemu-]debootstrap --variant=minbase ...
>> - inject all additional apt sources, including user-provided ones
>> - set pinnings and priority, including user-provided ones
>> - apt-get dist-upgrade
>>
>> That would kick out all the multistrap complications.
> 
> I would prefer using debootstrap as well. I am currently looking at its
> code, and maybe it could be possible to further split up debootstraps
> execution into downloading, extracting, configuring steps. This way only
> configuring has to be done with root permissions (because of chroot) and
> qemu if needed.>
> Of course overwriting any base packages with generated isar ones might
> not be as convenient, because packages might have to be replaced at a
> later step.

That was the idea behind "dist-upgrade", though I cannot say for sure it
will work reliably enough.

> 
> If some additional functionality in debootstrap is needed, then it could
> make sense to fetch debootstrap in isar, like any other recipe, and
> apply some of our own patches to it. Of course we should try to upstream
> those too.

Sounds promising. I'm sure there will be traps and pitfalls, but so
there are with multistrap, and its code become rather a barrier than a
help for us.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux