Merge branch 'smooth-partition-crash'

9 files changed, 66 insertions, 35 deletions

diff --git a/CHANGES b/CHANGES
index 205727ff8..b165c98b7 100644
--- a/CHANGES
+++ b/CHANGES
@@ -47,7 +47,7 @@ This release contains contributions from (alphabetically by first name):
  * The currently-selected disk device is remembered between manual partitioning
    and the partitioning-overview pages. (Thanks to Arnaud)
  * *partition* There is new support for partitioning layout presets.
-   See `partitionc.conf` for documentation and details.
+   See `partition.conf` for documentation and details.
  * The *keyboard* module now handles the (bogus) Austrian keymap for
    the system console properly. (Thanks to Kevin)
  * The *preservefiles* module now has a mechanism for setting the permissions
@@ -56,7 +56,8 @@ This release contains contributions from (alphabetically by first name):
    for use in OEM installs where an image of fixed size is created,
    and then sized to the actual SD card the user has used.
  * The *mount* module now handles missing *extraMounts* and *extraMountsEfi*
-   keys gracefully (this is probably a misconfiguration issue).
+   keys gracefully (this is probably a misconfiguration, though, and gives a
+   warning).
  * The *packages* module now supports pre- and post-script options
    for all operations, not just during install (keep in mind that
    these run as three separate shells, though).
diff --git a/src/modules/partition/core/PartitionCoreModule.cpp b/src/modules/partition/core/PartitionCoreModule.cpp
index 16e7c7f34..07cb0fcfd 100644
--- a/src/modules/partition/core/PartitionCoreModule.cpp
+++ b/src/modules/partition/core/PartitionCoreModule.cpp
@@ -880,7 +880,7 @@ PartitionCoreModule::revertAllDevices()
             }
         }
 
-        revertDevice( ( *it )->device.data() );
+        revertDevice( ( *it )->device.data(), false );
         ++it;
     }
 
@@ -889,7 +889,7 @@ PartitionCoreModule::revertAllDevices()
 
 
 void
-PartitionCoreModule::revertDevice( Device* dev )
+PartitionCoreModule::revertDevice( Device* dev, bool individualRevert )
 {
     QMutexLocker locker( &m_revertMutex );
     DeviceInfo* devInfo = infoForDevice( dev );
@@ -915,7 +915,8 @@ PartitionCoreModule::revertDevice( Device* dev )
 
     m_bootLoaderModel->init( devices );
 
-    refreshAfterModelChange();
+    if ( individualRevert )
+        refreshAfterModelChange();
     emit deviceReverted( newDev );
 }
 
@@ -931,7 +932,7 @@ PartitionCoreModule::asyncRevertDevice( Device* dev, std::function< void() > cal
         watcher->deleteLater();
     } );
 
-    QFuture< void > future = QtConcurrent::run( this, &PartitionCoreModule::revertDevice, dev );
+    QFuture< void > future = QtConcurrent::run( this, &PartitionCoreModule::revertDevice, dev, true );
     watcher->setFuture( future );
 }
 
diff --git a/src/modules/partition/core/PartitionCoreModule.h b/src/modules/partition/core/PartitionCoreModule.h
index 52cb47a59..c48c1562c 100644
--- a/src/modules/partition/core/PartitionCoreModule.h
+++ b/src/modules/partition/core/PartitionCoreModule.h
@@ -191,7 +191,12 @@ public:
 
     void revert();                      // full revert, thread safe, calls doInit
     void revertAllDevices();            // convenience function, calls revertDevice
-    void revertDevice( Device* dev );   // rescans a single Device and updates DeviceInfo
+    /** @brief rescans a single Device and updates DeviceInfo
+     *
+     * When @p individualRevert is true, calls refreshAfterModelChange(),
+     * used to reduce number of refreshes when calling revertAllDevices().
+     */
+    void revertDevice( Device* dev, bool individualRevert=true );
     void asyncRevertDevice( Device* dev, std::function< void() > callback ); //like revertDevice, but asynchronous
 
     void clearJobs();   // only clear jobs, the Device* states are preserved
diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp
index f8ff79d2c..c2489620f 100644
--- a/src/modules/partition/core/PartitionLayout.cpp
+++ b/src/modules/partition/core/PartitionLayout.cpp
@@ -53,7 +53,7 @@ PartitionLayout::addEntry( PartitionLayout::PartitionEntry entry )
 }
 
 static double
-parseSizeString( QString sizeString, PartitionLayout::SizeUnit *unit )
+parseSizeString( const QString& sizeString, PartitionLayout::SizeUnit* unit )
 {
     double value;
     bool ok;
@@ -102,35 +102,32 @@ parseSizeString( QString sizeString, PartitionLayout::SizeUnit *unit )
     return value;
 }
 
+PartitionLayout::PartitionEntry::PartitionEntry(const QString& size, const QString& min)
+{
+    partSize = parseSizeString( size , &partSizeUnit );
+    if ( !min.isEmpty() )
+        partMinSize = parseSizeString( min , &partMinSizeUnit );
+}
+
 void
-PartitionLayout::addEntry( QString mountPoint, QString size, QString min )
+PartitionLayout::addEntry( const QString& mountPoint, const QString& size, const QString& min )
 {
-    PartitionLayout::PartitionEntry entry;
+    PartitionLayout::PartitionEntry entry( size, min );
 
     entry.partMountPoint = mountPoint;
     entry.partFileSystem = FileSystem::Ext4;
-    entry.partSize = parseSizeString( size , &entry.partSizeUnit );
-    if (min.isEmpty())
-        entry.partMinSize = 0;
-    else
-        entry.partMinSize = parseSizeString( min , &entry.partMinSizeUnit );
 
     partLayout.append( entry );
 }
 
 void
-PartitionLayout::addEntry( QString label, QString mountPoint, QString fs, QString size, QString min )
+PartitionLayout::addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min )
 {
-    PartitionLayout::PartitionEntry entry;
+    PartitionLayout::PartitionEntry entry( size, min );
 
     entry.partLabel = label;
     entry.partMountPoint = mountPoint;
     entry.partFileSystem = FileSystem::typeForName( fs );
-    entry.partSize = parseSizeString( size , &entry.partSizeUnit );
-    if (min.isEmpty())
-        entry.partMinSize = 0;
-    else
-        entry.partMinSize = parseSizeString( min , &entry.partMinSizeUnit );
 
     partLayout.append( entry );
 }
diff --git a/src/modules/partition/core/PartitionLayout.h b/src/modules/partition/core/PartitionLayout.h
index 5ec65cf22..5e216122c 100644
--- a/src/modules/partition/core/PartitionLayout.h
+++ b/src/modules/partition/core/PartitionLayout.h
@@ -1,6 +1,7 @@
 /* === This file is part of Calamares - <https://github.com/calamares> ===
  *
  *   Copyright 2018, Collabora Ltd
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -47,11 +48,16 @@ public:
     {
         QString partLabel;
         QString partMountPoint;
-        int partFileSystem;
-        double partSize;
-        SizeUnit partSizeUnit;
-        double partMinSize;
-        SizeUnit partMinSizeUnit;
+        int partFileSystem = 0;
+        double partSize = 0.0L;
+        SizeUnit partSizeUnit = Percent;
+        double partMinSize = 0.0L;
+        SizeUnit partMinSizeUnit = Percent;
+
+        /// @brief All-zeroes PartitionEntry
+        PartitionEntry() {};
+        /// @brief Parse @p size and @p min to their respective member variables
+        PartitionEntry( const QString& size, const QString& min );
     };
 
     PartitionLayout();
@@ -60,8 +66,8 @@ public:
     ~PartitionLayout();
 
     void addEntry( PartitionEntry entry );
-    void addEntry( QString mountPoint, QString size, QString min = "" );
-    void addEntry( QString label, QString mountPoint, QString fs, QString size, QString min = "" );
+    void addEntry( const QString& mountPoint, const QString& size, const QString& min = QString() );
+    void addEntry( const QString& label, const QString& mountPoint, const QString& fs, const QString& size, const QString& min = QString() );
 
     /**
      * @brief Apply the current partition layout to the selected drive space.
diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp
index 1ef1a00e5..10ffbcee3 100644
--- a/src/modules/partition/gui/ChoicePage.cpp
+++ b/src/modules/partition/gui/ChoicePage.cpp
@@ -107,9 +107,9 @@ ChoicePage::ChoicePage( const SwapChoiceSet& swapChoices, QWidget* parent )
     , m_bootloaderComboBox( nullptr )
     , m_lastSelectedDeviceIndex( -1 )
     , m_enableEncryptionWidget( true )
-    , m_allowManualPartitioning( true )
     , m_availableSwapChoices( swapChoices )
     , m_eraseSwapChoice( pickOne( swapChoices ) )
+    , m_allowManualPartitioning( true )
 {
     setupUi( this );
 
@@ -1246,6 +1246,7 @@ ChoicePage::setupActions()
     if ( osproberEntriesForCurrentDevice.count() == 0 )
     {
         CALAMARES_RETRANSLATE(
+            cDebug() << "Setting texts for 0 osprober entries";
             m_messageLabel->setText( tr( "This storage device does not seem to have an operating system on it. "
                                          "What would you like to do?<br/>"
                                          "You will be able to review and confirm your choices "
@@ -1278,6 +1279,7 @@ ChoicePage::setupActions()
         if ( !osName.isEmpty() )
         {
             CALAMARES_RETRANSLATE(
+                cDebug() << "Setting texts for 1 non-empty osprober entry";
                 m_messageLabel->setText( tr( "This storage device has %1 on it. "
                                              "What would you like to do?<br/>"
                                              "You will be able to review and confirm your choices "
@@ -1301,6 +1303,7 @@ ChoicePage::setupActions()
         else
         {
             CALAMARES_RETRANSLATE(
+                cDebug() << "Setting texts for 1 empty osprober entry";
                 m_messageLabel->setText( tr( "This storage device already has an operating system on it. "
                                              "What would you like to do?<br/>"
                                              "You will be able to review and confirm your choices "
@@ -1325,6 +1328,8 @@ ChoicePage::setupActions()
         // osproberEntriesForCurrentDevice has at least 2 items.
 
         CALAMARES_RETRANSLATE(
+            cDebug() << "Setting texts for >= 2 osprober entries";
+
             m_messageLabel->setText( tr( "This storage device has multiple operating systems on it. "
                                          "What would you like to do?<br/>"
                                          "You will be able to review and confirm your choices "
diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp
index 86de992e6..a2f2eab60 100644
--- a/src/modules/partition/gui/PartitionPage.cpp
+++ b/src/modules/partition/gui/PartitionPage.cpp
@@ -165,9 +165,13 @@ PartitionPage::updateButtons()
 
     if ( m_ui->deviceComboBox->currentIndex() >= 0 )
     {
+        Device* device = nullptr;
         QModelIndex deviceIndex = m_core->deviceModel()->index( m_ui->deviceComboBox->currentIndex(), 0 );
-        auto device = m_core->deviceModel()->deviceForIndex( deviceIndex );
-        if ( device->type() != Device::Type::LVM_Device )
+        if ( deviceIndex.isValid() )
+            device = m_core->deviceModel()->deviceForIndex( deviceIndex );
+        if ( !device )
+            cWarning() << "Device for updateButtons is nullptr";
+        else if ( device->type() != Device::Type::LVM_Device )
         {
             createTable = true;
 
@@ -577,7 +581,7 @@ void
 PartitionPage::onPartitionModelReset()
 {
     m_ui->partitionTreeView->expandAll();
-    updateButtons();
+    // updateButtons();
     updateBootLoaderIndex();
 }
 
diff --git a/src/modules/partition/gui/PartitionViewStep.cpp b/src/modules/partition/gui/PartitionViewStep.cpp
index 503924860..b7433450d 100644
--- a/src/modules/partition/gui/PartitionViewStep.cpp
+++ b/src/modules/partition/gui/PartitionViewStep.cpp
@@ -538,7 +538,7 @@ findFS( QString defaultFS )
     // This bit is for distro's debugging their settings, and shows
     // all the strings that KPMCore is matching against for FS type.
     {
-        Logger::CLog d( Logger::LOGDEBUG );
+        Logger::CDebug d;
         using TR = Logger::DebugRow< int, QString >;
         const auto fstypes = FileSystem::types();
         d << "Available types (" << fstypes.count() << ')';
@@ -612,6 +612,18 @@ PartitionViewStep::setConfigurationMap( const QVariantMap& configurationMap )
         else
             choices.insert( PartitionActions::Choices::SwapChoice::SmallSwap );
     }
+
+    // Not all are supported right now // FIXME
+    static const char unsupportedSetting[] = "Partition-module does not support *userSwapChoices* setting";
+
+#define COMPLAIN_UNSUPPORTED(x) \
+    if ( choices.contains( x ) ) \
+    { cWarning() << unsupportedSetting << PartitionActions::Choices::choiceToName( x ); choices.remove( x ); }
+
+    COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::SwapFile )
+    COMPLAIN_UNSUPPORTED( PartitionActions::Choices::SwapChoice::ReuseSwap )
+#undef COMPLAIN_UNSUPPORTED
+
     m_swapChoices = choices;
 
     // These gs settings seem to be unused (in upstream Calamares) outside of
diff --git a/src/modules/partition/jobs/FillGlobalStorageJob.cpp b/src/modules/partition/jobs/FillGlobalStorageJob.cpp
index 597d62a82..1f4026dec 100644
--- a/src/modules/partition/jobs/FillGlobalStorageJob.cpp
+++ b/src/modules/partition/jobs/FillGlobalStorageJob.cpp
@@ -96,7 +96,7 @@ mapForPartition( Partition* partition, const QString& uuid )
 
     // Debugging for inside the loop in createPartitionList(),
     // so indent a bit
-    Logger::CLog deb = cDebug();
+    Logger::CDebug deb;
     using TR = Logger::DebugRow<const char *const, const QString&>;
     deb << "  .. mapping for" << partition->partitionPath() << partition->deviceNode()
         << TR( "mtpoint:", PartitionInfo::mountPoint( partition ) )