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

FIXES #1701

There's now a check in place that suppresses the GPT-for-BIOS
message if the user is going to follow its advice already.

src/modules/partition/core/BootLoaderModel.cpp

@@ -18,6 +18,7 @@
 
 // KPMcore
 #include <kpmcore/core/device.h>
+#include <kpmcore/core/partition.h>
 
 #include <QComboBox>
 
@@ -148,28 +149,39 @@ BootLoaderModel::data( const QModelIndex& index, int role ) const
     return QStandardItemModel::data( index, role );
 }
 
-namespace Calamares
+std::pair< int, Device* >
+BootLoaderModel::findBootLoader( const QString& path ) const
 {
-int
+    int r = 0;
-findBootloader( const QAbstractItemModel* model, const QString& path )
+    for ( Device* d : m_devices )
-{
-    for ( int i = 0; i < model->rowCount(); ++i )
     {
-        const auto index = model->index( i, 0, QModelIndex() );
+        if ( d && d->deviceNode() == path )
-        if ( !index.isValid() )
         {
-            continue;
+            return std::make_pair( r, d );
-        }
-        QVariant var = model->data( index, BootLoaderModel::BootLoaderPathRole );
-        if ( var.isValid() && var.toString() == path )
-        {
-            return i;
         }
+        r++;
     }
 
-    return -1;
+    Partition* partition = KPMHelpers::findPartitionByMountPoint( m_devices, path );
+    if ( partition )
+    {
+        const QString partition_device_path = partition->deviceNode();
+        r = 0;
+        for ( Device* d : m_devices )
+        {
+            if ( d && d->deviceNode() == partition_device_path )
+            {
+                return std::make_pair( r, d );
+            }
+            r++;
+        }
+    }
+    return std::make_pair( -1, nullptr );
 }
 
+
+namespace Calamares
+{
 void
 restoreSelectedBootLoader( QComboBox& combo, const QString& path )
 {
@@ -180,12 +192,16 @@ restoreSelectedBootLoader( QComboBox& combo, const QString& path )
         return;
     }
 
-    int r = -1;
     if ( path.isEmpty() )
     {
+        cDebug() << "No path to restore, choosing default";
         combo.setCurrentIndex( 0 );
+        return;
     }
-    else if ( ( r = findBootloader( model, path ) ) >= 0 )
+
+    const BootLoaderModel* bmodel = qobject_cast< const BootLoaderModel* >( model );
+    int r = bmodel ? bmodel->findBootLoader( path ).first : -1;
+    if ( r >= 0 )
     {
         combo.setCurrentIndex( r );
     }

src/modules/partition/core/BootLoaderModel.h

@@ -26,6 +26,8 @@ class BootLoaderModel : public QStandardItemModel
 {
     Q_OBJECT
 public:
+    using DeviceList = QList< Device* >;
+
     enum
     {
         BootLoaderPathRole = Qt::UserRole + 1,
@@ -39,13 +41,19 @@ public:
      * Init the model with the list of devices. Does *not* take ownership of the
      * devices.
      */
-    void init( const QList< Device* >& devices );
+    void init( const DeviceList& devices );
 
     void update();
 
     QVariant data( const QModelIndex& index, int role = Qt::DisplayRole ) const override;
 
-    using DeviceList = QList< Device* >;
+    /** @brief Looks up a boot-loader by device-name @p path (e.g. /dev/sda)
+    *
+    * Returns a row number (index) in the model and a Device*: if there **is** a
+    * device for the given @p path, index will be in range of the model and
+    * Device* non-null. Returns (-1, nullptr) otherwise.
+    */
+    std::pair< int, Device* > findBootLoader( const QString& path ) const;
 
 private:
     DeviceList m_devices;
@@ -57,13 +65,6 @@ private:
 
 namespace Calamares
 {
-/** @brief Returns the row number of boot-loader @p path (e.g. /dev/sda)
- *
- * Assuming the @p model is a BootLoaderModel, will return a row number
- * in the model. Returns -1 otherwise.
- */
-int findBootloader( const QAbstractItemModel* model, const QString& path );
-
 /** @brief Tries to set @p path as selected item in @p combo
  *
  * Matches a boot-loader install path (e.g. /dev/sda) with a model

src/modules/partition/core/PartitionCoreModule.cpp

@@ -340,7 +340,7 @@ PartitionCoreModule::deviceModel() const
     return m_deviceModel;
 }
 
-QAbstractItemModel*
+BootLoaderModel*
 PartitionCoreModule::bootLoaderModel() const
 {
     return m_bootLoaderModel;

src/modules/partition/core/PartitionCoreModule.h

@@ -122,7 +122,7 @@ public:
      * The single BootLoaderModel instance belongs to the PCM.
      * @return the BootLoaderModel.
      */
-    QAbstractItemModel* bootLoaderModel() const;
+    BootLoaderModel* bootLoaderModel() const;
 
     void createPartitionTable( Device* device, PartitionTable::TableType type );
 

src/modules/partition/gui/PartitionViewStep.cpp

@@ -13,6 +13,7 @@
 
 #include "gui/PartitionViewStep.h"
 
+#include "core/BootLoaderModel.h"
 #include "core/Config.h"
 #include "core/DeviceModel.h"
 #include "core/KPMHelpers.h"
@@ -36,6 +37,7 @@
 #include "utils/NamedEnum.h"
 #include "utils/QtCompat.h"
 #include "utils/Retranslator.h"
+#include "utils/Units.h"
 #include "utils/Variant.h"
 #include "widgets/WaitingWidget.h"
 
@@ -395,6 +397,44 @@ PartitionViewStep::onActivate()
     }
 }
 
+static bool
+shouldWarnForGPTOnBIOS( const PartitionCoreModule* core )
+{
+    if ( PartUtils::isEfiSystem() )
+    {
+        return false;
+    }
+
+    auto [ r, device ] = core->bootLoaderModel()->findBootLoader( core->bootLoaderInstallPath() );
+    if ( device )
+    {
+        auto* table = device->partitionTable();
+        cDebug() << "Found device for bootloader" << device->deviceNode();
+        if ( table && table->type() == PartitionTable::TableType::gpt )
+        {
+            // So this is a BIOS system, and the bootloader will be installed on a GPT system
+            for ( const auto& partition : qAsConst( table->children() ) )
+            {
+                using CalamaresUtils::Units::operator""_MiB;
+                if ( ( partition->activeFlags() & PartitionTable::Flag::BiosGrub )
+                     && ( partition->fileSystem().type() == FileSystem::Unformatted )
+                     && ( partition->capacity() >= 8_MiB ) )
+                {
+                    cDebug() << Logger::SubEntry << "Partition" << partition->devicePath()
+                             << partition->partitionPath()
+                             << "is a suitable bios_grub partition";
+                    return false;
+                }
+            }
+        }
+        cDebug() << Logger::SubEntry << "No suitable partition for bios_grub found";
+    }
+    else
+    {
+        cDebug() << "Found no device for" << core->bootLoaderInstallPath();
+    }
+    return true;
+}
 
 void
 PartitionViewStep::onLeave()
@@ -462,24 +502,25 @@ PartitionViewStep::onLeave()
         {
 
             cDebug() << "device: BIOS";
-            // TODO: this *always* warns, which might be annoying, so it'd be
-            //       best to find a way to detect that bios_grub partition.
 
-            QString message = tr( "Option to use GPT on BIOS" );
+            if ( shouldWarnForGPTOnBIOS( m_core ) )
-            QString description = tr( "A GPT partition table is the best option for all "
+            {
-                                      "systems. This installer supports such a setup for "
+                QString message = tr( "Option to use GPT on BIOS" );
-                                      "BIOS systems too."
+                QString description = tr( "A GPT partition table is the best option for all "
-                                      "<br/><br/>"
+                                          "systems. This installer supports such a setup for "
-                                      "To configure a GPT partition table on BIOS, "
+                                          "BIOS systems too."
-                                      "(if not done so already) go back "
+                                          "<br/><br/>"
-                                      "and set the partition table to GPT, next create a 8 MB "
+                                          "To configure a GPT partition table on BIOS, "
-                                      "unformatted partition with the "
+                                          "(if not done so already) go back "
-                                      "<strong>bios_grub</strong> flag enabled.<br/><br/>"
+                                          "and set the partition table to GPT, next create a 8 MB "
-                                      "An unformatted 8 MB partition is necessary "
+                                          "unformatted partition with the "
-                                      "to start %1 on a BIOS system with GPT." )
+                                          "<strong>bios_grub</strong> flag enabled.<br/><br/>"
-                                      .arg( branding->shortProductName() );
+                                          "An unformatted 8 MB partition is necessary "
+                                          "to start %1 on a BIOS system with GPT." )
+                                          .arg( branding->shortProductName() );
 
-            QMessageBox::information( m_manualPartitionPage, message, description );
+                QMessageBox::information( m_manualPartitionPage, message, description );
+            }
         }
 
         Partition* root_p = m_core->findPartitionByMountPoint( "/" );
@@ -593,7 +634,7 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap )
     // because it could take a while. Then when it's done, we can set up the widgets
     // and remove the spinner.
     m_future = new QFutureWatcher< void >();
-    connect( m_future, &QFutureWatcher< void >::finished, this, [this] {
+    connect( m_future, &QFutureWatcher< void >::finished, this, [ this ] {
         continueLoading();
         this->m_future->deleteLater();
         this->m_future = nullptr;