Re: [PATCH 2/9] base-apt: Introduce base implementaiton

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

Hi Maxim,

On 10/2/18 2:19 PM, Maxim Yu. Osipov wrote:
> From: Alexander Smirnov <asmirnov@ilbers.de>
> 
> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
> ---
>   meta-isar/conf/layer.conf                          | 10 ++++---
>   meta/recipes-devtools/base-apt/base-apt.bb         | 31 ++++++++++++++++++++++
>   .../base-apt/files/distributions.in                |  3 +++
>   3 files changed, 40 insertions(+), 4 deletions(-)
>   create mode 100644 meta/recipes-devtools/base-apt/base-apt.bb
>   create mode 100644 meta/recipes-devtools/base-apt/files/distributions.in
> 
> diff --git a/meta-isar/conf/layer.conf b/meta-isar/conf/layer.conf
> index cd42f06..ae1b3c5 100644
> --- a/meta-isar/conf/layer.conf
> +++ b/meta-isar/conf/layer.conf
> @@ -20,8 +20,10 @@ LAYERDIR_isar = "${LAYERDIR}"
>   # Codename of the repository created by the caching class
>   DEBDISTRONAME = "isar"
>   
> -# Path to the Isar apt repository
> -DEPLOY_DIR_APT ?= "${DEPLOY_DIR}/apt"
> +# Isar apt repository paths
> +DEPLOY_DIR_APT ?= "${DEPLOY_DIR}/isar-apt/apt"
> +DEPLOY_DIR_DB ?= "${DEPLOY_DIR}/isar-apt/db"
>   
> -# Path to the Isar databases used by `reprepro`
> -DEPLOY_DIR_DB ?= "${DEPLOY_DIR}/db"
> +# Base apt repository paths
> +BASE_APT_DIR ?= "${DEPLOY_DIR}/base-apt/apt"
> +BASE_APT_DB ?= "${DEPLOY_DIR}/base-apt/db"

I don't like those variable names. IMO it would be ok to break the 
variable backwards compatibility here and unify that.

How about:

     REPO_ISAR_DIR
     REPO_ISAR_DB_DIR
     REPO_CACHE_DIR
     REPO_CACHE_DB_DIR

The db directory is reprepro specific so the main variable should point 
the the correct repository path. IMO


> diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
> new file mode 100644
> index 0000000..6ff1164
> --- /dev/null
> +++ b/meta/recipes-devtools/base-apt/base-apt.bb
> @@ -0,0 +1,31 @@
> +# This software is a part of ISAR.
> +# Copyright (C) 2018 ilbers GmbH
> +
> +SRC_URI = "file://distributions.in"
> +
> +inherit base-apt-helper
> +
> +CACHE_CONF_DIR = "${BASE_APT_DIR}/${DISTRO_NAME}/conf"
> +do_cache_config[dirs] = "${CACHE_CONF_DIR}"
> +do_cache_config[stamp-extra-info] = "${DISTRO}"
> +do_cache_config[lockfiles] = "${BASE_APT_DIR}/isar.lock"
> +
> +# Generate reprepro config for current distro if it doesn't exist. Once it's
> +# generated, this task should do nothing.
> +do_cache_config() {
> +    if [ ! -e "${CACHE_CONF_DIR}/distributions" ]; then
> +        sed -e "s#{DISTRO_NAME}#"${DISTRO_SUITE}"#g" \

Mixing DISTRO_NAME and DISTRO_SUITE. They are different, try to be 
consistent.

Cheers,
Claudius

> +            ${WORKDIR}/distributions.in > ${CACHE_CONF_DIR}/distributions
> +    fi
> +
> +    path_cache="${BASE_APT_DIR}/${DISTRO_NAME}"
> +    path_databases="${BASE_APT_DB}/${DISTRO_NAME}"
> +
> +    if [ ! -d "${path_databases}" ]; then
> +        reprepro -b ${path_cache} \
> +                 --dbdir ${path_databases} \
> +                 export ${DISTRO_SUITE}
> +    fi
> +}
> +
> +addtask cache_config after do_build
> diff --git a/meta/recipes-devtools/base-apt/files/distributions.in b/meta/recipes-devtools/base-apt/files/distributions.in
> new file mode 100644
> index 0000000..cc82c57
> --- /dev/null
> +++ b/meta/recipes-devtools/base-apt/files/distributions.in
> @@ -0,0 +1,3 @@
> +Codename: {DISTRO_NAME}
> +Architectures: i386 armhf arm64 amd64 source
> +Components: main
> 

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