[PATCH 0/1] acquire database lock when calling reprepro

[Cedric Hombourger]

As bitbake performs multi-threaded builds, packages may be built
while others get deployed. A lock was already acquired within
dpkg_runbuild but accesses to the isar-apt database were not
protected from do_deploy_deb. Move the lock from the buildchroot
to the isar-apt repository and acquire the (same) lock from
dpkg_runbuild, repo_clean and do_deploy_deb.

Cedric Hombourger (1):
dpkg: acquire lock when calling reprepro

meta/classes/dpkg-base.bbclass | 8 ++++++--
meta/recipes-devtools/buildchroot/files/build.sh | 2 +-
meta/recipes-kernel/linux/files/build-kernel.sh | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)

--
2.11.0

[Cedric Hombourger]

As bitbake performs multi-threaded builds, packages may be built
while others get deployed. A lock was already acquired within
dpkg_runbuild but accesses to the isar-apt database were not
protected from do_deploy_deb. Move the lock from the buildchroot
to the isar-apt repository and acquire the (same) lock from
dpkg_runbuild, repo_clean and do_deploy_deb.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

meta/classes/dpkg-base.bbclass | 8 ++++++--
meta/recipes-devtools/buildchroot/files/build.sh | 2 +-
meta/recipes-kernel/linux/files/build-kernel.sh | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index a41df2c..7137968 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -71,6 +71,8 @@ do_build() {
mount -t proc none ${BUILDCHROOT_DIR}/proc
fi'

+ # Create database lock file here so we own it (not root) and perform actual build
+ touch ${DEPLOY_DIR_APT}/${DISTRO}/db.lock
dpkg_runbuild

sudo umount ${BUILDROOT} 2>/dev/null || true
@@ -82,7 +84,8 @@ CLEANFUNCS += "repo_clean"
repo_clean() {
PACKAGES=$(cd ${S}/..; ls *.deb | sed 's/\([^_]*\).*/\1/')
if [ -n "${PACKAGES}" ]; then
- reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
+ flock ${DEPLOY_DIR_APT}/${DISTRO}/db.lock \
+ reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
--dbdir ${DEPLOY_DIR_DB}/${DISTRO} \
-C main -A ${DISTRO_ARCH} \
remove ${DEBDISTRONAME} \
@@ -93,7 +96,8 @@ repo_clean() {
# Install package to Isar-apt
do_deploy_deb() {
repo_clean
- reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
+ flock ${DEPLOY_DIR_APT}/${DISTRO}/db.lock \
+ reprepro -b ${DEPLOY_DIR_APT}/${DISTRO} \
--dbdir ${DEPLOY_DIR_DB}/${DISTRO} \
-C main \
includedeb ${DEBDISTRONAME} \
diff --git a/meta/recipes-devtools/buildchroot/files/build.sh b/meta/recipes-devtools/buildchroot/files/build.sh
index e2dabab..e8a9c17 100644
--- a/meta/recipes-devtools/buildchroot/files/build.sh
+++ b/meta/recipes-devtools/buildchroot/files/build.sh
@@ -50,7 +50,7 @@ install_cmd="apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y

# Install all build deps
mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
-) 42>/dpkg.lock
+) 42>/isar-apt/db.lock

# If autotools files have been created, update their timestamp to
# prevent them from being regenerated
diff --git a/meta/recipes-kernel/linux/files/build-kernel.sh b/meta/recipes-kernel/linux/files/build-kernel.sh
index 8b58e93..2ff5a1e 100644
--- a/meta/recipes-kernel/linux/files/build-kernel.sh
+++ b/meta/recipes-kernel/linux/files/build-kernel.sh
@@ -34,7 +34,7 @@ REPACK_LINUX_IMAGE_DIR="${REPACK_DIR}/linux-image"
REPACK_LINUX_HEADERS_DIR="${REPACK_DIR}/linux-headers"

# Lock-protected because apt and dpkg do not wait in case of contention
-flock /dpkg.lock \
+flock /isar-apt/db.lock \
apt-get install -y -o Debug::pkgProblemResolver=yes \
--no-install-recommends ${KBUILD_DEPENDS}

--
2.11.0

[Alexander Smirnov]

Hi,

On 22.08.2018 14:34, Cedric Hombourger wrote:
> As bitbake performs multi-threaded builds, packages may be built
> while others get deployed. A lock was already acquired within
> dpkg_runbuild but accesses to the isar-apt database were not
> protected from do_deploy_deb. Move the lock from the buildchroot
> to the isar-apt repository and acquire the (same) lock from
> dpkg_runbuild, repo_clean and do_deploy_deb.
>

Have you observed any issues with current implementation? IIRC reprepro
has it's own built-in lock file.

Alex

[Hombourger, Cedric]

Hi Alex,

Yes I observed a dpkg_runbuild failure for a custom recipe because it was running apt-get update (from /build.sh) while the isar-apt database was being updated by another bitbake thread running do_deploy_deb

apt-get update was complaining that the hash of the isar-apt files was changed in the background

We should therefore let reprepro finish its job for the current recipe/package(s) before we update the database in /build.sh

I was lucky enough to get a test-case failing 100% (in our large Isar-based project)
With the lock rework, the issue was not observed anymore (neither on my machine nor on any of Jenkins nodes we have our CI running on)

Hope that helps
Cedric

[Jan Kiszka]

On 2018-08-22 13:34, Cedric Hombourger wrote:
> As bitbake performs multi-threaded builds, packages may be built
> while others get deployed. A lock was already acquired within
> dpkg_runbuild but accesses to the isar-apt database were not
> protected from do_deploy_deb. Move the lock from the buildchroot
> to the isar-apt repository and acquire the (same) lock from
> dpkg_runbuild, repo_clean and do_deploy_deb.

I think I've seen this issue once but forgot to dig into the details. It
looks consistent.

There is also

do_deploy_deb[lockfiles] = "${DEPLOY_DIR_APT}/isar.lock"

in that file. I suppose that can now be dropped, can't it?

> diff --git a/meta/recipes-devtools/buildchroot/files/build.sh b/meta/recipes-devtools/buildchroot/files/build.sh
> index e2dabab..e8a9c17 100644
> --- a/meta/recipes-devtools/buildchroot/files/build.sh
> +++ b/meta/recipes-devtools/buildchroot/files/build.sh
> @@ -50,7 +50,7 @@ install_cmd="apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y
>
> # Install all build deps
> mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
> -) 42>/dpkg.lock
> +) 42>/isar-apt/db.lock
>
> # If autotools files have been created, update their timestamp to
> # prevent them from being regenerated
> diff --git a/meta/recipes-kernel/linux/files/build-kernel.sh b/meta/recipes-kernel/linux/files/build-kernel.sh
> index 8b58e93..2ff5a1e 100644
> --- a/meta/recipes-kernel/linux/files/build-kernel.sh
> +++ b/meta/recipes-kernel/linux/files/build-kernel.sh
> @@ -34,7 +34,7 @@ REPACK_LINUX_IMAGE_DIR="${REPACK_DIR}/linux-image"
> REPACK_LINUX_HEADERS_DIR="${REPACK_DIR}/linux-headers"
>
> # Lock-protected because apt and dpkg do not wait in case of contention
> -flock /dpkg.lock \
> +flock /isar-apt/db.lock \
> apt-get install -y -o Debug::pkgProblemResolver=yes \
> --no-install-recommends ${KBUILD_DEPENDS}
>
>

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[Cedric Hombourger]

As bitbake performs multi-threaded builds, packages may be built
while others get deployed. A lock was already acquired within
dpkg_runbuild but accesses to the isar-apt database were not
protected from do_deploy_deb. Move the lock from the buildchroot
to the isar-apt repository and acquire the (same) lock from
dpkg_runbuild, repo_clean and do_deploy_deb.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

meta/classes/dpkg-base.bbclass | 9 ++++++---
meta/recipes-devtools/buildchroot/files/build.sh | 2 +-
meta/recipes-devtools/isar-apt/isar-apt-host.bb | 4 ++--
meta/recipes-devtools/isar-apt/isar-apt.bb | 4 ++--
meta/recipes-kernel/linux/files/build-kernel.sh | 2 +-
5 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index a41df2c..108d942 100644

@@ -102,6 +106,5 @@ do_deploy_deb() {

addtask deploy_deb after do_build
do_deploy_deb[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
-do_deploy_deb[lockfiles] = "${DEPLOY_DIR_APT}/isar.lock"
do_deploy_deb[depends] = "isar-apt:do_cache_config"
do_deploy_deb[dirs] = "${S}"

diff --git a/meta/recipes-devtools/buildchroot/files/build.sh b/meta/recipes-devtools/buildchroot/files/build.sh
index e2dabab..e8a9c17 100644
--- a/meta/recipes-devtools/buildchroot/files/build.sh
+++ b/meta/recipes-devtools/buildchroot/files/build.sh
@@ -50,7 +50,7 @@ install_cmd="apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y

# Install all build deps
mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
-) 42>/dpkg.lock
+) 42>/isar-apt/db.lock

# If autotools files have been created, update their timestamp to
# prevent them from being regenerated

diff --git a/meta/recipes-devtools/isar-apt/isar-apt-host.bb b/meta/recipes-devtools/isar-apt/isar-apt-host.bb
index 0cf5ce1..cab84a7 100644
--- a/meta/recipes-devtools/isar-apt/isar-apt-host.bb
+++ b/meta/recipes-devtools/isar-apt/isar-apt-host.bb
@@ -8,7 +8,6 @@ SRC_URI = "file://distributions.in"
CACHE_CONF_DIR = "${DEPLOY_DIR_APT}/${HOST_DISTRO}/conf"
do_cache_config[dirs] = "${CACHE_CONF_DIR}"
do_cache_config[stamp-extra-info] = "${HOST_DISTRO}"
-do_cache_config[lockfiles] = "${DEPLOY_DIR_APT}/isar.lock"

# Generate reprepro config for current host distro if it doesn't exist. Once it's
# generated, this task should do nothing.
@@ -22,7 +21,8 @@ do_cache_config() {
path_databases="${DEPLOY_DIR_DB}/${HOST_DISTRO}"

if [ ! -d "${path_databases}" ]; then
- reprepro -b ${path_cache} \
+ flock ${DEPLOY_DIR_APT}/db.lock \
+ reprepro -b ${path_cache} \
--dbdir ${path_databases} \
export ${DEBDISTRONAME}
fi
diff --git a/meta/recipes-devtools/isar-apt/isar-apt.bb b/meta/recipes-devtools/isar-apt/isar-apt.bb
index 9c31d12..e9dfec1 100644
--- a/meta/recipes-devtools/isar-apt/isar-apt.bb
+++ b/meta/recipes-devtools/isar-apt/isar-apt.bb
@@ -6,7 +6,6 @@ SRC_URI = "file://distributions.in"
CACHE_CONF_DIR = "${DEPLOY_DIR_APT}/${DISTRO}/conf"
do_cache_config[dirs] = "${CACHE_CONF_DIR}"
do_cache_config[stamp-extra-info] = "${DISTRO}"
-do_cache_config[lockfiles] = "${DEPLOY_DIR_APT}/isar.lock"

# Generate reprepro config for current distro if it doesn't exist. Once it's
# generated, this task should do nothing.
@@ -20,7 +19,8 @@ do_cache_config() {
path_databases="${DEPLOY_DIR_DB}/${DISTRO}"

if [ ! -d "${path_databases}" ]; then
- reprepro -b ${path_cache} \
+ flock ${DEPLOY_DIR_APT}/db.lock \
+ reprepro -b ${path_cache} \
--dbdir ${path_databases} \
export ${DEBDISTRONAME}
fi

diff --git a/meta/recipes-kernel/linux/files/build-kernel.sh b/meta/recipes-kernel/linux/files/build-kernel.sh
index 8b58e93..2ff5a1e 100644
--- a/meta/recipes-kernel/linux/files/build-kernel.sh
+++ b/meta/recipes-kernel/linux/files/build-kernel.sh
@@ -34,7 +34,7 @@ REPACK_LINUX_IMAGE_DIR="${REPACK_DIR}/linux-image"
REPACK_LINUX_HEADERS_DIR="${REPACK_DIR}/linux-headers"

# Lock-protected because apt and dpkg do not wait in case of contention
-flock /dpkg.lock \
+flock /isar-apt/db.lock \
apt-get install -y -o Debug::pkgProblemResolver=yes \
--no-install-recommends ${KBUILD_DEPENDS}

--
2.11.0

[Hombourger, Cedric]

Thanks Jan for your quick feedback. I have submitted a v2 with isar.lock removed

Cedric

-----Original Message-----
From: Jan Kiszka [mailto:jan.k...@siemens.com]
Sent: Wednesday, August 22, 2018 2:20 PM
To: Hombourger, Cedric <Cedric_H...@mentor.com>; isar-...@googlegroups.com

[Henning Schild]

My first reaction was the same, but in the other direction. I would
prefer a bitbake lock over the flock, if possible.

Henning

Am Wed, 22 Aug 2018 12:48:14 +0000
schrieb "Hombourger, Cedric" <Cedric_H...@mentor.com>:

[Hombourger, Cedric]

Hi Henning,

I am not sure this can be easily done without additional some additional refactoring
More specifically, I believe this would require splitting dpkg_runbuild & build.sh into 2 distinct tasks:

1) run apt-get update build the package dependencies package (aka the 1st section of build.sh)
2) perform the actual package build

Without this, we would keep the lock acquired for as long as the full package build goes
(several minutes for a kernel build)

I will investigate
This may make it easier to implement the other thing I had in mind: do not perform builds as root

Cedric

[Jan Kiszka]

On 2018-08-22 15:41, Henning Schild wrote:
> My first reaction was the same, but in the other direction. I would
> prefer a bitbake lock over the flock, if possible.

That would at least involve factoring out the critical section into
dedicated tasks - otherwise we to over-locking and serialize harmless
(and potentially lengthy) steps needlessly.

For now, I would be infavor of fixing in-place and trying to cleanup on top.

Jan

[Cedric Hombourger]

In preparation for a rework of locking mechanisms around apt
operations (installation of packages or inclusion of new
packages into the isar-apt database), move the code for
installing build dependencies out of dpkg_runbuild and into
a new function: dpkg_prepare.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

doc/technical_overview.md | 29 ++++++++++----
meta/classes/dpkg-base.bbclass | 30 +++++++++++---
meta/classes/dpkg-raw.bbclass | 15 ++++---
meta/classes/dpkg.bbclass | 8 +++-
meta/recipes-devtools/buildchroot/buildchroot.inc | 9 ++++-
meta/recipes-devtools/buildchroot/files/build.sh | 48 +----------------------
meta/recipes-devtools/buildchroot/files/common.sh | 28 +++++++++++++
meta/recipes-devtools/buildchroot/files/deps.sh | 32 +++++++++++++++
meta/recipes-kernel/linux-module/module.inc | 2 +-
9 files changed, 128 insertions(+), 73 deletions(-)
create mode 100644 meta/recipes-devtools/buildchroot/files/common.sh
create mode 100644 meta/recipes-devtools/buildchroot/files/deps.sh

diff --git a/doc/technical_overview.md b/doc/technical_overview.md
index ddd2c79..4f6d954 100644
--- a/doc/technical_overview.md
+++ b/doc/technical_overview.md
@@ -217,24 +217,37 @@ Both consist of the following steps:
3. Task `do_install` _only_ for `dpkg-raw`: copy all you want in your
debian package to `${D}`, install hooks in `${D}/DEBIAN`

-4. Task `do_build`: mount folder with unpacked files to buildchroot, execute
- the actual build function `dpkg_runbuild`, and finally umount again
+4. Task `do_prepare': perform any preparation steps to the unpacked/patched
+ sources before the build. This task calls the dpkg_prepare shell function
+ with the buildchroot mounts in place (`dpkg_do_mounts')

- 4.1. the `dpkg_runbuild` function of `dpkg.bbclass` runs `build.sh` in the
- buildchroot. That performs the following:
+ 4.1. the `dpkg_prepare` function of `dpkg.bbclass` runs `/isar/deps.sh` in
+ the buildchroot. That performs the following:

1. Go to `/home/build/${PN}`

2. Get list of dependencies from debian/control and install them

- 3. Run dpkg-buildpackage
+ 4.2. the `dpkg_prepare` function of `dpkg-raw.bbclass` translate the
+ recipe meta-data into a debian/control file suitable for packaging
+ with dpkg-deb
+
+5. Task `do_build`: mount folder (`dpkg_do_mounts') with unpacked files to buildchroot,
+ execute the actual build function `dpkg_runbuild`, and finally umount again
+ (`dpkg_undo_mounts')
+
+ 5.1. the `dpkg_runbuild` function of `dpkg.bbclass` runs `build.sh` in the
+ buildchroot. That performs the following:
+
+ 1. Go to `/home/build/${PN}`
+
+ 2. Run dpkg-buildpackage

- 4.2. the `dpkg_runbuild` function of `dpkg-raw.bbclass` basically runs
+ 5.2. the `dpkg_runbuild` function of `dpkg-raw.bbclass` basically runs
`dpkg-deb` to construct a Debian package from a folder of files,
without compiling anything

-
-5. Task `do_deploy_deb`: install successfully built packages
+6. Task `do_deploy_deb`: install successfully built packages
`${WORKDIR}/*.deb` to deploy directory `${DEPLOY_DIR_DEB}`

## 3.6 Populate Target Filesystem
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index a41df2c..5826357 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -13,7 +13,7 @@ python __anonymous() {
dep = "buildchroot-host:do_build"
rootfs = d.getVar('BUILDCHROOT_HOST_DIR', True)

- d.setVarFlag('do_build', 'depends', dep)
+ d.setVarFlag('do_prepare', 'depends', dep)
d.setVar('BUILDCHROOT_DIR', rootfs)
}

@@ -32,7 +32,7 @@ addtask patch after do_adjust_git before do_build

# Add dependency between Isar recipes
DEPENDS ?= ""
-do_build[deptask] = "do_deploy_deb"
+do_prepare[deptask] = "do_deploy_deb"

def get_package_srcdir(d):
s = d.getVar("S", True)
@@ -50,7 +50,10 @@ PPS ?= "${@get_package_srcdir(d)}"
BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"

-# default to "emtpy" implementation
+# default to "emtpy" implementation for dpkg_prepare() and dpkg_runbuild()
+dpkg_prepare() {
+ true
+}
dpkg_runbuild() {
die "This should never be called, overwrite it in your derived class"
}
@@ -58,7 +61,7 @@ dpkg_runbuild() {
MOUNT_LOCKFILE = "${BUILDCHROOT_DIR}/mount.lock"

# Wrap the function dpkg_runbuild with the bind mount for buildroot
-do_build() {
+dpkg_do_mounts() {
mkdir -p ${BUILDROOT}
sudo mount --bind ${WORKDIR} ${BUILDROOT}

@@ -70,13 +73,28 @@ do_build() {
mount -t devtmpfs -o mode=0755,nosuid devtmpfs ${BUILDCHROOT_DIR}/dev

mount -t proc none ${BUILDCHROOT_DIR}/proc
fi'
+}

- dpkg_runbuild
-
+dpkg_undo_mounts() {

sudo umount ${BUILDROOT} 2>/dev/null || true

sudo rmdir ${BUILDROOT} 2>/dev/null || true
}

+do_prepare() {
+ dpkg_do_mounts
+ dpkg_prepare
+ dpkg_undo_mounts
+}
+
+addtask prepare after do_patch before do_build
+do_prepare[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
+
+do_build() {
+ dpkg_do_mounts
+ dpkg_runbuild
+ dpkg_undo_mounts
+}
+
CLEANFUNCS += "repo_clean"

repo_clean() {
diff --git a/meta/classes/dpkg-raw.bbclass b/meta/classes/dpkg-raw.bbclass
index 28233ac..c5f1c88 100644
--- a/meta/classes/dpkg-raw.bbclass
+++ b/meta/classes/dpkg-raw.bbclass
@@ -14,9 +14,9 @@ do_install() {
}

do_install[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
-addtask install after do_unpack before do_deb_package_prepare
+addtask install after do_unpack before do_prepare

-do_deb_package_prepare() {
+deb_package_prepare() {
sudo rm -rf ${D}/DEBIAN
mkdir -p ${D}/DEBIAN
cat<<-__EOF__ > ${D}/DEBIAN/control
@@ -42,17 +42,16 @@ do_deb_package_prepare() {
done
}

-do_deb_package_prepare[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
-addtask deb_package_prepare after do_install before do_deb_package_conffiles
-
-do_deb_package_conffiles() {
+deb_package_conffiles() {
CONFFILES=${D}/DEBIAN/conffiles
find ${D} -type f -path '${D}/etc/*' | sed -e 's|^${D}|/|' >> $CONFFILES
test -s $CONFFILES || rm $CONFFILES
}

-do_deb_package_conffiles[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
-addtask deb_package_conffiles after do_deb_package_prepare before do_build
+dpkg_prepare() {
+ deb_package_prepare
+ deb_package_conffiles
+}

dpkg_runbuild() {
sudo chown -R root:root ${D}/DEBIAN/
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index ab70645..e8bd6ba 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -3,8 +3,14 @@

inherit dpkg-base

+# Install build dependencies for package
+dpkg_prepare() {
+ E="${@ bb.utils.export_proxies(d)}"
+ sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh ${PP}/${PPS} ${DISTRO_ARCH}
+}
+
# Build package from sources using build script
dpkg_runbuild() {
E="${@ bb.utils.export_proxies(d)}"
- sudo -E chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${PPS} ${DISTRO_ARCH}
+ sudo -E chroot ${BUILDCHROOT_DIR} /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
}
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 43e3cd6..7dd909e 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -8,7 +8,9 @@ LIC_FILES_CHKSUM = "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260

FILESPATH_prepend := "${THISDIR}/files:"
SRC_URI = "file://configscript.sh \
- file://build.sh"
+ file://build.sh \
+ file://common.sh \
+ file://deps.sh"
PV = "1.0"

inherit isar-bootstrap-helper
@@ -27,7 +29,10 @@ do_build() {

# Install package builder script
sudo chmod -R a+rw "${BUILDCHROOT_DIR}/home/builder"
- sudo install -m 755 ${WORKDIR}/build.sh ${BUILDCHROOT_DIR}
+ sudo install -m 755 -d ${BUILDCHROOT_DIR}/isar
+ sudo install -m 755 ${WORKDIR}/build.sh ${BUILDCHROOT_DIR}/isar/
+ sudo install -m 755 ${WORKDIR}/common.sh ${BUILDCHROOT_DIR}/isar/
+ sudo install -m 755 ${WORKDIR}/deps.sh ${BUILDCHROOT_DIR}/isar/

# Configure root filesystem
sudo install -m 755 ${WORKDIR}/configscript.sh ${BUILDCHROOT_DIR}
diff --git a/meta/recipes-devtools/buildchroot/files/build.sh b/meta/recipes-devtools/buildchroot/files/build.sh
index e2dabab..3c80bd7 100644
--- a/meta/recipes-devtools/buildchroot/files/build.sh
+++ b/meta/recipes-devtools/buildchroot/files/build.sh
@@ -4,53 +4,7 @@
# Copyright (C) 2015-2017 ilbers GmbH
# Copyright (c) 2018 Siemens AG

-set -e
-
-# Create human-readable names
-target_arch=$2
-
-# Notes:
-# mk-build-deps for jessie and jtretch has different parameter name to specify
-# host architecture.
-debian_version=$(cut -c1 /etc/debian_version)
-if [ $(($debian_version)) -ge 9 ]; then
- set_arch="--host-arch $target_arch"
-else
- set_arch="-a $target_arch"
-fi
-
-# Go to build directory
-cd $1
-
-# To avoid Perl locale warnings:
-export LC_ALL=C
-export LANG=C
-export LANGUAGE=C
-
-# Install command to be used by mk-build-deps
-# Notes:
-# 1) everything before the -y switch is unchanged from the defaults
-# 2) we add -y to go non-interactive
-install_cmd="apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y"
-
-(
- # Lock-protected because apt and dpkg do not wait in case of contention
- flock 42 || exit 1
-
- # Make sure that we have latest isar-apt content.
- # Options meaning:
- # Dir::Etc::sourcelist - specifies which source to be used
- # Dir::Etc::sourceparts - disables looking for the other sources
- # APT::Get::List-Cleanup - do not erase obsolete packages list for
- # upstream in '/var/lib/apt/lists'
- apt-get update \
- -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
- -o Dir::Etc::sourceparts="-" \
- -o APT::Get::List-Cleanup="0"
-
- # Install all build deps
- mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
-) 42>/dpkg.lock
+source /isar/common.sh

# If autotools files have been created, update their timestamp to
# prevent them from being regenerated

diff --git a/meta/recipes-devtools/buildchroot/files/common.sh b/meta/recipes-devtools/buildchroot/files/common.sh
new file mode 100644
index 0000000..b7551eb
--- /dev/null
+++ b/meta/recipes-devtools/buildchroot/files/common.sh
@@ -0,0 +1,28 @@
+#!/bin/bash
+#
+# This software is a part of ISAR.
+# Copyright (C) 2015-2017 ilbers GmbH
+# Copyright (c) 2018 Siemens AG
+
+set -e
+
+# Create human-readable names
+target_arch=$2
+
+# Notes:
+# mk-build-deps for jessie and jtretch has different parameter name to specify
+# host architecture.
+debian_version=$(cut -c1 /etc/debian_version)
+if [ $(($debian_version)) -ge 9 ]; then
+ set_arch="--host-arch $target_arch"
+else
+ set_arch="-a $target_arch"
+fi
+
+# Go to build directory
+cd $1
+
+# To avoid Perl locale warnings:
+export LC_ALL=C
+export LANG=C
+export LANGUAGE=C
diff --git a/meta/recipes-devtools/buildchroot/files/deps.sh b/meta/recipes-devtools/buildchroot/files/deps.sh
new file mode 100644
index 0000000..854a4d5
--- /dev/null
+++ b/meta/recipes-devtools/buildchroot/files/deps.sh
@@ -0,0 +1,32 @@
+#!/bin/bash
+#
+# This software is a part of ISAR.
+# Copyright (C) 2015-2017 ilbers GmbH
+# Copyright (c) 2018 Siemens AG
+
+source /isar/common.sh
+
+# Install command to be used by mk-build-deps
+# Notes:
+# 1) everything before the -y switch is unchanged from the defaults
+# 2) we add -y to go non-interactive
+install_cmd="apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y"
+
+(
+ # Lock-protected because apt and dpkg do not wait in case of contention
+ flock 42 || exit 1
+
+ # Make sure that we have latest isar-apt content.
+ # Options meaning:
+ # Dir::Etc::sourcelist - specifies which source to be used
+ # Dir::Etc::sourceparts - disables looking for the other sources
+ # APT::Get::List-Cleanup - do not erase obsolete packages list for
+ # upstream in '/var/lib/apt/lists'
+ apt-get update \
+ -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
+ -o Dir::Etc::sourceparts="-" \
+ -o APT::Get::List-Cleanup="0"
+
+ # Install all build deps
+ mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
+) 42>/dpkg.lock
diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
index 3075f44..3a3cab1 100644
--- a/meta/recipes-kernel/linux-module/module.inc
+++ b/meta/recipes-kernel/linux-module/module.inc
@@ -17,7 +17,7 @@ AUTOLOAD ?= "0"

inherit dpkg

-dpkg_runbuild_prepend() {
+do_prepare_prepend() {
cp -r ${WORKDIR}/debian ${S}/
sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
-e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
--
2.11.0

[Cedric Hombourger]

To prepare removal of flock calls around dpkg/apt operations, move
the code installing build dependencies to dpkg_prepare()

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

meta/recipes-kernel/linux/files/build-kernel.sh | 5 -----
meta/recipes-kernel/linux/linux-custom.inc | 8 ++++++++
2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/meta/recipes-kernel/linux/files/build-kernel.sh b/meta/recipes-kernel/linux/files/build-kernel.sh
index 8b58e93..498067c 100644
--- a/meta/recipes-kernel/linux/files/build-kernel.sh
+++ b/meta/recipes-kernel/linux/files/build-kernel.sh
@@ -33,11 +33,6 @@ REPACK_DIR="$1/../repack"

REPACK_LINUX_IMAGE_DIR="${REPACK_DIR}/linux-image"
REPACK_LINUX_HEADERS_DIR="${REPACK_DIR}/linux-headers"

-# Lock-protected because apt and dpkg do not wait in case of contention
-flock /dpkg.lock \
- apt-get install -y -o Debug::pkgProblemResolver=yes \
- --no-install-recommends ${KBUILD_DEPENDS}
-
cd $1
make olddefconfig

diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index 87d4377..26c6ad8 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -28,6 +28,14 @@ KBUILD_DEPENDS ?= "libssl-dev libelf-dev bc"
KERNEL_DEBIAN_DEPENDS ?= "initramfs-tools | linux-initramfs-tool, kmod, linux-base (>= 4.3~)"
KERNEL_HEADERS_DEBIAN_DEPENDS ?= "libc6, libssl1.1"

+dpkg_prepare() {

+ # Lock-protected because apt and dpkg do not wait in case of contention

+ sudo -E chroot ${BUILDCHROOT_DIR} \
+ flock /dpkg.lock apt-get install \
+ -y -o Debug::pkgProblemResolver=yes \
+ --no-install-recommends ${KBUILD_DEPENDS}
+}
+
dpkg_runbuild() {

# Install package builder script

sudo install -m 755 ${WORKDIR}/build-kernel.sh ${BUILDCHROOT_DIR}
--
2.11.0

[Cedric Hombourger]

reprepro and apt operations may not run in parallel since this could result
in the apt database being updated (by reprepro) while it is being read (by
apt-get update). reprepro calls were already serialized with Isar's lock (via
do_deploy_deb[lockfiles]). Use the same lock for do_prepare in the dpkg and
linux-custom packaging code. This allows us to drop the /dpkg.lock flock and
use bitbake's locking scheme instead.

Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com>
---

meta/classes/dpkg.bbclass | 3 +++
meta/recipes-devtools/buildchroot/files/deps.sh | 29 ++++++++++---------------
meta/recipes-kernel/linux/linux-custom.inc | 6 +++--
3 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index e8bd6ba..2e53c1b 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -9,6 +9,9 @@ dpkg_prepare() {

sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh ${PP}/${PPS} ${DISTRO_ARCH}
}

+# apt and reprepro may not run in parallel, acquire the Isar lock
+do_prepare[lockfiles] += "${DEPLOY_DIR_APT}/isar.lock"

+
# Build package from sources using build script
dpkg_runbuild() {
E="${@ bb.utils.export_proxies(d)}"

diff --git a/meta/recipes-devtools/buildchroot/files/deps.sh b/meta/recipes-devtools/buildchroot/files/deps.sh
index 854a4d5..4bd604f 100644
--- a/meta/recipes-devtools/buildchroot/files/deps.sh
+++ b/meta/recipes-devtools/buildchroot/files/deps.sh
@@ -12,21 +12,16 @@ source /isar/common.sh

# 2) we add -y to go non-interactive

install_cmd="apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y"

-(
- # Lock-protected because apt and dpkg do not wait in case of contention
- flock 42 || exit 1

+# Make sure that we have latest isar-apt content.
+# Options meaning:
+# Dir::Etc::sourcelist - specifies which source to be used
+# Dir::Etc::sourceparts - disables looking for the other sources
+# APT::Get::List-Cleanup - do not erase obsolete packages list for
+# upstream in '/var/lib/apt/lists'
+apt-get update \

+ -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
+ -o Dir::Etc::sourceparts="-" \
+ -o APT::Get::List-Cleanup="0"

- # Make sure that we have latest isar-apt content.
- # Options meaning:
- # Dir::Etc::sourcelist - specifies which source to be used
- # Dir::Etc::sourceparts - disables looking for the other sources
- # APT::Get::List-Cleanup - do not erase obsolete packages list for
- # upstream in '/var/lib/apt/lists'
- apt-get update \
- -o Dir::Etc::sourcelist="sources.list.d/isar-apt.list" \
- -o Dir::Etc::sourceparts="-" \
- -o APT::Get::List-Cleanup="0"
-
- # Install all build deps
- mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
-) 42>/dpkg.lock

+# Install all build deps
+mk-build-deps $set_arch -t "${install_cmd}" -i -r debian/control
diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index 26c6ad8..8438143 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -29,13 +29,15 @@ KERNEL_DEBIAN_DEPENDS ?= "initramfs-tools | linux-initramfs-tool, kmod, linux-ba
KERNEL_HEADERS_DEBIAN_DEPENDS ?= "libc6, libssl1.1"

dpkg_prepare() {

- # Lock-protected because apt and dpkg do not wait in case of contention

sudo -E chroot ${BUILDCHROOT_DIR} \
- flock /dpkg.lock apt-get install \
+ apt-get install \

-y -o Debug::pkgProblemResolver=yes \
--no-install-recommends ${KBUILD_DEPENDS}
}

+# apt and reprepro may not run in parallel, acquire the Isar lock
+do_prepare[lockfiles] += "${DEPLOY_DIR_APT}/isar.lock"
+
dpkg_runbuild() {

# Install package builder script

[Maxim Yu. Osipov]

On 08/26/2018 09:21 AM, Cedric Hombourger wrote:
> In preparation for a rework of locking mechanisms around apt
> operations (installation of packages or inclusion of new
> packages into the isar-apt database), move the code for
> installing build dependencies out of dpkg_runbuild and into
> a new function: dpkg_prepare.

v3 series was applied to the 'next'.

Thanks,
Maxim.

Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mos...@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov

[Jan Kiszka]

This patches causes regression with jailhouse-images which I do not
understand yet:

2018-08-31 07:53:54 - ERROR - ERROR: mc:espressobin-jailhouse:non-root-initramfs-2018.05.1-r0 do_prepare: Function failed: do_prepare (log file is located at /out/build/tmp/work/debian-stretch-arm64/non-root-initramfs-2018.05.1-r0/temp/log.do_prepare.28928)
2018-08-31 07:53:54 - ERROR - ERROR: Logfile of failure stored in: /out/build/tmp/work/debian-stretch-arm64/non-root-initramfs-2018.05.1-r0/temp/log.do_prepare.28928
2018-08-31 07:53:54 - INFO - Log data follows:
2018-08-31 07:53:54 - INFO - | DEBUG: Executing shell function do_prepare
2018-08-31 07:53:54 - INFO - | Get:1 file:/isar-apt isar InRelease
2018-08-31 07:53:54 - INFO - | Ign:1 file:/isar-apt isar InRelease
2018-08-31 07:53:54 - INFO - | Get:2 file:/isar-apt isar Release [3554 B]
2018-08-31 07:53:54 - INFO - | Get:2 file:/isar-apt isar Release [3554 B]
2018-08-31 07:53:54 - INFO - | Get:3 file:/isar-apt isar Release.gpg
2018-08-31 07:53:54 - INFO - | Ign:3 file:/isar-apt isar Release.gpg
2018-08-31 07:53:54 - INFO - | Reading package lists...
2018-08-31 07:53:54 - INFO - | E: You must put some 'source' URIs in your sources.list
2018-08-31 07:53:54 - INFO - | mk-build-deps: Unable to find package name in `apt-cache showsrc debian/control'
2018-08-31 07:53:54 - INFO - | WARNING: exit code 29 from a shell command.
2018-08-31 07:53:54 - INFO - | ERROR: Function failed: do_prepare (log file is located at /out/build/tmp/work/debian-stretch-arm64/non-root-initramfs-2018.05.1-r0/temp/log.do_prepare.28928)
2018-08-31 07:53:54 - INFO - NOTE: recipe non-root-initramfs-2018.05.1-r0: task do_prepare: Failed
2018-08-31 07:53:54 - ERROR - ERROR: Task (multiconfig:espressobin-jailhouse:/jailhouse-images/recipes-core/non-root-initramfs/non-root-initramfs_2018.05.1.bb:do_prepare) failed with exit code '1'


I also wonder why this didn't trigger with the isar core tests. Any idea?

[chomb...@gmail.com]

Hi Jan,

if your recipe is editing files in debian/, you would then want them to be done before we install build dependencies

this can be done by using a "do_prepare_prepend" to make the tweaks you need

maybe we could add a do_amend_debian_meta (defaulting to a no-op) to make it easy for people to tweak meta-data and not having to know about do_prepare?

Cedric

[Jan Kiszka]

On 2018-08-31 10:15, chomb...@gmail.com wrote:
> Hi Jan,
>
> if your recipe is editing files in debian/, you would then want them to
> be done before we install build dependencies
> this can be done by using a "do_prepare_prepend" to make the tweaks you need
>
> maybe we could add a do_amend_debian_meta (defaulting to a no-op) to
> make it easy for people to tweak meta-data and not having to know about
> do_prepare?

Ah, so every ad-hoc debianization is now a preparation task - that
should probably be stated somewhere more explicitly, it's a fairly
common pattern.

As I received internal complaints already: Now would be a good time to
start documenting recipe API changes in a central file. Then users can
quickly look up what they need to do in order to make their layers
compatible with the latest version.

Thanks,
Jan

>
> Cedric
>
> On Friday, August 31, 2018 at 10:10:45 AM UTC+2, Jan Kiszka wrote:
>
> On 2018-08-26 08:21, Cedric Hombourger wrote:
> > In preparation for a rework of locking mechanisms around apt
> > operations (installation of packages or inclusion of new
> > packages into the isar-apt database), move the code for
> > installing build dependencies out of dpkg_runbuild and into
> > a new function: dpkg_prepare.
> >
> > Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com

> <javascript:>>

> --
> You received this message because you are subscribed to the Google
> Groups "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to isar-users+...@googlegroups.com
> <mailto:isar-users+...@googlegroups.com>.
> To post to this group, send email to isar-...@googlegroups.com
> <mailto:isar-...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/isar-users/1bb4283f-ac5c-444c-8890-f36231f0e66c%40googlegroups.com
> <https://groups.google.com/d/msgid/isar-users/1bb4283f-ac5c-444c-8890-f36231f0e66c%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

[Jan Kiszka]

Re-reading the patch with my new knowledge:

On 2018-08-26 08:21, Cedric Hombourger wrote:

> diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
> index 3075f44..3a3cab1 100644
> --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -17,7 +17,7 @@ AUTOLOAD ?= "0"
>
> inherit dpkg
>
> -dpkg_runbuild_prepend() {
> +do_prepare_prepend() {

Shouldn't that be dpgk_prepare_append()?

> cp -r ${WORKDIR}/debian ${S}/
> sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
> -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
>

[chomb...@gmail.com]

On Friday, August 31, 2018 at 10:28:51 AM UTC+2, Jan Kiszka wrote:

Re-reading the patch with my new knowledge:

On 2018-08-26 08:21, Cedric Hombourger wrote:
> diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
> index 3075f44..3a3cab1 100644
> --- a/meta/recipes-kernel/linux-module/module.inc
> +++ b/meta/recipes-kernel/linux-module/module.inc
> @@ -17,7 +17,7 @@ AUTOLOAD ?= "0"
>  
>   inherit dpkg
>  
> -dpkg_runbuild_prepend() {
> +do_prepare_prepend() {

Shouldn't that be dpgk_prepare_append()?

Wouldn't it be too late?

I am afraid that debian/control wouldn't be complete and prevent installation of build dependencies

 

>       cp -r ${WORKDIR}/debian ${S}/
>       sed -i -e 's/@PN@/${PN}/g' -e 's/@PV@/${PV}/g' \
>              -e 's/@KERNEL_NAME@/${KERNEL_NAME}/g' \
>

Jan

--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

[Jan Kiszka]

On 2018-08-31 10:33, chomb...@gmail.com wrote:
>
>
> On Friday, August 31, 2018 at 10:28:51 AM UTC+2, Jan Kiszka wrote:
>
> Re-reading the patch with my new knowledge:
>
> On 2018-08-26 08:21, Cedric Hombourger wrote:
> > diff --git a/meta/recipes-kernel/linux-module/module.inc
> b/meta/recipes-kernel/linux-module/module.inc
> > index 3075f44..3a3cab1 100644
> > --- a/meta/recipes-kernel/linux-module/module.inc
> > +++ b/meta/recipes-kernel/linux-module/module.inc
> > @@ -17,7 +17,7 @@ AUTOLOAD ?= "0"
> >
> >   inherit dpkg
> >
> > -dpkg_runbuild_prepend() {
> > +do_prepare_prepend() {
>
> Shouldn't that be dpgk_prepare_append()?
>
>
> Wouldn't it be too late?
> I am afraid that debian/control wouldn't be complete and prevent
> installation of build dependencies
>

Right, we need that for the installation...

I was wondering, though, if it wouldn't be cleaner to hook additions to
the inner handler.

Anyway, all this is not mature yet:

- debianization does not require locking, only the dependency
installation does

- I'm quite sure something is broken now with dpkg-raw, because there is
no lock involved, but we can pull packages that way as well

- we should cleanly name the steps, as you suggested, so that users know
what to overwrite / append /prepend

[Jan Kiszka]

Yep, confirmed: We so far installed the dependencies of both regular and
raw packages in the build script under the lock. Now you pulled that
installation into deps.sh, but that script is not executed by dpkg-raw,
nor is there any lock on the related prepare step.

Can we fix this quickly? Or better revert and redo the series (given the
other issues)?

Jan

[Jan Kiszka]

I have to correct myself: dpkg-raw packages only have runtime
dependencies, no Build-Depends. Thus they never installed anything into
the buildchroot and also never used the build script - we are fine in
this regard.

I'll write a few patches to address the other issues.

Jan

[Cedric Hombourger]

Hi Jan,

Please let me know if there is anything that I can help you with

I was going to review existing documentation and propose updates around recipe creation / APIs

Cedric

[Jan Kiszka]

Hi Cedric,

On 2018-09-01 10:44, Cedric Hombourger wrote:
> Hi Jan,
>
> Please let me know if there is anything that I can help you with
> I was going to review existing documentation and propose updates around
> recipe creation / APIs
>

Thanks a lot, I would specifically appreciate your review. I've written
some changes already and also started a RECIPE-API-CHANGELOG.md from the
last release onward. Series is currently in CI, I will send later.

Jan

>> On Sep 1, 2018, at 8:58 AM, Jan Kiszka <jan.k...@web.de

>> <mailto:jan.k...@web.de>> wrote:
>>
>> On 2018-08-31 11:05, [ext] Jan Kiszka wrote:
>>> On 2018-08-31 10:41, Jan Kiszka wrote:
>>>> On 2018-08-31 10:33, chomb...@gmail.com