diff --git a/CHANGES b/CHANGES index b67d2d30a..4e38c24eb 100644 --- a/CHANGES +++ b/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) # diff --git a/src/modules/partition/core/DeviceList.cpp b/src/modules/partition/core/DeviceList.cpp index d58b22868..6b770a982 100644 --- a/src/modules/partition/core/DeviceList.cpp +++ b/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 #include @@ -133,11 +133,11 @@ getDevices( DeviceType which ) #endif // Unsafe partitioning - auto removeInAllModes = []( DeviceList& l, DeviceList::iterator& it) { return erase(l, it); }; - auto removeInSafeMode = []( DeviceList&, DeviceList::iterator& it) { return ++it; }; + auto removeInAllModes = []( DeviceList& l, DeviceList::iterator& it ) { return erase( l, it ); }; + auto removeInSafeMode = []( DeviceList&, DeviceList::iterator& it ) { return ++it; }; #else // Safe partitioning - auto removeInAllModes = []( DeviceList& l, DeviceList::iterator& it) { return erase(l, it); }; + auto removeInAllModes = []( DeviceList& l, DeviceList::iterator& it ) { return erase( l, it ); }; auto& removeInSafeMode = removeInAllModes; #endif diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index 368d04af2..47654a2be 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/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( - { prettyName, path, file, QString(), canBeResized( dm, path, o ), lineColumns, fstabEntries, homePath } ); + osproberEntries.append( { prettyName, + path, + file, + QString(), + canBeResized( dm, path, o ), + lineColumns, + fstabEntries, + homePath } ); osproberCleanLines.append( line ); } } diff --git a/src/modules/partition/core/PartUtils.h b/src/modules/partition/core/PartUtils.h index aec345882..404dd5a77 100644 --- a/src/modules/partition/core/PartUtils.h +++ b/src/modules/partition/core/PartUtils.h @@ -26,7 +26,7 @@ class DeviceModel; class Partition; namespace Logger { - class Once; +class Once; } namespace PartUtils diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index 912d46546..363d9daef 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -258,7 +258,7 @@ PartitionCoreModule::doInit() cDebug() << Logger::SubEntry << "node\tcapacity\tname\tprettyName"; for ( auto device : devices ) { - cDebug() << Logger::SubEntry << Logger::Pointer(device); + cDebug() << Logger::SubEntry << Logger::Pointer( device ); if ( device ) { // Gives ownership of the Device* to the DeviceInfo object diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 245ee0b92..d2f5c99e7 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -144,14 +144,15 @@ 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 ) { QStyleOptionViewItem options; options.initFrom( box ); - auto delegateSize = box->itemDelegate()->sizeHint(options, model->index(0, 0) ); + auto delegateSize = box->itemDelegate()->sizeHint( options, model->index( 0, 0 ) ); box->setMinimumWidth( delegateSize.width() ); } } @@ -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 ); diff --git a/src/modules/partition/gui/CreatePartitionDialog.cpp b/src/modules/partition/gui/CreatePartitionDialog.cpp index c765bf1f7..5ebe15336 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.cpp +++ b/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. - checkMountPointSelection(); +CreatePartitionDialog::CreatePartitionDialog( Device* device, + 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 - = KPMHelpers::createNewPartition( m_parent, *m_device, m_role, fsType, fsLabel, first, last, newFlags() ); + partition = KPMHelpers::createNewPartition( + 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(); -} diff --git a/src/modules/partition/gui/CreatePartitionDialog.h b/src/modules/partition/gui/CreatePartitionDialog.h index bee70f61b..38c65aaf6 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.h +++ b/src/modules/partition/gui/CreatePartitionDialog.h @@ -33,31 +33,48 @@ class Ui_CreatePartitionDialog; class CreatePartitionDialog : public QDialog { Q_OBJECT -public: - /** - * @brief Dialog for editing a new partition. + +private: + /** @brief Delegated constructor * - * For the (unlikely) case that a newly created partition is being re-edited, - * pass a pointer to that @p partition, otherwise pass nullptr. + * This does all the shared UI setup. */ 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; - /** - * 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(); + Partition* getNewlyCreatedPartition(); PartitionTable::Flags newFlags() const; diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index f5a3b0f43..8444b9ddb 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -391,13 +391,14 @@ PartitionPage::onCreateClicked() return; } - CreatePartitionDialog dlg( model->device(), partition->parent(), nullptr, getCurrentUsedMountpoints(), this ); - dlg.initFromFreeSpace( partition ); - if ( dlg.exec() == QDialog::Accepted ) + QPointer< CreatePartitionDialog > dlg = new CreatePartitionDialog( + model->device(), CreatePartitionDialog::FreeSpace { partition }, getCurrentUsedMountpoints(), this ); + if ( dlg->exec() == QDialog::Accepted ) { - Partition* newPart = dlg.createPartition(); - m_core->createPartition( model->device(), newPart, dlg.newFlags() ); + Partition* newPart = dlg->getNewlyCreatedPartition(); + 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 ); - dlg->initFromPartitionToCreate( partition ); + = new CreatePartitionDialog( device, CreatePartitionDialog::FreshPartition { partition }, mountPoints, this ); 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() ); } diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index b4eefb3b0..bbb865f30 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -388,7 +388,7 @@ shouldWarnForGPTOnBIOS( const PartitionCoreModule* core ) } auto [ r, device ] = core->bootLoaderModel()->findBootLoader( core->bootLoaderInstallPath() ); - Q_UNUSED(r); + Q_UNUSED( r ); if ( device ) { auto* table = device->partitionTable(); @@ -403,8 +403,7 @@ shouldWarnForGPTOnBIOS( const PartitionCoreModule* core ) && ( partition->fileSystem().type() == FileSystem::Unformatted ) && ( partition->capacity() >= 8_MiB ) ) { - cDebug() << Logger::SubEntry << "Partition" << partition->devicePath() - << partition->partitionPath() + cDebug() << Logger::SubEntry << "Partition" << partition->devicePath() << partition->partitionPath() << "is a suitable bios_grub partition"; return false; } @@ -619,7 +618,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;