Merge branch 'fix-btrfs-default-layout' into calamares

FIXES #1682

src/modules/mount/main.py

@@ -29,13 +29,44 @@ _ = gettext.translation("calamares-python",
 def pretty_name():
     return _("Mounting partitions.")
 
+
+def get_btrfs_subvolumes(partitions):
+    """
+    Gets the job-configuration for btrfs subvolumes, or if there is
+    none given, returns a default configuration that matches
+    the setup (/ and /home) from before configurability was introduced.
+
+    @param partitions
+        The partitions (from the partitioning module) that will exist on disk.
+        This is used to filter out subvolumes that don't need to be created
+        because they get a dedicated partition instead.
+    """
+    btrfs_subvolumes = libcalamares.job.configuration.get("btrfsSubvolumes", None)
+    # Warn if there's no configuration at all, and empty configurations are
+    # replaced by a simple root-only layout.
+    if btrfs_subvolumes is None:
+        libcalamares.utils.warning("No configuration for btrfsSubvolumes")
+    if not btrfs_subvolumes:
+        btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@"), dict(mountPoint="/home", subvolume="/@home") ]
+
+    # Filter out the subvolumes which have a dedicated partition
+    non_root_partition_mounts = [ m for m in [ p.get("mountPoint", None) for p in partitions ] if m is not None and m != '/' ]
+    btrfs_subvolumes = list(filter(lambda s : s["mountPoint"] not in non_root_partition_mounts, btrfs_subvolumes))
+
+    # If we have a swap **file**, give it a separate subvolume.
+    swap_choice = libcalamares.globalstorage.value( "partitionChoices" )
+    if swap_choice and swap_choice.get( "swap", None ) == "file":
+        btrfs_subvolumes.append({'mountPoint': '/swap', 'subvolume': '/@swap'})
+
+    return btrfs_subvolumes
+
+
 def mount_partition(root_mount_point, partition, partitions):
     """
     Do a single mount of @p partition inside @p root_mount_point.
     """
     # Create mount point with `+` rather than `os.path.join()` because
     # `partition["mountPoint"]` starts with a '/'.
-    global_storage = libcalamares.globalstorage
     raw_mount_point = partition["mountPoint"]
     if not raw_mount_point:
         return
@@ -74,27 +105,8 @@ def mount_partition(root_mount_point, partition, partitions):
     # Special handling for btrfs subvolumes. Create the subvolumes listed in mount.conf
     if fstype == "btrfs" and partition["mountPoint"] == '/':
         # Root has been mounted to btrfs volume -> create subvolumes from configuration
-        btrfs_subvolumes = libcalamares.job.configuration.get("btrfsSubvolumes", None)
-        # Warn if there's no configuration at all, and empty configurations are
-        # replaced by a simple root-only layout.
-        if btrfs_subvolumes is None:
-            libcalamares.utils.warning("No configuration for btrfsSubvolumes")
-        if not btrfs_subvolumes:
-            btrfs_subvolumes = [ dict(mountPoint="/", subvolume="/@") ]
+        btrfs_subvolumes = get_btrfs_subvolumes(partitions)
 
-        subvolume_mountpoints = [d['mountPoint'] for d in btrfs_subvolumes]
-        # Check if listed mountpoints besides / are already present and don't create subvolume for those
-        for p in partitions:
-            if "mountPoint" not in p or not p["mountPoint"]:
-                continue
-            if p["mountPoint"] in subvolume_mountpoints and p["mountPoint"] != '/':
-                btrfs_subvolumes = [d for d in btrfs_subvolumes if d['mountPoint'] != p["mountPoint"]]
-        # Check if we need a subvolume for swap file
-        swap_choice = global_storage.value( "partitionChoices" )
-        if swap_choice:
-            swap_choice = swap_choice.get( "swap", None )
-            if swap_choice and swap_choice == "file":
-                btrfs_subvolumes.append({'mountPoint': '/swap', 'subvolume': '/@swap'})
         # Store created list in global storage so it can be used in the fstab module
         libcalamares.globalstorage.insert("btrfsSubvolumes", btrfs_subvolumes)
         # Create the subvolumes that are in the completed list

src/modules/mount/tests/3.global

@@ -0,0 +1,8 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+partitions:
+    - device: "/dev/sdb1"
+      mountPoint: "/"
+      fs: "btrfs"
+partitionChoices:
+    swap: file

src/modules/mount/tests/3.job

@@ -0,0 +1,6 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+bogus: true
+
+# No configuration needed because the partitions are
+# all filesystems that require no special handling.

src/modules/mount/tests/4.global

@@ -0,0 +1,11 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+partitions:
+    - device: "/dev/sdb1"
+      mountPoint: "/"
+      fs: "btrfs"
+    - device: "/dev/sdb2"
+      mountPoint: "/home"
+      fs: "ext4"
+partitionChoices:
+    swap: file

src/modules/mount/tests/4.job

@@ -0,0 +1,12 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+
+btrfsSubvolumes:
+    - mountPoint: /
+      subvolume: /@
+    - mountPoint: /home
+      subvolume: /@home
+    - mountPoint: /var/cache
+      subvolume: /@cache
+    - mountPoint: /var/log
+      subvolume: /@log