From bf7eed934235eb0d67a9024d4384437a6525b993 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 15:50:13 +0200 Subject: [PATCH 01/13] [partition] Tidy debug output when creating table --- src/modules/partition/jobs/CreatePartitionTableJob.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/partition/jobs/CreatePartitionTableJob.cpp b/src/modules/partition/jobs/CreatePartitionTableJob.cpp index 0913a1cfc..76c0ab9ee 100644 --- a/src/modules/partition/jobs/CreatePartitionTableJob.cpp +++ b/src/modules/partition/jobs/CreatePartitionTableJob.cpp @@ -75,13 +75,13 @@ CreatePartitionTableJob::exec() QString message = tr( "The installer failed to create a partition table on %1." ).arg( m_device->name() ); PartitionTable* table = m_device->partitionTable(); - cDebug() << "Creating new partition table of type" << table->typeName() << ", uncommitted yet:"; if ( Logger::logLevelEnabled( Logger::LOGDEBUG ) ) { + cDebug() << "Creating new partition table of type" << table->typeName() << ", uncommitted partitions:"; for ( auto it = PartitionIterator::begin( table ); it != PartitionIterator::end( table ); ++it ) { - cDebug() << it; + cDebug() << Logger::SubEntry << it; } QProcess lsblk; @@ -89,14 +89,14 @@ CreatePartitionTableJob::exec() lsblk.setProcessChannelMode( QProcess::MergedChannels ); lsblk.start(); lsblk.waitForFinished(); - cDebug() << "lsblk:\n" << lsblk.readAllStandardOutput(); + cDebug() << Logger::SubEntry << "lsblk output:\n" << Logger::NoQuote << lsblk.readAllStandardOutput(); QProcess mount; mount.setProgram( "mount" ); // Debug output only, not mounting something mount.setProcessChannelMode( QProcess::MergedChannels ); mount.start(); mount.waitForFinished(); - cDebug() << "mount:\n" << mount.readAllStandardOutput(); + cDebug() << Logger::SubEntry << "mount output:\n" << Logger::NoQuote << mount.readAllStandardOutput(); } CreatePartitionTableOperation op( *m_device, table ); From 17cc0470da1f8e64c411b4b82f78aa66b582b695 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 15:54:51 +0200 Subject: [PATCH 02/13] [partition] Log names of partition flags, not just a number --- src/modules/partition/jobs/SetPartitionFlagsJob.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/jobs/SetPartitionFlagsJob.cpp b/src/modules/partition/jobs/SetPartitionFlagsJob.cpp index 7b6101241..de77d86f8 100644 --- a/src/modules/partition/jobs/SetPartitionFlagsJob.cpp +++ b/src/modules/partition/jobs/SetPartitionFlagsJob.cpp @@ -144,8 +144,9 @@ SetPartFlagsJob::prettyStatusMessage() const Calamares::JobResult SetPartFlagsJob::exec() { - cDebug() << "Setting flags on" << m_device->deviceNode() << "partition" << partition()->deviceNode() << "to" - << m_flags; + QStringList flagsList = PartitionTable::flagNames( m_flags ); + cDebug() << "Setting flags on" << m_device->deviceNode() << "partition" << partition()->deviceNode() + << Logger::DebugList( flagsList ); Report report( nullptr ); SetPartFlagsOperation op( *m_device, *partition(), m_flags ); From 66f96e339ca9e00498e1fed79af4e0403abeec4d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 16:01:04 +0200 Subject: [PATCH 03/13] [libcalamares] Introduce cVerbose() convenience macro (like cDebug()) --- src/libcalamares/utils/Logger.h | 1 + src/modules/partition/jobs/AutoMountManagementJob.cpp | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/utils/Logger.h b/src/libcalamares/utils/Logger.h index fb49248e1..8ef0edaf9 100644 --- a/src/libcalamares/utils/Logger.h +++ b/src/libcalamares/utils/Logger.h @@ -327,6 +327,7 @@ operator<<( CDebug&& s, const Once& o ) } // namespace Logger +#define cVerbose() Logger::CDebug( Logger::LOGVERBOSE, Q_FUNC_INFO ) #define cDebug() Logger::CDebug( Logger::LOGDEBUG, Q_FUNC_INFO ) #define cWarning() Logger::CDebug( Logger::LOGWARNING, Q_FUNC_INFO ) #define cError() Logger::CDebug( Logger::LOGERROR, Q_FUNC_INFO ) diff --git a/src/modules/partition/jobs/AutoMountManagementJob.cpp b/src/modules/partition/jobs/AutoMountManagementJob.cpp index 9472eae9c..e276447db 100644 --- a/src/modules/partition/jobs/AutoMountManagementJob.cpp +++ b/src/modules/partition/jobs/AutoMountManagementJob.cpp @@ -25,8 +25,8 @@ AutoMountManagementJob::prettyName() const Calamares::JobResult AutoMountManagementJob::exec() { - Logger::CDebug( Logger::LOGVERBOSE ) << "this" << Logger::Pointer( this ) << "value" << Logger::Pointer( m_stored ) - << ( m_stored ? "restore" : m_disable ? "disable" : "enable" ); + cVerbose() << "this" << Logger::Pointer( this ) << "value" << Logger::Pointer( m_stored ) + << ( m_stored ? "restore" : m_disable ? "disable" : "enable" ); if ( m_stored ) { CalamaresUtils::Partition::automountRestore( m_stored ); From 432154e50ac8d51598a2d6507b097f18468892e0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 16:10:13 +0200 Subject: [PATCH 04/13] [libcalamares] Improve Once-logging Sending a Once to a logger that isn't enabled should not "consume" that Once; it's still available for a subsequent logger that **is** enabled (useful if you're using more than one log-level in a function). --- src/libcalamares/utils/Logger.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libcalamares/utils/Logger.h b/src/libcalamares/utils/Logger.h index 8ef0edaf9..871cc6fb3 100644 --- a/src/libcalamares/utils/Logger.h +++ b/src/libcalamares/utils/Logger.h @@ -60,6 +60,8 @@ public: friend CDebug& operator<<( CDebug&&, const FuncSuppressor& ); friend CDebug& operator<<( CDebug&&, const Once& ); + inline unsigned int level() const { return m_debugLevel; } + private: QString m_msg; unsigned int m_debugLevel; @@ -315,6 +317,12 @@ private: inline CDebug& operator<<( CDebug&& s, const Once& o ) { + if ( !logLevelEnabled( s.level() ) ) + { + // This won't print, so it's not using the "onceness" + return s; + } + if ( o.m ) { o.m = false; From 7deb6c0e9efdd7241fb0ec77d5fd38d52aadf5f8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 16:18:06 +0200 Subject: [PATCH 05/13] [partition] Improve logging in clearmounts job --- .../partition/jobs/ClearTempMountsJob.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/modules/partition/jobs/ClearTempMountsJob.cpp b/src/modules/partition/jobs/ClearTempMountsJob.cpp index 2f90278fe..ffbc35044 100644 --- a/src/modules/partition/jobs/ClearTempMountsJob.cpp +++ b/src/modules/partition/jobs/ClearTempMountsJob.cpp @@ -43,6 +43,7 @@ ClearTempMountsJob::prettyStatusMessage() const Calamares::JobResult ClearTempMountsJob::exec() { + Logger::Once o; // Fetch a list of current mounts to Calamares temporary directories. QList< QPair< QString, QString > > lst; QFile mtab( "/etc/mtab" ); @@ -51,23 +52,27 @@ ClearTempMountsJob::exec() return Calamares::JobResult::error( tr( "Cannot get list of temporary mounts." ) ); } - cDebug() << "Opened mtab. Lines:"; + cVerbose() << o << "Opened mtab. Lines:"; QTextStream in( &mtab ); QString lineIn = in.readLine(); while ( !lineIn.isNull() ) { QStringList line = lineIn.split( ' ', SplitSkipEmptyParts ); - cDebug() << line.join( ' ' ); + cVerbose() << o << line.join( ' ' ); QString device = line.at( 0 ); QString mountPoint = line.at( 1 ); if ( mountPoint.startsWith( "/tmp/calamares-" ) ) { - cDebug() << "INSERTING pair (device, mountPoint)" << device << mountPoint; lst.append( qMakePair( device, mountPoint ) ); } lineIn = in.readLine(); } + if ( lst.empty() ) + { + return Calamares::JobResult::ok(); + } + std::sort( lst.begin(), lst.end(), []( const QPair< QString, QString >& a, const QPair< QString, QString >& b ) -> bool { return a.first > b.first; @@ -76,10 +81,10 @@ ClearTempMountsJob::exec() QStringList goodNews; QProcess process; - foreach ( auto line, lst ) + for ( const auto& line : qAsConst( lst ) ) { QString partPath = line.second; - cDebug() << "Will try to umount path" << partPath; + cDebug() << o << "Will try to umount path" << partPath; process.start( "umount", { "-lv", partPath } ); process.waitForFinished(); if ( process.exitCode() == 0 ) @@ -92,7 +97,7 @@ ClearTempMountsJob::exec() ok.setMessage( tr( "Cleared all temporary mounts." ) ); ok.setDetails( goodNews.join( "\n" ) ); - cDebug() << "ClearTempMountsJob finished. Here's what was done:\n" << goodNews.join( "\n" ); + cDebug() << o << "ClearTempMountsJob finished. Here's what was done:\n" << Logger::DebugList( goodNews ); return ok; } From 6936915dd641e4f6cf4f8a3bd13134f0da8262ae Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 17:41:22 +0200 Subject: [PATCH 06/13] [partition] Fix logging (type of debug stream changed) --- src/modules/partition/jobs/CreatePartitionTableJob.cpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/modules/partition/jobs/CreatePartitionTableJob.cpp b/src/modules/partition/jobs/CreatePartitionTableJob.cpp index 76c0ab9ee..118ec8823 100644 --- a/src/modules/partition/jobs/CreatePartitionTableJob.cpp +++ b/src/modules/partition/jobs/CreatePartitionTableJob.cpp @@ -60,14 +60,6 @@ CreatePartitionTableJob::prettyStatusMessage() const } -static inline QDebug& -operator<<( QDebug&& s, PartitionIterator& it ) -{ - s << ( ( *it ) ? ( *it )->deviceNode() : QString( "" ) ); - return s; -} - - Calamares::JobResult CreatePartitionTableJob::exec() { @@ -81,7 +73,7 @@ CreatePartitionTableJob::exec() cDebug() << "Creating new partition table of type" << table->typeName() << ", uncommitted partitions:"; for ( auto it = PartitionIterator::begin( table ); it != PartitionIterator::end( table ); ++it ) { - cDebug() << Logger::SubEntry << it; + cDebug() << Logger::SubEntry << ( ( *it ) ? ( *it )->deviceNode() : QString( "" ) ); } QProcess lsblk; From 60f8a7c5fb276cac4b9c802f5d3fcedb54e56668 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 18 Jun 2021 22:20:11 +0200 Subject: [PATCH 07/13] [partition] Don't offer /boot if EFI wants something else - Don't leave /boot in the list always; EFI might be configured for /boot/efi on this system - While here, apply coding style. --- src/modules/partition/gui/PartitionDialogHelpers.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/modules/partition/gui/PartitionDialogHelpers.cpp b/src/modules/partition/gui/PartitionDialogHelpers.cpp index c5c35279d..0d25c3c8d 100644 --- a/src/modules/partition/gui/PartitionDialogHelpers.cpp +++ b/src/modules/partition/gui/PartitionDialogHelpers.cpp @@ -23,11 +23,15 @@ QStringList standardMountPoints() { - QStringList mountPoints { "/", "/boot", "/home", "/opt", "/srv", "/usr", "/var" }; + QStringList mountPoints { "/", "/home", "/opt", "/srv", "/usr", "/var" }; if ( PartUtils::isEfiSystem() ) { mountPoints << Calamares::JobQueue::instance()->globalStorage()->value( "efiSystemPartition" ).toString(); } + else + { + mountPoints << QStringLiteral( "/boot" ); + } mountPoints.removeDuplicates(); mountPoints.sort(); return mountPoints; @@ -68,11 +72,13 @@ setSelectedMountPoint( QComboBox& combo, const QString& selected ) else { for ( int i = 0; i < combo.count(); ++i ) + { if ( selected == combo.itemText( i ) ) { combo.setCurrentIndex( i ); return; } + } combo.addItem( selected ); combo.setCurrentIndex( combo.count() - 1 ); } @@ -85,10 +91,12 @@ flagsFromList( const QListWidget& list ) PartitionTable::Flags flags; for ( int i = 0; i < list.count(); i++ ) + { if ( list.item( i )->checkState() == Qt::Checked ) { flags |= static_cast< PartitionTable::Flag >( list.item( i )->data( Qt::UserRole ).toInt() ); } + } return flags; } From 54e66ff1c0577bf36ee2b1336c73b3dbf4de1a31 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Jun 2021 10:09:44 +0200 Subject: [PATCH 08/13] [calamares] Python pre-script only if Python is enabled If Python support isn't enabled, you can include the PythonJob header, but the symbols aren't in libcalamares so fails at link time. FIXES #1729 --- src/calamares/testmain.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/calamares/testmain.cpp b/src/calamares/testmain.cpp index 6dae0af68..250b047eb 100644 --- a/src/calamares/testmain.cpp +++ b/src/calamares/testmain.cpp @@ -18,18 +18,25 @@ #include "GlobalStorage.h" #include "Job.h" #include "JobQueue.h" -#include "PythonJob.h" #include "Settings.h" #include "ViewManager.h" #include "modulesystem/Module.h" #include "modulesystem/ModuleManager.h" #include "modulesystem/ViewModule.h" #include "utils/Logger.h" +#include "utils/Yaml.h" +#include "viewpages/ExecutionViewStep.h" + +// Optional features of Calamares +// - Python support +// - QML support +#ifdef WITH_PYTHON +#include "PythonJob.h" +#endif #ifdef WITH_QML #include "utils/Qml.h" #endif -#include "utils/Yaml.h" -#include "viewpages/ExecutionViewStep.h" + #include #include @@ -366,10 +373,15 @@ createApplication( int& argc, char* argv[] ) return new QCoreApplication( argc, argv ); } +#ifdef WITH_PYTHON static const char pythonPreScript[] = R"( # This is Python code executed by Python modules *before* the # script file (e.g. main.py) is executed. Beware " before ) # because it's a C++ raw-string. +# +# Calls to suprocess methods that execute something are +# suppressed and logged -- scripts should really be using libcalamares +# methods instead. _calamares_subprocess = __import__("subprocess", globals(), locals(), [], 0) import sys import libcalamares @@ -386,6 +398,7 @@ sys.modules["subprocess"] = fake_subprocess libcalamares.utils.debug('pre-script for testing purposes injected') )"; +#endif int main( int argc, char* argv[] ) @@ -416,10 +429,12 @@ main( int argc, char* argv[] ) gs->insert( "localeConf", vm ); } +#ifdef WITH_PYTHON + Calamares::PythonJob::setInjectedPreScript(pythonPreScript); +#endif #ifdef WITH_QML CalamaresUtils::initQmlModulesDir(); // don't care if failed #endif - Calamares::PythonJob::setInjectedPreScript(pythonPreScript); cDebug() << "Calamares module-loader testing" << module.moduleName(); Calamares::Module* m = load_module( module ); From acb731d8237ce638decaa32514215b2c05048d0a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Jun 2021 22:21:06 +0200 Subject: [PATCH 09/13] [libcalamaresui] Provide the logFile path This is intended for consumption by QML; the ViewManager object acts as a proxy for a handful of global Settings values already, so throw in global Logger values as well. A QML module that would like to read the log file (e.g. for tailing it as part of a slide-show) can get the path via this property. --- src/libcalamaresui/ViewManager.cpp | 6 ++++++ src/libcalamaresui/ViewManager.h | 3 +++ 2 files changed, 9 insertions(+) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index c55b5dd67..f6edbfc2a 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -613,4 +613,10 @@ ViewManager::isSetupMode() const return s ? s->isSetupMode() : false; } +QString +ViewManager::logFilePath() const +{ + return Logger::logFile(); +} + } // namespace Calamares diff --git a/src/libcalamaresui/ViewManager.h b/src/libcalamaresui/ViewManager.h index 9a77cbb5a..5a449a153 100644 --- a/src/libcalamaresui/ViewManager.h +++ b/src/libcalamaresui/ViewManager.h @@ -54,6 +54,7 @@ class UIDLLEXPORT ViewManager : public QAbstractListModel Q_PROPERTY( bool isDebugMode READ isDebugMode CONSTANT FINAL ) Q_PROPERTY( bool isChrootMode READ isChrootMode CONSTANT FINAL ) Q_PROPERTY( bool isSetupMode READ isSetupMode CONSTANT FINAL ) + Q_PROPERTY( QString logFilePath READ logFilePath CONSTANT FINAL ) public: /** @@ -208,6 +209,8 @@ public Q_SLOTS: bool isChrootMode() const; /// @brief Proxy to Settings::isSetupMode() default @c false bool isSetupMode() const; + /// @brief Proxy to Logger::logFile(), default empty + QString logFilePath() const; signals: void currentStepChanged(); From f06766085ab0d52e00e8f75bdfda288880501201 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Jun 2021 23:20:32 +0200 Subject: [PATCH 10/13] [partition] Rename function, to track down consumers --- src/modules/partition/gui/CreatePartitionDialog.cpp | 2 +- src/modules/partition/gui/CreatePartitionDialog.h | 2 +- src/modules/partition/gui/PartitionPage.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/partition/gui/CreatePartitionDialog.cpp b/src/modules/partition/gui/CreatePartitionDialog.cpp index c765bf1f7..e7aab89cd 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.cpp +++ b/src/modules/partition/gui/CreatePartitionDialog.cpp @@ -188,7 +188,7 @@ CreatePartitionDialog::initGptPartitionTypeUi() } Partition* -CreatePartitionDialog::createPartition() +CreatePartitionDialog::getNewlyCreatedPartition() { if ( m_role.roles() == PartitionRole::None ) { diff --git a/src/modules/partition/gui/CreatePartitionDialog.h b/src/modules/partition/gui/CreatePartitionDialog.h index bee70f61b..45e3f7cf4 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.h +++ b/src/modules/partition/gui/CreatePartitionDialog.h @@ -57,7 +57,7 @@ public: * 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..d9ff65f88 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -395,7 +395,7 @@ PartitionPage::onCreateClicked() dlg.initFromFreeSpace( partition ); if ( dlg.exec() == QDialog::Accepted ) { - Partition* newPart = dlg.createPartition(); + Partition* newPart = dlg.getNewlyCreatedPartition(); m_core->createPartition( model->device(), newPart, dlg.newFlags() ); } } @@ -496,7 +496,7 @@ PartitionPage::updatePartitionToCreate( Device* device, Partition* partition ) 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() ); } From dbfd8bea030b9513a8f088723de5f0c9f1c37e01 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Jun 2021 23:22:40 +0200 Subject: [PATCH 11/13] [partition] Newly-created (fresh) partitions don't have flags yet When a partition is set as "freshly created", the dialog was passing in newFlags() as the **already-active** flags on the partition; then the caller was setting those same flags as "set these in the future", so that afterwards, no flags would actually be set (because they're already active -- see the first sentence). Now, fresh partitions have no flags. --- src/modules/partition/gui/CreatePartitionDialog.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/gui/CreatePartitionDialog.cpp b/src/modules/partition/gui/CreatePartitionDialog.cpp index e7aab89cd..37547ef8c 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.cpp +++ b/src/modules/partition/gui/CreatePartitionDialog.cpp @@ -204,17 +204,21 @@ CreatePartitionDialog::getNewlyCreatedPartition() : 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() ); + = KPMHelpers::createNewPartition( m_parent, *m_device, m_role, fsType, fsLabel, first, last, PartitionTable::Flags() ); } if ( m_device->type() == Device::Type::LVM_Device ) From e2bf717ea0c9f820c7cb6d08ebc409bec70e9295 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 22 Jun 2021 00:06:17 +0200 Subject: [PATCH 12/13] [partition] Rewrite new-partition API The existing API required calling the one constructor with specific pointers (nullptr for a partition-from-free-space) followed by calling one of the initFrom*() functions. This is fragile design. Use tag-classes to distinguish create-from-free-space and edit-another-freshly-created-partition cases, refactor to merge the initFrom*() methods into the constructors and factor out the shared UI creation. Callers can now use the tag-class to distinguish. While here, adjust both callers to use QPointer, avoiding some very specific dialog-on-the-stack crash possibilities. --- .../partition/gui/CreatePartitionDialog.cpp | 76 +++++++++---------- .../partition/gui/CreatePartitionDialog.h | 45 ++++++----- src/modules/partition/gui/PartitionPage.cpp | 14 ++-- 3 files changed, 73 insertions(+), 62 deletions(-) diff --git a/src/modules/partition/gui/CreatePartitionDialog.cpp b/src/modules/partition/gui/CreatePartitionDialog.cpp index 37547ef8c..c09724463 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,48 @@ 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() {} @@ -288,34 +319,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 45e3f7cf4..e18840564 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.h +++ b/src/modules/partition/gui/CreatePartitionDialog.h @@ -33,30 +33,41 @@ 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, + PartitionNode* parentPartition, + 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* getNewlyCreatedPartition(); PartitionTable::Flags newFlags() const; diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index d9ff65f88..06aa7b1a8 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.getNewlyCreatedPartition(); - m_core->createPartition( model->device(), newPart, dlg.newFlags() ); + Partition* newPart = dlg->getNewlyCreatedPartition(); + m_core->createPartition( model->device(), newPart, dlg->newFlags() ); } + delete dlg; } void @@ -492,8 +493,7 @@ 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->getNewlyCreatedPartition(); From 131352ca03884dc8c0ace700b5dc41534e35affc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 22 Jun 2021 00:21:01 +0200 Subject: [PATCH 13/13] [partition] Apply coding style --- src/modules/partition/core/DeviceList.cpp | 8 ++++---- src/modules/partition/core/PartUtils.cpp | 13 ++++++++++--- src/modules/partition/core/PartUtils.h | 2 +- .../partition/core/PartitionCoreModule.cpp | 2 +- src/modules/partition/gui/ChoicePage.cpp | 11 +++++++---- .../partition/gui/CreatePartitionDialog.cpp | 11 +++++------ .../partition/gui/CreatePartitionDialog.h | 16 +++++++++++----- src/modules/partition/gui/PartitionPage.cpp | 6 +++--- src/modules/partition/gui/PartitionViewStep.cpp | 7 +++---- 9 files changed, 45 insertions(+), 31 deletions(-) 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 c09724463..5ebe15336 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.cpp +++ b/src/modules/partition/gui/CreatePartitionDialog.cpp @@ -135,7 +135,7 @@ CreatePartitionDialog::CreatePartitionDialog( Device* device, const FreeSpace& freeSpacePartition, const QStringList& usedMountPoints, QWidget* parentWidget ) - : CreatePartitionDialog(device, freeSpacePartition.p->parent(), usedMountPoints, parentWidget ) + : CreatePartitionDialog( device, freeSpacePartition.p->parent(), usedMountPoints, parentWidget ) { standardMountPoints( *( m_ui->mountPointComboBox ), QString() ); setFlagList( *( m_ui->m_listFlags ), @@ -148,10 +148,9 @@ CreatePartitionDialog::CreatePartitionDialog( Device* device, const FreshPartition& existingNewPartition, const QStringList& usedMountPoints, QWidget* parentWidget ) - : CreatePartitionDialog(device, existingNewPartition.p->parent(), usedMountPoints, parentWidget ) + : CreatePartitionDialog( device, existingNewPartition.p->parent(), usedMountPoints, parentWidget ) { - standardMountPoints( *( m_ui->mountPointComboBox ), - PartitionInfo::mountPoint( existingNewPartition.p ) ); + 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 ) ); @@ -248,8 +247,8 @@ CreatePartitionDialog::getNewlyCreatedPartition() } else { - partition - = KPMHelpers::createNewPartition( m_parent, *m_device, m_role, fsType, fsLabel, first, last, PartitionTable::Flags() ); + partition = KPMHelpers::createNewPartition( + m_parent, *m_device, m_role, fsType, fsLabel, first, last, PartitionTable::Flags() ); } if ( m_device->type() == Device::Type::LVM_Device ) diff --git a/src/modules/partition/gui/CreatePartitionDialog.h b/src/modules/partition/gui/CreatePartitionDialog.h index e18840564..38c65aaf6 100644 --- a/src/modules/partition/gui/CreatePartitionDialog.h +++ b/src/modules/partition/gui/CreatePartitionDialog.h @@ -40,13 +40,19 @@ private: * This does all the shared UI setup. */ CreatePartitionDialog( Device* device, - PartitionNode* parentPartition, - const QStringList& usedMountPoints, - QWidget* parentWidget ); + PartitionNode* parentPartition, + const QStringList& usedMountPoints, + QWidget* parentWidget ); public: - struct FreeSpace { Partition* p; }; - struct FreshPartition { Partition* p; }; + struct FreeSpace + { + Partition* p; + }; + struct FreshPartition + { + Partition* p; + }; /** @brief Dialog for editing a new partition based on free space. * diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index 06aa7b1a8..8444b9ddb 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -391,8 +391,8 @@ PartitionPage::onCreateClicked() return; } - QPointer< CreatePartitionDialog > dlg = new - CreatePartitionDialog ( model->device(), CreatePartitionDialog::FreeSpace{ partition }, getCurrentUsedMountpoints(), this ); + QPointer< CreatePartitionDialog > dlg = new CreatePartitionDialog( + model->device(), CreatePartitionDialog::FreeSpace { partition }, getCurrentUsedMountpoints(), this ); if ( dlg->exec() == QDialog::Accepted ) { Partition* newPart = dlg->getNewlyCreatedPartition(); @@ -493,7 +493,7 @@ PartitionPage::updatePartitionToCreate( Device* device, Partition* partition ) mountPoints.removeOne( PartitionInfo::mountPoint( partition ) ); QPointer< CreatePartitionDialog > dlg - = new CreatePartitionDialog( device, CreatePartitionDialog::FreshPartition{ partition }, mountPoints, this ); + = new CreatePartitionDialog( device, CreatePartitionDialog::FreshPartition { partition }, mountPoints, this ); if ( dlg->exec() == QDialog::Accepted ) { Partition* newPartition = dlg->getNewlyCreatedPartition(); 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;