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

[Alexander Smirnov <asmirnov@ilbers.de>]

On 02/06/2018 03:10 PM, Jan Kiszka wrote:
> On 2018-02-06 12:01, Alexander Smirnov wrote:
>> 8<--
>>
>> Changes since v2:
>>   - Move do_prepare after do_build. This will not trigger do_build for
>>     reoccurent builds.
> 
> Hmm, aren't you also moving it *before every* build? That's how I read
> the code.
> 

Yes, there is dependency dpkg-base:

do_build[depends] = "buildchroot:do_prepare"

so it's a mandatory dependency for each recipe to be built.

>>
>> 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-isar/conf/local.conf.sample                     |  3 +++
>>   meta/classes/dpkg-base.bbclass                       |  2 +-
>>   meta/classes/isar-events.bbclass                     | 20 ++++++++++++++++++++
>>   meta/recipes-devtools/buildchroot/buildchroot.bb     | 20 +++++++++++++++++++-
>>   .../buildchroot/files/multistrap.conf.in             |  8 +++++++-
>>   5 files changed, 50 insertions(+), 3 deletions(-)
>>   create mode 100644 meta/classes/isar-events.bbclass
>>
>> diff --git a/meta-isar/conf/local.conf.sample b/meta-isar/conf/local.conf.sample
>> index 2ae43e7..fc760ed 100644
>> --- a/meta-isar/conf/local.conf.sample
>> +++ b/meta-isar/conf/local.conf.sample
>> @@ -158,3 +158,6 @@ IMAGE_INSTALL = "example-hello example-raw"
>>   #
>>   # Default parallel jobs for bitbake:
>>   BB_NUMBER_THREADS = "4"
>> +
>> +# Add event handlers for bitbake
>> +INHERIT += "isar-events"
> 
> If you are already updating, please move this to a proper place
> (isar-bitbake.conf) like I suggested via my other patch.

Will be done in v4.

> 
>> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
>> index 026028f..74dedc5 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_prepare"
>>   
>>   # 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/recipes-devtools/buildchroot/buildchroot.bb b/meta/recipes-devtools/buildchroot/buildchroot.bb
>> index 51f9d5d..60195de 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_prepare
>> +
>>       # Create root filesystem
>>       sudo -E multistrap -a ${DISTRO_ARCH} -d "${BUILDCHROOT_DIR}" -f "${WORKDIR}/multistrap.conf"
>>   
>> @@ -76,4 +80,18 @@ do_build() {
>>       # Configure root filesystem
>>       sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
>>       _do_build_cleanup
>> +
>> +    do_cleanup
>> +}
>> +
>> +do_prepare[nostamp] = "1"
>> +
>> +do_prepare() {
>> +    sudo mount --bind ${DEPLOY_DIR_APT}/${DISTRO} ${BUILDCHROOT_DIR}/isar-apt
> 
> What prevents that this is run multiple times? Or will it simply
> silently fail if the mount already exists?
> 

Here nothing, will fix in v4.

>> +}
> 
> Maybe better call this "do_setup_mounts" or so
> 

Agreed.

>> +
>> +addtask prepare after do_build
> 
> That is weird. Don't we call this prior to every build anyway?
> 

We do. See the first comment above.

Alex

>> +
>> +do_cleanup() {
>> +    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
>>
> 
> Jan
>