Filter and sort all mounts in sandbox_cmd()

We don't want users of sandbox_cmd() to have to care about mount
ordering. Currently, if mounts with a more general destination are
ordered after mounts with a more specific destination, the earlier
mount is hidden by the later mount. By sorting by destination, we
avoid this issue.

diff --git a/mkosi/sandbox.py b/mkosi/sandbox.py
index 350240ca5054a76b179f0e63babbdb950fc4951e..fdad185d642d05f4ddd1ad3036321501eb2443ee 100644 (file)
--- a/mkosi/sandbox.py
+++ b/mkosi/sandbox.py
@@ -96,6 +96,28 @@ def finalize_crypto_mounts(tools: Path = Path("/")) -> list[Mount]:
     ]
 
 
+def finalize_mounts(mounts: Sequence[Mount]) -> list[PathString]:
+    mounts = list(set(mounts))
+
+    mounts = [
+        m for m in mounts
+        if not any(
+            m != n and
+            m.devices == n.devices and
+            m.ro == n.ro and
+            m.required == n.required and
+            Path(m.src).is_relative_to(n.src) and
+            Path(m.dst).is_relative_to(n.dst) and
+            Path(m.src).relative_to(n.src) == Path(m.dst).relative_to(n.dst)
+            for n in mounts
+        )
+    ]
+
+    mounts = sorted(mounts, key=lambda m: (Path(m.dst), m.devices, not m.ro, m.required, Path(m.src)))
+
+    return flatten(m.options() for m in mounts)
+
+
 def sandbox_cmd(
     *,
     network: bool = False,
@@ -107,6 +129,7 @@ def sandbox_cmd(
     options: Sequence[PathString] = (),
 ) -> list[PathString]:
     cmdline: list[PathString] = []
+    mounts = list(mounts)
 
     if not relaxed:
         # We want to use an empty subdirectory in the host's temporary directory as the sandbox's /var/tmp. To make
@@ -126,10 +149,10 @@ def sandbox_cmd(
         # We mounted a subdirectory of TMPDIR to /var/tmp so we unset TMPDIR so that /tmp or /var/tmp are used instead.
         "--unsetenv", "TMPDIR",
     ]
-    allmounts = [Mount(tools / "usr", "/usr", ro=True)]
+    mounts += [Mount(tools / "usr", "/usr", ro=True)]
 
     if relaxed:
-        allmounts += [Mount("/tmp", "/tmp")]
+        mounts += [Mount("/tmp", "/tmp")]
     else:
         cmdline += [
             "--tmpfs", "/tmp",
@@ -137,10 +160,10 @@ def sandbox_cmd(
         ]
 
     if (tools / "nix/store").exists():
-        allmounts += [Mount(tools / "nix/store", "/nix/store")]
+        mounts += [Mount(tools / "nix/store", "/nix/store")]
 
     if devices or relaxed:
-        allmounts += [
+        mounts += [
             Mount("/sys", "/sys"),
             Mount("/run", "/run"),
             Mount("/dev", "/dev", devices=True),
@@ -153,7 +176,7 @@ def sandbox_cmd(
 
         for d in dirs:
             if Path(d).exists():
-                allmounts += [Mount(d, d)]
+                mounts += [Mount(d, d)]
 
         if len(Path.cwd().parents) >= 2:
             # `Path.parents` only supports slices and negative indexing from Python 3.10 onwards.
@@ -165,21 +188,20 @@ def sandbox_cmd(
             d = ""
 
         if d and d not in (*dirs, "/home", "/usr", "/nix", "/tmp"):
-            allmounts += [Mount(d, d)]
+            mounts += [Mount(d, d)]
 
     if vartmp:
-        allmounts += [Mount(vartmp, "/var/tmp")]
+        mounts += [Mount(vartmp, "/var/tmp")]
 
     for d in ("bin", "sbin", "lib", "lib32", "lib64"):
         if (p := tools / d).is_symlink():
             cmdline += ["--symlink", p.readlink(), Path("/") / p.relative_to(tools)]
         elif p.is_dir():
-            allmounts += [Mount(p, Path("/") / p.relative_to(tools), ro=True)]
+            mounts += [Mount(p, Path("/") / p.relative_to(tools), ro=True)]
 
     path = "/usr/bin:/usr/sbin" if tools != Path("/") else os.environ["PATH"]
 
     cmdline += ["--setenv", "PATH", f"/scripts:{path}", *options]
-    allmounts = [*allmounts, *mounts]
 
     if not relaxed:
         cmdline += ["--symlink", "../proc/self/mounts", "/etc/mtab"]
@@ -190,15 +212,15 @@ def sandbox_cmd(
     # already exists on the host as otherwise we'd modify the host's /etc by creating the mountpoint ourselves (or
     # fail when trying to create it).
     if (tools / "etc/alternatives").exists() and (not relaxed or Path("/etc/alternatives").exists()):
-        allmounts += [Mount(tools / "etc/alternatives", "/etc/alternatives", ro=True)]
+        mounts += [Mount(tools / "etc/alternatives", "/etc/alternatives", ro=True)]
 
     if scripts:
-        allmounts += [Mount(scripts, "/scripts", ro=True)]
+        mounts += [Mount(scripts, "/scripts", ro=True)]
 
     if network and not relaxed and Path("/etc/resolv.conf").exists():
-        allmounts += [Mount("/etc/resolv.conf", "/etc/resolv.conf")]
+        mounts += [Mount("/etc/resolv.conf", "/etc/resolv.conf")]
 
-    cmdline += flatten(mount.options() for mount in allmounts)
+    cmdline += finalize_mounts(mounts)
 
     # bubblewrap creates everything with a restricted mode so relax stuff as needed.
     ops = []

diff --git a/mkosi/tree.py b/mkosi/tree.py
index ab4b80a19e6f67f8dfdbeb2af0f8b33cebbe8c9b..1b6f651180bf9ba8cf56dabd6431f8b9ad20c1d6 100644 (file)
--- a/mkosi/tree.py
+++ b/mkosi/tree.py
@@ -150,23 +150,18 @@ def rmtree(*paths: Path, tools: Path = Path("/"), sandbox: SandboxProtocol = nos
         return
 
     if find_binary("btrfs", root=tools) and (subvolumes := [p for p in paths if is_subvolume(p)]):
-        parents = sorted(set(p.parent for p in subvolumes))
-        parents = [p for p in parents if all(p == q or not p.is_relative_to(q) for q in parents)]
-
         # Silence and ignore failures since when not running as root, this will fail with a permission error unless the
         # btrfs filesystem is mounted with user_subvol_rm_allowed.
         run(["btrfs", "subvolume", "delete", *subvolumes],
             check=False,
-            sandbox=sandbox(mounts=[Mount(p, p) for p in parents]),
+            sandbox=sandbox(mounts=[Mount(p.parent, p.parent) for p in subvolumes]),
             stdout=subprocess.DEVNULL if not ARG_DEBUG.get() else None,
             stderr=subprocess.DEVNULL if not ARG_DEBUG.get() else None)
 
     paths = tuple(p for p in paths if p.exists())
     if paths:
-        parents = sorted(set(p.parent for p in paths))
-        parents = [p for p in parents if all(p == q or not p.is_relative_to(q) for q in parents)]
         run(["rm", "-rf", "--", *paths],
-            sandbox=sandbox(mounts=[Mount(p, p) for p in parents]))
+            sandbox=sandbox(mounts=[Mount(p.parent, p.parent) for p in paths]))
 
 
 def move_tree(