Re: [PATCH 3/6] classes/dpkg: Split install for cache

[Alexander Smirnov <asmirnov@ilbers.de>]

On 08/28/2017 11:00 AM, Claudius Heine wrote:
> Hi,
> 
> On 08/27/2017 05:13 PM, Alexander Smirnov wrote:
>> Split install function into two parts:
>>   - install_to_cache: if caching is enabled
>>   - install_to_deploy: if caching is disabled
>>
>> This patch brings flexibility to the implementation and makes it possible
>> to move all the caching implementation code to dedicated class. The magic
>> behavior depending on the value of DEBCACHE_ENABLED is now transparent in
>> the bitbake pipeline (can be inspected via "bitbake -g").
>>
>> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
>> ---
>>   meta/classes/dpkg.bbclass  | 66 
>> ++++++++++++++++++++++++++--------------------
>>   meta/classes/image.bbclass |  2 +-
>>   2 files changed, 38 insertions(+), 30 deletions(-)
>>
>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
>> index b1e201d..118ba2f 100644
>> --- a/meta/classes/dpkg.bbclass
>> +++ b/meta/classes/dpkg.bbclass
>> @@ -56,46 +56,53 @@ do_build() {
>>   }
>>   # Install package to dedicated deploy directory
>> -do_binary_deb_install() {
>> +do_install_to_deploy() {
>> +    # Deb caching is disabled, simply copy all binary packages to the 
>> deploy
>> +    # directory
>> +    install -m 644 "${BUILDROOT}"/*.deb "${DEPLOY_DIR_DEB}/"
>> +}
>> +
>> +addtask install_to_deploy after do_build
>> +do_install_to_deploy[dirs] = "${DEPLOY_DIR_DEB}"
>> +do_install_to_deploy[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> +do_install_to_deploy[noexec] = "1"
>> +
>> +# Install package to dedicated apt repo
>> +do_install_to_cache() {
>>       readonly DIR_CACHE="${DEBCACHEDIR}/${DISTRO}"
>>       readonly DIR_DB="${DEBDBDIR}/${DISTRO}"
>> -    if [ "${DEBCACHE_ENABLED}" != "0" ]; then
>> -        # If `bitbake` is running for the first time, the cache 
>> doesn't exist
>> -        # yet and needs to be configured using a `distributions` file.
>> -        # A template stored in the layer directory is pre-processed to
>> -        # generate the configuration file, which is then placed in the
>> -        # appropriate directory.
>> -        if [ ! -e "${DIR_CACHE}/conf/distributions" ]; then
>> -            mkdir -p "${DIR_CACHE}/conf"
>> -            sed -e "s#{DISTRO_NAME}#${DEBDISTRONAME}#g" \
>> -                "${DEBFILESDIR}/distributions.in" \
>> -                > "${DIR_CACHE}/conf/distributions"
>> -        fi
>> -
>> -        # Add binary and source packages to the deb cache
>> -        # If the cache doesn't exist yet, it will be created using the
>> -        # `distributions` file generated above.
>> -        ls -1 "${BUILDROOT}"/*.deb "${BUILDROOT}"/*.dsc | while read 
>> -r p; do
>> -            reprepro --waitforlock 3 -b "${DIR_CACHE}" --dbdir 
>> "${DIR_DB}" \
>> -                -C main "include${p##*.}" "${DEBDISTRONAME}" "${p}"
>> -        done
>> -    else
>> -        # Deb caching is disabled, simply copy all binary packages to 
>> the
>> -        # deploy directory
>> -        mkdir -p "${DEPLOY_DIR_DEB}"
>> -        install -m 644 "${BUILDROOT}"/*.deb "${DEPLOY_DIR_DEB}/"
>> +    # If `bitbake` is running for the first time, the cache doesn't 
>> exist
>> +    # yet and needs to be configured using a `distributions` file.
>> +    # A template stored in the layer directory is pre-processed to
>> +    # generate the configuration file, which is then placed in the
>> +    # appropriate directory.
>> +    if [ ! -e "${DIR_CACHE}/conf/distributions" ]; then
>> +        mkdir -p "${DIR_CACHE}/conf"
>> +        sed -e "s#{DISTRO_NAME}#${DEBDISTRONAME}#g" \
>> +            "${DEBFILESDIR}/distributions.in" \
>> +            > "${DIR_CACHE}/conf/distributions"
>>       fi
>> +
>> +    # Add binary and source packages to the deb cache
>> +    # If the cache doesn't exist yet, it will be created using the
>> +    # `distributions` file generated above.
>> +    ls -1 "${BUILDROOT}"/*.deb "${BUILDROOT}"/*.dsc | while read -r 
>> p; do
>> +        reprepro --waitforlock 3 -b "${DIR_CACHE}" --dbdir "${DIR_DB}" \
>> +            -C main "include${p##*.}" "${DEBDISTRONAME}" "${p}"
>> +    done
>>   }
>> -addtask binary_deb_install after do_build
>> -do_binary_deb_install[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> +addtask install_to_cache after do_build
> 
> What was the reason why we put stuff after the 'build' task? In OE the 
> 'build' task is the default task triggering everything necessary for a 
> recipe. In Isar that changed. Why?

Actually AFAIK do_build is not OE tasks, it's default bitbake task. 
Install is after do_build, because in Isar do_build is used to build the 
packages, so it's not empty.

> 
>> +do_install_to_cache[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> +do_install_to_cache[noexec] = "1"
>>   # Deb caching lambda run during the parsing phase that checks 
>> whether the
>>   # current package has to be rebuilt, or taken from the cache
>>   python __anonymous () {
>>       if d.getVar("DEBCACHE_ENABLED", True) == "0":
>> -        # Deb caching is disabled, do nothing
>> +        # Deb caching is disabled
>> +        d.delVarFlag('do_install_to_deploy', 'noexec')
>>           return True
>>       PN = d.getVar("PN", True)
>> @@ -108,6 +115,7 @@ python __anonymous () {
>>       path_cache = os.path.join(DEBCACHEDIR, DISTRO)
>>       path_databases = os.path.join(DEBDBDIR, DISTRO)
>>       path_distributions = os.path.join(path_cache, "conf", 
>> "distributions")
>> +    d.delVarFlag('do_install_to_cache', 'noexec')
>>       # The distributions file is needed by `reprepro` to know what types
>>       # of packages are supported, what the distribution name is, etc.
>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>> index c2ff453..6b1b5eb 100644
>> --- a/meta/classes/image.bbclass
>> +++ b/meta/classes/image.bbclass
>> @@ -46,4 +46,4 @@ do_populate() {
>>   }
>>   addtask populate before do_build
>> -do_populate[deptask] = "do_install"
>> +do_populate[deptask] = "do_install_to_cache do_install_to_deploy"
> 
> I would rather still have a, maybe empty, 'do_install' step that just 
> depends on the 'do_install_to_cache' and 'do_install_to_deploy' tasks. I 
> don't see why ever small change in one class should change the complete 
> pipeline for every recipe.

It's a bit difficult question. In general meaning when building 
something from sources, the install means a storing of build artifacts 
to some specific location. So in this case, we  just copy binary deb 
package to repo, and there is nothing common with installation of artifacts.

Also Isar has in roadmap cross-compilation, so the pipeline will 
probably look like:

fetch - unpack - compile - install - install_to_cache

so in this case the install task probably should go before install_to_cache.

So probably it makes sense to avoid _install_ in this task, for example 
use do_populate_deb_cache, what do you think?

-- 
With best regards,
Alexander Smirnov

ilbers GmbH
Baierbrunner Str. 28c
D-81379 Munich
+49 (89) 122 67 24-0
http://ilbers.de/
Commercial register Munich, HRB 214197
General manager: Baurzhan Ismagulov