Merge remote-tracking branch 'origin/issue-1761' into calamares

FIXES #1761

src/modules/partition/PartitionViewStep.cpp

@@ -434,50 +434,66 @@ PartitionViewStep::onLeave()
         {
             const QString espMountPoint
                 = Calamares::JobQueue::instance()->globalStorage()->value( "efiSystemPartition" ).toString();
-            const QString espFlagName = PartitionTable::flagName(
 #ifdef WITH_KPMCORE4API
-                PartitionTable::Flag::Boot
+            const auto espFlag = PartitionTable::Flag::Boot;
 #else
-                PartitionTable::FlagEsp
+            const auto espFlag = PartitionTable::FlagEsp;
 #endif
-                                                        );
             Partition* esp = m_core->findPartitionByMountPoint( espMountPoint );
 
             QString message;
             QString description;
-            if ( !esp || ( esp && !PartUtils::isEfiFilesystemSuitable( esp ) ) )
+
+            Logger::Once o;
+
+            const bool okType = esp && PartUtils::isEfiFilesystemSuitableType( esp );
+            const bool okSize = esp && PartUtils::isEfiFilesystemSuitableSize( esp );
+            const bool okFlag = esp && PartUtils::isEfiBootable( esp );
+
+            if ( !esp )
             {
                 message = tr( "No EFI system partition configured" );
+            }
+            else if ( !(okType && okSize && okFlag ) )
+            {
+                message = tr( "EFI system partition configured incorrectly" );
+            }
+
+            if ( !esp || !(okType&&okSize &&okFlag)) {
             description = tr( "An EFI system partition is necessary to start %1."
                                 "<br/><br/>"
                                 "To configure an EFI system partition, go back and "
-                                  "select or create a FAT32 filesystem with the "
+                                "select or create a suitable filesystem.").arg( branding->shortProductName() );
-                                  "<strong>%3</strong> flag enabled and mount point "
-                                  "<strong>%2</strong>.<br/><br/>"
-                                  "You can continue without setting up an EFI system "
-                                  "partition but your system may fail to start." )
-                                  .arg( branding->shortProductName() )
-                                  .arg( espMountPoint, espFlagName );
             }
-            else if ( esp && !PartUtils::isEfiBootable( esp ) )
+            if (!esp) {
+                cDebug() << o << "No ESP mounted";
+                description.append(' ');
+                description.append(tr("The filesystem must be mounted on <strong>%1</strong>.").arg(espMountPoint));
+            }
+            if (!okType) {
+                cDebug() << o << "ESP wrong type";
+                description.append(' ');
+                description.append(tr("The filesystem must have type FAT32."));
+            }
+            if (!okSize) {
+                cDebug() << o << "ESP too small";
+                description.append(' ');
+                description.append(tr("The filesystem must be at least %1 MiB in size.").arg( PartUtils::efiFilesystemMinimumSize() ));
+            }
+            if (!okFlag)
             {
-                message = tr( "EFI system partition flag not set" );
+                cDebug() << o << "ESP missing flag";
-                description = tr( "An EFI system partition is necessary to start %1."
+                description.append(' ');
-                                  "<br/><br/>"
+                description.append(tr("The filesystem must have flag <strong>%1</strong> set.").arg(PartitionTable::flagName( espFlag )));
-                                  "A partition was configured with mount point "
+            }
-                                  "<strong>%2</strong> but its <strong>%3</strong> "
+            if (!description.isEmpty()) {
-                                  "flag is not set.<br/>"
+                description.append( "<br/><br/>" );
-                                  "To set the flag, go back and edit the partition."
+                description.append( tr(
-                                  "<br/><br/>"
+                                  "You can continue without setting up an EFI system "
-                                  "You can continue without setting the flag but your "
+                                  "partition but your system may fail to start." ));
-                                  "system may fail to start." )
-                                  .arg( branding->shortProductName() )
-                                  .arg( espMountPoint, espFlagName );
             }
-
             if ( !message.isEmpty() )
             {
-                cWarning() << message;
                 QMessageBox::warning( m_manualPartitionPage, message, description );
             }
         }

src/modules/partition/core/PartUtils.cpp

@@ -447,22 +447,14 @@ isEfiSystem()
 }
 
 bool
-isEfiFilesystemSuitable(const Partition* candidate)
+isEfiFilesystemSuitableType( 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
@@ -475,6 +467,23 @@ isEfiFilesystemSuitable(const Partition* candidate)
     }
 }
 
+bool
+isEfiFilesystemSuitableSize( const Partition* candidate )
+{
+    auto size = candidate->capacity();  // bytes
+
+    using CalamaresUtils::Units::operator""_MiB;
+    if ( size >= 300_MiB )
+    {
+        return true;
+    }
+    else
+    {
+        cWarning() << "Filesystem for EFI is too small (" << size << "bytes)";
+        return false;
+    }
+}
+
 
 bool
 isEfiBootable( const Partition* candidate )
@@ -508,6 +517,15 @@ isEfiBootable( const Partition* candidate )
 #endif
 }
 
+// TODO: this is configurable via the config file **already**
+size_t
+efiFilesystemMinimumSize()
+{
+    using CalamaresUtils::Units::operator""_MiB;
+    return 300_MiB;
+}
+
+
 QString
 canonicalFilesystemName( const QString& fsName, FileSystem::Type* fsType )
 {

src/modules/partition/core/PartUtils.h

@@ -84,9 +84,24 @@ bool isEfiSystem();
 
 /**
  * @brief Is the @p partition suitable as an EFI boot partition?
- * Checks for filesystem type (FAT32) and size (300MiB at least).
+ * Checks for filesystem type (FAT32).
  */
-bool isEfiFilesystemSuitable( const Partition* candidate );
+bool isEfiFilesystemSuitableType( const Partition* candidate );
+
+/**
+ * @brief Is the @p partition suitable as an EFI boot partition?
+ * Checks for filesystem size (300MiB, see efiFilesystemMinimumSize).
+ */
+bool isEfiFilesystemSuitableSize( const Partition* candidate );
+
+/** @brief Returns the minimum size of an EFI boot partition.
+ *
+ * This is determined as 300MiB, based on the FAT32 standard
+ * and EFI documentation (and not a little discussion in Calamares
+ * issues about what works, what is effective, and what is mandated
+ * by the standard and how all of those are different).
+ */
+size_t efiFilesystemMinimumSize();
 
 /**
  * @brief Is the given @p partition bootable in EFI? Depending on

src/modules/partition/core/PartitionLayout.cpp

@@ -158,7 +158,8 @@ PartitionLayout::setDefaultFsType(FileSystem::Type defaultFsType)
     case FileSystem::BitLocker:
 #endif
         // bad bad
-            cWarning() << "The selected default FS" << defaultFsType << "is not suitable." << "Using ext4 instead.";
+        cWarning() << "The selected default FS" << defaultFsType << "is not suitable."
+                   << "Using ext4 instead.";
         defaultFsType = FileSystem::Ext4;
         break;
     case FileSystem::Ext2:
@@ -191,7 +192,8 @@ PartitionLayout::setDefaultFsType(FileSystem::Type defaultFsType)
         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.";
+        cWarning() << "The selected default FS" << defaultFsType << "is not known to Calamares."
+                   << "Using ext4 instead.";
         defaultFsType = FileSystem::Ext4;
     }