Merge branch 'calamares' of https://github.com/calamares/calamares into development

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)
+        btrfs_subvolumes = get_btrfs_subvolumes(partitions)
-        # 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="/@") ]
 
-        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

src/modules/partition/PartitionViewStep.cpp

@@ -432,19 +432,20 @@ PartitionViewStep::onLeave()
     {
         if ( PartUtils::isEfiSystem() )
         {
-            QString espMountPoint
+            const QString espMountPoint
                 = Calamares::JobQueue::instance()->globalStorage()->value( "efiSystemPartition" ).toString();
+            const QString espFlagName = PartitionTable::flagName(
 #ifdef WITH_KPMCORE4API
-            auto espFlag = PartitionTable::Flag::Boot;
+                PartitionTable::Flag::Boot
 #else
-            auto espFlag = PartitionTable::FlagEsp;
+                PartitionTable::FlagEsp
 #endif
-            QString espFlagName = PartitionTable::flagName( espFlag );
+                                                        );
             Partition* esp = m_core->findPartitionByMountPoint( espMountPoint );
 
             QString message;
             QString description;
-            if ( !esp )
+            if ( !esp || ( esp && !PartUtils::isEfiFilesystemSuitable( esp ) ) )
             {
                 message = tr( "No EFI system partition configured" );
                 description = tr( "An EFI system partition is necessary to start %1."

src/modules/partition/core/PartUtils.cpp

@@ -446,18 +446,47 @@ isEfiSystem()
     return QDir( "/sys/firmware/efi/efivars" ).exists();
 }
 
+bool
+isEfiFilesystemSuitable(const Partition* candidate)
+{
+    auto type = candidate->fileSystem().type();
+    auto size = candidate->capacity();  // bytes
+
+    using CalamaresUtils::Units::operator""_MiB;
+
+    switch( type )
+    {
+        case FileSystem::Type::Fat32:
+            if ( size >= 300_MiB )
+            {
+                return true;
+            }
+            cWarning() << "FAT32 filesystem is too small (" << size << "bytes)";
+            return false;
+#ifdef WITH_KPMCORE4API
+        case FileSystem::Type::Fat12:
+#endif
+        case FileSystem::Type::Fat16:
+            cWarning() << "FAT12 and FAT16 are probably not supported by EFI";
+            return false;
+        default:
+            cWarning() << "EFI boot partition must be FAT32";
+            return false;
+    }
+}
+
+
 bool
 isEfiBootable( const Partition* candidate )
 {
     const auto flags = PartitionInfo::flags( candidate );
 
-    // TODO: with KPMCore 4, this comment is wrong: the flags
-    //       are remapped, and the ESP flag is the same as Boot.
 #if defined( WITH_KPMCORE4API )
+    // In KPMCore4, the flags are remapped, and the ESP flag is the same as Boot.
     static_assert( KPM_PARTITION_FLAG_ESP == KPM_PARTITION_FLAG( Boot ), "KPMCore API enum changed" );
     return flags.testFlag( KPM_PARTITION_FLAG_ESP );
 #else
-    /* If bit 17 is set, old-style Esp flag, it's OK */
+    // In KPMCore3, bit 17 is the old-style Esp flag, and it's OK
     if ( flags.testFlag( KPM_PARTITION_FLAG_ESP ) )
     {
         return true;

src/modules/partition/core/PartUtils.h

@@ -82,6 +82,12 @@ OsproberEntryList runOsprober( DeviceModel* dm );
  */
 bool isEfiSystem();
 
+/**
+ * @brief Is the @p partition suitable as an EFI boot partition?
+ * Checks for filesystem type (FAT32) and size (300MiB at least).
+ */
+bool isEfiFilesystemSuitable( const Partition* candidate );
+
 /**
  * @brief Is the given @p partition bootable in EFI? Depending on
  * the partition table layout, this may mean different flags.

src/modules/partition/core/PartitionLayout.cpp

@@ -140,55 +140,59 @@ PartitionLayout::init( FileSystem::Type defaultFsType, const QVariantList& confi
 void
 PartitionLayout::setDefaultFsType(FileSystem::Type defaultFsType)
 {
-    using T = FileSystem::Type;
+    using FileSystem = FileSystem::Type;
     switch ( defaultFsType )
     {
-        case T::Unknown:
+        case FileSystem::Unknown:
-        case T::Unformatted:
+        case FileSystem::Unformatted:
-        case T::Extended:
+        case FileSystem::Extended:
-        case T::LinuxSwap:
+        case FileSystem::LinuxSwap:
-        case T::Luks:
+        case FileSystem::Luks:
-        case T::Ocfs2:
+        case FileSystem::Ocfs2:
-        case T::Lvm2_PV:
+        case FileSystem::Lvm2_PV:
-        case T::Udf:
+        case FileSystem::Udf:
-        case T::Iso9660:
+        case FileSystem::Iso9660:
-        case T::Luks2:
+#ifdef WITH_KPMCORE4API
-        case T::LinuxRaidMember:
+        case FileSystem::Luks2:
-        case T::BitLocker:
+        case FileSystem::LinuxRaidMember:
+        case FileSystem::BitLocker:
+#endif
             // bad bad
             cWarning() << "The selected default FS" << defaultFsType << "is not suitable." << "Using ext4 instead.";
-            defaultFsType = T::Ext4;
+            defaultFsType = FileSystem::Ext4;
             break;
-        case T::Ext2:
+        case FileSystem::Ext2:
-        case T::Ext3:
+        case FileSystem::Ext3:
-        case T::Ext4:
+        case FileSystem::Ext4:
-        case T::Fat32:
+        case FileSystem::Fat32:
-        case T::Ntfs:
+        case FileSystem::Ntfs:
-        case T::Reiser4:
+        case FileSystem::Reiser4:
-        case T::ReiserFS:
+        case FileSystem::ReiserFS:
-        case T::Xfs:
+        case FileSystem::Xfs:
-        case T::Jfs:
+        case FileSystem::Jfs:
-        case T::Btrfs:
+        case FileSystem::Btrfs:
-        case T::Exfat:
+        case FileSystem::Exfat:
-        case T::F2fs:
+        case FileSystem::F2fs:
             // ok
             break;
-        case T::Fat12:
+        case FileSystem::Fat16:
-        case T::Fat16:
+        case FileSystem::Hfs:
-        case T::Hfs:
+        case FileSystem::HfsPlus:
-        case T::HfsPlus:
+        case FileSystem::Ufs:
-        case T::Ufs:
+        case FileSystem::Hpfs:
-        case T::Hpfs:
+        case FileSystem::Zfs:
-        case T::Zfs:
+        case FileSystem::Nilfs2:
-        case T::Nilfs2:
+#ifdef WITH_KPMCORE4API
-        case T::Apfs:
+        case FileSystem::Fat12:
-        case T::Minix:
+        case FileSystem::Apfs:
+        case FileSystem::Minix:
+#endif
             // weird
             cWarning() << "The selected default FS" << defaultFsType << "is unusual, but not wrong.";
             break;
         default:
             cWarning() << "The selected default FS" << defaultFsType << "is not known to Calamares." << "Using ext4 instead.";
-            defaultFsType = T::Ext4;
+            defaultFsType = FileSystem::Ext4;
     }
 
     m_defaultFsType = defaultFsType;

src/modules/partition/partition.conf

@@ -147,7 +147,10 @@ defaultFileSystemType:  "ext4"
 #
 # The value *defaultFileSystemType* is added to this list (with a warning)
 # if not present; the default pick is the *defaultFileSystemType*.
-availableFileSystemTypes:  ["ext4", "btrfs", "f2fs"]
+#
+# If not specified at all, uses *defaultFileSystemType* without a
+# warning (this matches traditional no-choice-available behavior best).
+# availableFileSystemTypes:  ["ext4","f2fs"]
 
 # Show/hide LUKS related functionality in automated partitioning modes.
 # Disable this if you choose not to deploy early unlocking support in GRUB2