From c63d4ad2cc41ad8e6c2cbc6696336abf71554383 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 3 Aug 2020 12:31:30 +0200 Subject: [PATCH 01/26] [partition] Enable 'file' swap choice SEE #1166 --- src/modules/partition/partition.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/partition.conf b/src/modules/partition/partition.conf index 363ef7db1..3115afa98 100644 --- a/src/modules/partition/partition.conf +++ b/src/modules/partition/partition.conf @@ -37,7 +37,7 @@ userSwapChoices: - small # Up to 4GB - suspend # At least main memory size # - reuse # Re-use existing swap, but don't create any (unsupported right now) - # - file # To swap file instead of partition (unsupported right now) + - file # To swap file instead of partition # LEGACY SETTINGS (these will generate a warning) # ensureSuspendToDisk: true From c2929e93b3f409514c6c31189f9b348a9bce4ba7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 3 Aug 2020 13:13:56 +0200 Subject: [PATCH 02/26] [partition] Start sanitizing the Jobs on a Device - having a struct with an obtuse API for adding jobs-that-need-to-happen- to-this-device is just not good for maintainability. - break the build by making things private. --- src/modules/partition/core/PartitionCoreModule.cpp | 6 ++++-- src/modules/partition/core/PartitionCoreModule.h | 6 +++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index 8856778b3..f29db1812 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -120,7 +120,7 @@ PartitionCoreModule::DeviceInfo::~DeviceInfo() {} void PartitionCoreModule::DeviceInfo::forgetChanges() { - jobs.clear(); + m_jobs.clear(); for ( auto it = PartitionIterator::begin( device.data() ); it != PartitionIterator::end( device.data() ); ++it ) { PartitionInfo::reset( *it ); @@ -132,16 +132,18 @@ PartitionCoreModule::DeviceInfo::forgetChanges() bool PartitionCoreModule::DeviceInfo::isDirty() const { - if ( !jobs.isEmpty() ) + if ( !m_jobs.isEmpty() ) { return true; } for ( auto it = PartitionIterator::begin( device.data() ); it != PartitionIterator::end( device.data() ); ++it ) + { if ( PartitionInfo::isDirty( *it ) ) { return true; } + } return false; } diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h index f88544ae8..35764424c 100644 --- a/src/modules/partition/core/PartitionCoreModule.h +++ b/src/modules/partition/core/PartitionCoreModule.h @@ -255,13 +255,17 @@ private: QScopedPointer< Device > device; QScopedPointer< PartitionModel > partitionModel; const QScopedPointer< Device > immutableDevice; - Calamares::JobList jobs; // To check if LVM VGs are deactivated bool isAvailable; void forgetChanges(); bool isDirty() const; + + const Calamares::JobList& jobs() const { return m_jobs; } + + private: + Calamares::JobList m_jobs; }; QList< DeviceInfo* > m_deviceInfos; QList< Partition* > m_efiSystemPartitions; From 22ba3cc62d43345fce9ff50aaf29d7a1625904b2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 3 Aug 2020 13:20:04 +0200 Subject: [PATCH 03/26] [partition] Make private struct type private - no need for the definition to be in public header, move to implementation - while here, sort the members and private methods - add a makeJob() to add jobs to the queue --- .../partition/core/PartitionCoreModule.cpp | 33 ++++++++++++++++ .../partition/core/PartitionCoreModule.h | 38 +++++-------------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index f29db1812..b1124a6c6 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -106,6 +106,39 @@ private: //- DeviceInfo --------------------------------------------- +/** + * Owns the Device, PartitionModel and the jobs + */ +struct PartitionCoreModule::DeviceInfo +{ + DeviceInfo( Device* ); + ~DeviceInfo(); + QScopedPointer< Device > device; + QScopedPointer< PartitionModel > partitionModel; + const QScopedPointer< Device > immutableDevice; + + // To check if LVM VGs are deactivated + bool isAvailable; + + void forgetChanges(); + bool isDirty() const; + + const Calamares::JobList& jobs() const { return m_jobs; } + + template< typename Job, typename... Args > + Calamares::Job* makeJob(Args... a) + { + auto* job = new Job( device.get(), a... ); + job->updatePreview(); + m_jobs << Calamares::job_ptr( job ); + return job; + } + +private: + Calamares::JobList m_jobs; +}; + + PartitionCoreModule::DeviceInfo::DeviceInfo( Device* _device ) : device( _device ) , partitionModel( new PartitionModel ) diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h index 35764424c..4a900ef18 100644 --- a/src/modules/partition/core/PartitionCoreModule.h +++ b/src/modules/partition/core/PartitionCoreModule.h @@ -24,6 +24,7 @@ #include "core/KPMHelpers.h" #include "core/PartitionLayout.h" #include "core/PartitionModel.h" +#include "jobs/PartitionJob.h" #include "Job.h" #include "partition/KPMManager.h" @@ -241,32 +242,19 @@ Q_SIGNALS: void deviceReverted( Device* device ); private: - CalamaresUtils::Partition::KPMManager m_kpmcore; - + struct DeviceInfo; void refreshAfterModelChange(); - /** - * Owns the Device, PartitionModel and the jobs - */ - struct DeviceInfo - { - DeviceInfo( Device* ); - ~DeviceInfo(); - QScopedPointer< Device > device; - QScopedPointer< PartitionModel > partitionModel; - const QScopedPointer< Device > immutableDevice; - - // To check if LVM VGs are deactivated - bool isAvailable; + void doInit(); + void updateHasRootMountPoint(); + void updateIsDirty(); + void scanForEfiSystemPartitions(); + void scanForLVMPVs(); - void forgetChanges(); - bool isDirty() const; + DeviceInfo* infoForDevice( const Device* ) const; - const Calamares::JobList& jobs() const { return m_jobs; } + CalamaresUtils::Partition::KPMManager m_kpmcore; - private: - Calamares::JobList m_jobs; - }; QList< DeviceInfo* > m_deviceInfos; QList< Partition* > m_efiSystemPartitions; QVector< const Partition* > m_lvmPVs; @@ -278,14 +266,6 @@ private: QString m_bootLoaderInstallPath; PartitionLayout* m_partLayout; - void doInit(); - void updateHasRootMountPoint(); - void updateIsDirty(); - void scanForEfiSystemPartitions(); - void scanForLVMPVs(); - - DeviceInfo* infoForDevice( const Device* ) const; - OsproberEntryList m_osproberLines; QMutex m_revertMutex; From 6e31d9de4bb2ee7fe79fe291d2b4e99239815c50 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 3 Aug 2020 13:41:34 +0200 Subject: [PATCH 04/26] [partition] Name deviceInfo consistently, auto* - use auto* for pointer type where we already say "device info" twice --- .../partition/core/PartitionCoreModule.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index b1124a6c6..078faba27 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -328,24 +328,24 @@ PartitionCoreModule::immutableDeviceCopy( const Device* device ) void PartitionCoreModule::createPartitionTable( Device* device, PartitionTable::TableType type ) { - DeviceInfo* info = infoForDevice( device ); - if ( info ) + auto* deviceInfo = infoForDevice( device ); + if ( deviceInfo ) { // Creating a partition table wipes all the disk, so there is no need to // keep previous changes - info->forgetChanges(); + deviceInfo->forgetChanges(); OperationHelper helper( partitionModelForDevice( device ), this ); CreatePartitionTableJob* job = new CreatePartitionTableJob( device, type ); job->updatePreview(); - info->jobs << Calamares::job_ptr( job ); + deviceInfo->jobs << Calamares::job_ptr( job ); } } void PartitionCoreModule::createPartition( Device* device, Partition* partition, PartitionTable::Flags flags ) { - auto deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); @@ -396,7 +396,7 @@ PartitionCoreModule::createVolumeGroup( QString& vgName, QVector< const Partitio void PartitionCoreModule::resizeVolumeGroup( LvmDevice* device, QVector< const Partition* >& pvList ) { - DeviceInfo* deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); ResizeVolumeGroupJob* job = new ResizeVolumeGroupJob( device, pvList ); @@ -409,7 +409,7 @@ PartitionCoreModule::resizeVolumeGroup( LvmDevice* device, QVector< const Partit void PartitionCoreModule::deactivateVolumeGroup( LvmDevice* device ) { - DeviceInfo* deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); deviceInfo->isAvailable = false; @@ -425,7 +425,7 @@ PartitionCoreModule::deactivateVolumeGroup( LvmDevice* device ) void PartitionCoreModule::removeVolumeGroup( LvmDevice* device ) { - DeviceInfo* deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); RemoveVolumeGroupJob* job = new RemoveVolumeGroupJob( device ); @@ -438,7 +438,7 @@ PartitionCoreModule::removeVolumeGroup( LvmDevice* device ) void PartitionCoreModule::deletePartition( Device* device, Partition* partition ) { - auto deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); @@ -525,7 +525,7 @@ PartitionCoreModule::deletePartition( Device* device, Partition* partition ) void PartitionCoreModule::formatPartition( Device* device, Partition* partition ) { - auto deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); @@ -536,7 +536,7 @@ PartitionCoreModule::formatPartition( Device* device, Partition* partition ) void PartitionCoreModule::resizePartition( Device* device, Partition* partition, qint64 first, qint64 last ) { - auto deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); @@ -548,7 +548,7 @@ PartitionCoreModule::resizePartition( Device* device, Partition* partition, qint void PartitionCoreModule::setPartitionFlags( Device* device, Partition* partition, PartitionTable::Flags flags ) { - auto deviceInfo = infoForDevice( device ); + auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper( partitionModelForDevice( device ), this ); From 20b477d0637f2bda686edda459ce86b2c9feb50c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 16:20:04 +0200 Subject: [PATCH 05/26] [partition] Distinguish jobs with updatePreview() --- .../partition/core/PartitionCoreModule.cpp | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index 078faba27..ac5e3b65b 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -50,6 +50,7 @@ #include "partition/PartitionIterator.h" #include "partition/PartitionQuery.h" #include "utils/Logger.h" +#include "utils/Traits.h" #include "utils/Variant.h" // KPMcore @@ -106,9 +107,29 @@ private: //- DeviceInfo --------------------------------------------- +// Some jobs have an updatePreview some don't +DECLARE_HAS_METHOD(updatePreview) + +template< typename Job > +void updatePreview( Job* job, const std::true_type& ) +{ + job->updatePreview(); +} + +template< typename Job > +void updatePreview( Job* job, const std::false_type& ) +{ +} + +template< typename Job > +void updatePreview( Job* job ) +{ + updatePreview(job, has_updatePreview{}); +} + /** - * Owns the Device, PartitionModel and the jobs - */ + * Owns the Device, PartitionModel and the jobs + */ struct PartitionCoreModule::DeviceInfo { DeviceInfo( Device* ); @@ -129,7 +150,7 @@ struct PartitionCoreModule::DeviceInfo Calamares::Job* makeJob(Args... a) { auto* job = new Job( device.get(), a... ); - job->updatePreview(); + updatePreview( job ); m_jobs << Calamares::job_ptr( job ); return job; } From 23b507ae8ee12d0933d67de5b07098ebcebf5970 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 16:53:29 +0200 Subject: [PATCH 06/26] [partition] Chase constness, makeJob() --- .../partition/core/PartitionCoreModule.cpp | 64 ++++++------------- 1 file changed, 18 insertions(+), 46 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index ac5e3b65b..ca8f9f131 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -357,9 +357,7 @@ PartitionCoreModule::createPartitionTable( Device* device, PartitionTable::Table deviceInfo->forgetChanges(); OperationHelper helper( partitionModelForDevice( device ), this ); - CreatePartitionTableJob* job = new CreatePartitionTableJob( device, type ); - job->updatePreview(); - deviceInfo->jobs << Calamares::job_ptr( job ); + deviceInfo->makeJob< CreatePartitionTableJob >( type ); } } @@ -370,15 +368,11 @@ PartitionCoreModule::createPartition( Device* device, Partition* partition, Part Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); - CreatePartitionJob* job = new CreatePartitionJob( device, partition ); - job->updatePreview(); - - deviceInfo->jobs << Calamares::job_ptr( job ); + deviceInfo->makeJob< CreatePartitionJob >( partition ); if ( flags != KPM_PARTITION_FLAG( None ) ) { - SetPartFlagsJob* fJob = new SetPartFlagsJob( device, partition, flags ); - deviceInfo->jobs << Calamares::job_ptr( fJob ); + deviceInfo->makeJob< SetPartFlagsJob >( partition, flags ); PartitionInfo::setFlags( partition, flags ); } } @@ -392,25 +386,18 @@ PartitionCoreModule::createVolumeGroup( QString& vgName, QVector< const Partitio vgName.append( '_' ); } - CreateVolumeGroupJob* job = new CreateVolumeGroupJob( vgName, pvList, peSize ); - job->updatePreview(); - LvmDevice* device = new LvmDevice( vgName ); - for ( const Partition* p : pvList ) { device->physicalVolumes() << p; } DeviceInfo* deviceInfo = new DeviceInfo( device ); - deviceInfo->partitionModel->init( device, osproberEntries() ); - m_deviceModel->addDevice( device ); - m_deviceInfos << deviceInfo; - deviceInfo->jobs << Calamares::job_ptr( job ); + deviceInfo->makeJob< CreateVolumeGroupJob >( vgName, pvList, peSize ); refreshAfterModelChange(); } @@ -419,11 +406,7 @@ PartitionCoreModule::resizeVolumeGroup( LvmDevice* device, QVector< const Partit { auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); - - ResizeVolumeGroupJob* job = new ResizeVolumeGroupJob( device, pvList ); - - deviceInfo->jobs << Calamares::job_ptr( job ); - + deviceInfo->makeJob< ResizeVolumeGroupJob >( device, pvList ); refreshAfterModelChange(); } @@ -435,6 +418,7 @@ PartitionCoreModule::deactivateVolumeGroup( LvmDevice* device ) deviceInfo->isAvailable = false; + // TODO: this leaks DeactivateVolumeGroupJob* job = new DeactivateVolumeGroupJob( device ); // DeactivateVolumeGroupJob needs to be immediately called @@ -448,11 +432,7 @@ PartitionCoreModule::removeVolumeGroup( LvmDevice* device ) { auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); - - RemoveVolumeGroupJob* job = new RemoveVolumeGroupJob( device ); - - deviceInfo->jobs << Calamares::job_ptr( job ); - + deviceInfo->makeJob< RemoveVolumeGroupJob >( device ); refreshAfterModelChange(); } @@ -482,7 +462,7 @@ PartitionCoreModule::deletePartition( Device* device, Partition* partition ) } } - Calamares::JobList& jobs = deviceInfo->jobs; + const Calamares::JobList& jobs = deviceInfo->jobs(); if ( partition->state() == KPM_PARTITION_STATE( New ) ) { // First remove matching SetPartFlagsJobs @@ -537,9 +517,8 @@ PartitionCoreModule::deletePartition( Device* device, Partition* partition ) ++it; } } - DeletePartitionJob* job = new DeletePartitionJob( device, partition ); - job->updatePreview(); - jobs << Calamares::job_ptr( job ); + + deviceInfo->makeJob< DeletePartitionJob >( partition ); } } @@ -549,9 +528,7 @@ PartitionCoreModule::formatPartition( Device* device, Partition* partition ) auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); - - FormatPartitionJob* job = new FormatPartitionJob( device, partition ); - deviceInfo->jobs << Calamares::job_ptr( job ); + deviceInfo->makeJob< FormatPartitionJob >( partition ); } void @@ -560,10 +537,7 @@ PartitionCoreModule::resizePartition( Device* device, Partition* partition, qint auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper helper( partitionModelForDevice( device ), this ); - - ResizePartitionJob* job = new ResizePartitionJob( device, partition, first, last ); - job->updatePreview(); - deviceInfo->jobs << Calamares::job_ptr( job ); + deviceInfo->makeJob< ResizePartitionJob >( partition, first, last ); } void @@ -572,9 +546,7 @@ PartitionCoreModule::setPartitionFlags( Device* device, Partition* partition, Pa auto* deviceInfo = infoForDevice( device ); Q_ASSERT( deviceInfo ); OperationHelper( partitionModelForDevice( device ), this ); - - SetPartFlagsJob* job = new SetPartFlagsJob( device, partition, flags ); - deviceInfo->jobs << Calamares::job_ptr( job ); + deviceInfo->makeJob< SetPartFlagsJob >( partition, flags ); PartitionInfo::setFlags( partition, flags ); } @@ -607,7 +579,7 @@ PartitionCoreModule::jobs() const for ( auto info : m_deviceInfos ) { - lst << info->jobs; + lst << info->jobs(); devices << info->device.data(); } lst << Calamares::job_ptr( new FillGlobalStorageJob( devices, m_bootLoaderInstallPath ) ); @@ -661,7 +633,7 @@ PartitionCoreModule::dumpQueue() const for ( auto info : m_deviceInfos ) { cDebug() << "## Device:" << info->device->name(); - for ( auto job : info->jobs ) + for ( const auto& job : info->jobs() ) { cDebug() << "-" << job->prettyName(); } @@ -801,7 +773,7 @@ PartitionCoreModule::scanForLVMPVs() for ( DeviceInfo* d : m_deviceInfos ) { - for ( auto job : d->jobs ) + for ( const auto& job : d->jobs() ) { // Including new LVM PVs CreatePartitionJob* partJob = dynamic_cast< CreatePartitionJob* >( job.data() ); @@ -1028,9 +1000,9 @@ PartitionCoreModule::revertAllDevices() { ( *it )->isAvailable = true; - if ( !( *it )->jobs.empty() ) + if ( !( *it )->jobs().empty() ) { - CreateVolumeGroupJob* vgJob = dynamic_cast< CreateVolumeGroupJob* >( ( *it )->jobs[ 0 ].data() ); + CreateVolumeGroupJob* vgJob = dynamic_cast< CreateVolumeGroupJob* >( ( *it )->jobs().first().data() ); if ( vgJob ) { From 17914b9cf9be83c7ff962fff2ac6a2a07731621b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Sep 2020 16:15:09 +0200 Subject: [PATCH 07/26] CI: adjust to clang-format-10 automatically - leave clang-format file alone, but dynamically modify it when clang-format 10 or later is present - ignore the resulting .bak file --- .gitignore | 1 + ci/calamaresstyle | 12 +++++++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 5bf3c57ca..4023c2c49 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,7 @@ CMakeLists.txt.user # Backup files *~ +*.bak # Kate *.kate-swp diff --git a/ci/calamaresstyle b/ci/calamaresstyle index d2ce360bb..bd715eee1 100755 --- a/ci/calamaresstyle +++ b/ci/calamaresstyle @@ -35,7 +35,13 @@ test -n "$CF" || { echo "! No clang-format ($CF_VERSIONS) found in PATH"; exit 1 test -x "$AS" || { echo "! $AS is not executable."; exit 1 ; } test -x "$CF" || { echo "! $CF is not executable."; exit 1 ; } -expr `"$CF" --version | tr -dc '[^.0-9]' | cut -d . -f 1` '<' 10 > /dev/null || { echo "! $CF is version 10 or later, needs different .clang-format" ; exit 1 ; } +unmangle_clang_format="" +if expr `"$CF" --version | tr -dc '[^.0-9]' | cut -d . -f 1` '<' 10 > /dev/null ; then + : +else + unmangle_clang_format=$( dirname $0 )/../.clang-format + echo "SpaceInEmptyBlock: false" >> "$unmangle_clang_format" +fi set -e @@ -65,3 +71,7 @@ if test "x$any_dirs" = "xyes" ; then else style_some "$@" fi + +if test -n "$unmangle_clang_format" ; then + sed -i.bak '/^SpaceInEmptyBlock/d' "$unmangle_clang_format" +fi From 1f7744133382a195cdc2e5492fc150e974ba2595 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Sep 2020 16:36:43 +0200 Subject: [PATCH 08/26] [partition] add job-removal to the support classes --- .../partition/core/PartitionCoreModule.cpp | 99 +++++++++++-------- 1 file changed, 59 insertions(+), 40 deletions(-) diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index b4cd201b4..653df9c3b 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -99,23 +99,26 @@ private: //- DeviceInfo --------------------------------------------- // Some jobs have an updatePreview some don't -DECLARE_HAS_METHOD(updatePreview) +DECLARE_HAS_METHOD( updatePreview ) -template< typename Job > -void updatePreview( Job* job, const std::true_type& ) +template < typename Job > +void +updatePreview( Job* job, const std::true_type& ) { job->updatePreview(); } -template< typename Job > -void updatePreview( Job* job, const std::false_type& ) +template < typename Job > +void +updatePreview( Job* job, const std::false_type& ) { } -template< typename Job > -void updatePreview( Job* job ) +template < typename Job > +void +updatePreview( Job* job ) { - updatePreview(job, has_updatePreview{}); + updatePreview( job, has_updatePreview< Job > {} ); } /** @@ -137,8 +140,35 @@ struct PartitionCoreModule::DeviceInfo const Calamares::JobList& jobs() const { return m_jobs; } - template< typename Job, typename... Args > - Calamares::Job* makeJob(Args... a) + /** @brief Take the jobs of the given type that apply to @p partition + * + * Returns a job pointer to the job that has just been removed. + */ + template < typename Job > + Calamares::job_ptr takeJob( Partition* partition ) + { + for ( auto it = m_jobs.begin(); it != m_jobs.end(); ) + { + Job* job = qobject_cast< Job* >( it->data() ); + if ( job && job->partition() == partition ) + { + Calamares::job_ptr p = *it; + it = m_jobs.erase( it ); + return p; + } + else + { + ++it; + } + } + + return Calamares::job_ptr( nullptr ); + } + + /** @brief Add a job of given type to the job list + */ + template < typename Job, typename... Args > + Calamares::Job* makeJob( Args... a ) { auto* job = new Job( device.get(), a... ); updatePreview( job ); @@ -456,26 +486,20 @@ PartitionCoreModule::deletePartition( Device* device, Partition* partition ) const Calamares::JobList& jobs = deviceInfo->jobs(); if ( partition->state() == KPM_PARTITION_STATE( New ) ) { - // First remove matching SetPartFlagsJobs - for ( auto it = jobs.begin(); it != jobs.end(); ) + // Take all the SetPartFlagsJob from the list and delete them + do { - SetPartFlagsJob* job = qobject_cast< SetPartFlagsJob* >( it->data() ); - if ( job && job->partition() == partition ) + auto job_ptr = deviceInfo->takeJob< SetPartFlagsJob >( partition ); + if ( job_ptr.data() ) { - it = jobs.erase( it ); + continue; } - else - { - ++it; - } - } + } while ( false ); + // Find matching CreatePartitionJob - auto it = std::find_if( jobs.begin(), jobs.end(), [partition]( Calamares::job_ptr job ) { - CreatePartitionJob* createJob = qobject_cast< CreatePartitionJob* >( job.data() ); - return createJob && createJob->partition() == partition; - } ); - if ( it == jobs.end() ) + auto job_ptr = deviceInfo->takeJob< CreatePartitionJob >( partition ); + if ( !job_ptr.data() ) { cDebug() << "Failed to find a CreatePartitionJob matching the partition to remove"; return; @@ -488,7 +512,6 @@ PartitionCoreModule::deletePartition( Device* device, Partition* partition ) } device->partitionTable()->updateUnallocated( *device ); - jobs.erase( it ); // The partition is no longer referenced by either a job or the device // partition list, so we have to delete it delete partition; @@ -496,18 +519,14 @@ PartitionCoreModule::deletePartition( Device* device, Partition* partition ) else { // Remove any PartitionJob on this partition - for ( auto it = jobs.begin(); it != jobs.end(); ) + do { - PartitionJob* job = qobject_cast< PartitionJob* >( it->data() ); - if ( job && job->partition() == partition ) + auto job_ptr = deviceInfo->takeJob< PartitionJob >( partition ); + if ( job_ptr.data() ) { - it = jobs.erase( it ); + continue; } - else - { - ++it; - } - } + } while ( false ); deviceInfo->makeJob< DeletePartitionJob >( partition ); } @@ -599,7 +618,7 @@ PartitionCoreModule::lvmPVs() const bool PartitionCoreModule::hasVGwithThisName( const QString& name ) const { - auto condition = [name]( DeviceInfo* d ) { + auto condition = [ name ]( DeviceInfo* d ) { return dynamic_cast< LvmDevice* >( d->device.data() ) && d->device.data()->name() == name; }; @@ -609,7 +628,7 @@ PartitionCoreModule::hasVGwithThisName( const QString& name ) const bool PartitionCoreModule::isInVG( const Partition* partition ) const { - auto condition = [partition]( DeviceInfo* d ) { + auto condition = [ partition ]( DeviceInfo* d ) { LvmDevice* vg = dynamic_cast< LvmDevice* >( d->device.data() ); return vg && vg->physicalVolumes().contains( partition ); }; @@ -942,9 +961,9 @@ PartitionCoreModule::layoutApply( Device* dev, const QString boot = QStringLiteral( "/boot" ); const QString root = QStringLiteral( "/" ); const auto is_boot - = [&]( Partition* p ) -> bool { return PartitionInfo::mountPoint( p ) == boot || p->mountPoint() == boot; }; + = [ & ]( Partition* p ) -> bool { return PartitionInfo::mountPoint( p ) == boot || p->mountPoint() == boot; }; const auto is_root - = [&]( Partition* p ) -> bool { return PartitionInfo::mountPoint( p ) == root || p->mountPoint() == root; }; + = [ & ]( Partition* p ) -> bool { return PartitionInfo::mountPoint( p ) == root || p->mountPoint() == root; }; const bool separate_boot_partition = std::find_if( partList.constBegin(), partList.constEnd(), is_boot ) != partList.constEnd(); @@ -1059,7 +1078,7 @@ void PartitionCoreModule::asyncRevertDevice( Device* dev, std::function< void() > callback ) { QFutureWatcher< void >* watcher = new QFutureWatcher< void >(); - connect( watcher, &QFutureWatcher< void >::finished, this, [watcher, callback] { + connect( watcher, &QFutureWatcher< void >::finished, this, [ watcher, callback ] { callback(); watcher->deleteLater(); } ); From e37c7da60dde2f9daf1e819fd11f54fa194bed18 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 21 Sep 2020 16:46:24 +0200 Subject: [PATCH 09/26] [partition] Introduce dummy argument to LVM jobs - Give LVM jobs a dummy argument Device* so that they fit the functionality of makeJob for partitioning. For those jobs that already take an LVMDevice*, this should be the self-same device, but that isn't checked. --- src/modules/partition/jobs/CreateVolumeGroupJob.cpp | 5 ++++- src/modules/partition/jobs/CreateVolumeGroupJob.h | 3 ++- src/modules/partition/jobs/RemoveVolumeGroupJob.cpp | 2 +- src/modules/partition/jobs/RemoveVolumeGroupJob.h | 3 ++- src/modules/partition/jobs/ResizeVolumeGroupJob.cpp | 2 +- src/modules/partition/jobs/ResizeVolumeGroupJob.h | 3 ++- 6 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/modules/partition/jobs/CreateVolumeGroupJob.cpp b/src/modules/partition/jobs/CreateVolumeGroupJob.cpp index af9997df6..36d79b7b7 100644 --- a/src/modules/partition/jobs/CreateVolumeGroupJob.cpp +++ b/src/modules/partition/jobs/CreateVolumeGroupJob.cpp @@ -15,7 +15,10 @@ #include #include -CreateVolumeGroupJob::CreateVolumeGroupJob( QString& vgName, QVector< const Partition* > pvList, const qint32 peSize ) +CreateVolumeGroupJob::CreateVolumeGroupJob( Device*, + QString& vgName, + QVector< const Partition* > pvList, + const qint32 peSize ) : m_vgName( vgName ) , m_pvList( pvList ) , m_peSize( peSize ) diff --git a/src/modules/partition/jobs/CreateVolumeGroupJob.h b/src/modules/partition/jobs/CreateVolumeGroupJob.h index e9682043c..987c937c6 100644 --- a/src/modules/partition/jobs/CreateVolumeGroupJob.h +++ b/src/modules/partition/jobs/CreateVolumeGroupJob.h @@ -15,13 +15,14 @@ #include +class Device; class Partition; class CreateVolumeGroupJob : public Calamares::Job { Q_OBJECT public: - CreateVolumeGroupJob( QString& vgName, QVector< const Partition* > pvList, const qint32 peSize ); + CreateVolumeGroupJob( Device*, QString& vgName, QVector< const Partition* > pvList, const qint32 peSize ); QString prettyName() const override; QString prettyDescription() const override; diff --git a/src/modules/partition/jobs/RemoveVolumeGroupJob.cpp b/src/modules/partition/jobs/RemoveVolumeGroupJob.cpp index a3b5b8d73..3c4e7b036 100644 --- a/src/modules/partition/jobs/RemoveVolumeGroupJob.cpp +++ b/src/modules/partition/jobs/RemoveVolumeGroupJob.cpp @@ -13,7 +13,7 @@ #include #include -RemoveVolumeGroupJob::RemoveVolumeGroupJob( LvmDevice* device ) +RemoveVolumeGroupJob::RemoveVolumeGroupJob( Device*, LvmDevice* device ) : m_device( device ) { } diff --git a/src/modules/partition/jobs/RemoveVolumeGroupJob.h b/src/modules/partition/jobs/RemoveVolumeGroupJob.h index 03f52135b..8582e3635 100644 --- a/src/modules/partition/jobs/RemoveVolumeGroupJob.h +++ b/src/modules/partition/jobs/RemoveVolumeGroupJob.h @@ -13,13 +13,14 @@ #include "Job.h" #include "partition/KPMManager.h" +class Device; class LvmDevice; class RemoveVolumeGroupJob : public Calamares::Job { Q_OBJECT public: - RemoveVolumeGroupJob( LvmDevice* device ); + RemoveVolumeGroupJob( Device*, LvmDevice* device ); QString prettyName() const override; QString prettyDescription() const override; diff --git a/src/modules/partition/jobs/ResizeVolumeGroupJob.cpp b/src/modules/partition/jobs/ResizeVolumeGroupJob.cpp index 0c017877e..1aa4541b8 100644 --- a/src/modules/partition/jobs/ResizeVolumeGroupJob.cpp +++ b/src/modules/partition/jobs/ResizeVolumeGroupJob.cpp @@ -15,7 +15,7 @@ #include #include -ResizeVolumeGroupJob::ResizeVolumeGroupJob( LvmDevice* device, QVector< const Partition* >& partitionList ) +ResizeVolumeGroupJob::ResizeVolumeGroupJob( Device*, LvmDevice* device, QVector< const Partition* >& partitionList ) : m_device( device ) , m_partitionList( partitionList ) { diff --git a/src/modules/partition/jobs/ResizeVolumeGroupJob.h b/src/modules/partition/jobs/ResizeVolumeGroupJob.h index 9e3f038c2..bb3e09d75 100644 --- a/src/modules/partition/jobs/ResizeVolumeGroupJob.h +++ b/src/modules/partition/jobs/ResizeVolumeGroupJob.h @@ -15,6 +15,7 @@ #include +class Device; class LvmDevice; class Partition; @@ -22,7 +23,7 @@ class ResizeVolumeGroupJob : public Calamares::Job { Q_OBJECT public: - ResizeVolumeGroupJob( LvmDevice* device, QVector< const Partition* >& partitionList ); + ResizeVolumeGroupJob( Device*, LvmDevice* device, QVector< const Partition* >& partitionList ); QString prettyName() const override; QString prettyDescription() const override; From f155c8351b8ec78a97b1fc33df0f1421fbda4ce4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 28 Sep 2020 14:48:55 +0200 Subject: [PATCH 10/26] [partition] Only one setting for partitionLayout is supported --- src/modules/partition/gui/PartitionViewStep.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 1b70124dd..0fe8f2466 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -597,7 +597,7 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) if ( configurationMap.contains( "partitionLayout" ) ) { - m_core->initLayout( configurationMap.values( "partitionLayout" ).at( 0 ).toList() ); + m_core->initLayout( configurationMap.value( "partitionLayout" ).toList() ); } else { From 3bb5adcfca74b5d7fc5220307e4f54ce8168a463 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 28 Sep 2020 14:52:18 +0200 Subject: [PATCH 11/26] [partition] Simplify *efiSystemPartition* settings --- src/modules/partition/gui/PartitionViewStep.cpp | 6 +----- src/modules/partition/partition.conf | 2 ++ 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 0fe8f2466..6138498e3 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -523,11 +523,7 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) // Copy the efiSystemPartition setting to the global storage. It is needed not only in // the EraseDiskPage, but also in the bootloader configuration modules (grub, bootloader). Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); - QString efiSP = CalamaresUtils::getString( configurationMap, "efiSystemPartition" ); - if ( efiSP.isEmpty() ) - { - efiSP = QStringLiteral( "/boot/efi" ); - } + QString efiSP = CalamaresUtils::getString( configurationMap, "efiSystemPartition", QStringLiteral( "/boot/efi" ) ); gs->insert( "efiSystemPartition", efiSP ); // Set up firmwareType global storage entry. This is used, e.g. by the bootloader module. diff --git a/src/modules/partition/partition.conf b/src/modules/partition/partition.conf index 5f9842394..007d7b36e 100644 --- a/src/modules/partition/partition.conf +++ b/src/modules/partition/partition.conf @@ -4,6 +4,8 @@ # This setting specifies the mount point of the EFI system partition. Some # distributions (Fedora, Debian, Manjaro, etc.) use /boot/efi, others (KaOS, # etc.) use just /boot. +# +# Defaults to "/boot/efi", may be empty (but weird effects ensue) efiSystemPartition: "/boot/efi" # This optional setting specifies the size of the EFI system partition. From 9f0f600aa4e73af7b63232b62a853700dae67415 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 28 Sep 2020 14:53:38 +0200 Subject: [PATCH 12/26] [partition] Remove the 'swapfile-unsupported' message --- src/modules/partition/core/Config.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index 6dcad2d66..686d76716 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -100,7 +100,6 @@ getSwapChoices( const QVariantMap& configurationMap ) choices.remove( x ); \ } - COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::SwapFile ) COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::ReuseSwap ) #undef COMPLAIN_UNSUPPORTED From b518ef7dfe069feab18b9d8e9a6ef9b7c057b742 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 28 Sep 2020 15:32:47 +0200 Subject: [PATCH 13/26] [partition] Select initial swap choice --- src/modules/partition/core/Config.cpp | 7 +++++++ src/modules/partition/core/Config.h | 12 +++++++++--- src/modules/partition/gui/ChoicePage.cpp | 2 +- src/modules/partition/partition.conf | 11 +++++++++-- src/modules/partition/partition.schema.yaml | 1 + 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index 686d76716..8907a978a 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -116,6 +116,13 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) bool nameFound = false; // In the name table (ignored, falls back to first entry in table) m_initialInstallChoice = PartitionActions::Choices::installChoiceNames().find( CalamaresUtils::getString( configurationMap, "initialPartitioningChoice" ), nameFound ); + m_initialSwapChoice = PartitionActions::Choices::swapChoiceNames().find( + CalamaresUtils::getString( configurationMap, "initialSwapChoice" ), nameFound ); + if ( !m_swapChoices.contains( m_initialSwapChoice ) ) + { + cWarning() << "Configuration for *initialSwapChoice* is not one of the *userSwapChoices*"; + m_initialSwapChoice = PartitionActions::Choices::pickOne( m_swapChoices ); + } } void diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index f2ba9cf58..d338dd57b 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -28,14 +28,20 @@ public: PartitionActions::Choices::SwapChoiceSet swapChoices() const { return m_swapChoices; } - /** - * @brief What kind of installation (partitioning) is requested **initially**? + /** @brief What kind of installation (partitioning) is requested **initially**? * - * @return the partitioning choice (may by @c NoChoice) + * @return the partitioning choice (may be @c NoChoice) */ PartitionActions::Choices::InstallChoice initialInstallChoice() const { return m_initialInstallChoice; } + /** @brief What kind of swap selection is requested **initially**? + * + * @return The swap choice (may be @c NoSwap ) + */ + PartitionActions::Choices::SwapChoice initialSwapChoice() const { return m_initialSwapChoice; } + private: + PartitionActions::Choices::SwapChoice m_initialSwapChoice; PartitionActions::Choices::SwapChoiceSet m_swapChoices; PartitionActions::Choices::InstallChoice m_initialInstallChoice = PartitionActions::Choices::NoChoice; qreal m_requiredStorageGiB = 0.0; // May duplicate setting in the welcome module diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 2e965ad93..cb263f32e 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -86,7 +86,7 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) , m_lastSelectedDeviceIndex( -1 ) , m_enableEncryptionWidget( true ) , m_availableSwapChoices( config->swapChoices() ) - , m_eraseSwapChoice( PartitionActions::Choices::pickOne( m_availableSwapChoices ) ) + , m_eraseSwapChoice( config->initialSwapChoice() ) , m_allowManualPartitioning( true ) { setupUi( this ); diff --git a/src/modules/partition/partition.conf b/src/modules/partition/partition.conf index 007d7b36e..276ee3458 100644 --- a/src/modules/partition/partition.conf +++ b/src/modules/partition/partition.conf @@ -74,8 +74,15 @@ alwaysShowPartitionLabels: true # # The default is "none" # -# TODO: this isn't implemented -# initialPartitioningChoice: none +initialPartitioningChoice: none +# +# Similarly, some of the installation choices may offer a choice of swap; +# the available choices depend on *userSwapChoices*, above, and this +# setting can be used to pick a specific one. +# +# The default is "none" (no swap) if that is one of the enabled options, otherwise +# one of the items from the options. +initialSwapChoice: none # Default filesystem type, used when a "new" partition is made. # diff --git a/src/modules/partition/partition.schema.yaml b/src/modules/partition/partition.schema.yaml index 770b8a645..16cc08319 100644 --- a/src/modules/partition/partition.schema.yaml +++ b/src/modules/partition/partition.schema.yaml @@ -22,6 +22,7 @@ properties: allowManualPartitioning: { type: boolean, default: true } partitionLayout: { type: array } # TODO: specify items initialPartitioningChoice: { type: string, enum: [ none, erase, replace, alongside, manual ] } + initialSwapChoice: { type: string, enum: [ none, small, suspend, reuse, file ] } requiredStorage: { type: number } required: From a92cb32cef99cba9cea53e5fa36732f8c6dab210 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 28 Sep 2020 17:46:42 +0200 Subject: [PATCH 14/26] [partition] set the right buttons if an action is pre-selected --- src/modules/partition/gui/ChoicePage.cpp | 78 ++++++++++++++++++------ src/modules/partition/gui/ChoicePage.h | 4 +- 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index cb263f32e..b41e7bfa2 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -83,7 +83,6 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) , m_beforePartitionBarsView( nullptr ) , m_beforePartitionLabelsView( nullptr ) , m_bootloaderComboBox( nullptr ) - , m_lastSelectedDeviceIndex( -1 ) , m_enableEncryptionWidget( true ) , m_availableSwapChoices( config->swapChoices() ) , m_eraseSwapChoice( config->initialSwapChoice() ) @@ -154,7 +153,7 @@ ChoicePage::init( PartitionCoreModule* core ) // We need to do this because a PCM revert invalidates the deviceModel. - connect( core, &PartitionCoreModule::reverted, this, [=] { + connect( core, &PartitionCoreModule::reverted, this, [ = ] { m_drivesCombo->setModel( core->deviceModel() ); m_drivesCombo->setCurrentIndex( m_lastSelectedDeviceIndex ); } ); @@ -275,7 +274,7 @@ ChoicePage::setupChoices() #else auto buttonSignal = &QButtonGroup::idToggled; #endif - connect( m_grp, buttonSignal, this, [this]( int id, bool checked ) { + connect( m_grp, buttonSignal, this, [ this ]( int id, bool checked ) { if ( checked ) // An action was picked. { m_choice = static_cast< InstallChoice >( id ); @@ -339,6 +338,38 @@ ChoicePage::hideButtons() m_somethingElseButton->hide(); } +void +ChoicePage::checkInstallChoiceRadioButton( InstallChoice c ) +{ + QSignalBlocker b( m_grp ); + PrettyRadioButton* button = nullptr; + switch ( c ) + { + case InstallChoice::Alongside: + button = m_alongsideButton; + break; + case InstallChoice::Replace: + button = m_replaceButton; + break; + case InstallChoice::Erase: + button = m_eraseButton; + break; + case InstallChoice::Manual: + button = m_somethingElseButton; + break; + case InstallChoice::NoChoice: + // Nothing + ; + } + + m_grp->setExclusive( false ); + m_eraseButton->setChecked( button == m_eraseButton ); + m_replaceButton->setChecked( button == m_replaceButton ); + m_alongsideButton->setChecked( button == m_alongsideButton ); + m_somethingElseButton->setChecked( button == m_somethingElseButton ); + m_grp->setExclusive( true ); +} + /** * @brief ChoicePage::applyDeviceChoice handler for the selected event of the device @@ -359,11 +390,11 @@ ChoicePage::applyDeviceChoice() if ( m_core->isDirty() ) { ScanningDialog::run( - QtConcurrent::run( [=] { + QtConcurrent::run( [ = ] { QMutexLocker locker( &m_coreMutex ); m_core->revertAllDevices(); } ), - [this] { continueApplyDeviceChoice(); }, + [ this ] { continueApplyDeviceChoice(); }, this ); } else @@ -392,7 +423,14 @@ ChoicePage::continueApplyDeviceChoice() // Preview setup done. Now we show/hide choices as needed. setupActions(); - m_lastSelectedDeviceIndex = m_drivesCombo->currentIndex(); + cDebug() << "Previous device" << m_lastSelectedDeviceIndex << "new device" << m_drivesCombo->currentIndex(); + if ( m_lastSelectedDeviceIndex != m_drivesCombo->currentIndex() ) + { + m_lastSelectedDeviceIndex = m_drivesCombo->currentIndex(); + m_lastSelectedActionIndex = -1; + m_choice = m_config->initialInstallChoice(); + checkInstallChoiceRadioButton( m_choice ); + } emit actionChosen(); emit deviceChosen(); @@ -423,6 +461,8 @@ ChoicePage::onEraseSwapChoiceChanged() void ChoicePage::applyActionChoice( ChoicePage::InstallChoice choice ) { + cDebug() << "Prev" << m_lastSelectedActionIndex << "InstallChoice" << choice + << PartitionActions::Choices::installChoiceNames().find( choice ); m_beforePartitionBarsView->selectionModel()->disconnect( SIGNAL( currentRowChanged( QModelIndex, QModelIndex ) ) ); m_beforePartitionBarsView->selectionModel()->clearSelection(); m_beforePartitionBarsView->selectionModel()->clearCurrentIndex(); @@ -443,11 +483,11 @@ ChoicePage::applyActionChoice( ChoicePage::InstallChoice choice ) if ( m_core->isDirty() ) { ScanningDialog::run( - QtConcurrent::run( [=] { + QtConcurrent::run( [ = ] { QMutexLocker locker( &m_coreMutex ); m_core->revertDevice( selectedDevice() ); } ), - [=] { + [ = ] { PartitionActions::doAutopartition( m_core, selectedDevice(), options ); emit deviceChosen(); }, @@ -464,7 +504,7 @@ ChoicePage::applyActionChoice( ChoicePage::InstallChoice choice ) if ( m_core->isDirty() ) { ScanningDialog::run( - QtConcurrent::run( [=] { + QtConcurrent::run( [ = ] { QMutexLocker locker( &m_coreMutex ); m_core->revertDevice( selectedDevice() ); } ), @@ -484,11 +524,11 @@ ChoicePage::applyActionChoice( ChoicePage::InstallChoice choice ) if ( m_core->isDirty() ) { ScanningDialog::run( - QtConcurrent::run( [=] { + QtConcurrent::run( [ = ] { QMutexLocker locker( &m_coreMutex ); m_core->revertDevice( selectedDevice() ); } ), - [this] { + [ this ] { // We need to reupdate after reverting because the splitter widget is // not a true view. updateActionChoicePreview( currentChoice() ); @@ -724,7 +764,7 @@ ChoicePage::doReplaceSelectedPartition( const QModelIndex& current ) // doReuseHomePartition *after* the device revert, for later use. ScanningDialog::run( QtConcurrent::run( - [this, current]( QString* homePartitionPath, bool doReuseHomePartition ) { + [ this, current ]( QString* homePartitionPath, bool doReuseHomePartition ) { QMutexLocker locker( &m_coreMutex ); if ( m_core->isDirty() ) @@ -805,7 +845,7 @@ ChoicePage::doReplaceSelectedPartition( const QModelIndex& current ) }, homePartitionPath, doReuseHomePartition ), - [=] { + [ = ] { m_reuseHomeCheckBox->setVisible( !homePartitionPath->isEmpty() ); if ( !homePartitionPath->isEmpty() ) m_reuseHomeCheckBox->setText( tr( "Reuse %1 as home partition for %2." ) @@ -955,7 +995,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) connect( m_afterPartitionSplitterWidget, &PartitionSplitterWidget::partitionResized, this, - [this, sizeLabel]( const QString& path, qint64 size, qint64 sizeNext ) { + [ this, sizeLabel ]( const QString& path, qint64 size, qint64 sizeNext ) { Q_UNUSED( path ) sizeLabel->setText( tr( "%1 will be shrunk to %2MiB and a new " @@ -969,7 +1009,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) m_previewAfterFrame->show(); m_previewAfterLabel->show(); - SelectionFilter filter = [this]( const QModelIndex& index ) { + SelectionFilter filter = [ this ]( const QModelIndex& index ) { return PartUtils::canBeResized( static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ) ); }; @@ -1017,7 +1057,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) eraseBootloaderLabel->setText( tr( "Boot loader location:" ) ); m_bootloaderComboBox = createBootloaderComboBox( eraseWidget ); - connect( m_core->bootLoaderModel(), &QAbstractItemModel::modelReset, [this]() { + connect( m_core->bootLoaderModel(), &QAbstractItemModel::modelReset, [ this ]() { if ( !m_bootloaderComboBox.isNull() ) { Calamares::restoreSelectedBootLoader( *m_bootloaderComboBox, m_core->bootLoaderInstallPath() ); @@ -1027,7 +1067,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) m_core, &PartitionCoreModule::deviceReverted, this, - [this]( Device* dev ) { + [ this ]( Device* dev ) { Q_UNUSED( dev ) if ( !m_bootloaderComboBox.isNull() ) { @@ -1058,7 +1098,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) } else { - SelectionFilter filter = [this]( const QModelIndex& index ) { + SelectionFilter filter = [ this ]( const QModelIndex& index ) { return PartUtils::canBeReplaced( static_cast< Partition* >( index.data( PartitionModel::PartitionPtrRole ).value< void* >() ) ); }; @@ -1160,7 +1200,7 @@ ChoicePage::createBootloaderComboBox( QWidget* parent ) bcb->setModel( m_core->bootLoaderModel() ); // When the chosen bootloader device changes, we update the choice in the PCM - connect( bcb, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, [this]( int newIndex ) { + connect( bcb, QOverload< int >::of( &QComboBox::currentIndexChanged ), this, [ this ]( int newIndex ) { QComboBox* bcb = qobject_cast< QComboBox* >( sender() ); if ( bcb ) { diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index 7c364cc1f..87e50ca8a 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -114,6 +114,7 @@ private: bool calculateNextEnabled() const; void updateNextEnabled(); void setupChoices(); + void checkInstallChoiceRadioButton( ChoicePage::InstallChoice choice ); ///< Sets the chosen button to "on" QComboBox* createBootloaderComboBox( QWidget* parentButton ); Device* selectedDevice(); @@ -161,7 +162,8 @@ private: QPointer< QLabel > m_efiLabel; QPointer< QComboBox > m_efiComboBox; - int m_lastSelectedDeviceIndex; + int m_lastSelectedDeviceIndex = -1; + int m_lastSelectedActionIndex = -1; QString m_defaultFsType; bool m_enableEncryptionWidget; From 10d194d693d4d5138f55f45a2c587c1b6678c7ac Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 29 Sep 2020 12:22:50 +0200 Subject: [PATCH 15/26] [partition] Simplify button-selection --- src/modules/partition/gui/ChoicePage.cpp | 29 ++++-------------------- 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index b41e7bfa2..60069cd2b 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -342,31 +342,12 @@ void ChoicePage::checkInstallChoiceRadioButton( InstallChoice c ) { QSignalBlocker b( m_grp ); - PrettyRadioButton* button = nullptr; - switch ( c ) - { - case InstallChoice::Alongside: - button = m_alongsideButton; - break; - case InstallChoice::Replace: - button = m_replaceButton; - break; - case InstallChoice::Erase: - button = m_eraseButton; - break; - case InstallChoice::Manual: - button = m_somethingElseButton; - break; - case InstallChoice::NoChoice: - // Nothing - ; - } - m_grp->setExclusive( false ); - m_eraseButton->setChecked( button == m_eraseButton ); - m_replaceButton->setChecked( button == m_replaceButton ); - m_alongsideButton->setChecked( button == m_alongsideButton ); - m_somethingElseButton->setChecked( button == m_somethingElseButton ); + // If c == InstallChoice::NoChoice none will match and all are deselected + m_eraseButton->setChecked( InstallChoice::Erase == c); + m_replaceButton->setChecked( InstallChoice::Replace == c ); + m_alongsideButton->setChecked( InstallChoice::Alongside == c ); + m_somethingElseButton->setChecked( InstallChoice::Manual == c ); m_grp->setExclusive( true ); } From b41e4624c92c7bbca73deb3aa6f004f16e46ebee Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 29 Sep 2020 14:00:49 +0200 Subject: [PATCH 16/26] [partition] Move 'selected installation option' to Config --- src/modules/partition/core/Config.cpp | 25 +++++++++ src/modules/partition/core/Config.h | 25 +++++++++ src/modules/partition/gui/ChoicePage.cpp | 53 +++++++++---------- src/modules/partition/gui/ChoicePage.h | 9 ---- .../partition/gui/PartitionViewStep.cpp | 16 +++--- 5 files changed, 84 insertions(+), 44 deletions(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index 8907a978a..6b1b59c1b 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -106,6 +106,29 @@ getSwapChoices( const QVariantMap& configurationMap ) return choices; } +void +Config::setInstallChoice( int c ) +{ + if ( ( c < PartitionActions::Choices::InstallChoice::NoChoice ) + || ( c > PartitionActions::Choices::InstallChoice::Manual ) ) + { + cWarning() << "Invalid install choice (int)" << c; + c = PartitionActions::Choices::InstallChoice::NoChoice; + } + setInstallChoice( static_cast< PartitionActions::Choices::InstallChoice >( c ) ); +} + +void +Config::setInstallChoice( PartitionActions::Choices::InstallChoice c ) +{ + if ( c != m_installChoice ) + { + m_installChoice = c; + emit installChoiceChanged( c ); + } +} + + void Config::setConfigurationMap( const QVariantMap& configurationMap ) { @@ -116,6 +139,8 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) bool nameFound = false; // In the name table (ignored, falls back to first entry in table) m_initialInstallChoice = PartitionActions::Choices::installChoiceNames().find( CalamaresUtils::getString( configurationMap, "initialPartitioningChoice" ), nameFound ); + setInstallChoice( m_initialInstallChoice ); + m_initialSwapChoice = PartitionActions::Choices::swapChoiceNames().find( CalamaresUtils::getString( configurationMap, "initialSwapChoice" ), nameFound ); if ( !m_swapChoices.contains( m_initialSwapChoice ) ) diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index d338dd57b..8869ad9be 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -23,6 +23,13 @@ public: Config( QObject* parent ); virtual ~Config() = default; + /** @brief The installation choice (Erase, Alongside, ...) + * + * This is an int because exposing the enum values is slightly complicated + * by the source layout. + */ + Q_PROPERTY( int installChoice READ installChoice WRITE setInstallChoice NOTIFY installChoiceChanged ) + void setConfigurationMap( const QVariantMap& ); void updateGlobalStorage() const; @@ -34,16 +41,34 @@ public: */ PartitionActions::Choices::InstallChoice initialInstallChoice() const { return m_initialInstallChoice; } + /** @brief What kind of installation (partition) is requested **now**? + * + * This changes depending on what the user selects (unlike the initial choice, + * which is fixed by the configuration). + * + * @return the partitioning choice (may be @c NoChoice) + */ + PartitionActions::Choices::InstallChoice installChoice() const { return m_installChoice; } + + /** @brief What kind of swap selection is requested **initially**? * * @return The swap choice (may be @c NoSwap ) */ PartitionActions::Choices::SwapChoice initialSwapChoice() const { return m_initialSwapChoice; } +public Q_SLOTS: + void setInstallChoice( int ); + void setInstallChoice( PartitionActions::Choices::InstallChoice ); + +Q_SIGNALS: + void installChoiceChanged( PartitionActions::Choices::InstallChoice ); + private: PartitionActions::Choices::SwapChoice m_initialSwapChoice; PartitionActions::Choices::SwapChoiceSet m_swapChoices; PartitionActions::Choices::InstallChoice m_initialInstallChoice = PartitionActions::Choices::NoChoice; + PartitionActions::Choices::InstallChoice m_installChoice = PartitionActions::Choices::NoChoice; qreal m_requiredStorageGiB = 0.0; // May duplicate setting in the welcome module }; diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 60069cd2b..1d5f3ea25 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -71,7 +71,6 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) , m_config( config ) , m_nextEnabled( false ) , m_core( nullptr ) - , m_choice( InstallChoice::NoChoice ) , m_isEfi( false ) , m_grp( nullptr ) , m_alongsideButton( nullptr ) @@ -277,7 +276,7 @@ ChoicePage::setupChoices() connect( m_grp, buttonSignal, this, [ this ]( int id, bool checked ) { if ( checked ) // An action was picked. { - m_choice = static_cast< InstallChoice >( id ); + m_config->setInstallChoice( id ); updateNextEnabled(); emit actionChosen(); @@ -287,7 +286,7 @@ ChoicePage::setupChoices() if ( m_grp->checkedButton() == nullptr ) // If no other action is chosen, we must { // set m_choice to NoChoice and reset previews. - m_choice = InstallChoice::NoChoice; + m_config->setInstallChoice( InstallChoice::NoChoice ); updateNextEnabled(); emit actionChosen(); @@ -344,7 +343,7 @@ ChoicePage::checkInstallChoiceRadioButton( InstallChoice c ) QSignalBlocker b( m_grp ); m_grp->setExclusive( false ); // If c == InstallChoice::NoChoice none will match and all are deselected - m_eraseButton->setChecked( InstallChoice::Erase == c); + m_eraseButton->setChecked( InstallChoice::Erase == c ); m_replaceButton->setChecked( InstallChoice::Replace == c ); m_alongsideButton->setChecked( InstallChoice::Alongside == c ); m_somethingElseButton->setChecked( InstallChoice::Manual == c ); @@ -409,8 +408,8 @@ ChoicePage::continueApplyDeviceChoice() { m_lastSelectedDeviceIndex = m_drivesCombo->currentIndex(); m_lastSelectedActionIndex = -1; - m_choice = m_config->initialInstallChoice(); - checkInstallChoiceRadioButton( m_choice ); + m_config->setInstallChoice( m_config->initialInstallChoice() ); + checkInstallChoiceRadioButton( m_config->installChoice() ); } emit actionChosen(); @@ -424,7 +423,7 @@ ChoicePage::onActionChanged() Device* currd = selectedDevice(); if ( currd ) { - applyActionChoice( currentChoice() ); + applyActionChoice( m_config->installChoice() ); } } @@ -512,7 +511,7 @@ ChoicePage::applyActionChoice( ChoicePage::InstallChoice choice ) [ this ] { // We need to reupdate after reverting because the splitter widget is // not a true view. - updateActionChoicePreview( currentChoice() ); + updateActionChoicePreview( m_config->installChoice() ); updateNextEnabled(); }, this ); @@ -585,14 +584,14 @@ void ChoicePage::onEncryptWidgetStateChanged() { EncryptWidget::Encryption state = m_encryptWidget->state(); - if ( m_choice == InstallChoice::Erase ) + if ( m_config->installChoice() == InstallChoice::Erase ) { if ( state == EncryptWidget::Encryption::Confirmed || state == EncryptWidget::Encryption::Disabled ) { - applyActionChoice( m_choice ); + applyActionChoice( m_config->installChoice() ); } } - else if ( m_choice == InstallChoice::Replace ) + else if ( m_config->installChoice() == InstallChoice::Replace ) { if ( m_beforePartitionBarsView && m_beforePartitionBarsView->selectionModel()->currentIndex().isValid() && ( state == EncryptWidget::Encryption::Confirmed || state == EncryptWidget::Encryption::Disabled ) ) @@ -607,7 +606,7 @@ ChoicePage::onEncryptWidgetStateChanged() void ChoicePage::onHomeCheckBoxStateChanged() { - if ( currentChoice() == InstallChoice::Replace + if ( m_config->installChoice() == InstallChoice::Replace && m_beforePartitionBarsView->selectionModel()->currentIndex().isValid() ) { doReplaceSelectedPartition( m_beforePartitionBarsView->selectionModel()->currentIndex() ); @@ -618,12 +617,14 @@ ChoicePage::onHomeCheckBoxStateChanged() void ChoicePage::onLeave() { - if ( m_choice == InstallChoice::Alongside ) + if ( m_config->installChoice() == InstallChoice::Alongside ) { doAlongsideApply(); } - if ( m_isEfi && ( m_choice == InstallChoice::Alongside || m_choice == InstallChoice::Replace ) ) + if ( m_isEfi + && ( m_config->installChoice() == InstallChoice::Alongside + || m_config->installChoice() == InstallChoice::Replace ) ) { QList< Partition* > efiSystemPartitions = m_core->efiSystemPartitions(); if ( efiSystemPartitions.count() == 1 ) @@ -899,7 +900,7 @@ ChoicePage::updateDeviceStatePreview() sm->deleteLater(); } - switch ( m_choice ) + switch ( m_config->installChoice() ) { case InstallChoice::Replace: case InstallChoice::Alongside: @@ -1073,7 +1074,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) m_previewAfterFrame->show(); m_previewAfterLabel->show(); - if ( m_choice == InstallChoice::Erase ) + if ( m_config->installChoice() == InstallChoice::Erase ) { m_selectLabel->hide(); } @@ -1102,7 +1103,9 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) break; } - if ( m_isEfi && ( m_choice == InstallChoice::Alongside || m_choice == InstallChoice::Replace ) ) + if ( m_isEfi + && ( m_config->installChoice() == InstallChoice::Alongside + || m_config->installChoice() == InstallChoice::Replace ) ) { QHBoxLayout* efiLayout = new QHBoxLayout; layout->addLayout( efiLayout ); @@ -1117,7 +1120,7 @@ ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) // Also handle selection behavior on beforeFrame. QAbstractItemView::SelectionMode previewSelectionMode; - switch ( m_choice ) + switch ( m_config->installChoice() ) { case InstallChoice::Replace: case InstallChoice::Alongside: @@ -1457,19 +1460,13 @@ ChoicePage::isNextEnabled() const } -ChoicePage::InstallChoice -ChoicePage::currentChoice() const -{ - return m_choice; -} - bool ChoicePage::calculateNextEnabled() const { bool enabled = false; auto sm_p = m_beforePartitionBarsView ? m_beforePartitionBarsView->selectionModel() : nullptr; - switch ( m_choice ) + switch ( m_config->installChoice() ) { case InstallChoice::NoChoice: cDebug() << "No partitioning choice"; @@ -1495,7 +1492,9 @@ ChoicePage::calculateNextEnabled() const } - if ( m_isEfi && ( m_choice == InstallChoice::Alongside || m_choice == InstallChoice::Replace ) ) + if ( m_isEfi + && ( m_config->installChoice() == InstallChoice::Alongside + || m_config->installChoice() == InstallChoice::Replace ) ) { if ( m_core->efiSystemPartitions().count() == 0 ) { @@ -1504,7 +1503,7 @@ ChoicePage::calculateNextEnabled() const } } - if ( m_choice != InstallChoice::Manual && m_encryptWidget->isVisible() ) + if ( m_config->installChoice() != InstallChoice::Manual && m_encryptWidget->isVisible() ) { switch ( m_encryptWidget->state() ) { diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index 87e50ca8a..4a967f91c 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -72,13 +72,6 @@ public: */ bool isNextEnabled() const; - /** - * @brief currentChoice returns the enum Choice value corresponding to the - * currently selected partitioning mode (with a PrettyRadioButton). - * @return the enum Choice value. - */ - InstallChoice currentChoice() const; - /** * @brief onLeave runs when control passes from this page to another one. */ @@ -139,8 +132,6 @@ private: QMutex m_previewsMutex; - InstallChoice m_choice; - bool m_isEfi; QComboBox* m_drivesCombo; diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 6138498e3..996c1601c 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -140,7 +140,7 @@ PartitionViewStep::createSummaryWidget() const widget->setLayout( mainLayout ); mainLayout->setMargin( 0 ); - ChoicePage::InstallChoice choice = m_choicePage->currentChoice(); + ChoicePage::InstallChoice choice = m_config->installChoice(); QFormLayout* formLayout = new QFormLayout( widget ); const int MARGIN = CalamaresUtils::defaultFontHeight() / 2; @@ -286,7 +286,7 @@ PartitionViewStep::next() { if ( m_choicePage == m_widget->currentWidget() ) { - if ( m_choicePage->currentChoice() == ChoicePage::InstallChoice::Manual ) + if ( m_config->installChoice() == ChoicePage::InstallChoice::Manual ) { if ( !m_manualPartitionPage ) { @@ -301,7 +301,7 @@ PartitionViewStep::next() m_manualPartitionPage->onRevertClicked(); } } - cDebug() << "Choice applied: " << m_choicePage->currentChoice(); + cDebug() << "Choice applied: " << m_config->installChoice(); } } @@ -368,9 +368,9 @@ PartitionViewStep::isAtEnd() const { if ( m_widget->currentWidget() == m_choicePage ) { - if ( m_choicePage->currentChoice() == ChoicePage::InstallChoice::Erase - || m_choicePage->currentChoice() == ChoicePage::InstallChoice::Replace - || m_choicePage->currentChoice() == ChoicePage::InstallChoice::Alongside ) + auto choice = m_config->installChoice(); + if ( ChoicePage::InstallChoice::Erase == choice || ChoicePage::InstallChoice::Replace == choice + || ChoicePage::InstallChoice::Alongside == choice ) { return true; } @@ -387,7 +387,7 @@ PartitionViewStep::onActivate() // if we're coming back to PVS from the next VS if ( m_widget->currentWidget() == m_choicePage - && m_choicePage->currentChoice() == ChoicePage::InstallChoice::Alongside ) + && m_config->installChoice() == ChoicePage::InstallChoice::Alongside ) { m_choicePage->applyActionChoice( ChoicePage::InstallChoice::Alongside ); // m_choicePage->reset(); @@ -582,7 +582,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; From 010526ee2a2e4bd1d9774612a157a470bb8cf789 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 29 Sep 2020 14:04:12 +0200 Subject: [PATCH 17/26] [partition] Coding style --- .../partition/core/PartitionLayout.cpp | 32 +++++++++---------- .../gui/EditExistingPartitionDialog.cpp | 4 +-- .../partition/gui/PartitionBarsView.cpp | 4 +-- .../partition/gui/PartitionLabelsView.cpp | 2 +- src/modules/partition/gui/PartitionPage.cpp | 8 ++--- .../partition/gui/PartitionSplitterWidget.cpp | 6 ++-- src/modules/partition/gui/ReplaceWidget.cpp | 2 +- src/modules/partition/gui/ScanningDialog.cpp | 2 +- .../partition/gui/VolumeGroupBaseDialog.cpp | 6 ++-- 9 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index eaed27af6..92c3e2bab 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -165,13 +165,13 @@ PartitionLayout::execute( Device* dev, { QList< Partition* > partList; // Map each partition entry to its requested size (0 when calculated later) - QMap< const PartitionLayout::PartitionEntry *, qint64 > partSizeMap; + QMap< const PartitionLayout::PartitionEntry*, qint64 > partSizeMap; qint64 minSize, maxSize, end; qint64 totalSize = lastSector - firstSector + 1; qint64 availableSize = totalSize; // Let's check if we have enough space for each partSize - for( const PartitionLayout::PartitionEntry& part : m_partLayout ) + for ( const PartitionLayout::PartitionEntry& part : m_partLayout ) { qint64 size = -1; // Calculate partition size @@ -179,7 +179,7 @@ PartitionLayout::execute( Device* dev, if ( part.partSize.isValid() ) { // We need to ignore the percent-defined - if ( part.partSize.unit() != CalamaresUtils::Partition::SizeUnit::Percent) + if ( part.partSize.unit() != CalamaresUtils::Partition::SizeUnit::Percent ) { size = part.partSize.toSectors( totalSize, dev->logicalSize() ); } @@ -201,15 +201,15 @@ PartitionLayout::execute( Device* dev, continue; } - partSizeMap.insert (&part, size); + partSizeMap.insert( &part, size ); availableSize -= size; } // Use partMinSize and see if we can do better afterward. - if (availableSize < 0) + if ( availableSize < 0 ) { availableSize = totalSize; - for( const PartitionLayout::PartitionEntry& part : m_partLayout ) + for ( const PartitionLayout::PartitionEntry& part : m_partLayout ) { qint64 size; @@ -219,7 +219,7 @@ PartitionLayout::execute( Device* dev, } else if ( part.partSize.isValid() ) { - if ( part.partSize.unit() != CalamaresUtils::Partition::SizeUnit::Percent) + if ( part.partSize.unit() != CalamaresUtils::Partition::SizeUnit::Percent ) { size = part.partSize.toSectors( totalSize, dev->logicalSize() ); } @@ -233,23 +233,23 @@ PartitionLayout::execute( Device* dev, size = 0; } - partSizeMap.insert (&part, size); + partSizeMap.insert( &part, size ); availableSize -= size; } } // Assign size for percentage-defined partitions - for( const PartitionLayout::PartitionEntry& part : m_partLayout ) + for ( const PartitionLayout::PartitionEntry& part : m_partLayout ) { - if ( part.partSize.unit() == CalamaresUtils::Partition::SizeUnit::Percent) + if ( part.partSize.unit() == CalamaresUtils::Partition::SizeUnit::Percent ) { - qint64 size = partSizeMap.value (&part); + qint64 size = partSizeMap.value( &part ); size = part.partSize.toSectors( availableSize + size, dev->logicalSize() ); - partSizeMap.insert (&part, size); + partSizeMap.insert( &part, size ); if ( part.partMinSize.isValid() ) { qint64 minSize = part.partMinSize.toSectors( totalSize, dev->logicalSize() ); - if (minSize > size) + if ( minSize > size ) { size = minSize; } @@ -257,7 +257,7 @@ PartitionLayout::execute( Device* dev, if ( part.partMaxSize.isValid() ) { qint64 maxSize = part.partMaxSize.toSectors( totalSize, dev->logicalSize() ); - if (maxSize < size) + if ( maxSize < size ) { size = maxSize; } @@ -270,9 +270,9 @@ PartitionLayout::execute( Device* dev, // TODO: Refine partition sizes to make sure there is room for every partition // Use a default (200-500M ?) minimum size for partition without minSize - for( const PartitionLayout::PartitionEntry& part : m_partLayout ) + for ( const PartitionLayout::PartitionEntry& part : m_partLayout ) { - qint64 size = partSizeMap.value (&part); + qint64 size = partSizeMap.value( &part ); Partition* currentPartition = nullptr; // Adjust partition size based on user-defined boundaries and available space diff --git a/src/modules/partition/gui/EditExistingPartitionDialog.cpp b/src/modules/partition/gui/EditExistingPartitionDialog.cpp index 1e66c539c..287a0e488 100644 --- a/src/modules/partition/gui/EditExistingPartitionDialog.cpp +++ b/src/modules/partition/gui/EditExistingPartitionDialog.cpp @@ -64,7 +64,7 @@ EditExistingPartitionDialog::EditExistingPartitionDialog( Device* device, replacePartResizerWidget(); - connect( m_ui->formatRadioButton, &QAbstractButton::toggled, [this]( bool doFormat ) { + connect( m_ui->formatRadioButton, &QAbstractButton::toggled, [ this ]( bool doFormat ) { replacePartResizerWidget(); m_ui->fileSystemLabel->setEnabled( doFormat ); @@ -79,7 +79,7 @@ EditExistingPartitionDialog::EditExistingPartitionDialog( Device* device, } ); connect( - m_ui->fileSystemComboBox, &QComboBox::currentTextChanged, [this]( QString ) { updateMountPointPicker(); } ); + m_ui->fileSystemComboBox, &QComboBox::currentTextChanged, [ this ]( QString ) { updateMountPointPicker(); } ); // File system QStringList fsNames; diff --git a/src/modules/partition/gui/PartitionBarsView.cpp b/src/modules/partition/gui/PartitionBarsView.cpp index 81f518acc..03e06ee64 100644 --- a/src/modules/partition/gui/PartitionBarsView.cpp +++ b/src/modules/partition/gui/PartitionBarsView.cpp @@ -54,7 +54,7 @@ PartitionBarsView::PartitionBarsView( QWidget* parent ) setSelectionMode( QAbstractItemView::SingleSelection ); // Debug - connect( this, &PartitionBarsView::clicked, this, [=]( const QModelIndex& index ) { + connect( this, &PartitionBarsView::clicked, this, [ = ]( const QModelIndex& index ) { cDebug() << "Clicked row" << index.row(); } ); setMouseTracking( true ); @@ -399,7 +399,7 @@ void PartitionBarsView::setSelectionModel( QItemSelectionModel* selectionModel ) { QAbstractItemView::setSelectionModel( selectionModel ); - connect( selectionModel, &QItemSelectionModel::selectionChanged, this, [=] { viewport()->repaint(); } ); + connect( selectionModel, &QItemSelectionModel::selectionChanged, this, [ = ] { viewport()->repaint(); } ); } diff --git a/src/modules/partition/gui/PartitionLabelsView.cpp b/src/modules/partition/gui/PartitionLabelsView.cpp index 7e861d994..1fb5c6f3e 100644 --- a/src/modules/partition/gui/PartitionLabelsView.cpp +++ b/src/modules/partition/gui/PartitionLabelsView.cpp @@ -520,7 +520,7 @@ void PartitionLabelsView::setSelectionModel( QItemSelectionModel* selectionModel ) { QAbstractItemView::setSelectionModel( selectionModel ); - connect( selectionModel, &QItemSelectionModel::selectionChanged, this, [=] { viewport()->repaint(); } ); + connect( selectionModel, &QItemSelectionModel::selectionChanged, this, [ = ] { viewport()->repaint(); } ); } diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index b9930504f..2c2df5b97 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -438,7 +438,7 @@ void PartitionPage::onRevertClicked() { ScanningDialog::run( - QtConcurrent::run( [this] { + QtConcurrent::run( [ this ] { QMutexLocker locker( &m_revertMutex ); int oldIndex = m_ui->deviceComboBox->currentIndex(); @@ -446,7 +446,7 @@ PartitionPage::onRevertClicked() m_ui->deviceComboBox->setCurrentIndex( ( oldIndex < 0 ) ? 0 : oldIndex ); updateFromCurrentDevice(); } ), - [this] { + [ this ] { m_lastSelectedBootLoaderIndex = -1; if ( m_ui->bootLoaderComboBox->currentIndex() < 0 ) { @@ -594,7 +594,7 @@ PartitionPage::updateFromCurrentDevice() m_ui->partitionBarsView->selectionModel(), &QItemSelectionModel::currentChanged, this, - [=] { + [ = ] { QModelIndex selectedIndex = m_ui->partitionBarsView->selectionModel()->currentIndex(); selectedIndex = selectedIndex.sibling( selectedIndex.row(), 0 ); m_ui->partitionBarsView->setCurrentIndex( selectedIndex ); @@ -613,7 +613,7 @@ PartitionPage::updateFromCurrentDevice() // model changes connect( m_ui->partitionTreeView->selectionModel(), &QItemSelectionModel::currentChanged, - [this]( const QModelIndex&, const QModelIndex& ) { updateButtons(); } ); + [ this ]( const QModelIndex&, const QModelIndex& ) { updateButtons(); } ); connect( model, &QAbstractItemModel::modelReset, this, &PartitionPage::onPartitionModelReset ); } diff --git a/src/modules/partition/gui/PartitionSplitterWidget.cpp b/src/modules/partition/gui/PartitionSplitterWidget.cpp index 93c77bb69..0d6ea84d1 100644 --- a/src/modules/partition/gui/PartitionSplitterWidget.cpp +++ b/src/modules/partition/gui/PartitionSplitterWidget.cpp @@ -159,7 +159,7 @@ PartitionSplitterWidget::setSplitPartition( const QString& path, qint64 minSize, m_itemToResizePath.clear(); } - PartitionSplitterItem itemToResize = _findItem( m_items, [path]( PartitionSplitterItem& item ) -> bool { + PartitionSplitterItem itemToResize = _findItem( m_items, [ path ]( PartitionSplitterItem& item ) -> bool { if ( path == item.itemPath ) { item.status = PartitionSplitterItem::Resizing; @@ -184,7 +184,7 @@ PartitionSplitterWidget::setSplitPartition( const QString& path, qint64 minSize, qint64 newSize = m_itemToResize.size - preferredSize; m_itemToResize.size = preferredSize; - int opCount = _eachItem( m_items, [preferredSize]( PartitionSplitterItem& item ) -> bool { + int opCount = _eachItem( m_items, [ preferredSize ]( PartitionSplitterItem& item ) -> bool { if ( item.status == PartitionSplitterItem::Resizing ) { item.size = preferredSize; @@ -358,7 +358,7 @@ PartitionSplitterWidget::mouseMoveEvent( QMouseEvent* event ) m_itemToResize.size = qRound64( span * percent ); m_itemToResizeNext.size -= m_itemToResize.size - oldsize; - _eachItem( m_items, [this]( PartitionSplitterItem& item ) -> bool { + _eachItem( m_items, [ this ]( PartitionSplitterItem& item ) -> bool { if ( item.status == PartitionSplitterItem::Resizing ) { item.size = m_itemToResize.size; diff --git a/src/modules/partition/gui/ReplaceWidget.cpp b/src/modules/partition/gui/ReplaceWidget.cpp index 7e4fb48d6..a316d98b2 100644 --- a/src/modules/partition/gui/ReplaceWidget.cpp +++ b/src/modules/partition/gui/ReplaceWidget.cpp @@ -46,7 +46,7 @@ ReplaceWidget::ReplaceWidget( PartitionCoreModule* core, QComboBox* devicesCombo m_ui->bootStatusLabel->clear(); updateFromCurrentDevice( devicesComboBox ); - connect( devicesComboBox, &QComboBox::currentTextChanged, this, [=]( const QString& /* text */ ) { + connect( devicesComboBox, &QComboBox::currentTextChanged, this, [ = ]( const QString& /* text */ ) { updateFromCurrentDevice( devicesComboBox ); } ); diff --git a/src/modules/partition/gui/ScanningDialog.cpp b/src/modules/partition/gui/ScanningDialog.cpp index cd22bb861..df3d7b082 100644 --- a/src/modules/partition/gui/ScanningDialog.cpp +++ b/src/modules/partition/gui/ScanningDialog.cpp @@ -47,7 +47,7 @@ ScanningDialog::run( const QFuture< void >& future, theDialog->show(); QFutureWatcher< void >* watcher = new QFutureWatcher< void >(); - connect( watcher, &QFutureWatcher< void >::finished, theDialog, [watcher, theDialog, callback] { + connect( watcher, &QFutureWatcher< void >::finished, theDialog, [ watcher, theDialog, callback ] { watcher->deleteLater(); theDialog->hide(); theDialog->deleteLater(); diff --git a/src/modules/partition/gui/VolumeGroupBaseDialog.cpp b/src/modules/partition/gui/VolumeGroupBaseDialog.cpp index 6277c30e5..3043a1c5e 100644 --- a/src/modules/partition/gui/VolumeGroupBaseDialog.cpp +++ b/src/modules/partition/gui/VolumeGroupBaseDialog.cpp @@ -46,17 +46,17 @@ VolumeGroupBaseDialog::VolumeGroupBaseDialog( QString& vgName, QVector< const Pa updateOkButton(); updateTotalSize(); - connect( ui->pvList, &QListWidget::itemChanged, this, [&]( QListWidgetItem* ) { + connect( ui->pvList, &QListWidget::itemChanged, this, [ & ]( QListWidgetItem* ) { updateTotalSize(); updateOkButton(); } ); - connect( ui->peSize, qOverload< int >( &QSpinBox::valueChanged ), this, [&]( int ) { + connect( ui->peSize, qOverload< int >( &QSpinBox::valueChanged ), this, [ & ]( int ) { updateTotalSectors(); updateOkButton(); } ); - connect( ui->vgName, &QLineEdit::textChanged, this, [&]( const QString& ) { updateOkButton(); } ); + connect( ui->vgName, &QLineEdit::textChanged, this, [ & ]( const QString& ) { updateOkButton(); } ); } VolumeGroupBaseDialog::~VolumeGroupBaseDialog() From 881661e94bec05031ecd050c860a65eae8ef77b3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Oct 2020 12:08:42 +0200 Subject: [PATCH 18/26] [partition] Migrate InstallChoice to the Config object --- src/modules/partition/core/Config.cpp | 24 +++++++++---- src/modules/partition/core/Config.h | 34 +++++++++++------- .../partition/core/PartitionActions.cpp | 13 ------- src/modules/partition/core/PartitionActions.h | 10 ------ .../partition/core/PartitionCoreModule.cpp | 4 +-- .../partition/core/PartitionCoreModule.h | 3 +- src/modules/partition/gui/ChoicePage.cpp | 7 ++-- src/modules/partition/gui/ChoicePage.h | 11 +++--- .../partition/gui/PartitionViewStep.cpp | 35 +++++++++---------- .../partition/jobs/FillGlobalStorageJob.cpp | 2 +- .../partition/jobs/FillGlobalStorageJob.h | 4 ++- 11 files changed, 74 insertions(+), 73 deletions(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index 6b1b59c1b..5099b48e6 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -19,6 +19,19 @@ Config::Config( QObject* parent ) { } +const NamedEnumTable< Config::InstallChoice >& +Config::installChoiceNames() +{ + static const NamedEnumTable< InstallChoice > names { { QStringLiteral( "none" ), InstallChoice::NoChoice }, + { QStringLiteral( "nochoice" ), InstallChoice::NoChoice }, + { QStringLiteral( "alongside" ), InstallChoice::Alongside }, + { QStringLiteral( "erase" ), InstallChoice::Erase }, + { QStringLiteral( "replace" ), InstallChoice::Replace }, + { QStringLiteral( "manual" ), InstallChoice::Manual } }; + return names; +} + + static PartitionActions::Choices::SwapChoiceSet getSwapChoices( const QVariantMap& configurationMap ) { @@ -109,17 +122,16 @@ getSwapChoices( const QVariantMap& configurationMap ) void Config::setInstallChoice( int c ) { - if ( ( c < PartitionActions::Choices::InstallChoice::NoChoice ) - || ( c > PartitionActions::Choices::InstallChoice::Manual ) ) + if ( ( c < InstallChoice::NoChoice ) || ( c > InstallChoice::Manual ) ) { cWarning() << "Invalid install choice (int)" << c; - c = PartitionActions::Choices::InstallChoice::NoChoice; + c = InstallChoice::NoChoice; } - setInstallChoice( static_cast< PartitionActions::Choices::InstallChoice >( c ) ); + setInstallChoice( static_cast< InstallChoice >( c ) ); } void -Config::setInstallChoice( PartitionActions::Choices::InstallChoice c ) +Config::setInstallChoice( InstallChoice c ) { if ( c != m_installChoice ) { @@ -137,7 +149,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) m_swapChoices = getSwapChoices( configurationMap ); bool nameFound = false; // In the name table (ignored, falls back to first entry in table) - m_initialInstallChoice = PartitionActions::Choices::installChoiceNames().find( + m_initialInstallChoice = Config::installChoiceNames().find( CalamaresUtils::getString( configurationMap, "initialPartitioningChoice" ), nameFound ); setInstallChoice( m_initialInstallChoice ); diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index 8869ad9be..00f81b897 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -18,17 +18,27 @@ class Config : public QObject { Q_OBJECT - -public: - Config( QObject* parent ); - virtual ~Config() = default; - /** @brief The installation choice (Erase, Alongside, ...) * * This is an int because exposing the enum values is slightly complicated * by the source layout. */ - Q_PROPERTY( int installChoice READ installChoice WRITE setInstallChoice NOTIFY installChoiceChanged ) + Q_PROPERTY( InstallChoice installChoice READ installChoice WRITE setInstallChoice NOTIFY installChoiceChanged ) + +public: + Config( QObject* parent ); + virtual ~Config() = default; + + enum InstallChoice + { + NoChoice, + Alongside, + Erase, + Replace, + Manual + }; + Q_ENUM( InstallChoice ) + static const NamedEnumTable< InstallChoice >& installChoiceNames(); void setConfigurationMap( const QVariantMap& ); void updateGlobalStorage() const; @@ -39,7 +49,7 @@ public: * * @return the partitioning choice (may be @c NoChoice) */ - PartitionActions::Choices::InstallChoice initialInstallChoice() const { return m_initialInstallChoice; } + InstallChoice initialInstallChoice() const { return m_initialInstallChoice; } /** @brief What kind of installation (partition) is requested **now**? * @@ -48,7 +58,7 @@ public: * * @return the partitioning choice (may be @c NoChoice) */ - PartitionActions::Choices::InstallChoice installChoice() const { return m_installChoice; } + InstallChoice installChoice() const { return m_installChoice; } /** @brief What kind of swap selection is requested **initially**? @@ -59,16 +69,16 @@ public: public Q_SLOTS: void setInstallChoice( int ); - void setInstallChoice( PartitionActions::Choices::InstallChoice ); + void setInstallChoice( InstallChoice ); Q_SIGNALS: - void installChoiceChanged( PartitionActions::Choices::InstallChoice ); + void installChoiceChanged( InstallChoice ); private: PartitionActions::Choices::SwapChoice m_initialSwapChoice; PartitionActions::Choices::SwapChoiceSet m_swapChoices; - PartitionActions::Choices::InstallChoice m_initialInstallChoice = PartitionActions::Choices::NoChoice; - PartitionActions::Choices::InstallChoice m_installChoice = PartitionActions::Choices::NoChoice; + InstallChoice m_initialInstallChoice = NoChoice; + InstallChoice m_installChoice = NoChoice; qreal m_requiredStorageGiB = 0.0; // May duplicate setting in the welcome module }; diff --git a/src/modules/partition/core/PartitionActions.cpp b/src/modules/partition/core/PartitionActions.cpp index 748df2d95..168c3804a 100644 --- a/src/modules/partition/core/PartitionActions.cpp +++ b/src/modules/partition/core/PartitionActions.cpp @@ -279,19 +279,6 @@ pickOne( const SwapChoiceSet& s ) return *( s.begin() ); } -const NamedEnumTable< InstallChoice >& -installChoiceNames() -{ - static const NamedEnumTable< InstallChoice > names { { QStringLiteral( "none" ), InstallChoice::NoChoice }, - { QStringLiteral( "nochoice" ), InstallChoice::NoChoice }, - { QStringLiteral( "alongside" ), InstallChoice::Alongside }, - { QStringLiteral( "erase" ), InstallChoice::Erase }, - { QStringLiteral( "replace" ), InstallChoice::Replace }, - { QStringLiteral( "manual" ), InstallChoice::Manual } }; - return names; -} - - } // namespace Choices } // namespace PartitionActions diff --git a/src/modules/partition/core/PartitionActions.h b/src/modules/partition/core/PartitionActions.h index d86e51048..12b0f682e 100644 --- a/src/modules/partition/core/PartitionActions.h +++ b/src/modules/partition/core/PartitionActions.h @@ -47,16 +47,6 @@ const NamedEnumTable< SwapChoice >& swapChoiceNames(); */ SwapChoice pickOne( const SwapChoiceSet& s ); -enum InstallChoice -{ - NoChoice, - Alongside, - Erase, - Replace, - Manual -}; -const NamedEnumTable< InstallChoice >& installChoiceNames(); - struct ReplacePartitionOptions { QString defaultFsType; // e.g. "ext4" or "btrfs" diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp index 653df9c3b..d2c5ad81f 100644 --- a/src/modules/partition/core/PartitionCoreModule.cpp +++ b/src/modules/partition/core/PartitionCoreModule.cpp @@ -561,7 +561,7 @@ PartitionCoreModule::setPartitionFlags( Device* device, Partition* partition, Pa } Calamares::JobList -PartitionCoreModule::jobs() const +PartitionCoreModule::jobs( const Config* config ) const { Calamares::JobList lst; QList< Device* > devices; @@ -592,7 +592,7 @@ PartitionCoreModule::jobs() const lst << info->jobs(); devices << info->device.data(); } - lst << Calamares::job_ptr( new FillGlobalStorageJob( devices, m_bootLoaderInstallPath ) ); + lst << Calamares::job_ptr( new FillGlobalStorageJob( config, devices, m_bootLoaderInstallPath ) ); return lst; } diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h index a51dd0d2d..1dc61db6f 100644 --- a/src/modules/partition/core/PartitionCoreModule.h +++ b/src/modules/partition/core/PartitionCoreModule.h @@ -32,6 +32,7 @@ #include class BootLoaderModel; +class Config; class CreatePartitionJob; class Device; class DeviceModel; @@ -171,7 +172,7 @@ public: * requested by the user. * @return a list of jobs. */ - Calamares::JobList jobs() const; + Calamares::JobList jobs( const Config* ) const; bool hasRootMountPoint() const; diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 1d5f3ea25..ccdd50251 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -59,6 +59,7 @@ using Calamares::PrettyRadioButton; using CalamaresUtils::Partition::findPartitionByPath; using CalamaresUtils::Partition::isPartitionFreeSpace; using CalamaresUtils::Partition::PartitionIterator; +using InstallChoice = Config::InstallChoice; using PartitionActions::Choices::SwapChoice; /** @@ -439,10 +440,10 @@ ChoicePage::onEraseSwapChoiceChanged() } void -ChoicePage::applyActionChoice( ChoicePage::InstallChoice choice ) +ChoicePage::applyActionChoice( InstallChoice choice ) { cDebug() << "Prev" << m_lastSelectedActionIndex << "InstallChoice" << choice - << PartitionActions::Choices::installChoiceNames().find( choice ); + << Config::installChoiceNames().find( choice ); m_beforePartitionBarsView->selectionModel()->disconnect( SIGNAL( currentRowChanged( QModelIndex, QModelIndex ) ) ); m_beforePartitionBarsView->selectionModel()->clearSelection(); m_beforePartitionBarsView->selectionModel()->clearCurrentIndex(); @@ -925,7 +926,7 @@ ChoicePage::updateDeviceStatePreview() * @param choice the chosen partitioning action. */ void -ChoicePage::updateActionChoicePreview( ChoicePage::InstallChoice choice ) +ChoicePage::updateActionChoicePreview( InstallChoice choice ) { Device* currentDevice = selectedDevice(); Q_ASSERT( currentDevice ); diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index 4a967f91c..5c20d817a 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -15,8 +15,9 @@ #include "ui_ChoicePage.h" +#include "core/Config.h" #include "core/OsproberEntry.h" -#include "core/PartitionActions.h" +// #include "core/PartitionActions.h" #include #include @@ -53,8 +54,6 @@ class ChoicePage : public QWidget, private Ui::ChoicePage { Q_OBJECT public: - using InstallChoice = PartitionActions::Choices::InstallChoice; - explicit ChoicePage( Config* config, QWidget* parent = nullptr ); virtual ~ChoicePage(); @@ -81,7 +80,7 @@ public: * @brief applyActionChoice reacts to a choice of partitioning mode. * @param choice the partitioning action choice. */ - void applyActionChoice( ChoicePage::InstallChoice choice ); + void applyActionChoice( Config::InstallChoice choice ); int lastSelectedDeviceIndex(); void setLastSelectedDeviceIndex( int index ); @@ -107,7 +106,7 @@ private: bool calculateNextEnabled() const; void updateNextEnabled(); void setupChoices(); - void checkInstallChoiceRadioButton( ChoicePage::InstallChoice choice ); ///< Sets the chosen button to "on" + void checkInstallChoiceRadioButton( Config::InstallChoice choice ); ///< Sets the chosen button to "on" QComboBox* createBootloaderComboBox( QWidget* parentButton ); Device* selectedDevice(); @@ -117,7 +116,7 @@ private: void continueApplyDeviceChoice(); // .. called after scan void updateDeviceStatePreview(); - void updateActionChoicePreview( ChoicePage::InstallChoice choice ); + void updateActionChoicePreview( Config::InstallChoice choice ); void setupActions(); OsproberEntryList getOsproberEntriesForDevice( Device* device ) const; void doAlongsideApply(); diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 996c1601c..946e567f7 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -140,7 +140,7 @@ PartitionViewStep::createSummaryWidget() const widget->setLayout( mainLayout ); mainLayout->setMargin( 0 ); - ChoicePage::InstallChoice choice = m_config->installChoice(); + Config::InstallChoice choice = m_config->installChoice(); QFormLayout* formLayout = new QFormLayout( widget ); const int MARGIN = CalamaresUtils::defaultFontHeight() / 2; @@ -158,18 +158,18 @@ PartitionViewStep::createSummaryWidget() const QString modeText; switch ( choice ) { - case ChoicePage::InstallChoice::Alongside: + case Config::InstallChoice::Alongside: modeText = tr( "Install %1 alongside another operating system." ) .arg( branding->shortVersionedName() ); break; - case ChoicePage::InstallChoice::Erase: + case Config::InstallChoice::Erase: modeText = tr( "Erase disk and install %1." ).arg( branding->shortVersionedName() ); break; - case ChoicePage::InstallChoice::Replace: + case Config::InstallChoice::Replace: modeText = tr( "Replace a partition with %1." ).arg( branding->shortVersionedName() ); break; - case ChoicePage::InstallChoice::NoChoice: - case ChoicePage::InstallChoice::Manual: + case Config::InstallChoice::NoChoice: + case Config::InstallChoice::Manual: modeText = tr( "Manual partitioning." ); } modeLabel->setText( modeText ); @@ -182,27 +182,27 @@ PartitionViewStep::createSummaryWidget() const QString modeText; switch ( choice ) { - case ChoicePage::InstallChoice::Alongside: + case Config::InstallChoice::Alongside: modeText = tr( "Install %1 alongside another operating system on disk " "%2 (%3)." ) .arg( branding->shortVersionedName() ) .arg( info.deviceNode ) .arg( info.deviceName ); break; - case ChoicePage::InstallChoice::Erase: + case Config::InstallChoice::Erase: modeText = tr( "Erase disk %2 (%3) and install %1." ) .arg( branding->shortVersionedName() ) .arg( info.deviceNode ) .arg( info.deviceName ); break; - case ChoicePage::InstallChoice::Replace: + case Config::InstallChoice::Replace: modeText = tr( "Replace a partition on disk %2 (%3) with %1." ) .arg( branding->shortVersionedName() ) .arg( info.deviceNode ) .arg( info.deviceName ); break; - case ChoicePage::InstallChoice::NoChoice: - case ChoicePage::InstallChoice::Manual: + case Config::InstallChoice::NoChoice: + case Config::InstallChoice::Manual: modeText = tr( "Manual partitioning on disk %1 (%2)." ) .arg( info.deviceNode ) .arg( info.deviceName ); @@ -286,7 +286,7 @@ PartitionViewStep::next() { if ( m_choicePage == m_widget->currentWidget() ) { - if ( m_config->installChoice() == ChoicePage::InstallChoice::Manual ) + if ( m_config->installChoice() == Config::InstallChoice::Manual ) { if ( !m_manualPartitionPage ) { @@ -369,8 +369,8 @@ PartitionViewStep::isAtEnd() const if ( m_widget->currentWidget() == m_choicePage ) { auto choice = m_config->installChoice(); - if ( ChoicePage::InstallChoice::Erase == choice || ChoicePage::InstallChoice::Replace == choice - || ChoicePage::InstallChoice::Alongside == choice ) + if ( Config::InstallChoice::Erase == choice || Config::InstallChoice::Replace == choice + || Config::InstallChoice::Alongside == choice ) { return true; } @@ -386,10 +386,9 @@ PartitionViewStep::onActivate() m_config->updateGlobalStorage(); // if we're coming back to PVS from the next VS - if ( m_widget->currentWidget() == m_choicePage - && m_config->installChoice() == ChoicePage::InstallChoice::Alongside ) + if ( m_widget->currentWidget() == m_choicePage && m_config->installChoice() == Config::InstallChoice::Alongside ) { - m_choicePage->applyActionChoice( ChoicePage::InstallChoice::Alongside ); + m_choicePage->applyActionChoice( Config::InstallChoice::Alongside ); // m_choicePage->reset(); //FIXME: ReplaceWidget should be reset maybe? } @@ -605,7 +604,7 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) Calamares::JobList PartitionViewStep::jobs() const { - return m_core->jobs(); + return m_core->jobs( m_config ); } Calamares::RequirementsList diff --git a/src/modules/partition/jobs/FillGlobalStorageJob.cpp b/src/modules/partition/jobs/FillGlobalStorageJob.cpp index f26b70a97..8e7958e83 100644 --- a/src/modules/partition/jobs/FillGlobalStorageJob.cpp +++ b/src/modules/partition/jobs/FillGlobalStorageJob.cpp @@ -126,7 +126,7 @@ mapForPartition( Partition* partition, const QString& uuid ) return map; } -FillGlobalStorageJob::FillGlobalStorageJob( QList< Device* > devices, const QString& bootLoaderPath ) +FillGlobalStorageJob::FillGlobalStorageJob( const Config*, QList< Device* > devices, const QString& bootLoaderPath ) : m_devices( devices ) , m_bootLoaderPath( bootLoaderPath ) { diff --git a/src/modules/partition/jobs/FillGlobalStorageJob.h b/src/modules/partition/jobs/FillGlobalStorageJob.h index c1256de12..039fb18d8 100644 --- a/src/modules/partition/jobs/FillGlobalStorageJob.h +++ b/src/modules/partition/jobs/FillGlobalStorageJob.h @@ -16,6 +16,7 @@ #include #include +class Config; class Device; class Partition; @@ -30,7 +31,8 @@ class FillGlobalStorageJob : public Calamares::Job { Q_OBJECT public: - FillGlobalStorageJob( QList< Device* > devices, const QString& bootLoaderPath ); + FillGlobalStorageJob( const Config* config, QList< Device* > devices, const QString& bootLoaderPath ); + QString prettyName() const override; QString prettyDescription() const override; QString prettyStatusMessage() const override; From 0f4fe6294c2bfee6974e6f1c53131537bdaf1009 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Oct 2020 12:22:53 +0200 Subject: [PATCH 19/26] [partition] Migrate type for SwapChoice to Config object --- src/modules/partition/core/Config.cpp | 61 ++++++++++++++----- src/modules/partition/core/Config.h | 31 ++++++++-- .../partition/core/PartitionActions.cpp | 44 ++----------- src/modules/partition/core/PartitionActions.h | 26 +------- src/modules/partition/gui/ChoicePage.cpp | 5 +- src/modules/partition/gui/ChoicePage.h | 5 +- 6 files changed, 84 insertions(+), 88 deletions(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index 5099b48e6..d74ae504e 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -31,8 +31,39 @@ Config::installChoiceNames() return names; } +const NamedEnumTable< Config::SwapChoice >& +Config::swapChoiceNames() +{ + static const NamedEnumTable< SwapChoice > names { { QStringLiteral( "none" ), SwapChoice::NoSwap }, + { QStringLiteral( "small" ), SwapChoice::SmallSwap }, + { QStringLiteral( "suspend" ), SwapChoice::FullSwap }, + { QStringLiteral( "reuse" ), SwapChoice::ReuseSwap }, + { QStringLiteral( "file" ), SwapChoice::SwapFile } }; + + return names; +} + +Config::SwapChoice +pickOne( const Config::SwapChoiceSet& s ) +{ + if ( s.count() == 0 ) + { + return Config::SwapChoice::NoSwap; + } + if ( s.count() == 1 ) + { + return *( s.begin() ); + } + if ( s.contains( Config::SwapChoice::NoSwap ) ) + { + return Config::SwapChoice::NoSwap; + } + // Here, count > 1 but NoSwap is not a member. + return *( s.begin() ); +} + -static PartitionActions::Choices::SwapChoiceSet +static Config::SwapChoiceSet getSwapChoices( const QVariantMap& configurationMap ) { // SWAP SETTINGS @@ -57,7 +88,7 @@ getSwapChoices( const QVariantMap& configurationMap ) } bool neverCreateSwap = CalamaresUtils::getBool( configurationMap, "neverCreateSwap", false ); - PartitionActions::Choices::SwapChoiceSet choices; // Available swap choices + Config::SwapChoiceSet choices; // Available swap choices if ( configurationMap.contains( "userSwapChoices" ) ) { // We've already warned about overlapping settings with the @@ -67,7 +98,7 @@ getSwapChoices( const QVariantMap& configurationMap ) for ( const auto& item : l ) { bool ok = false; - auto v = PartitionActions::Choices::swapChoiceNames().find( item, ok ); + auto v = Config::swapChoiceNames().find( item, ok ); if ( ok ) { choices.insert( v ); @@ -77,28 +108,28 @@ getSwapChoices( const QVariantMap& configurationMap ) if ( choices.isEmpty() ) { cWarning() << "Partition-module configuration for *userSwapChoices* is empty:" << l; - choices.insert( PartitionActions::Choices::SwapChoice::FullSwap ); + choices.insert( Config::SwapChoice::FullSwap ); } // suspend if it's one of the possible choices; suppress swap only if it's // the **only** choice available. - ensureSuspendToDisk = choices.contains( PartitionActions::Choices::SwapChoice::FullSwap ); - neverCreateSwap = ( choices.count() == 1 ) && choices.contains( PartitionActions::Choices::SwapChoice::NoSwap ); + ensureSuspendToDisk = choices.contains( Config::SwapChoice::FullSwap ); + neverCreateSwap = ( choices.count() == 1 ) && choices.contains( Config::SwapChoice::NoSwap ); } else { // Convert the legacy settings into a single setting for now. if ( neverCreateSwap ) { - choices.insert( PartitionActions::Choices::SwapChoice::NoSwap ); + choices.insert( Config::SwapChoice::NoSwap ); } else if ( ensureSuspendToDisk ) { - choices.insert( PartitionActions::Choices::SwapChoice::FullSwap ); + choices.insert( Config::SwapChoice::FullSwap ); } else { - choices.insert( PartitionActions::Choices::SwapChoice::SmallSwap ); + choices.insert( Config::SwapChoice::SmallSwap ); } } @@ -109,11 +140,11 @@ getSwapChoices( const QVariantMap& configurationMap ) if ( choices.contains( x ) ) \ { \ bool bogus = false; \ - cWarning() << unsupportedSetting << PartitionActions::Choices::swapChoiceNames().find( x, bogus ); \ + cWarning() << unsupportedSetting << Config::swapChoiceNames().find( x, bogus ); \ choices.remove( x ); \ } - COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::ReuseSwap ) + COMPLAIN_UNSUPPORTED( Config::SwapChoice::ReuseSwap ) #undef COMPLAIN_UNSUPPORTED return choices; @@ -149,16 +180,16 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) m_swapChoices = getSwapChoices( configurationMap ); bool nameFound = false; // In the name table (ignored, falls back to first entry in table) - m_initialInstallChoice = Config::installChoiceNames().find( + m_initialInstallChoice = installChoiceNames().find( CalamaresUtils::getString( configurationMap, "initialPartitioningChoice" ), nameFound ); setInstallChoice( m_initialInstallChoice ); - m_initialSwapChoice = PartitionActions::Choices::swapChoiceNames().find( - CalamaresUtils::getString( configurationMap, "initialSwapChoice" ), nameFound ); + m_initialSwapChoice + = swapChoiceNames().find( CalamaresUtils::getString( configurationMap, "initialSwapChoice" ), nameFound ); if ( !m_swapChoices.contains( m_initialSwapChoice ) ) { cWarning() << "Configuration for *initialSwapChoice* is not one of the *userSwapChoices*"; - m_initialSwapChoice = PartitionActions::Choices::pickOne( m_swapChoices ); + m_initialSwapChoice = pickOne( m_swapChoices ); } } diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index 00f81b897..04c6a0bd0 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -10,7 +10,7 @@ #ifndef PARTITION_CONFIG_H #define PARTITION_CONFIG_H -#include "core/PartitionActions.h" +#include "utils/NamedEnum.h" #include #include @@ -40,10 +40,23 @@ public: Q_ENUM( InstallChoice ) static const NamedEnumTable< InstallChoice >& installChoiceNames(); + /** @brief Choice of swap (size and type) */ + enum SwapChoice + { + NoSwap, // don't create any swap, don't use any + ReuseSwap, // don't create, but do use existing + SmallSwap, // up to 8GiB of swap + FullSwap, // ensureSuspendToDisk -- at least RAM size + SwapFile // use a file (if supported) + }; + Q_ENUM( SwapChoice ) + static const NamedEnumTable< SwapChoice >& swapChoiceNames(); + using SwapChoiceSet = QSet< SwapChoice >; + void setConfigurationMap( const QVariantMap& ); void updateGlobalStorage() const; - PartitionActions::Choices::SwapChoiceSet swapChoices() const { return m_swapChoices; } + SwapChoiceSet swapChoices() const { return m_swapChoices; } /** @brief What kind of installation (partitioning) is requested **initially**? * @@ -65,7 +78,7 @@ public: * * @return The swap choice (may be @c NoSwap ) */ - PartitionActions::Choices::SwapChoice initialSwapChoice() const { return m_initialSwapChoice; } + SwapChoice initialSwapChoice() const { return m_initialSwapChoice; } public Q_SLOTS: void setInstallChoice( int ); @@ -75,12 +88,20 @@ Q_SIGNALS: void installChoiceChanged( InstallChoice ); private: - PartitionActions::Choices::SwapChoice m_initialSwapChoice; - PartitionActions::Choices::SwapChoiceSet m_swapChoices; + SwapChoice m_initialSwapChoice; + SwapChoiceSet m_swapChoices; InstallChoice m_initialInstallChoice = NoChoice; InstallChoice m_installChoice = NoChoice; qreal m_requiredStorageGiB = 0.0; // May duplicate setting in the welcome module }; +/** @brief Given a set of swap choices, return a sensible value from it. + * + * "Sensible" here means: if there is one value, use it; otherwise, use + * NoSwap if there are no choices, or if NoSwap is one of the choices, in the set. + * If that's not possible, any value from the set. + */ +Config::SwapChoice pickOne( const Config::SwapChoiceSet& s ); + #endif diff --git a/src/modules/partition/core/PartitionActions.cpp b/src/modules/partition/core/PartitionActions.cpp index 168c3804a..9f6e63c91 100644 --- a/src/modules/partition/core/PartitionActions.cpp +++ b/src/modules/partition/core/PartitionActions.cpp @@ -35,9 +35,9 @@ using CalamaresUtils::operator""_GiB; using CalamaresUtils::operator""_MiB; qint64 -swapSuggestion( const qint64 availableSpaceB, Choices::SwapChoice swap ) +swapSuggestion( const qint64 availableSpaceB, Config::SwapChoice swap ) { - if ( ( swap != Choices::SmallSwap ) && ( swap != Choices::FullSwap ) ) + if ( ( swap != Config::SwapChoice::SmallSwap ) && ( swap != Config::SwapChoice::FullSwap ) ) { return 0; } @@ -48,7 +48,7 @@ swapSuggestion( const qint64 availableSpaceB, Choices::SwapChoice swap ) qint64 availableRamB = memory.first; qreal overestimationFactor = memory.second; - bool ensureSuspendToDisk = swap == Choices::FullSwap; + bool ensureSuspendToDisk = swap == Config::SwapChoice::FullSwap; // Ramp up quickly to 8GiB, then follow memory size if ( availableRamB <= 4_GiB ) @@ -149,7 +149,8 @@ doAutopartition( PartitionCoreModule* core, Device* dev, Choices::AutoPartitionO core->createPartitionTable( dev, PartitionTable::msdos ); } - const bool mayCreateSwap = ( o.swap == Choices::SmallSwap ) || ( o.swap == Choices::FullSwap ); + const bool mayCreateSwap + = ( o.swap == Config::SwapChoice::SmallSwap ) || ( o.swap == Config::SwapChoice::FullSwap ); bool shouldCreateSwap = false; qint64 suggestedSwapSizeB = 0; @@ -246,39 +247,4 @@ doReplacePartition( PartitionCoreModule* core, Device* dev, Partition* partition core->dumpQueue(); } -namespace Choices -{ -const NamedEnumTable< SwapChoice >& -swapChoiceNames() -{ - static const NamedEnumTable< SwapChoice > names { { QStringLiteral( "none" ), SwapChoice::NoSwap }, - { QStringLiteral( "small" ), SwapChoice::SmallSwap }, - { QStringLiteral( "suspend" ), SwapChoice::FullSwap }, - { QStringLiteral( "reuse" ), SwapChoice::ReuseSwap }, - { QStringLiteral( "file" ), SwapChoice::SwapFile } }; - - return names; -} - -SwapChoice -pickOne( const SwapChoiceSet& s ) -{ - if ( s.count() == 0 ) - { - return SwapChoice::NoSwap; - } - if ( s.count() == 1 ) - { - return *( s.begin() ); - } - if ( s.contains( SwapChoice::NoSwap ) ) - { - return SwapChoice::NoSwap; - } - // Here, count > 1 but NoSwap is not a member. - return *( s.begin() ); -} - -} // namespace Choices - } // namespace PartitionActions diff --git a/src/modules/partition/core/PartitionActions.h b/src/modules/partition/core/PartitionActions.h index 12b0f682e..15b7c1e1e 100644 --- a/src/modules/partition/core/PartitionActions.h +++ b/src/modules/partition/core/PartitionActions.h @@ -10,7 +10,7 @@ #ifndef PARTITIONACTIONS_H #define PARTITIONACTIONS_H -#include "utils/NamedEnum.h" +#include "core/Config.h" #include #include @@ -27,26 +27,6 @@ namespace PartitionActions */ namespace Choices { -/** @brief Choice of swap (size and type) */ -enum SwapChoice -{ - NoSwap, // don't create any swap, don't use any - ReuseSwap, // don't create, but do use existing - SmallSwap, // up to 8GiB of swap - FullSwap, // ensureSuspendToDisk -- at least RAM size - SwapFile // use a file (if supported) -}; -using SwapChoiceSet = QSet< SwapChoice >; -const NamedEnumTable< SwapChoice >& swapChoiceNames(); - -/** @brief Given a set of swap choices, return a sensible value from it. - * - * "Sensible" here means: if there is one value, use it; otherwise, use - * NoSwap if there are no choices, or if NoSwap is one of the choices, in the set. - * If that's not possible, any value from the set. - */ -SwapChoice pickOne( const SwapChoiceSet& s ); - struct ReplacePartitionOptions { QString defaultFsType; // e.g. "ext4" or "btrfs" @@ -63,13 +43,13 @@ struct AutoPartitionOptions : ReplacePartitionOptions { QString efiPartitionMountPoint; // optional, e.g. "/boot" quint64 requiredSpaceB; // estimated required space for root partition - SwapChoice swap; + Config::SwapChoice swap; AutoPartitionOptions( const QString& fs, const QString& luks, const QString& efi, qint64 requiredBytes, - SwapChoice s ) + Config::SwapChoice s ) : ReplacePartitionOptions( fs, luks ) , efiPartitionMountPoint( efi ) , requiredSpaceB( requiredBytes > 0 ? static_cast< quint64 >( requiredBytes ) : 0 ) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index ccdd50251..d8926145b 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -60,7 +60,7 @@ using CalamaresUtils::Partition::findPartitionByPath; using CalamaresUtils::Partition::isPartitionFreeSpace; using CalamaresUtils::Partition::PartitionIterator; using InstallChoice = Config::InstallChoice; -using PartitionActions::Choices::SwapChoice; +using SwapChoice = Config::SwapChoice; /** * @brief ChoicePage::ChoicePage is the default constructor. Called on startup as part of @@ -433,8 +433,7 @@ ChoicePage::onEraseSwapChoiceChanged() { if ( m_eraseSwapChoiceComboBox ) { - m_eraseSwapChoice - = static_cast< PartitionActions::Choices::SwapChoice >( m_eraseSwapChoiceComboBox->currentData().toInt() ); + m_eraseSwapChoice = static_cast< Config::SwapChoice >( m_eraseSwapChoiceComboBox->currentData().toInt() ); onActionChanged(); } } diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index 5c20d817a..b598c92fb 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -17,7 +17,6 @@ #include "core/Config.h" #include "core/OsproberEntry.h" -// #include "core/PartitionActions.h" #include #include @@ -43,7 +42,7 @@ class PartitionCoreModule; class Device; -using SwapChoiceSet = QSet< PartitionActions::Choices::SwapChoice >; +using SwapChoiceSet = Config::SwapChoiceSet; /** * @brief The ChoicePage class is the first page of the partitioning interface. @@ -158,7 +157,7 @@ private: QString m_defaultFsType; bool m_enableEncryptionWidget; SwapChoiceSet m_availableSwapChoices; // What is available - PartitionActions::Choices::SwapChoice m_eraseSwapChoice; // what is selected + Config::SwapChoice m_eraseSwapChoice; // what is selected bool m_allowManualPartitioning; From f79fbd4105ec6ea13310394a05909debd97f1ea6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Oct 2020 12:40:13 +0200 Subject: [PATCH 20/26] [partition] Add swap choice to config object --- src/modules/partition/core/Config.cpp | 21 +++++++++++++++++++++ src/modules/partition/core/Config.h | 26 ++++++++++++++++++-------- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index d74ae504e..8bf093be1 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -171,6 +171,26 @@ Config::setInstallChoice( InstallChoice c ) } } +void +Config::setSwapChoice( int c ) +{ + if ( ( c < SwapChoice::NoSwap ) || ( c > SwapChoice::SwapFile ) ) + { + cWarning() << "Instalid swap choice (int)" << c; + c = SwapChoice::NoSwap; + } + setSwapChoice( static_cast< SwapChoice >( c ) ); +} + +void +Config::setSwapChoice( Config::SwapChoice c ) +{ + if ( c != m_swapChoice ) + { + m_swapChoice = c; + emit swapChoiceChanged( c ); + } +} void Config::setConfigurationMap( const QVariantMap& configurationMap ) @@ -191,6 +211,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) cWarning() << "Configuration for *initialSwapChoice* is not one of the *userSwapChoices*"; m_initialSwapChoice = pickOne( m_swapChoices ); } + setSwapChoice( m_initialSwapChoice ); } void diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index 04c6a0bd0..c949fc77f 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -18,13 +18,12 @@ class Config : public QObject { Q_OBJECT - /** @brief The installation choice (Erase, Alongside, ...) - * - * This is an int because exposing the enum values is slightly complicated - * by the source layout. - */ + ///@brief The installation choice (Erase, Alongside, ...) Q_PROPERTY( InstallChoice installChoice READ installChoice WRITE setInstallChoice NOTIFY installChoiceChanged ) + ///@brief The swap choice (None, Small, Hibernate, ...) which only makes sense when Erase is chosen + Q_PROPERTY( SwapChoice swapChoice READ swapChoice WRITE setSwapChoice NOTIFY swapChoiceChanged ) + public: Config( QObject* parent ); virtual ~Config() = default; @@ -73,23 +72,34 @@ public: */ InstallChoice installChoice() const { return m_installChoice; } - /** @brief What kind of swap selection is requested **initially**? * * @return The swap choice (may be @c NoSwap ) */ SwapChoice initialSwapChoice() const { return m_initialSwapChoice; } + /** @brief What kind of swap selection is requested **now**? + * + * A choice of swap only makes sense when install choice Erase is made. + * + * @return The swap choice (may be @c NoSwap). + */ + SwapChoice swapChoice() const { return m_swapChoice; } + public Q_SLOTS: - void setInstallChoice( int ); + void setInstallChoice( int ); ///< Translates a button ID or so to InstallChoice void setInstallChoice( InstallChoice ); + void setSwapChoice( int ); ///< Translates a button ID or so to SwapChoice + void setSwapChoice( SwapChoice ); Q_SIGNALS: void installChoiceChanged( InstallChoice ); + void swapChoiceChanged( SwapChoice ); private: - SwapChoice m_initialSwapChoice; SwapChoiceSet m_swapChoices; + SwapChoice m_initialSwapChoice = NoSwap; + SwapChoice m_swapChoice = NoSwap; InstallChoice m_initialInstallChoice = NoChoice; InstallChoice m_installChoice = NoChoice; qreal m_requiredStorageGiB = 0.0; // May duplicate setting in the welcome module From 6e30a7b8f6fb2dad31a61df99ce8b97d749128f3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 2 Oct 2020 13:04:12 +0200 Subject: [PATCH 21/26] [partition] Move is-manual-partitioning-allowed to the Config object --- src/modules/partition/core/Config.cpp | 12 ++++++++++++ src/modules/partition/core/Config.h | 5 +++++ src/modules/partition/gui/ChoicePage.cpp | 4 +--- src/modules/partition/gui/ChoicePage.h | 2 -- src/modules/partition/gui/PartitionViewStep.cpp | 2 -- 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index 8bf093be1..d0d6fac11 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -192,6 +192,14 @@ Config::setSwapChoice( Config::SwapChoice c ) } } +bool +Config::allowManualPartitioning() const +{ + Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); + return gs->value( "allowManualPartitioning" ).toBool(); +} + + void Config::setConfigurationMap( const QVariantMap& configurationMap ) { @@ -212,6 +220,10 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) m_initialSwapChoice = pickOne( m_swapChoices ); } setSwapChoice( m_initialSwapChoice ); + + Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); + gs->insert( "allowManualPartitioning", + CalamaresUtils::getBool( configurationMap, "allowManualPartitioning", true ) ); } void diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index c949fc77f..87e0d4047 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -24,6 +24,8 @@ class Config : public QObject ///@brief The swap choice (None, Small, Hibernate, ...) which only makes sense when Erase is chosen Q_PROPERTY( SwapChoice swapChoice READ swapChoice WRITE setSwapChoice NOTIFY swapChoiceChanged ) + Q_PROPERTY( bool allowManualPartitioning READ allowManualPartitioning CONSTANT FINAL ) + public: Config( QObject* parent ); virtual ~Config() = default; @@ -86,6 +88,9 @@ public: */ SwapChoice swapChoice() const { return m_swapChoice; } + ///@brief Is manual partitioning allowed (not explicitly disnabled in the config file)? + bool allowManualPartitioning() const; + public Q_SLOTS: void setInstallChoice( int ); ///< Translates a button ID or so to InstallChoice void setInstallChoice( InstallChoice ); diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index d8926145b..9467720db 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -86,7 +86,6 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) , m_enableEncryptionWidget( true ) , m_availableSwapChoices( config->swapChoices() ) , m_eraseSwapChoice( config->initialSwapChoice() ) - , m_allowManualPartitioning( true ) { setupUi( this ); @@ -94,7 +93,6 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) m_defaultFsType = gs->value( "defaultFileSystemType" ).toString(); m_enableEncryptionWidget = gs->value( "enableLuksAutomatedPartitioning" ).toBool(); - m_allowManualPartitioning = gs->value( "allowManualPartitioning" ).toBool(); if ( FileSystem::typeForName( m_defaultFsType ) == FileSystem::Unknown ) { @@ -1241,7 +1239,7 @@ ChoicePage::setupActions() m_deviceInfoWidget->setPartitionTableType( PartitionTable::unknownTableType ); } - if ( m_allowManualPartitioning ) + if ( m_config->allowManualPartitioning() ) { m_somethingElseButton->show(); } diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index b598c92fb..d79b56c56 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -159,8 +159,6 @@ private: SwapChoiceSet m_availableSwapChoices; // What is available Config::SwapChoice m_eraseSwapChoice; // what is selected - bool m_allowManualPartitioning; - QMutex m_coreMutex; }; diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp index 946e567f7..d0390aa86 100644 --- a/src/modules/partition/gui/PartitionViewStep.cpp +++ b/src/modules/partition/gui/PartitionViewStep.cpp @@ -549,8 +549,6 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap ) CalamaresUtils::getBool( configurationMap, "alwaysShowPartitionLabels", true ) ); gs->insert( "enableLuksAutomatedPartitioning", CalamaresUtils::getBool( configurationMap, "enableLuksAutomatedPartitioning", true ) ); - gs->insert( "allowManualPartitioning", - CalamaresUtils::getBool( configurationMap, "allowManualPartitioning", true ) ); // The defaultFileSystemType setting needs a bit more processing, // as we want to cover various cases (such as different cases) From 92a874dae7ab13587cf7938048ff282a674befc2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Oct 2020 15:44:14 +0200 Subject: [PATCH 22/26] [partition] move the swap-choice to Config --- src/modules/partition/core/Config.h | 10 ++++++++-- src/modules/partition/gui/ChoicePage.cpp | 11 ++++------- src/modules/partition/gui/ChoicePage.h | 2 -- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/modules/partition/core/Config.h b/src/modules/partition/core/Config.h index 87e0d4047..23ebdedf8 100644 --- a/src/modules/partition/core/Config.h +++ b/src/modules/partition/core/Config.h @@ -57,8 +57,6 @@ public: void setConfigurationMap( const QVariantMap& ); void updateGlobalStorage() const; - SwapChoiceSet swapChoices() const { return m_swapChoices; } - /** @brief What kind of installation (partitioning) is requested **initially**? * * @return the partitioning choice (may be @c NoChoice) @@ -74,6 +72,14 @@ public: */ InstallChoice installChoice() const { return m_installChoice; } + /** @brief The set of swap choices enabled for this install + * + * Not all swap choices are supported by each distro, so they + * can choose to enable or disable them. This method + * returns a set (hopefully non-empty) of configured swap choices. + */ + SwapChoiceSet swapChoices() const { return m_swapChoices; } + /** @brief What kind of swap selection is requested **initially**? * * @return The swap choice (may be @c NoSwap ) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index 9467720db..6501a12f6 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -84,8 +84,6 @@ ChoicePage::ChoicePage( Config* config, QWidget* parent ) , m_beforePartitionLabelsView( nullptr ) , m_bootloaderComboBox( nullptr ) , m_enableEncryptionWidget( true ) - , m_availableSwapChoices( config->swapChoices() ) - , m_eraseSwapChoice( config->initialSwapChoice() ) { setupUi( this ); @@ -247,10 +245,9 @@ ChoicePage::setupChoices() m_replaceButton->addToGroup( m_grp, InstallChoice::Replace ); // Fill up swap options - // .. TODO: only if enabled in the config - if ( m_availableSwapChoices.count() > 1 ) + if ( m_config->swapChoices().count() > 1 ) { - m_eraseSwapChoiceComboBox = createCombo( m_availableSwapChoices, m_eraseSwapChoice ); + m_eraseSwapChoiceComboBox = createCombo( m_config->swapChoices(), m_config->swapChoice() ); m_eraseButton->addOptionsComboBox( m_eraseSwapChoiceComboBox ); } @@ -431,7 +428,7 @@ ChoicePage::onEraseSwapChoiceChanged() { if ( m_eraseSwapChoiceComboBox ) { - m_eraseSwapChoice = static_cast< Config::SwapChoice >( m_eraseSwapChoiceComboBox->currentData().toInt() ); + m_config->setSwapChoice( m_eraseSwapChoiceComboBox->currentData().toInt() ); onActionChanged(); } } @@ -456,7 +453,7 @@ ChoicePage::applyActionChoice( InstallChoice choice ) gs->value( "efiSystemPartition" ).toString(), CalamaresUtils::GiBtoBytes( gs->value( "requiredStorageGiB" ).toDouble() ), - m_eraseSwapChoice }; + m_config->swapChoice() }; if ( m_core->isDirty() ) { diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index d79b56c56..cce91e9cc 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -156,8 +156,6 @@ private: QString m_defaultFsType; bool m_enableEncryptionWidget; - SwapChoiceSet m_availableSwapChoices; // What is available - Config::SwapChoice m_eraseSwapChoice; // what is selected QMutex m_coreMutex; }; From 029c3f1c74796877d5a284c1c37a0c447299490a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Oct 2020 15:54:26 +0200 Subject: [PATCH 23/26] [partition] Write the install choices to Global Storage --- src/modules/partition/core/Config.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/modules/partition/core/Config.cpp b/src/modules/partition/core/Config.cpp index d0d6fac11..9f251229e 100644 --- a/src/modules/partition/core/Config.cpp +++ b/src/modules/partition/core/Config.cpp @@ -150,6 +150,19 @@ getSwapChoices( const QVariantMap& configurationMap ) return choices; } +void +updateGlobalStorage( Config::InstallChoice installChoice, Config::SwapChoice swapChoice ) +{ + auto* gs = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; + if ( gs ) + { + QVariantMap m; + m.insert( "install", Config::installChoiceNames().find( installChoice ) ); + m.insert( "swap", Config::swapChoiceNames().find( swapChoice ) ); + gs->insert( "partitionChoices", m ); + } +} + void Config::setInstallChoice( int c ) { @@ -168,6 +181,7 @@ Config::setInstallChoice( InstallChoice c ) { m_installChoice = c; emit installChoiceChanged( c ); + ::updateGlobalStorage( c, m_swapChoice ); } } @@ -189,6 +203,7 @@ Config::setSwapChoice( Config::SwapChoice c ) { m_swapChoice = c; emit swapChoiceChanged( c ); + ::updateGlobalStorage( m_installChoice, c ); } } From 0b3a6baeead7fc60ab19172ee8411f20c50d5c56 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Oct 2020 17:05:22 +0200 Subject: [PATCH 24/26] [fstab] If swap is "file" then create it here - handle swapfiles when writing /etc/fstab in the target system - special-case mountpoint - since swapfiles are not a partition, take the setting out of partitionChoices - create the physical swapfile as well (there's no other place where it would make sense) --- src/modules/fstab/main.py | 52 ++++++++++++++++++++++++++-- src/modules/partition/partition.conf | 4 +++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/src/modules/fstab/main.py b/src/modules/fstab/main.py index 26c357945..7b076c843 100644 --- a/src/modules/fstab/main.py +++ b/src/modules/fstab/main.py @@ -257,7 +257,7 @@ class FstabGenerator(object): if mount_point == "/": check = 1 - elif mount_point: + elif mount_point and mount_point != "swap": check = 2 else: check = 0 @@ -270,8 +270,10 @@ class FstabGenerator(object): if has_luks: device = "/dev/mapper/" + partition["luksMapperName"] - else: + elif partition["uuid"] is not None: device = "UUID=" + partition["uuid"] + else: + device = partition["device"] return dict(device=device, mount_point=mount_point, @@ -307,6 +309,29 @@ class FstabGenerator(object): self.mount_options["default"]) +def create_swapfile(root_mount_point, root_btrfs): + """ + Creates /swapfile in @p root_mount_point ; if the root filesystem + is on btrfs, then handle some btrfs specific features as well, + as documented in + https://wiki.archlinux.org/index.php/Swap#Swap_file + """ + swapfile_path = os.path.join(root_mount_point, "swapfile") + with open(swapfile_path, "wb") as f: + pass + if root_btrfs: + o = subprocess.check_output(["chattr", "+C", swapfile_path]) + libcalamares.utils.debug("swapfile attributes: {!s}".format(o)) + o = subprocess.check_output(["btrfs", "property", "set", swapfile_path, "compression", "none"]) + libcalamares.utils.debug("swapfile compression: {!s}".format(o)) + # Create the swapfile; swapfiles are small-ish + o = subprocess.check_output(["dd", "if=/dev/zero", "of=" + swapfile_path, "bs=1M", "count=512", "conv=notrunc"]) + libcalamares.utils.debug("swapfile dd: {!s}".format(o)) + os.chmod(swapfile_path, 0o600) + o = subprocess.check_output(["mkswap", swapfile_path]) + libcalamares.utils.debug("swapfile mkswap: {!s}".format(o)) + + def run(): """ Configures fstab. @@ -330,6 +355,17 @@ def run(): _("No root mount point is given for
{!s}
to use.") .format("fstab")) + # This follows the GS settings from the partition module's Config object + swap_choice = global_storage.value( "partitionChoices" ) + if swap_choice: + swap_choice = swap_choice.get( "swap", None ) + if swap_choice and swap_choice == "file": + # There's no formatted partition for it, so we'll sneak in an entry + partitions.append( dict(fs="swap", mountPoint=None, claimed=True, device="/swapfile", uuid=None) ) + else: + swap_choice = None + + libcalamares.job.setprogress(0.1) mount_options = conf["mountOptions"] ssd_extra_mount_options = conf.get("ssdExtraMountOptions", {}) crypttab_options = conf.get("crypttabOptions", "luks") @@ -339,4 +375,14 @@ def run(): ssd_extra_mount_options, crypttab_options) - return generator.run() + if swap_choice is not None: + libcalamares.job.setprogress(0.2) + root_partitions = [ p["fs"].lower() for p in partitions if p["mountPoint"] == "/" ] + root_btrfs = (root_partitions[0] == "btrfs") if root_partitions else False + create_swapfile(root_mount_point, root_btrfs) + + try: + libcalamares.job.setprogress(0.5) + return generator.run() + finally: + libcalamares.job.setprogress(1.0) diff --git a/src/modules/partition/partition.conf b/src/modules/partition/partition.conf index 276ee3458..efebe19f4 100644 --- a/src/modules/partition/partition.conf +++ b/src/modules/partition/partition.conf @@ -37,6 +37,10 @@ efiSystemPartition: "/boot/efi" # In both cases, a fudge factor (usually 10% extra) is applied so that there # is some space for administrative overhead (e.g. 8 GiB swap will allocate # 8.8GiB on disk in the end). +# +# If *file* is enabled here, make sure to have the *fstab* module +# as well (later in the exec phase) so that the swap file is +# actually created. userSwapChoices: - none # Create no swap, use no swap - small # Up to 4GB From 77e270136505518c4b6a4d640631ec077b04a049 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Oct 2020 17:21:54 +0200 Subject: [PATCH 25/26] [partition] Coding style - various clang-format versions battle for supremacy --- src/modules/partition/core/PartitionLayout.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index d06b0d043..182e7606b 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -165,12 +165,12 @@ PartitionLayout::execute( Device* dev, { QList< Partition* > partList; // Map each partition entry to its requested size (0 when calculated later) - QMap< const PartitionLayout::PartitionEntry *, qint64 > partSizeMap; + QMap< const PartitionLayout::PartitionEntry*, qint64 > partSizeMap; qint64 totalSize = lastSector - firstSector + 1; qint64 availableSize = totalSize; // Let's check if we have enough space for each partSize - for( const auto& part : qAsConst(m_partLayout) ) + for ( const auto& part : qAsConst( m_partLayout ) ) { qint64 size; // Calculate partition size @@ -208,7 +208,7 @@ PartitionLayout::execute( Device* dev, if ( availableSize < 0 ) { availableSize = totalSize; - for( const auto& part : qAsConst(m_partLayout) ) + for ( const auto& part : qAsConst( m_partLayout ) ) { qint64 size; @@ -238,11 +238,11 @@ PartitionLayout::execute( Device* dev, } // Assign size for percentage-defined partitions - for( const auto& part : qAsConst(m_partLayout) ) + for ( const auto& part : qAsConst( m_partLayout ) ) { if ( part.partSize.unit() == CalamaresUtils::Partition::SizeUnit::Percent ) { - qint64 size = partSizeMap.value(&part); + qint64 size = partSizeMap.value( &part ); size = part.partSize.toSectors( availableSize + size, dev->logicalSize() ); if ( part.partMinSize.isValid() ) { @@ -261,7 +261,7 @@ PartitionLayout::execute( Device* dev, } } - partSizeMap.insert(&part, size); + partSizeMap.insert( &part, size ); } } @@ -270,12 +270,12 @@ PartitionLayout::execute( Device* dev, // TODO: Refine partition sizes to make sure there is room for every partition // Use a default (200-500M ?) minimum size for partition without minSize - for( const auto& part : qAsConst(m_partLayout) ) + for ( const auto& part : qAsConst( m_partLayout ) ) { qint64 size, end; Partition* currentPartition = nullptr; - size = partSizeMap.value(&part); + size = partSizeMap.value( &part ); // Adjust partition size based on available space if ( size > availableSize ) @@ -283,7 +283,7 @@ PartitionLayout::execute( Device* dev, size = availableSize; } - end = firstSector + std::max(size - 1, Q_INT64_C(0)); + end = firstSector + std::max( size - 1, Q_INT64_C( 0 ) ); if ( luksPassphrase.isEmpty() ) { From 26e8a6bcb59bc4d467bc94a646c4ba43bb54661d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 6 Oct 2020 22:25:14 +0200 Subject: [PATCH 26/26] Changes: explain the swapfile settings (and how limited they are) --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index a96335e6e..0adfede4d 100644 --- a/CHANGES +++ b/CHANGES @@ -24,6 +24,10 @@ This release contains contributions from (alphabetically by first name): system) now supports more than one entropy file; generate them as needed (or copy a fixed value to all, depending on *entropy-copy*). Deprecate *entropy* (which generates a specific output file) as too inflexible. + - In the *partition* module, swap can now be chosen as *file*, which is + **not** create a swap partition, but write a `/swapfile` in the root + directory, 512MiB large, and set that as swap. There is as yet no + "smarts" about the size of the swap file. - Progress reporting from the *unpackfs* module has been revamped: it reports more often now, so that it is more obvious that files are being transferred even when the percentage progress does not