Re: [PATCH 2/4] meta-isar-bin: Generate cache repos

[Henning Schild <henning.schild@siemens.com>]

Am Wed, 4 Oct 2017 13:57:23 +0300
schrieb Alexander Smirnov <asmirnov@ilbers.de>:

> On 10/04/2017 12:03 PM, Henning Schild wrote:
> > Am Mon, 2 Oct 2017 18:45:29 +0300
> > schrieb Alexander Smirnov <asmirnov@ilbers.de>:
> >   
> >> Generate repos for each configuration in multiconfig.
> >>
> >> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
> >> ---
> >>   meta-isar-bin/conf/layer.conf    |  3 +++
> >>   meta-isar/conf/local.conf.sample |  4 ++++
> >>   meta/classes/image.bbclass       | 35
> >> ++++++++++++++++++++++++++++++++++- 3 files changed, 41
> >> insertions(+), 1 deletion(-)
> >>
> >> diff --git a/meta-isar-bin/conf/layer.conf
> >> b/meta-isar-bin/conf/layer.conf index 3518184..5ef6e60 100644
> >> --- a/meta-isar-bin/conf/layer.conf
> >> +++ b/meta-isar-bin/conf/layer.conf
> >> @@ -9,3 +9,6 @@ DEBCACHEDIR ?= "${LAYERDIR}/apt"
> >>   
> >>   # Path to the configuration files templates used by `reprepro`
> >>   DEBFILESDIR ?= "${LAYERDIR}/files"
> >> +
> >> +# Path to the databases used by `reprepro`
> >> +DEBDBDIR ?= "${LAYERDIR}/db"
> >> diff --git a/meta-isar/conf/local.conf.sample
> >> b/meta-isar/conf/local.conf.sample index a456b1b..660958f 100644
> >> --- a/meta-isar/conf/local.conf.sample
> >> +++ b/meta-isar/conf/local.conf.sample
> >> @@ -158,3 +158,7 @@ IMAGE_INSTALL = "hello example-raw"
> >>   #
> >>   # Default parallel jobs for bitbake:
> >>   BB_NUMBER_THREADS = "4"
> >> +
> >> +#
> >> +# Number of attempts to try to get reprepro lock for access to apt
> >> cache +REPREPRO_LOCK_ATTEMPTS = "16"
> >> diff --git a/meta/classes/image.bbclass
> >> b/meta/classes/image.bbclass index d30b139..b07775a 100644
> >> --- a/meta/classes/image.bbclass
> >> +++ b/meta/classes/image.bbclass
> >> @@ -1,5 +1,5 @@
> >>   # This software is a part of ISAR.
> >> -# Copyright (C) 2015-2016 ilbers GmbH
> >> +# Copyright (C) 2015-2017 ilbers GmbH
> >>   
> >>   KERNEL_IMAGE ?= ""
> >>   INITRD_IMAGE ?= ""
> >> @@ -14,6 +14,30 @@ CACHE_CONF_DIR = "${DEBCACHEDIR}/${DISTRO}/conf"
> >>   do_cache_config[dirs] = "${CACHE_CONF_DIR}"
> >>   do_cache_config[stamp-extra-info] = "${DISTRO}"
> >>   
> >> +call_reprepro() {
> >> +    for i in $(seq 1 ${REPREPRO_LOCK_ATTEMPTS}); do
> >> +        #  According to `sh` manual page, shell exit statuses fall
> >> between
> >> +        # 0-255. The EEXIST error code is (-17), so casting to
> >> usigned 8-bit  
> > 
> > cant you use python to get that number and use the name "EEXIST"
> > instead of a hardcoded number? I think i sent a sniplet in an
> > earlier mail.  
> 
> No we can't. In python I've checked the code EEXIST, and it is (17), 
> while return value from subprocess call is still (239). So using
> python here is only overhead in correct wrapping of reprepro call.

Just do the math in python, skip the long comment and get rid of the
magic number.
python -c "import errno; print(256-errno.EEXIST)"

> >   
> >> +        # integer results value (239).
> >> +        eexist="239"
> >> +        retval="0"
> >> +        reprepro --waitforlock 1 $* || retval="$?"  
> > 
> > The waitforlock should be skipped altogether, which means try it
> > once. The current version means try twice. I got that wrong when i
> > suggested to go for "1", sorry.  
> 
> 'waitforlock 1' adds 10 seconds delay between attempts. So the idea
> to use this form to avoid extra sleep calls with magic timeout value.

Now you have two nested loops again and the name REPREPRO_LOCK_ATTEMPTS
is wrong, because you do it 2*REPREPRO_LOCK_ATTEMPTS. Either divide
that number by two or introduce a sleep on the error-path.
 
> >   
> >> +        # If reprepro has failed to get database lock, it returns
> >> EEXIST code.
> >> +        # In this case we continue trying to get lock until max
> >> amount of
> >> +        # attempts is reached.
> >> +        if [ $retval -eq $eexist ]; then
> >> +            bbwarn "Failed to get reprepro lock, trying
> >> again..."  
> > 
> > Are there other cases where reprepro returns -EEXIST?
> >   
> 
> AFAIK there is only one case when reprepro could return EEXIST - it's 
> morgue feature which is not used here. All the other EEXIST checks
> are processed internally.

Ok, just wanted to make sure someone checked that.

Henning

> Alex
> 
> > Henning
> >   
> >> +        else
> >> +            break
> >> +        fi
> >> +    done
> >> +
> >> +    if [ $retval -ne 0 ]; then
> >> +        bbfatal "reprepro failed"
> >> +    fi
> >> +}
> >> +
> >>   # Generate reprepro config for current distro if it doesn't
> >> exist. Once it's # generated, this task should do nothing.
> >>   do_cache_config() {
> >> @@ -21,6 +45,15 @@ do_cache_config() {
> >>           sed -e "s#{DISTRO_NAME}#"${DEBDISTRONAME}"#g" \
> >>               ${DEBFILESDIR}/distributions.in >
> >> ${CACHE_CONF_DIR}/distributions fi
> >> +
> >> +    path_cache="${DEBCACHEDIR}/${DISTRO}"
> >> +    path_databases="${DEBDBDIR}/${DISTRO}"
> >> +
> >> +    if [ ! -d "${path_databases}" ]; then
> >> +        call_reprepro -b ${path_cache} \
> >> +                      --dbdir ${path_databases} \
> >> +                      export ${DEBDISTRONAME}
> >> +    fi
> >>   }
> >>   
> >>   addtask cache_config before do_fetch  
> >