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

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

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?

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

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