[PATCH] dpkg: acquire a read (shared) locks while building packages

[PATCH] dpkg: acquire a read (shared) locks while building packages

[Cedric Hombourger]

As debhelper scripts involved during the actual build process
(dpkg-buildpackage) may access the dpkg database, we need to
make sure it does not get modified in the background by other
build tasks for other packages (such as install_build_deps).
An example of a reader is dh_shlibdeps/dpkg-shlibdeps: it
needs to find packages providing the shared libraries the
package depends on to substitue shlibs variables found in its
control file with package dependencies. Acquire the isar-apt
lock as a shared lock to block writers.

Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
---
 meta/classes/dpkg.bbclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 24b9fe3..f4e3d7a 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
 # Build package from sources using build script
 dpkg_runbuild() {
     E="${@ bb.utils.export_proxies(d)}"
+    flock -s "${REPO_ISAR_DIR}/isar.lock" \
     sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
 }
-- 
2.11.0

Re: [PATCH] dpkg: acquire a read (shared) locks while building packages

[Jan Kiszka]

On 08.03.19 10:57, Cedric Hombourger wrote:
> As debhelper scripts involved during the actual build process
> (dpkg-buildpackage) may access the dpkg database, we need to
> make sure it does not get modified in the background by other
> build tasks for other packages (such as install_build_deps).
> An example of a reader is dh_shlibdeps/dpkg-shlibdeps: it
> needs to find packages providing the shared libraries the
> package depends on to substitue shlibs variables found in its

substitute

> control file with package dependencies. Acquire the isar-apt
> lock as a shared lock to block writers.

Ah... Understood now.

> 
> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> ---
>   meta/classes/dpkg.bbclass | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 24b9fe3..f4e3d7a 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>   # Build package from sources using build script
>   dpkg_runbuild() {
>       E="${@ bb.utils.export_proxies(d)}"
> +    flock -s "${REPO_ISAR_DIR}/isar.lock" \
>       sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
>   }
> 

Couldn't we have the same issue with dpkg-base derived classes? The kernel? Or 
downstream classes? IOW, shouldn't this go to the caller of dpkg_runbuild?

Jan

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

Re: [PATCH] dpkg: acquire a read (shared) locks while building packages

[cedric_hombourger]

[-- Attachment #1.1: Type: text/plain, Size: 1062 bytes --]



On Friday, March 8, 2019 at 11:04:12 AM UTC+1, Jan Kiszka wrote:
>
> > diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass 
> > index 24b9fe3..f4e3d7a 100644 
> > --- a/meta/classes/dpkg.bbclass 
> > +++ b/meta/classes/dpkg.bbclass 
> > @@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = 
> "${DISTRO}-${DISTRO_ARCH}" 
> >   # Build package from sources using build script 
> >   dpkg_runbuild() { 
> >       E="${@ bb.utils.export_proxies(d)}" 
> > +    flock -s "${REPO_ISAR_DIR}/isar.lock" \ 
> >       sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} 
> /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH} 
> >   } 
> > 
>
> Couldn't we have the same issue with dpkg-base derived classes? The 
> kernel? Or 
> downstream classes? IOW, shouldn't this go to the caller of dpkg_runbuild?


I do understand the intent but I don't think we can do something like:
flock -s <lock-file> dpkg_runbuild

is there another flock construct we could use to get around this?

I can update the patch to correct the typo in the commit log
 

[-- Attachment #1.2: Type: text/html, Size: 1427 bytes --]

Re: [PATCH] dpkg: acquire a read (shared) locks while building packages

[Jan Kiszka]

On 08.03.19 14:33, cedric_hombourger@mentor.com wrote:
> 
> On Friday, March 8, 2019 at 11:04:12 AM UTC+1, Jan Kiszka wrote:
> 
>      > diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
>      > index 24b9fe3..f4e3d7a 100644
>      > --- a/meta/classes/dpkg.bbclass
>      > +++ b/meta/classes/dpkg.bbclass
>      > @@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] =
>     "${DISTRO}-${DISTRO_ARCH}"
>      >   # Build package from sources using build script
>      >   dpkg_runbuild() {
>      >       E="${@ bb.utils.export_proxies(d)}"
>      > +    flock -s "${REPO_ISAR_DIR}/isar.lock" \
>      >       sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR}
>     /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
>      >   }
>      >
> 
>     Couldn't we have the same issue with dpkg-base derived classes? The kernel? Or
>     downstream classes? IOW, shouldn't this go to the caller of dpkg_runbuild?
> 
> 
> I do understand the intent but I don't think we can do something like:
> flock -s <lock-file> dpkg_runbuild
> 
> is there another flock construct we could use to get around this?

Likely true. We would need a construct that bitbake can parse while unrolling 
this function - not aware of any.

> 
> I can update the patch to correct the typo in the commit log
> 

Then let's also patch meta/recipes-kernel/linux/linux-custom.inc analogously and 
leave external dpkg_runbuild to downstream.

Thanks,
Jan

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

Re: [PATCH] dpkg: acquire a read (shared) locks while building packages

[cedric_hombourger]

[-- Attachment #1.1: Type: text/plain, Size: 823 bytes --]


>
> Then let's also patch meta/recipes-kernel/linux/linux-custom.inc 
> analogously and 
> leave external dpkg_runbuild to downstream. 
>

it should be noted that "make dpkg-deb" does not use debhelper (as I 
recently found while modifying the kernel builddep script to also package 
perf and
wondering why my shlibs variables were not parsed)

So, to the best of my knowledge, the current (mainline) package scripts 
from the kernel will not access the dpkg database. so are we wanting to be
extra safe and grab a shared lock anyway (which will block other bitbake 
threads from running do_install_deps or do_deploy_deb while the kernel
builds since bitbake grabs an exclusive lock for these tasks) OR use this 
"knowledge" to keep some parallelism.

I am fine either way - just wanted to expose both options :)

Cedric 

[-- Attachment #1.2: Type: text/html, Size: 1121 bytes --]

Re: [PATCH] dpkg: acquire a read (shared) locks while building packages

[Jan Kiszka]

On 08.03.19 16:24, cedric_hombourger@mentor.com wrote:
>     Then let's also patch meta/recipes-kernel/linux/linux-custom.inc analogously
>     and
>     leave external dpkg_runbuild to downstream.
> 
> 
> it should be noted that "make dpkg-deb" does not use debhelper (as I recently 
> found while modifying the kernel builddep script to also package perf and
> wondering why my shlibs variables were not parsed)
> 
> So, to the best of my knowledge, the current (mainline) package scripts from the 
> kernel will not access the dpkg database. so are we wanting to be
> extra safe and grab a shared lock anyway (which will block other bitbake threads 
> from running do_install_deps or do_deploy_deb while the kernel
> builds since bitbake grabs an exclusive lock for these tasks) OR use this 
> "knowledge" to keep some parallelism.
> 
> I am fine either way - just wanted to expose both options :)

Indeed, then we are safe at this point. If you sent v2, please add this reason 
to the changelog. Hopefully, the person will read that who adds kernel tool 
packaging via debhelpers one day. ;)

Thanks,
Jan

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

[PATCH v2] dpkg: acquire a read (shared) locks while building packages

[Cedric Hombourger]

As debhelper scripts involved during the actual build process
(dpkg-buildpackage) may access the dpkg database, we need to
make sure it does not get modified in the background by other
build tasks for other packages (such as install_build_deps).
An example of a reader is dh_shlibdeps/dpkg-shlibdeps: it
needs to find packages providing the shared libraries the
package depends on to substitute shlibs variables found in its
control file with package dependencies. Acquire the isar-apt
lock as a shared lock to block writers.

Special care may be needed in other classes/users of the
dpkg_runbuild API. Due to some limitations with bitbake (to be
discussed with the maintainers), we could not grab the lock
from dpkg-base. A statement such as "flock <my-lock.lock>
dpkg_run_build" would fail as flock can't exec() dpkg_run_build
(since it is a shell function, not an executable). The popular
shell construct "( flock -n 9; dpkg_run_build ) 9> <my-lock.lock>"
cannot be parsed by bitbake. And finally the lockfiles task
attribute (implemented in bitbake) only handles exclusive locks.

This commit is only addressing dpkg.bbclass. dpkg-raw.bbclass
is left intact as the generated control file does require
dh_shlibdeps. linux-custom.inc was also left out as the Debian
packaging scripts only use low-level packaging tools and do not
use debhelper scripts during packaging (at this time). In
agreement with the Isar project maintainers, we will not grab
the Isar lock from linux-custom.inc to let kernel builds run
in parallel with do_install_deps() and do_deploy_deb() that
may be executed by other bitbake threads.

Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
---
 meta/classes/dpkg.bbclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 24b9fe3..f4e3d7a 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
 # Build package from sources using build script
 dpkg_runbuild() {
     E="${@ bb.utils.export_proxies(d)}"
+    flock -s "${REPO_ISAR_DIR}/isar.lock" \
     sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
 }
-- 
2.11.0

Re: [PATCH v2] dpkg: acquire a read (shared) locks while building packages

[Jan Kiszka]

On 08.03.19 16:49, Cedric Hombourger wrote:
> As debhelper scripts involved during the actual build process
> (dpkg-buildpackage) may access the dpkg database, we need to
> make sure it does not get modified in the background by other
> build tasks for other packages (such as install_build_deps).
> An example of a reader is dh_shlibdeps/dpkg-shlibdeps: it
> needs to find packages providing the shared libraries the
> package depends on to substitute shlibs variables found in its
> control file with package dependencies. Acquire the isar-apt
> lock as a shared lock to block writers.
> 
> Special care may be needed in other classes/users of the
> dpkg_runbuild API. Due to some limitations with bitbake (to be
> discussed with the maintainers), we could not grab the lock
> from dpkg-base. A statement such as "flock <my-lock.lock>
> dpkg_run_build" would fail as flock can't exec() dpkg_run_build
> (since it is a shell function, not an executable). The popular
> shell construct "( flock -n 9; dpkg_run_build ) 9> <my-lock.lock>"
> cannot be parsed by bitbake. And finally the lockfiles task
> attribute (implemented in bitbake) only handles exclusive locks.
> 
> This commit is only addressing dpkg.bbclass. dpkg-raw.bbclass
> is left intact as the generated control file does require

( Actually, dpkg-raw just uses dpkg these days, so no work to be done anyway. )

> dh_shlibdeps. linux-custom.inc was also left out as the Debian
> packaging scripts only use low-level packaging tools and do not
> use debhelper scripts during packaging (at this time). In
> agreement with the Isar project maintainers, we will not grab
> the Isar lock from linux-custom.inc to let kernel builds run
> in parallel with do_install_deps() and do_deploy_deb() that
> may be executed by other bitbake threads.
> 
> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> ---
>   meta/classes/dpkg.bbclass | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 24b9fe3..f4e3d7a 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>   # Build package from sources using build script
>   dpkg_runbuild() {
>       E="${@ bb.utils.export_proxies(d)}"
> +    flock -s "${REPO_ISAR_DIR}/isar.lock" \
>       sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
>   }
> 

Looks good to me.

Jan

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

Re: [PATCH v2] dpkg: acquire a read (shared) locks while building packages

[cedric_hombourger]

[-- Attachment #1.1: Type: text/plain, Size: 3024 bytes --]

observed another failure in our CI environment because of this missing lock
any objections to getting this applied to next given that it was acked by 
Jan?

On Friday, March 8, 2019 at 5:38:09 PM UTC+1, Jan Kiszka wrote:
>
> On 08.03.19 16:49, Cedric Hombourger wrote: 
> > As debhelper scripts involved during the actual build process 
> > (dpkg-buildpackage) may access the dpkg database, we need to 
> > make sure it does not get modified in the background by other 
> > build tasks for other packages (such as install_build_deps). 
> > An example of a reader is dh_shlibdeps/dpkg-shlibdeps: it 
> > needs to find packages providing the shared libraries the 
> > package depends on to substitute shlibs variables found in its 
> > control file with package dependencies. Acquire the isar-apt 
> > lock as a shared lock to block writers. 
> > 
> > Special care may be needed in other classes/users of the 
> > dpkg_runbuild API. Due to some limitations with bitbake (to be 
> > discussed with the maintainers), we could not grab the lock 
> > from dpkg-base. A statement such as "flock <my-lock.lock> 
> > dpkg_run_build" would fail as flock can't exec() dpkg_run_build 
> > (since it is a shell function, not an executable). The popular 
> > shell construct "( flock -n 9; dpkg_run_build ) 9> <my-lock.lock>" 
> > cannot be parsed by bitbake. And finally the lockfiles task 
> > attribute (implemented in bitbake) only handles exclusive locks. 
> > 
> > This commit is only addressing dpkg.bbclass. dpkg-raw.bbclass 
> > is left intact as the generated control file does require 
>
> ( Actually, dpkg-raw just uses dpkg these days, so no work to be done 
> anyway. ) 
>
> > dh_shlibdeps. linux-custom.inc was also left out as the Debian 
> > packaging scripts only use low-level packaging tools and do not 
> > use debhelper scripts during packaging (at this time). In 
> > agreement with the Isar project maintainers, we will not grab 
> > the Isar lock from linux-custom.inc to let kernel builds run 
> > in parallel with do_install_deps() and do_deploy_deb() that 
> > may be executed by other bitbake threads. 
> > 
> > Signed-off-by: Cedric Hombourger <Cedric_H...@mentor.com <javascript:>> 
> > --- 
> >   meta/classes/dpkg.bbclass | 1 + 
> >   1 file changed, 1 insertion(+) 
> > 
> > diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass 
> > index 24b9fe3..f4e3d7a 100644 
> > --- a/meta/classes/dpkg.bbclass 
> > +++ b/meta/classes/dpkg.bbclass 
> > @@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = 
> "${DISTRO}-${DISTRO_ARCH}" 
> >   # Build package from sources using build script 
> >   dpkg_runbuild() { 
> >       E="${@ bb.utils.export_proxies(d)}" 
> > +    flock -s "${REPO_ISAR_DIR}/isar.lock" \ 
> >       sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} 
> /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH} 
> >   } 
> > 
>
> Looks good to me. 
>
> Jan 
>
> -- 
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE 
> Corporate Competence Center Embedded Linux 
>

[-- Attachment #1.2: Type: text/html, Size: 3737 bytes --]

Re: [PATCH v2] dpkg: acquire a read (shared) locks while building packages

[Maxim Yu. Osipov]

On 3/8/19 4:49 PM, Cedric Hombourger wrote:
> As debhelper scripts involved during the actual build process
> (dpkg-buildpackage) may access the dpkg database, we need to
> make sure it does not get modified in the background by other
> build tasks for other packages (such as install_build_deps).
> An example of a reader is dh_shlibdeps/dpkg-shlibdeps: it
> needs to find packages providing the shared libraries the
> package depends on to substitute shlibs variables found in its
> control file with package dependencies. Acquire the isar-apt
> lock as a shared lock to block writers.
> 
> Special care may be needed in other classes/users of the
> dpkg_runbuild API. Due to some limitations with bitbake (to be
> discussed with the maintainers), we could not grab the lock
> from dpkg-base. A statement such as "flock <my-lock.lock>
> dpkg_run_build" would fail as flock can't exec() dpkg_run_build
> (since it is a shell function, not an executable). The popular
> shell construct "( flock -n 9; dpkg_run_build ) 9> <my-lock.lock>"
> cannot be parsed by bitbake. And finally the lockfiles task
> attribute (implemented in bitbake) only handles exclusive locks.
> 
> This commit is only addressing dpkg.bbclass. dpkg-raw.bbclass
> is left intact as the generated control file does require
> dh_shlibdeps. linux-custom.inc was also left out as the Debian
> packaging scripts only use low-level packaging tools and do not
> use debhelper scripts during packaging (at this time). In
> agreement with the Isar project maintainers, we will not grab
> the Isar lock from linux-custom.inc to let kernel builds run
> in parallel with do_install_deps() and do_deploy_deb() that
> may be executed by other bitbake threads.

Applied to the 'next'.

Thanks,
Maxim.

> Signed-off-by: Cedric Hombourger <Cedric_Hombourger@mentor.com>
> ---
>   meta/classes/dpkg.bbclass | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 24b9fe3..f4e3d7a 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -19,5 +19,6 @@ do_install_builddeps[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>   # Build package from sources using build script
>   dpkg_runbuild() {
>       E="${@ bb.utils.export_proxies(d)}"
> +    flock -s "${REPO_ISAR_DIR}/isar.lock" \
>       sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} /isar/build.sh ${PP}/${PPS} ${DISTRO_ARCH}
>   }
> 


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