From c3ad5f5c007cf6ca5d06f2f0c85fbf00546282ee Mon Sep 17 00:00:00 2001
From: Douglas Raillard <douglas.raillard@arm.com>
Date: Thu, 6 Jul 2023 12:53:17 +0100
Subject: [PATCH 1/2] tools/lisa-build-asset: Avoid emptying --build-dir

FIX

--build-dir folder was emptied before being used. In case of previous
build failure, we might still have bind mounts that we absolutely not
want to remove recursively as it might empty e.g. LISA_HOME
---
 tools/lisa-build-asset | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/lisa-build-asset b/tools/lisa-build-asset
index 69627879b..8f725b8d3 100755
--- a/tools/lisa-build-asset
+++ b/tools/lisa-build-asset
@@ -49,7 +49,7 @@ def get_env(asset=None, arch=None, build_dir=None, toolchain=None, recipe=None,
         'LISA_ASSET': asset,
         'LISA_ASSET_RECIPE': recipe,
         'LISA_ARCH_ASSETS': os.path.join(LISA_HOME, 'lisa', '_assets', 'binaries', arch),
-        'BUILD_DIR': os.path.abspath(build_dir),
+        'BUILD_DIR': os.path.abspath(build_dir) if build_dir is not None else None,
         'CROSS_COMPILE': toolchain + '-' if toolchain else None,
         'CONFIGURE_HOST': os.path.basename(toolchain) if toolchain else None,
         'USE_MUSL_LIB': '1' if use_musl else None,
@@ -227,8 +227,10 @@ def make(asset, arch, actions, build_dir, recipe_dir, toolchain=None, arch_chroo
 
 def make_asset(asset, arch_list, build_dir, recipe_dir, toolchains=None, native_build=False):
     if build_dir:
-        shutil.rmtree(build_dir, ignore_errors=True)
-        os.makedirs(build_dir, exist_ok=True)
+        # We do not want to empty an existing directory, as it might contain
+        # bind mounts and deleting that recursively may end up deleting e.g.
+        # LISA_HOME.
+        os.makedirs(build_dir, exist_ok=False)
         cm = nullcontext(build_dir)
     else:
         cm = tempfile.TemporaryDirectory()
-- 
GitLab


From d6bcc83540131ff88050488c14b1df00009ed1ae Mon Sep 17 00:00:00 2001
From: Douglas Raillard <douglas.raillard@arm.com>
Date: Thu, 6 Jul 2023 13:02:25 +0100
Subject: [PATCH 2/2] tools/lisa-build-asset: Try harder to remove bind mounts

FIX

If setting up the Alpine chroot fails, still try to destroy the chroot,
thereby removing any existing bind mounts. Otherwise, we leave mount
points active that could lead to data loss if someone tries to remove
the build folder without paying attention.
---
 tools/lisa-build-asset | 46 +++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/tools/lisa-build-asset b/tools/lisa-build-asset
index 8f725b8d3..d59d82dee 100755
--- a/tools/lisa-build-asset
+++ b/tools/lisa-build-asset
@@ -156,31 +156,35 @@ def make_chroot(asset, arch, build_dir, recipe_dir, arch_chroot_dir):
     # List the required env variables.
     env_args = ('-k', ' '.join(env.keys())) if env else []
 
-    # Install the chroot.
-    subprocess.check_call(
-        [
-            'sudo', os.path.join(get_alpine_dir(), 'alpine-chroot-install'),
-            '-b', alpine_version,
-            '-d', arch_chroot_dir,
-            '-i', LISA_HOME,
-            '-a', QEMU_ARCH_NAMES.get(arch, arch),
-            *alpine_dependencies,
-            *env_args,
-        ],
-        cwd=build_dir,
-    )
+    def setup():
+        # Install the chroot.
+        subprocess.check_call(
+            [
+                'sudo', os.path.join(get_alpine_dir(), 'alpine-chroot-install'),
+                '-b', alpine_version,
+                '-d', arch_chroot_dir,
+                '-i', LISA_HOME,
+                '-a', QEMU_ARCH_NAMES.get(arch, arch),
+                *alpine_dependencies,
+                *env_args,
+            ],
+            cwd=build_dir,
+        )
 
-    def bind_mount(path):
-        # join() will ignore everything before an absolute path so make it
-        # relative first
-        rel = os.path.relpath(path, start='/')
-        path_ = os.path.join(arch_chroot_dir, rel)
-        subprocess.check_call(['sudo', 'mkdir', '-p', path_])
-        subprocess.check_call(['sudo', 'mount', '--bind', path, path_])
+        def bind_mount(path):
+            # join() will ignore everything before an absolute path so make it
+            # relative first
+            rel = os.path.relpath(path, start='/')
+            path_ = os.path.join(arch_chroot_dir, rel)
+            subprocess.check_call(['sudo', 'mkdir', '-p', path_])
+            subprocess.check_call(['sudo', 'mount', '--bind', path, path_])
 
-    bind_mount(build_dir)
+        bind_mount(build_dir)
 
     try:
+        # If setup fails, we still destroy the chroot so we give a chance to
+        # cleanup bind mounts.
+        setup()
         yield arch_chroot_dir
     finally:
         # Clean-up the chroot
-- 
GitLab