public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
* [PATCH] dpkg: Make mount buildroot reliable
@ 2021-04-02 13:03 Anton Mikanovich
  2021-04-08  6:27 ` Jan Kiszka
  0 siblings, 1 reply; 3+ messages in thread
From: Anton Mikanovich @ 2021-04-02 13:03 UTC (permalink / raw)
  To: isar-users; +Cc: Anton Mikanovich

We use mounting for several tasks. Mounting and unmounting is performed
in shell scripts. If a command fails before unmounting, the directories
remain mounted.

Implement exception handling around the scripts to ensure that
unmounting is performed also in case of script failure. The number of
unmounting attempts has been limited to avoid infinite loops.

Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
 meta/classes/dpkg-base.bbclass | 93 +++++++++++++++++++---------------
 meta/classes/dpkg-gbp.bbclass  |  8 +--
 meta/classes/dpkg.bbclass      | 14 +++--
 3 files changed, 64 insertions(+), 51 deletions(-)

diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
index 5c7bddc..544ee36 100644
--- a/meta/classes/dpkg-base.bbclass
+++ b/meta/classes/dpkg-base.bbclass
@@ -54,13 +54,7 @@ addtask patch before do_adjust_git
 
 SRC_APT ?= ""
 
-do_apt_fetch() {
-    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
-        return 0
-    fi
-    rm -rf ${S}
-    dpkg_do_mounts
-    E="${@ isar_export_proxies(d)}"
+fetch_apt() {
     sudo -E chroot ${BUILDCHROOT_DIR} /usr/bin/apt-get update \
         -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
         -o Dir::Etc::SourceParts="-" \
@@ -72,8 +66,27 @@ do_apt_fetch() {
         sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} \
             sh -c 'cp /downloads/deb-src/"$1"/"$2"/* ${PP} && cd ${PP} && apt-get -y --only-source source "$2"' my_script "${DISTRO}" "${uri}"
     done
+}
+
+python do_apt_fetch() {
+    import glob
+    import shutil
+
+    src_apt = d.getVar("SRC_APT", True)
+    if not src_apt:
+        return 0
 
-    dpkg_undo_mounts
+    srcsubdir = d.getVar('S', True)
+    if os.path.exists(srcsubdir):
+        shutil.rmtree(srcsubdir)
+
+    dpkg_do_mounts(d)
+    try:
+        isar_export_proxies(d)
+        buildchroot = d.getVar('BUILDCHROOT_DIR', True)
+        bb.build.exec_func("fetch_apt", d)
+    finally:
+        dpkg_undo_mounts(d)
 }
 
 addtask apt_fetch after do_unpack before do_patch
@@ -118,25 +131,23 @@ do_prepare_build[deptask] = "do_deploy_deb"
 
 BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
 
-dpkg_do_mounts() {
-    mkdir -p ${BUILDROOT}
-    sudo mount --bind ${WORKDIR} ${BUILDROOT}
-
-    buildchroot_do_mounts
-}
-
-dpkg_undo_mounts() {
-    i=1
-    while ! sudo umount ${BUILDROOT}; do
-        sleep 0.1
-        i=`expr $i + 1`
-        if [ $i -gt 100 ]; then
-            bbwarn "${BUILDROOT}: Couldn't unmount, retrying..."
-            i=1
-        fi
-    done
-    sudo rmdir ${BUILDROOT}
-}
+def dpkg_do_mounts(d):
+    buildroot = d.getVar('BUILDROOT', True)
+    workdir = d.getVar("WORKDIR", True)
+    os.makedirs(buildroot, exist_ok=True)
+    os.system('sudo mount --bind %s %s' % (workdir, buildroot))
+    bb.build.exec_func("buildchroot_do_mounts", d)
+
+def dpkg_undo_mounts(d):
+    buildroot = d.getVar('BUILDROOT', True)
+    for i in range(200):
+        if not os.system('sudo umount %s' % buildroot):
+            os.rmdir(buildroot)
+            return
+        if i % 100 == 0:
+            bb.warn("%s: Couldn't unmount, retrying..." % buildroot)
+        time.sleep(0.1)
+    bb.fatal("Couldn't unmount, exiting...")
 
 # Placeholder for actual dpkg_runbuild() implementation
 dpkg_runbuild() {
@@ -146,10 +157,12 @@ dpkg_runbuild() {
 python do_dpkg_build() {
     lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
                              shared=True)
-    bb.build.exec_func("dpkg_do_mounts", d)
-    bb.build.exec_func("dpkg_runbuild", d)
-    bb.build.exec_func("dpkg_undo_mounts", d)
-    bb.utils.unlockfile(lock)
+    dpkg_do_mounts(d)
+    try:
+        bb.build.exec_func("dpkg_runbuild", d)
+    finally:
+        dpkg_undo_mounts(d)
+        bb.utils.unlockfile(lock)
 }
 
 addtask dpkg_build before do_build
@@ -193,16 +206,16 @@ python do_devshell() {
     oe_lib_path = os.path.join(d.getVar('LAYERDIR_core'), 'lib')
     sys.path.insert(0, oe_lib_path)
 
-    bb.build.exec_func('dpkg_do_mounts', d)
-
-    isar_export_proxies(d)
-
-    buildchroot = d.getVar('BUILDCHROOT_DIR')
-    pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS'))
-    termcmd = "sudo -E chroot {0} sh -c 'cd {1}; $SHELL -i'"
-    oe_terminal(termcmd.format(buildchroot, pp_pps), "Isar devshell", d)
+    dpkg_do_mounts(d)
+    try:
+        isar_export_proxies(d)
 
-    bb.build.exec_func('dpkg_undo_mounts', d)
+        buildchroot = d.getVar('BUILDCHROOT_DIR')
+        pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS'))
+        termcmd = "sudo -E chroot {0} sh -c 'cd {1}; $SHELL -i'"
+        oe_terminal(termcmd.format(buildchroot, pp_pps), "Isar devshell", d)
+    finally:
+        dpkg_undo_mounts(d)
 }
 
 addtask devshell after do_prepare_build
diff --git a/meta/classes/dpkg-gbp.bbclass b/meta/classes/dpkg-gbp.bbclass
index d956e8c..20d2d4c 100644
--- a/meta/classes/dpkg-gbp.bbclass
+++ b/meta/classes/dpkg-gbp.bbclass
@@ -12,12 +12,7 @@ PATCHTOOL ?= "git"
 GBP_DEPENDS ?= "git-buildpackage pristine-tar"
 GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
 
-do_install_builddeps_append() {
-    dpkg_do_mounts
-    distro="${DISTRO}"
-    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
-       distro="${HOST_DISTRO}"
-    fi
+builddeps_install_append() {
     deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
@@ -26,7 +21,6 @@ do_install_builddeps_append() {
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
                         --no-install-recommends ${GBP_DEPENDS}
-    dpkg_undo_mounts
 }
 
 dpkg_runbuild_prepend() {
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 4e7c2f7..29e2b89 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -6,9 +6,7 @@ inherit dpkg-base
 PACKAGE_ARCH ?= "${DISTRO_ARCH}"
 
 # Install build dependencies for package
-do_install_builddeps() {
-    dpkg_do_mounts
-    E="${@ isar_export_proxies(d)}"
+builddeps_install() {
     distro="${DISTRO}"
     if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
        distro="${HOST_DISTRO}"
@@ -19,7 +17,15 @@ do_install_builddeps() {
     deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
         ${PP}/${PPS} ${PACKAGE_ARCH}
-    dpkg_undo_mounts
+}
+
+python do_install_builddeps() {
+    dpkg_do_mounts(d)
+    isar_export_proxies(d)
+    try:
+        bb.build.exec_func("builddeps_install", d)
+    finally:
+        dpkg_undo_mounts(d)
 }
 
 addtask install_builddeps after do_prepare_build before do_dpkg_build
-- 
2.20.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] dpkg: Make mount buildroot reliable
  2021-04-02 13:03 [PATCH] dpkg: Make mount buildroot reliable Anton Mikanovich
@ 2021-04-08  6:27 ` Jan Kiszka
  2021-04-08  8:13   ` Baurzhan Ismagulov
  0 siblings, 1 reply; 3+ messages in thread
From: Jan Kiszka @ 2021-04-08  6:27 UTC (permalink / raw)
  To: Anton Mikanovich, isar-users

On 02.04.21 15:03, Anton Mikanovich wrote:
> We use mounting for several tasks. Mounting and unmounting is performed
> in shell scripts. If a command fails before unmounting, the directories
> remain mounted.

Right - but what is the problem to be solved? We are cleaning up in
build_completed() whatever is left by individual jobs.

I'm not against this refactoring, but the purpose should be made clearer.

Jan

> 
> Implement exception handling around the scripts to ensure that
> unmounting is performed also in case of script failure. The number of
> unmounting attempts has been limited to avoid infinite loops.
> 
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
>  meta/classes/dpkg-base.bbclass | 93 +++++++++++++++++++---------------
>  meta/classes/dpkg-gbp.bbclass  |  8 +--
>  meta/classes/dpkg.bbclass      | 14 +++--
>  3 files changed, 64 insertions(+), 51 deletions(-)
> 
> diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass
> index 5c7bddc..544ee36 100644
> --- a/meta/classes/dpkg-base.bbclass
> +++ b/meta/classes/dpkg-base.bbclass
> @@ -54,13 +54,7 @@ addtask patch before do_adjust_git
>  
>  SRC_APT ?= ""
>  
> -do_apt_fetch() {
> -    if [ -z "${@d.getVar("SRC_APT", True).strip()}" ]; then
> -        return 0
> -    fi
> -    rm -rf ${S}
> -    dpkg_do_mounts
> -    E="${@ isar_export_proxies(d)}"
> +fetch_apt() {
>      sudo -E chroot ${BUILDCHROOT_DIR} /usr/bin/apt-get update \
>          -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
>          -o Dir::Etc::SourceParts="-" \
> @@ -72,8 +66,27 @@ do_apt_fetch() {
>          sudo -E chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} \
>              sh -c 'cp /downloads/deb-src/"$1"/"$2"/* ${PP} && cd ${PP} && apt-get -y --only-source source "$2"' my_script "${DISTRO}" "${uri}"
>      done
> +}
> +
> +python do_apt_fetch() {
> +    import glob
> +    import shutil
> +
> +    src_apt = d.getVar("SRC_APT", True)
> +    if not src_apt:
> +        return 0
>  
> -    dpkg_undo_mounts
> +    srcsubdir = d.getVar('S', True)
> +    if os.path.exists(srcsubdir):
> +        shutil.rmtree(srcsubdir)
> +
> +    dpkg_do_mounts(d)
> +    try:
> +        isar_export_proxies(d)
> +        buildchroot = d.getVar('BUILDCHROOT_DIR', True)
> +        bb.build.exec_func("fetch_apt", d)
> +    finally:
> +        dpkg_undo_mounts(d)
>  }
>  
>  addtask apt_fetch after do_unpack before do_patch
> @@ -118,25 +131,23 @@ do_prepare_build[deptask] = "do_deploy_deb"
>  
>  BUILDROOT = "${BUILDCHROOT_DIR}/${PP}"
>  
> -dpkg_do_mounts() {
> -    mkdir -p ${BUILDROOT}
> -    sudo mount --bind ${WORKDIR} ${BUILDROOT}
> -
> -    buildchroot_do_mounts
> -}
> -
> -dpkg_undo_mounts() {
> -    i=1
> -    while ! sudo umount ${BUILDROOT}; do
> -        sleep 0.1
> -        i=`expr $i + 1`
> -        if [ $i -gt 100 ]; then
> -            bbwarn "${BUILDROOT}: Couldn't unmount, retrying..."
> -            i=1
> -        fi
> -    done
> -    sudo rmdir ${BUILDROOT}
> -}
> +def dpkg_do_mounts(d):
> +    buildroot = d.getVar('BUILDROOT', True)
> +    workdir = d.getVar("WORKDIR", True)
> +    os.makedirs(buildroot, exist_ok=True)
> +    os.system('sudo mount --bind %s %s' % (workdir, buildroot))
> +    bb.build.exec_func("buildchroot_do_mounts", d)
> +
> +def dpkg_undo_mounts(d):
> +    buildroot = d.getVar('BUILDROOT', True)
> +    for i in range(200):
> +        if not os.system('sudo umount %s' % buildroot):
> +            os.rmdir(buildroot)
> +            return
> +        if i % 100 == 0:
> +            bb.warn("%s: Couldn't unmount, retrying..." % buildroot)
> +        time.sleep(0.1)
> +    bb.fatal("Couldn't unmount, exiting...")
>  
>  # Placeholder for actual dpkg_runbuild() implementation
>  dpkg_runbuild() {
> @@ -146,10 +157,12 @@ dpkg_runbuild() {
>  python do_dpkg_build() {
>      lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock",
>                               shared=True)
> -    bb.build.exec_func("dpkg_do_mounts", d)
> -    bb.build.exec_func("dpkg_runbuild", d)
> -    bb.build.exec_func("dpkg_undo_mounts", d)
> -    bb.utils.unlockfile(lock)
> +    dpkg_do_mounts(d)
> +    try:
> +        bb.build.exec_func("dpkg_runbuild", d)
> +    finally:
> +        dpkg_undo_mounts(d)
> +        bb.utils.unlockfile(lock)
>  }
>  
>  addtask dpkg_build before do_build
> @@ -193,16 +206,16 @@ python do_devshell() {
>      oe_lib_path = os.path.join(d.getVar('LAYERDIR_core'), 'lib')
>      sys.path.insert(0, oe_lib_path)
>  
> -    bb.build.exec_func('dpkg_do_mounts', d)
> -
> -    isar_export_proxies(d)
> -
> -    buildchroot = d.getVar('BUILDCHROOT_DIR')
> -    pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS'))
> -    termcmd = "sudo -E chroot {0} sh -c 'cd {1}; $SHELL -i'"
> -    oe_terminal(termcmd.format(buildchroot, pp_pps), "Isar devshell", d)
> +    dpkg_do_mounts(d)
> +    try:
> +        isar_export_proxies(d)
>  
> -    bb.build.exec_func('dpkg_undo_mounts', d)
> +        buildchroot = d.getVar('BUILDCHROOT_DIR')
> +        pp_pps = os.path.join(d.getVar('PP'), d.getVar('PPS'))
> +        termcmd = "sudo -E chroot {0} sh -c 'cd {1}; $SHELL -i'"
> +        oe_terminal(termcmd.format(buildchroot, pp_pps), "Isar devshell", d)
> +    finally:
> +        dpkg_undo_mounts(d)
>  }
>  
>  addtask devshell after do_prepare_build
> diff --git a/meta/classes/dpkg-gbp.bbclass b/meta/classes/dpkg-gbp.bbclass
> index d956e8c..20d2d4c 100644
> --- a/meta/classes/dpkg-gbp.bbclass
> +++ b/meta/classes/dpkg-gbp.bbclass
> @@ -12,12 +12,7 @@ PATCHTOOL ?= "git"
>  GBP_DEPENDS ?= "git-buildpackage pristine-tar"
>  GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
>  
> -do_install_builddeps_append() {
> -    dpkg_do_mounts
> -    distro="${DISTRO}"
> -    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> -       distro="${HOST_DISTRO}"
> -    fi
> +builddeps_install_append() {
>      deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} \
>          apt-get install -y -o Debug::pkgProblemResolver=yes \
> @@ -26,7 +21,6 @@ do_install_builddeps_append() {
>      sudo -E chroot ${BUILDCHROOT_DIR} \
>          apt-get install -y -o Debug::pkgProblemResolver=yes \
>                          --no-install-recommends ${GBP_DEPENDS}
> -    dpkg_undo_mounts
>  }
>  
>  dpkg_runbuild_prepend() {
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 4e7c2f7..29e2b89 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -6,9 +6,7 @@ inherit dpkg-base
>  PACKAGE_ARCH ?= "${DISTRO_ARCH}"
>  
>  # Install build dependencies for package
> -do_install_builddeps() {
> -    dpkg_do_mounts
> -    E="${@ isar_export_proxies(d)}"
> +builddeps_install() {
>      distro="${DISTRO}"
>      if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
>         distro="${HOST_DISTRO}"
> @@ -19,7 +17,15 @@ do_install_builddeps() {
>      deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
>          ${PP}/${PPS} ${PACKAGE_ARCH}
> -    dpkg_undo_mounts
> +}
> +
> +python do_install_builddeps() {
> +    dpkg_do_mounts(d)
> +    isar_export_proxies(d)
> +    try:
> +        bb.build.exec_func("builddeps_install", d)
> +    finally:
> +        dpkg_undo_mounts(d)
>  }
>  
>  addtask install_builddeps after do_prepare_build before do_dpkg_build
> 

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] dpkg: Make mount buildroot reliable
  2021-04-08  6:27 ` Jan Kiszka
@ 2021-04-08  8:13   ` Baurzhan Ismagulov
  0 siblings, 0 replies; 3+ messages in thread
From: Baurzhan Ismagulov @ 2021-04-08  8:13 UTC (permalink / raw)
  To: isar-users

On Thu, Apr 08, 2021 at 08:27:29AM +0200, Jan Kiszka wrote:
> On 02.04.21 15:03, Anton Mikanovich wrote:
> > We use mounting for several tasks. Mounting and unmounting is performed
> > in shell scripts. If a command fails before unmounting, the directories
> > remain mounted.
> 
> Right - but what is the problem to be solved? We are cleaning up in
> build_completed() whatever is left by individual jobs.
> 
> I'm not against this refactoring, but the purpose should be made clearer.

This case is related to the dpkg unmounting WORKDIR and hanging forever with
"Couldn't unmount, retrying..."; it doesn't reach build_completed(). We'll
rework the commit message.

That said, the next patch addresses the hanging mounts issue by unmounting the
directories as soon as possible, so that build_completed() in most cases
doesn't have to do anything.

With kind regards,
Baurzhan.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-04-08  8:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02 13:03 [PATCH] dpkg: Make mount buildroot reliable Anton Mikanovich
2021-04-08  6:27 ` Jan Kiszka
2021-04-08  8:13   ` Baurzhan Ismagulov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox