Merge branch 'issue-1724' into calamares

FIXES #1724

CHANGES

@@ -47,6 +47,9 @@ results -- are sent to Matrix only.
  - A long-neglected pull request from Lisa Vitolo for the *partition*
    module -- allowing to set filesystem labels during manual partitioning --
    has been revived and merged.
+ - The *partition* manager has had a long-standing bug with partition-flags
+   and manual partitioning resolved. This may help resolve some installation
+   issues on UEFI systems. #1724
 
 
 # 3.2.39.3 (2021-04-14) #

src/modules/partition/core/DeviceList.cpp

@@ -10,8 +10,8 @@
 
 #include "DeviceList.h"
 
-#include "utils/Logger.h"
 #include "partition/PartitionIterator.h"
+#include "utils/Logger.h"
 
 #include <kpmcore/backend/corebackend.h>
 #include <kpmcore/backend/corebackendmanager.h>

src/modules/partition/core/PartUtils.cpp

@@ -177,7 +177,8 @@ canBeResized( Partition* candidate, const Logger::Once& o )
 
     if ( availableStorageB > advisedStorageB )
     {
-        cDebug() << o << "Partition" << convenienceName( candidate ) << "authorized for resize + autopartition install.";
+        cDebug() << o << "Partition" << convenienceName( candidate )
+                 << "authorized for resize + autopartition install.";
         return true;
     }
     else
@@ -412,8 +413,14 @@ runOsprober( DeviceModel* dm )
             FstabEntryList fstabEntries = lookForFstabEntries( path );
             QString homePath = findPartitionPathForMountPoint( fstabEntries, "/home" );
 
-            osproberEntries.append(
+            osproberEntries.append( { prettyName,
-                { prettyName, path, file, QString(), canBeResized( dm, path, o ), lineColumns, fstabEntries, homePath } );
+                                      path,
+                                      file,
+                                      QString(),
+                                      canBeResized( dm, path, o ),
+                                      lineColumns,
+                                      fstabEntries,
+                                      homePath } );
             osproberCleanLines.append( line );
         }
     }

src/modules/partition/gui/ChoicePage.cpp

@@ -144,7 +144,8 @@ ChoicePage::~ChoicePage() {}
  * this avoids cases where the popup would truncate data being drawn
  * because the overall box is sized too narrow.
  */
-void setModelToComboBox( QComboBox* box, QAbstractItemModel* model )
+void
+setModelToComboBox( QComboBox* box, QAbstractItemModel* model )
 {
     box->setModel( model );
     if ( model->rowCount() > 0 )
@@ -1005,7 +1006,8 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice )
 
         SelectionFilter filter = []( const QModelIndex& index ) {
             return PartUtils::canBeResized(
-                static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ), Logger::Once() );
+                static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ),
+                Logger::Once() );
         };
         m_beforePartitionBarsView->setSelectionFilter( filter );
         m_beforePartitionLabelsView->setSelectionFilter( filter );
@@ -1094,7 +1096,8 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice )
         {
             SelectionFilter filter = []( const QModelIndex& index ) {
                 return PartUtils::canBeReplaced(
-                    static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ), Logger::Once() );
+                    static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ),
+                    Logger::Once() );
             };
             m_beforePartitionBarsView->setSelectionFilter( filter );
             m_beforePartitionLabelsView->setSelectionFilter( filter );

src/modules/partition/gui/CreatePartitionDialog.cpp

@@ -52,7 +52,6 @@ static QSet< FileSystem::Type > s_unmountableFS( { FileSystem::Unformatted,
 
 CreatePartitionDialog::CreatePartitionDialog( Device* device,
                                               PartitionNode* parentPartition,
-                                              Partition* partition,
                                               const QStringList& usedMountPoints,
                                               QWidget* parentWidget )
     : QDialog( parentWidget )
@@ -81,9 +80,6 @@ CreatePartitionDialog::CreatePartitionDialog( Device* device,
         m_ui->lvNameLineEdit->setValidator( validator );
     }
 
-    standardMountPoints( *( m_ui->mountPointComboBox ),
-                         partition ? PartitionInfo::mountPoint( partition ) : QString() );
-
     if ( device->partitionTable()->type() == PartitionTable::msdos
          || device->partitionTable()->type() == PartitionTable::msdos_sectorbased )
     {
@@ -132,13 +128,47 @@ CreatePartitionDialog::CreatePartitionDialog( Device* device,
     // Select a default
     m_ui->fsComboBox->setCurrentIndex( defaultFsIndex );
     updateMountPointUi();
+    checkMountPointSelection();
+}
 
+CreatePartitionDialog::CreatePartitionDialog( Device* device,
+                                              const FreeSpace& freeSpacePartition,
+                                              const QStringList& usedMountPoints,
+                                              QWidget* parentWidget )
+    : CreatePartitionDialog( device, freeSpacePartition.p->parent(), usedMountPoints, parentWidget )
+{
+    standardMountPoints( *( m_ui->mountPointComboBox ), QString() );
     setFlagList( *( m_ui->m_listFlags ),
                  static_cast< PartitionTable::Flags >( ~PartitionTable::Flags::Int( 0 ) ),
-                 partition ? PartitionInfo::flags( partition ) : PartitionTable::Flags() );
+                 PartitionTable::Flags() );
+    initPartResizerWidget( freeSpacePartition.p );
+}
 
-    // Checks the initial selection.
+CreatePartitionDialog::CreatePartitionDialog( Device* device,
-    checkMountPointSelection();
+                                              const FreshPartition& existingNewPartition,
+                                              const QStringList& usedMountPoints,
+                                              QWidget* parentWidget )
+    : CreatePartitionDialog( device, existingNewPartition.p->parent(), usedMountPoints, parentWidget )
+{
+    standardMountPoints( *( m_ui->mountPointComboBox ), PartitionInfo::mountPoint( existingNewPartition.p ) );
+    setFlagList( *( m_ui->m_listFlags ),
+                 static_cast< PartitionTable::Flags >( ~PartitionTable::Flags::Int( 0 ) ),
+                 PartitionInfo::flags( existingNewPartition.p ) );
+
+    const bool isExtended = existingNewPartition.p->roles().has( PartitionRole::Extended );
+    if ( isExtended )
+    {
+        cDebug() << "Editing extended partitions is not supported.";
+        return;
+    }
+
+    initPartResizerWidget( existingNewPartition.p );
+
+    FileSystem::Type fsType = existingNewPartition.p->fileSystem().type();
+    m_ui->fsComboBox->setCurrentText( FileSystem::nameForType( fsType ) );
+
+    setSelectedMountPoint( m_ui->mountPointComboBox, PartitionInfo::mountPoint( existingNewPartition.p ) );
+    updateMountPointUi();
 }
 
 CreatePartitionDialog::~CreatePartitionDialog() {}
@@ -188,7 +218,7 @@ CreatePartitionDialog::initGptPartitionTypeUi()
 }
 
 Partition*
-CreatePartitionDialog::createPartition()
+CreatePartitionDialog::getNewlyCreatedPartition()
 {
     if ( m_role.roles() == PartitionRole::None )
     {
@@ -204,17 +234,21 @@ CreatePartitionDialog::createPartition()
         : FileSystem::typeForName( m_ui->fsComboBox->currentText() );
     const QString fsLabel = m_ui->filesystemLabelEdit->text();
 
+    // The newly-created partitions have no flags set (no **active** flags),
+    // because they're new. The desired flags can be retrieved from
+    // newFlags() and the consumer (see PartitionPage::onCreateClicked)
+    // does so, to set up the partition for create-and-then-set-flags.
     Partition* partition = nullptr;
     QString luksPassphrase = m_ui->encryptWidget->passphrase();
     if ( m_ui->encryptWidget->state() == EncryptWidget::Encryption::Confirmed && !luksPassphrase.isEmpty() )
     {
         partition = KPMHelpers::createNewEncryptedPartition(
-            m_parent, *m_device, m_role, fsType, fsLabel, first, last, luksPassphrase, newFlags() );
+            m_parent, *m_device, m_role, fsType, fsLabel, first, last, luksPassphrase, PartitionTable::Flags() );
     }
     else
     {
-        partition
+        partition = KPMHelpers::createNewPartition(
-            = KPMHelpers::createNewPartition( m_parent, *m_device, m_role, fsType, fsLabel, first, last, newFlags() );
+            m_parent, *m_device, m_role, fsType, fsLabel, first, last, PartitionTable::Flags() );
     }
 
     if ( m_device->type() == Device::Type::LVM_Device )
@@ -284,34 +318,3 @@ CreatePartitionDialog::initPartResizerWidget( Partition* partition )
     m_partitionSizeController->setPartResizerWidget( m_ui->partResizerWidget );
     m_partitionSizeController->setSpinBox( m_ui->sizeSpinBox );
 }
-
-void
-CreatePartitionDialog::initFromFreeSpace( Partition* freeSpacePartition )
-{
-    initPartResizerWidget( freeSpacePartition );
-}
-
-void
-CreatePartitionDialog::initFromPartitionToCreate( Partition* partition )
-{
-    Q_ASSERT( partition );
-
-    bool isExtended = partition->roles().has( PartitionRole::Extended );
-    Q_ASSERT( !isExtended );
-    if ( isExtended )
-    {
-        cDebug() << "Editing extended partitions is not supported for now";
-        return;
-    }
-
-    initPartResizerWidget( partition );
-
-    // File System
-    FileSystem::Type fsType = partition->fileSystem().type();
-    m_ui->fsComboBox->setCurrentText( FileSystem::nameForType( fsType ) );
-
-    // Mount point
-    setSelectedMountPoint( m_ui->mountPointComboBox, PartitionInfo::mountPoint( partition ) );
-
-    updateMountPointUi();
-}

src/modules/partition/gui/CreatePartitionDialog.h

@@ -33,31 +33,48 @@ class Ui_CreatePartitionDialog;
 class CreatePartitionDialog : public QDialog
 {
     Q_OBJECT
-public:
+
-    /**
+private:
-     * @brief Dialog for editing a new partition.
+    /** @brief Delegated constructor
      *
-     * For the (unlikely) case that a newly created partition is being re-edited,
+     * This does all the shared UI setup.
-     * pass a pointer to that @p partition, otherwise pass nullptr.
      */
     CreatePartitionDialog( Device* device,
                            PartitionNode* parentPartition,
-                           Partition* partition,
+                           const QStringList& usedMountPoints,
+                           QWidget* parentWidget );
+
+public:
+    struct FreeSpace
+    {
+        Partition* p;
+    };
+    struct FreshPartition
+    {
+        Partition* p;
+    };
+
+    /** @brief Dialog for editing a new partition based on free space.
+     *
+     * Creating from free space makes a wholly new partition with
+     * no flags set at all.
+     */
+    CreatePartitionDialog( Device* device,
+                           const FreeSpace& freeSpacePartition,
+                           const QStringList& usedMountPoints,
+                           QWidget* parentWidget = nullptr );
+    /** @brief Dialog for editing a newly-created partition.
+     *
+     * A partition previously newly created (e.g. via this dialog
+     * and the constructor above) can be re-edited.
+     */
+    CreatePartitionDialog( Device* device,
+                           const FreshPartition& existingNewPartition,
                            const QStringList& usedMountPoints,
                            QWidget* parentWidget = nullptr );
     ~CreatePartitionDialog() override;
 
-    /**
+    Partition* getNewlyCreatedPartition();
-     * Must be called when user wants to create a partition in
-     * freeSpacePartition.
-     */
-    void initFromFreeSpace( Partition* freeSpacePartition );
-
-    /**
-     * Must be called when user wants to edit a to-be-created partition.
-     */
-    void initFromPartitionToCreate( Partition* partition );
-    Partition* createPartition();
 
     PartitionTable::Flags newFlags() const;
 

src/modules/partition/gui/PartitionPage.cpp

@@ -391,13 +391,14 @@ PartitionPage::onCreateClicked()
         return;
     }
 
-    CreatePartitionDialog dlg( model->device(), partition->parent(), nullptr, getCurrentUsedMountpoints(), this );
+    QPointer< CreatePartitionDialog > dlg = new CreatePartitionDialog(
-    dlg.initFromFreeSpace( partition );
+        model->device(), CreatePartitionDialog::FreeSpace { partition }, getCurrentUsedMountpoints(), this );
-    if ( dlg.exec() == QDialog::Accepted )
+    if ( dlg->exec() == QDialog::Accepted )
     {
-        Partition* newPart = dlg.createPartition();
+        Partition* newPart = dlg->getNewlyCreatedPartition();
-        m_core->createPartition( model->device(), newPart, dlg.newFlags() );
+        m_core->createPartition( model->device(), newPart, dlg->newFlags() );
     }
+    delete dlg;
 }
 
 void
@@ -492,11 +493,10 @@ PartitionPage::updatePartitionToCreate( Device* device, Partition* partition )
     mountPoints.removeOne( PartitionInfo::mountPoint( partition ) );
 
     QPointer< CreatePartitionDialog > dlg
-        = new CreatePartitionDialog( device, partition->parent(), partition, mountPoints, this );
+        = new CreatePartitionDialog( device, CreatePartitionDialog::FreshPartition { partition }, mountPoints, this );
-    dlg->initFromPartitionToCreate( partition );
     if ( dlg->exec() == QDialog::Accepted )
     {
-        Partition* newPartition = dlg->createPartition();
+        Partition* newPartition = dlg->getNewlyCreatedPartition();
         m_core->deletePartition( device, partition );
         m_core->createPartition( device, newPartition, dlg->newFlags() );
     }

src/modules/partition/gui/PartitionViewStep.cpp

@@ -403,8 +403,7 @@ shouldWarnForGPTOnBIOS( const PartitionCoreModule* core )
                      && ( partition->fileSystem().type() == FileSystem::Unformatted )
                      && ( partition->capacity() >= 8_MiB ) )
                 {
-                    cDebug() << Logger::SubEntry << "Partition" << partition->devicePath()
+                    cDebug() << Logger::SubEntry << "Partition" << partition->devicePath() << partition->partitionPath()
-                             << partition->partitionPath()
                              << "is a suitable bios_grub partition";
                     return false;
                 }