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

[Claudius Heine <claudius.heine.ext@siemens.com>]

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.)

> 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.

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.

Cheers,
Claudius
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de