Re: [PATCH 3/5 v4] buildchroot: Enable isar-apt

[Alexander Smirnov <asmirnov@ilbers.de>]

On 02/06/2018 08:31 PM, Jan Kiszka wrote:
> On 2018-02-06 14:57, Alexander Smirnov wrote:
>> 8<--
>>
>> Branch to test: asmirnov/devel
>>
>> 8<--
>>
>> This patch provides access to isar-apt from buildchroot. It does the
>> following:
>>   - mount isar-apt during buildchroot building.
>>   - umount isar-apt using bitbake events.
>>
>> Also it needs to keep Isar build tree clean from any mounts despite on whether
>> build succeed of failed. bitbake provides various events that could trigger
>> custom python hooks. In this patch BuildCompleted event is used, which happened
>> when bitbake finished its execution despite on the result.
>>
>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>> ---
>>   meta/classes/dpkg-base.bbclass                     |  2 +-
>>   meta/classes/isar-events.bbclass                   | 20 ++++++++++++++++
>>   meta/conf/isar-bitbake.conf                        |  3 +++
>>   meta/recipes-devtools/buildchroot/buildchroot.bb   | 28 +++++++++++++++++++++-
>>   .../buildchroot/files/multistrap.conf.in           |  8 ++++++-
>>   5 files changed, 58 insertions(+), 3 deletions(-)
>>   create mode 100644 meta/classes/isar-events.bbclass
>>
>> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
>> index 026028f..5d5a924 100644
>> --- a/meta/classes/dpkg-base.bbclass
>> +++ b/meta/classes/dpkg-base.bbclass
>> @@ -2,7 +2,7 @@
>>   # Copyright (C) 2017 Siemens AG
>>   
>>   # Add dependency from buildchroot creation
>> -do_build[depends] = "buildchroot:do_build"
>> +do_build[depends] = "buildchroot:do_setup_mounts"
>>   
>>   # Each package should have its own unique build folder, so use
>>   # recipe name as identifier
>> diff --git a/meta/classes/isar-events.bbclass b/meta/classes/isar-events.bbclass
>> new file mode 100644
>> index 0000000..55fc106
>> --- /dev/null
>> +++ b/meta/classes/isar-events.bbclass
>> @@ -0,0 +1,20 @@
>> +# Isar event handlers.
>> +#
>> +# This software is a part of ISAR.
>> +# Copyright (C) 2015-2017 ilbers GmbH
>> +
>> +addhandler isar_handler
>> +
>> +python isar_handler () {
>> +    import subprocess
>> +
>> +    devnull = open(os.devnull, 'w')
>> +
>> +    if isinstance(e, bb.event.BuildCompleted):
>> +        bchroot = d.getVar('BUILDCHROOT_DIR', True)
>> +
>> +        # Clean up buildchroot
>> +        subprocess.call('/usr/bin/sudo /bin/umount ' + bchroot + '/isar-apt || /bin/true', stdout=devnull, stderr=devnull, shell=True)
>> +
>> +    devnull.close()
>> +}
>> diff --git a/meta/conf/isar-bitbake.conf b/meta/conf/isar-bitbake.conf
>> index b853c88..36cca29 100644
>> --- a/meta/conf/isar-bitbake.conf
>> +++ b/meta/conf/isar-bitbake.conf
>> @@ -27,6 +27,9 @@ BUILDCHROOT_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/buildchroot/rootfs"
>>   # Setup our default hash policy
>>   BB_SIGNATURE_HANDLER ?= "noop"
>>   
>> +# Add event handlers for bitbake
>> +INHERIT += "isar-events"
>> +
>>   include conf/local.conf
>>   include conf/multiconfig/${BB_CURRENT_MC}.conf
>>   include conf/machine/${MACHINE}.conf
>> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb b/meta/recipes-devtools/buildchroot/buildchroot.bb
>> index 51f9d5d..304c67e 100644
>> --- a/meta/recipes-devtools/buildchroot/buildchroot.bb
>> +++ b/meta/recipes-devtools/buildchroot/buildchroot.bb
>> @@ -32,7 +32,9 @@ BUILDCHROOT_PREINSTALL ?= "gcc \
>>   WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
>>   
>>   do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> -do_build[dirs] = "${WORKDIR}/hooks_multistrap"
>> +do_build[dirs] = "${WORKDIR}/hooks_multistrap \
>> +                  ${BUILDCHROOT_DIR}/isar-apt"
>> +do_build[depends] = "isar-apt:do_cache_config"
>>   
>>   do_build() {
>>       E="${@ bb.utils.export_proxies(d)}"
>> @@ -67,6 +69,8 @@ do_build() {
>>       }
>>       trap '_do_build_cleanup' EXIT
>>   
>> +    do_setup_mounts
>> +
>>       # Create root filesystem
>>       sudo -E multistrap -a ${DISTRO_ARCH} -d "${BUILDCHROOT_DIR}" -f "${WORKDIR}/multistrap.conf"
>>   
>> @@ -76,4 +80,26 @@ do_build() {
>>       # Configure root filesystem
>>       sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
>>       _do_build_cleanup
>> +
>> +    do_cleanup_mounts
>> +}
>> +
>> +# Invalidate stamp for do_setup_mounts before each build start.
>> +# This will guarantee that this function will be executed once
>> +# per build.
>> +python __anonymous() {
>> +    stamp = d.getVar("STAMP") + ".do_setup_mounts." + d.getVarFlag("do_setup_mounts", 'stamp-extra-info')
>> +    os.remove(stamp) if os.path.exists(stamp) else None
>> +}
>> +
>> +do_setup_mounts[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> +
>> +do_setup_mounts() {
>> +    sudo mount --bind ${DEPLOY_DIR_APT}/${DISTRO} ${BUILDCHROOT_DIR}/isar-apt
>> +}
>> +
>> +addtask setup_mounts after do_build
>> +
>> +do_cleanup_mounts() {
>> +    sudo umount ${BUILDCHROOT_DIR}/isar-apt 2>/dev/null || true
>>   }
>> diff --git a/meta/recipes-devtools/buildchroot/files/multistrap.conf.in b/meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>> index a0b28e3..480a4b8 100644
>> --- a/meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>> +++ b/meta/recipes-devtools/buildchroot/files/multistrap.conf.in
>> @@ -6,7 +6,7 @@ noauth=true
>>   unpack=true
>>   ignorenativearch=true
>>   bootstrap=##DISTRO_MULTICONF_BOOTSTRAP##
>> -aptsources=##DISTRO_MULTICONF_APTSOURCES##
>> +aptsources=isar-apt ##DISTRO_MULTICONF_APTSOURCES##
>>   configscript=##CONFIG_SCRIPT##
>>   setupscript=##SETUP_SCRIPT##
>>   hookdir=##DIR_HOOKS##
>> @@ -29,3 +29,9 @@ 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
>>
> 
> Looks good! I'm testing it via your devel branch, and I also
> successfully rebased "Permanently mount '/dev', '/sys' and '/proc'" on top.
> 

Thanks!

> Regarding the remaining cleanup of WORKDIR mounting: Can't each
> dpkg-base class register their own cleanup handler in the same fashion?
> 

I've remembered, that for Jenkins I've used something following to clean 
the build automatically:

8<--

#!/bin/bash

W=${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}

for m in $(cat /proc/mounts | grep $W); do
     m=$(echo $m | cut -d ' ' -f 2)
     umount $m
done

8<--

There are some facts:

1. Proc folder must be available for correct Isar operating.
2. /proc/mounts contains all the active mounts.
3. Folder $W contains only mounts created by current build.

Based on this, I think if we insert the code above to the event handler, 
it will be robust enough solution to clean up everything. Also we should 
not anymore care if somebody adds some extra mount.

What do you think?

Alex