Re: [PATCH 3/4 v2] buildchroot: Add prepare and cleanup tasks

[Alexander Smirnov <asmirnov@ilbers.de>]

On 11/27/2017 10:56 AM, Henning Schild wrote:
> Am Fri, 24 Nov 2017 19:51:26 +0300
> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> 
>> On 11/24/2017 07:34 PM, Henning Schild wrote:
>>> Am Thu, 23 Nov 2017 13:25:31 +0300
>>> schrieb Alexander Smirnov <asmirnov@ilbers.de>:
>>>    
>>>> Some packages could require builchroot filesystem tunning, for
>>>> example share Isar downloads folder with chroot filesystem.
>>>>
>>>> This patch adds two tasks for buildchroot:
>>>>
>>>> 1. 'do_prepare': This task is executed after buildchroot rootfs
>>>> generation and before 'do_build' from packages to be built.
>>>> Some notes:
>>>>    - An individual task is required for the case when you want to
>>>> rebuild some package, this would require to run this preparation
>>>> stuff for existing buildchroot rootfs.
>>>>    - This task should not have stamp. This means that anytime you
>>>> want to build something, 'do_prepare' should be executed.
>>>>
>>>> 2. 'do_cleanup': This task is executed after all the packages are
>>>> deployed. Some notes:
>>>>    - This task also should not have stamp.
>>>>    - This task depends from the recipes listed in IMAGE_INSTALL.
>>>>
>>>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>>>> ---
>>>>    meta/classes/dpkg-base.bbclass                   |  2 +-
>>>>    meta/classes/image.bbclass                       |  1 +
>>>>    meta/recipes-devtools/buildchroot/buildchroot.bb | 21
>>>> +++++++++++++++++++++ 3 files changed, 23 insertions(+), 1
>>>> deletion(-)
>>>>
>>>> diff --git a/meta/classes/dpkg-base.bbclass
>>>> b/meta/classes/dpkg-base.bbclass index 4941f9b..21ae88d 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/image.bbclass
>>>> b/meta/classes/image.bbclass index 7813b16..b0f645c 100644
>>>> --- a/meta/classes/image.bbclass
>>>> +++ b/meta/classes/image.bbclass
>>>> @@ -83,6 +83,7 @@ do_populate() {
>>>>    
>>>>    addtask populate before do_build after do_unpack
>>>>    do_populate[deptask] = "do_deploy_deb"
>>>> +do_populate[depends] = "buildchroot:do_cleanup"
>>>
>>> Why do we need that? To me that looks redundant.
>>
>> 1. buildchroot:do_cleanup task should be executed after all the
>> packages are build, so that's why the following is used:
>>
>>     DEPENDS += "${IMAGE_INSTALL}"
>>     do_cleanup[deptask] = "do_deploy_deb"
> 
> That one is clear.
> 
>> 2. buildchroot:do_cleanup should be somehow integrated to Isar
>> pipeline, so some dependency from late task (which runs after
>> packages building) should be added. I chose image:do_populate, so the
>> following line forces Isar to run do_cleanup during build:
>>
>>     do_populate[depends] = "buildchroot:do_cleanup"
> 
> If you do not depend on it in the image-recipe, will it not be executed?

It will not, buildchroot is joint to pipeline only by 'do_build' task, 
so all the tasks after are silently ignored. For sure, I've tested this.

> Please remember that we discussed dropping do_populate, so it might be
> a good idea to integrate that step somewhere else. Maybe give the image
> its own do_cleanup.
> 
> https://groups.google.com/d/msg/isar-users/efer3RF989o/mB5Fce9WBgAJ
> 

I remember this discussion, but at the moment I have no clear answer 
whether do_populate will go or not.

The concept that was discussed contains the following steps:

1. Buildchroot generation.
2. Building all the packages and put them to apt.
3. Provide this apt as the input for image's multistrap.

Thinking on this I've realized that this approach has one issue - it 
serializes buildchroot and image generation, so significantly increases 
Isar build time. Possible solution could be:

1. Buildchroot and base image rootfs generation.
2. Building all the packages and put them to apt.
3. Populate image rootfs via apt-get using Isar apt.

I'd like to discuss this in more details when I return to apt topic. So 
for now I'd rely on existing pipeline.

Alex

> Henning
> 
>> Alex
>>
>>>
>>> Henning
>>>    
>>>>    do_rootfs[stamp-extra-info] = "${MACHINE}-${DISTRO}"
>>>>    
>>>> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb
>>>> b/meta/recipes-devtools/buildchroot/buildchroot.bb index
>>>> 6a94733..fe88e98 100644 ---
>>>> a/meta/recipes-devtools/buildchroot/buildchroot.bb +++
>>>> b/meta/recipes-devtools/buildchroot/buildchroot.bb @@ -67,7 +67,28
>>>> @@ do_build() { # Install package builder script
>>>>        sudo install -m 755 ${WORKDIR}/build.sh ${BUILDCHROOT_DIR}
>>>>    
>>>> +    # Create share point for downloads
>>>> +    sudo install -d ${BUILDCHROOT_DIR}/git
>>>> +
>>>>        # Configure root filesystem
>>>>        sudo chroot ${BUILDCHROOT_DIR} /configscript.sh
>>>>        _do_build_cleanup
>>>>    }
>>>> +
>>>> +do_prepare[nostamp] = "1"
>>>> +
>>>> +do_prepare() {
>>>> +    sudo mount --bind ${GITDIR} ${BUILDCHROOT_DIR}/git
>>>> +}
>>>> +
>>>> +addtask prepare after do_build
>>>> +
>>>> +DEPENDS += "${IMAGE_INSTALL}"
>>>> +do_cleanup[deptask] = "do_deploy_deb"
>>>> +do_cleanup[nostamp] = "1"
>>>> +
>>>> +do_cleanup() {
>>>> +    sudo umount ${BUILDCHROOT_DIR}/git
>>>> +}
>>>> +
>>>> +addtask cleanup after do_prepare