From 0aaf24c4a5702a5d354f21fcea4aa5abbe3d4718 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 14:12:18 +0100
Subject: [PATCH 01/30] CMake: tell tests where their source-dir is

- Abuse BUILD_AS_TEST to pass in the value as a string
---
 CMakeModules/CalamaresAddTest.cmake | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CMakeModules/CalamaresAddTest.cmake b/CMakeModules/CalamaresAddTest.cmake
index 36be0f03e..65f9389e8 100644
--- a/CMakeModules/CalamaresAddTest.cmake
+++ b/CMakeModules/CalamaresAddTest.cmake
@@ -50,7 +50,7 @@ function( calamares_add_test )
                 Qt5::Test
             )
         calamares_automoc( ${TEST_NAME} )
-        target_compile_definitions( ${TEST_NAME} PRIVATE -DBUILD_AS_TEST ${TEST_DEFINITIONS} )
+        target_compile_definitions( ${TEST_NAME} PRIVATE -DBUILD_AS_TEST="${CMAKE_CURRENT_SOURCE_DIR}"  ${TEST_DEFINITIONS} )
         if( TEST_GUI )
             target_link_libraries( ${TEST_NAME} calamaresui Qt5::Gui )
         endif()

From 8825c9c995cd4c665983f6b5016fa5c05c734ebf Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 14:10:48 +0100
Subject: [PATCH 02/30] [netinstall] Apply coding style

---
 src/modules/netinstall/NetInstallPage.cpp     | 6 +++---
 src/modules/netinstall/NetInstallViewStep.cpp | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 352eb9f3e..ec1928dff 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -157,10 +157,10 @@ NetInstallPage::dataIsHere()
     // Go backwards because expanding a group may cause rows to appear below it
     for ( int i = m_groups->rowCount() - 1; i >= 0; --i )
     {
-        auto index = m_groups->index(i,0);
-        if ( m_groups->data(index, PackageModel::MetaExpandRole).toBool() )
+        auto index = m_groups->index( i, 0 );
+        if ( m_groups->data( index, PackageModel::MetaExpandRole ).toBool() )
         {
-            ui->groupswidget->setExpanded(index, true);
+            ui->groupswidget->setExpanded( index, true );
         }
     }
 
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index cb79982f9..66895dd96 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -56,7 +56,7 @@ NetInstallViewStep::prettyName() const
 {
     return m_sidebarLabel ? m_sidebarLabel->get() : tr( "Package selection" );
 
-#if defined(TABLE_OF_TRANSLATIONS)
+#if defined( TABLE_OF_TRANSLATIONS )
     NOTREACHED
     // This is a table of "standard" labels for this module. If you use them
     // in the label: sidebar: section of the config file, the existing

From c66ef5a20199019898fedf1f0e0b2674c50743c6 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 20 Mar 2020 19:48:29 +0100
Subject: [PATCH 03/30] [netinstall] Refactor: kill ItemData

- This doesn't compile right now.
- The nested class ItemData doesn't do anything useful or
  meaningful that having model items with the right data wouldn't.
---
 src/modules/netinstall/PackageTreeItem.cpp | 128 ++++++---------------
 src/modules/netinstall/PackageTreeItem.h   |  69 +++++------
 2 files changed, 74 insertions(+), 123 deletions(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 7e20d63e1..81a20257d 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -21,55 +21,26 @@
 
 #include "utils/Logger.h"
 
-QVariant
-PackageTreeItem::ItemData::toOperation() const
-{
-    // If it's a package with a pre- or post-script, replace
-    // with the more complicated datastructure.
-    if ( !preScript.isEmpty() || !postScript.isEmpty() )
-    {
-        QMap< QString, QVariant > sdetails;
-        sdetails.insert( "pre-script", preScript );
-        sdetails.insert( "package", packageName );
-        sdetails.insert( "post-script", postScript );
-        return sdetails;
-    }
-    else
-    {
-        return packageName;
-    }
-}
-
-PackageTreeItem::PackageTreeItem( const ItemData& data, PackageTreeItem* parent )
-    : m_parentItem( parent )
-    , m_data( data )
-{
-}
-
-PackageTreeItem::PackageTreeItem( const QString packageName, PackageTreeItem* parent )
+PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* parent )
     : m_parentItem( parent )
 {
-    m_data.packageName = packageName;
+    m_packageName = packageName;
     if ( parent != nullptr )
     {
-        m_data.selected = parent->isSelected();
+        // Avoid partially-checked .. a package can't be partial
+        m_selected = parent->isSelected() == Qt::Unchecked ? Qt::Unchecked : Qt::Checked;
     }
     else
     {
-        m_data.selected = Qt::Unchecked;
+        m_selected = Qt::Unchecked;
     }
 }
 
-PackageTreeItem::PackageTreeItem( PackageTreeItem* parent )
-    : m_parentItem( parent )
-{
-}
-
 PackageTreeItem::PackageTreeItem::PackageTreeItem()
     : PackageTreeItem( QString(), nullptr )
 {
-    m_data.selected = Qt::Checked;
-    m_data.name = QLatin1String( "<root>" );
+    m_selected = Qt::Checked;
+    m_name = QLatin1String( "<root>" );
 }
 
 PackageTreeItem::~PackageTreeItem()
@@ -123,7 +94,7 @@ PackageTreeItem::data( int column ) const
         switch ( column )  // group
         {
         case 0:
-            return QVariant( prettyName() );
+            return QVariant( name() );
         case 1:
             return QVariant( description() );
         default:
@@ -145,47 +116,15 @@ PackageTreeItem::parentItem() const
 }
 
 
-QString
-PackageTreeItem::prettyName() const
-{
-    return m_data.name;
-}
-
-QString
-PackageTreeItem::description() const
-{
-    return m_data.description;
-}
-
-QString
-PackageTreeItem::preScript() const
-{
-    return m_data.preScript;
-}
-
-QString
-PackageTreeItem::packageName() const
-{
-    return m_data.packageName;
-}
-
-QString
-PackageTreeItem::postScript() const
-{
-    return m_data.postScript;
-}
-
-bool
-PackageTreeItem::isHidden() const
-{
-    return m_data.isHidden;
-}
-
 bool
 PackageTreeItem::hiddenSelected() const
 {
-    Q_ASSERT( m_data.isHidden );
-    if ( !m_data.selected )
+    if ( !m_isHidden )
+    {
+        return m_selected;
+    }
+
+    if ( !m_selected )
     {
         return false;
     }
@@ -201,32 +140,20 @@ PackageTreeItem::hiddenSelected() const
     }
 
     /* Has no non-hiddent parents */
-    return m_data.selected;
+    return m_selected;
 }
 
 
-bool
-PackageTreeItem::isCritical() const
-{
-    return m_data.isCritical;
-}
-
-Qt::CheckState
-PackageTreeItem::isSelected() const
-{
-    return m_data.selected;
-}
-
 void
 PackageTreeItem::setSelected( Qt::CheckState isSelected )
 {
     if ( parentItem() == nullptr )
-    // This is the root, it is always checked so don't change state
     {
+        // This is the root, it is always checked so don't change state
         return;
     }
 
-    m_data.selected = isSelected;
+    m_selected = isSelected;
     setChildrenSelected( isSelected );
 
     // Look for suitable parent item which may change checked-state
@@ -277,7 +204,7 @@ PackageTreeItem::setChildrenSelected( Qt::CheckState isSelected )
         // Children are never root; don't need to use setSelected on them.
         for ( auto child : m_childItems )
         {
-            child->m_data.selected = isSelected;
+            child->m_selected = isSelected;
             child->setChildrenSelected( isSelected );
         }
 }
@@ -287,3 +214,22 @@ PackageTreeItem::type() const
 {
     return QStandardItem::UserType;
 }
+
+QVariant
+PackageTreeItem::toOperation() const
+{
+    // If it's a package with a pre- or post-script, replace
+    // with the more complicated datastructure.
+    if ( !m_preScript.isEmpty() || !m_postScript.isEmpty() )
+    {
+        QMap< QString, QVariant > sdetails;
+        sdetails.insert( "pre-script", m_preScript );
+        sdetails.insert( "package", m_packageName );
+        sdetails.insert( "post-script", m_postScript );
+        return sdetails;
+    }
+    else
+    {
+        return m_packageName;
+    }
+}
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index d9c1f9ec2..83d1cb3e7 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -27,29 +27,12 @@
 class PackageTreeItem : public QStandardItem
 {
 public:
-    struct ItemData
-    {
-        QString name;
-        QString description;
-        QString preScript;
-        QString packageName;
-        QString postScript;
-        bool isCritical = false;
-        bool isHidden = false;
-        bool startExpanded = false;  // Only for groups
-        Qt::CheckState selected = Qt::Unchecked;
+    using PackageList = QList< PackageTreeItem* >;
 
-        /** @brief Turns this item into a variant for PackageOperations use
-         *
-         * For "plain" items, this is just the package name; items with
-         * scripts return a map. See the package module for how it's interpreted.
-         */
-        QVariant toOperation() const;
-    };
-    explicit PackageTreeItem( const ItemData& data, PackageTreeItem* parent = nullptr );
-    explicit PackageTreeItem( const QString packageName, PackageTreeItem* parent = nullptr );
-    explicit PackageTreeItem( PackageTreeItem* parent );
-    explicit PackageTreeItem();  // The root of the tree; always selected, named <root>
+    ///@brief A package (individual package)
+    explicit PackageTreeItem( const QString& packageName, PackageTreeItem* parent = nullptr );
+    ///@brief A root item, always selected, named "<root>"
+    explicit PackageTreeItem();
     ~PackageTreeItem() override;
 
     void appendChild( PackageTreeItem* child );
@@ -61,18 +44,19 @@ public:
     PackageTreeItem* parentItem();
     const PackageTreeItem* parentItem() const;
 
-    QString prettyName() const;
-    QString description() const;
-    QString preScript() const;
-    QString packageName() const;
-    QString postScript() const;
+    QString name() const { return m_name; }
+    QString packageName() const { return m_packageName; }
+
+    QString description() const { return m_description; }
+    QString preScript() const { return m_preScript; }
+    QString postScript() const { return m_postScript; }
 
     /** @brief Is this item hidden?
      *
      * Hidden items (generally only groups) are maintained separately,
      * not shown to the user, but do enter into the package-installation process.
      */
-    bool isHidden() const;
+    bool isHidden() const { return m_isHidden; }
 
     /** @brief Is this hidden item, considered "selected"?
      *
@@ -87,7 +71,7 @@ public:
      * A critical group must be successfully installed, for the Calamares
      * installation to continue.
      */
-    bool isCritical() const;
+    bool isCritical() const { return m_isCritical; }
 
     /** @brief Is this group expanded on start?
      *
@@ -95,17 +79,38 @@ public:
      * that expands on start is shown expanded (not collapsed)
      * in the treeview when the page is loaded.
      */
-    bool expandOnStart() const { return m_data.startExpanded; }
+    bool expandOnStart() const { return m_startExpanded; }
+
+    /** @brief Turns this item into a variant for PackageOperations use
+     *
+     * For "plain" items, this is just the package name; items with
+     * scripts return a map. See the package module for how it's interpreted.
+     */
+    QVariant toOperation() const;
 
     Qt::CheckState isSelected() const;
     void setSelected( Qt::CheckState isSelected );
     void setChildrenSelected( Qt::CheckState isSelected );
+
+    // QStandardItem methods
     int type() const override;
 
 private:
     PackageTreeItem* m_parentItem;
-    QList< PackageTreeItem* > m_childItems;
-    ItemData m_data;
+    PackageList m_childItems;
+
+    // An entry can be a packkage, or a group.
+    QString m_name;
+    QString m_packageName;
+    Qt::CheckState m_selected = Qt::Unchecked;
+
+    // These are only useful for groups
+    QString m_description;
+    QString m_preScript;
+    QString m_postScript;
+    bool m_isCritical = false;
+    bool m_isHidden = false;
+    bool m_startExpanded = false;
 };
 
 #endif  // PACKAGETREEITEM_H

From 4cb2ed955276e8b73dae21feb5050f95ec3b4eb0 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 20 Mar 2020 23:03:47 +0100
Subject: [PATCH 04/30] [netinstall] Chase removal of ItemData

- Simplify creation of PackageTreeItems by interpreting the
  YAML directly (instead of via ItemData),
- Simplify list types,
- Drop superfluous API.
---
 src/modules/netinstall/NetInstallPage.cpp     |  4 +--
 src/modules/netinstall/NetInstallPage.h       |  2 +-
 src/modules/netinstall/NetInstallViewStep.cpp |  8 ++---
 src/modules/netinstall/PackageModel.cpp       | 34 ++++---------------
 src/modules/netinstall/PackageModel.h         |  8 ++---
 src/modules/netinstall/PackageTreeItem.cpp    | 33 ++++++++++++++----
 src/modules/netinstall/PackageTreeItem.h      | 13 +++++--
 7 files changed, 53 insertions(+), 49 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index ec1928dff..35086cb44 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -167,7 +167,7 @@ NetInstallPage::dataIsHere()
     emit checkReady( true );
 }
 
-PackageModel::PackageItemDataList
+PackageTreeItem::List
 NetInstallPage::selectedPackages() const
 {
     if ( m_groups )
@@ -177,7 +177,7 @@ NetInstallPage::selectedPackages() const
     else
     {
         cWarning() << "no netinstall groups are available.";
-        return PackageModel::PackageItemDataList();
+        return PackageTreeItem::List();
     }
 }
 
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index 12633c6b9..f9d7127e2 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -74,7 +74,7 @@ public:
     // Returns the list of packages belonging to groups that are
     // selected in the view in this given moment. No data is cached here, so
     // this function does not have constant time.
-    PackageModel::PackageItemDataList selectedPackages() const;
+    PackageTreeItem::List selectedPackages() const;
 
 public slots:
     void dataIsHere();
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 66895dd96..ba16d940c 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -127,7 +127,7 @@ NetInstallViewStep::onActivate()
 void
 NetInstallViewStep::onLeave()
 {
-    PackageModel::PackageItemDataList packages = m_widget->selectedPackages();
+    auto packages = m_widget->selectedPackages();
     cDebug() << "Netinstall: Processing" << packages.length() << "packages.";
 
     static const char PACKAGEOP[] = "packageOperations";
@@ -158,13 +158,13 @@ NetInstallViewStep::onLeave()
 
     for ( const auto& package : packages )
     {
-        if ( package.isCritical )
+        if ( package->isCritical() )
         {
-            installPackages.append( package.toOperation() );
+            installPackages.append( package->toOperation() );
         }
         else
         {
-            tryInstallPackages.append( package.toOperation() );
+            tryInstallPackages.append( package->toOperation() );
         }
     }
 
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 215ac2912..5fc204bdb 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -169,29 +169,21 @@ PackageModel::headerData( int section, Qt::Orientation orientation, int role ) c
     return QVariant();
 }
 
-QList< PackageTreeItem::ItemData >
+PackageTreeItem::List
 PackageModel::getPackages() const
 {
-    QList< PackageTreeItem* > items = getItemPackages( m_rootItem );
+    auto items = getItemPackages( m_rootItem );
     for ( auto package : m_hiddenItems )
+    {
         if ( package->hiddenSelected() )
         {
             items.append( getItemPackages( package ) );
         }
-    QList< PackageTreeItem::ItemData > packages;
-    for ( auto item : items )
-    {
-        PackageTreeItem::ItemData itemData;
-        itemData.preScript = item->parentItem()->preScript();  // Only groups have hooks
-        itemData.packageName = item->packageName();  // this seg faults
-        itemData.postScript = item->parentItem()->postScript();  // Only groups have hooks
-        itemData.isCritical = item->parentItem()->isCritical();  // Only groups are critical
-        packages.append( itemData );
     }
-    return packages;
+    return items;
 }
 
-QList< PackageTreeItem* >
+PackageTreeItem::List
 PackageModel::getItemPackages( PackageTreeItem* item ) const
 {
     QList< PackageTreeItem* > selectedPackages;
@@ -232,21 +224,7 @@ PackageModel::setupModelData( const YAML::Node& data, PackageTreeItem* parent )
     for ( YAML::const_iterator it = data.begin(); it != data.end(); ++it )
     {
         const YAML::Node itemDefinition = *it;
-
-        QString name( tr( CalamaresUtils::yamlToVariant( itemDefinition[ "name" ] ).toByteArray() ) );
-        QString description( tr( CalamaresUtils::yamlToVariant( itemDefinition[ "description" ] ).toByteArray() ) );
-
-        PackageTreeItem::ItemData itemData;
-        itemData.name = name;
-        itemData.description = description;
-
-        itemData.preScript = getString( itemDefinition, "pre-install" );
-        itemData.postScript = getString( itemDefinition, "post-install" );
-        itemData.isCritical = getBool( itemDefinition, "critical" );
-        itemData.isHidden = getBool( itemDefinition, "hidden" );
-        itemData.startExpanded = getBool( itemDefinition, "expanded" );
-
-        PackageTreeItem* item = new PackageTreeItem( itemData, parent );
+        PackageTreeItem* item = new PackageTreeItem( CalamaresUtils::yamlMapToVariant( itemDefinition ), parent );
 
         if ( itemDefinition[ "selected" ] )
         {
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index b76a58a42..09701ef7d 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -37,8 +37,6 @@ class PackageModel : public QAbstractItemModel
     Q_OBJECT
 
 public:
-    using PackageItemDataList = QList< PackageTreeItem::ItemData >;
-
     // Names for columns (unused in the code)
     static constexpr const int NameColumn = 0;
     static constexpr const int DescriptionColumn = 1;
@@ -63,14 +61,14 @@ public:
     int rowCount( const QModelIndex& parent = QModelIndex() ) const override;
     int columnCount( const QModelIndex& parent = QModelIndex() ) const override;
 
-    PackageItemDataList getPackages() const;
-    QList< PackageTreeItem* > getItemPackages( PackageTreeItem* item ) const;
+    PackageTreeItem::List getPackages() const;
+    PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
 
 private:
     void setupModelData( const YAML::Node& data, PackageTreeItem* parent );
 
     PackageTreeItem* m_rootItem;
-    QList< PackageTreeItem* > m_hiddenItems;
+    PackageTreeItem::List m_hiddenItems;
 };
 
 #endif  // PACKAGEMODEL_H
diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 81a20257d..089521505 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -20,22 +20,43 @@
 #include "PackageTreeItem.h"
 
 #include "utils/Logger.h"
+#include "utils/Variant.h"
 
-PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* parent )
-    : m_parentItem( parent )
+static Qt::CheckState
+parentCheckState( PackageTreeItem* parent )
 {
-    m_packageName = packageName;
-    if ( parent != nullptr )
+    if ( parent )
     {
         // Avoid partially-checked .. a package can't be partial
-        m_selected = parent->isSelected() == Qt::Unchecked ? Qt::Unchecked : Qt::Checked;
+        return parent->isSelected() == Qt::Unchecked ? Qt::Unchecked : Qt::Checked;
     }
     else
     {
-        m_selected = Qt::Unchecked;
+        return Qt::Unchecked;
     }
 }
 
+PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* parent )
+    : m_parentItem( parent )
+    , m_packageName( packageName )
+    , m_selected( parentCheckState( parent ) )
+{
+}
+
+PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem* parent )
+    : m_parentItem( parent )
+    , m_name( CalamaresUtils::getString( groupData, "name" ) )
+    , m_selected( parentCheckState( parent ) )
+    , m_description( CalamaresUtils::getString( groupData, "description" ) )
+    , m_preScript( CalamaresUtils::getString( groupData, "pre-install" ) )
+    , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) )
+    , m_isCritical( CalamaresUtils::getBool( groupData, "critical", false ) )
+    , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) )
+    , m_startExpanded( CalamaresUtils::getBool( groupData, "expanded", false ) )
+{
+}
+
+
 PackageTreeItem::PackageTreeItem::PackageTreeItem()
     : PackageTreeItem( QString(), nullptr )
 {
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index 83d1cb3e7..5f22201cf 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -27,10 +27,12 @@
 class PackageTreeItem : public QStandardItem
 {
 public:
-    using PackageList = QList< PackageTreeItem* >;
+    using List = QList< PackageTreeItem* >;
 
     ///@brief A package (individual package)
     explicit PackageTreeItem( const QString& packageName, PackageTreeItem* parent = nullptr );
+    ///@brief A group (sub-items and sub-groups are ignored)
+    explicit PackageTreeItem( const QVariantMap& groupData, PackageTreeItem* parent = nullptr );
     ///@brief A root item, always selected, named "<root>"
     explicit PackageTreeItem();
     ~PackageTreeItem() override;
@@ -81,6 +83,12 @@ public:
      */
     bool expandOnStart() const { return m_startExpanded; }
 
+    /** @brief is this item selected?
+     *
+     * Groups may be partially selected; packages are only on or off.
+     */
+    Qt::CheckState isSelected() const { return m_selected; }
+
     /** @brief Turns this item into a variant for PackageOperations use
      *
      * For "plain" items, this is just the package name; items with
@@ -88,7 +96,6 @@ public:
      */
     QVariant toOperation() const;
 
-    Qt::CheckState isSelected() const;
     void setSelected( Qt::CheckState isSelected );
     void setChildrenSelected( Qt::CheckState isSelected );
 
@@ -97,7 +104,7 @@ public:
 
 private:
     PackageTreeItem* m_parentItem;
-    PackageList m_childItems;
+    List m_childItems;
 
     // An entry can be a packkage, or a group.
     QString m_name;

From c7b646315ad55e13c47edfa7b84a784220e3730b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Sun, 22 Mar 2020 13:21:39 +0100
Subject: [PATCH 05/30] [netinstall] Add immutable to groups settings

---
 src/modules/netinstall/PackageTreeItem.cpp | 1 +
 src/modules/netinstall/PackageTreeItem.h   | 1 +
 src/modules/netinstall/README.md           | 6 +++++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 089521505..51fa2b13e 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -52,6 +52,7 @@ PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem*
     , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) )
     , m_isCritical( CalamaresUtils::getBool( groupData, "critical", false ) )
     , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) )
+    , m_showReadOnly( CalamaresUtils::getBool( groupData, "immutable", false ) )
     , m_startExpanded( CalamaresUtils::getBool( groupData, "expanded", false ) )
 {
 }
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index 5f22201cf..89a07aadc 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -117,6 +117,7 @@ private:
     QString m_postScript;
     bool m_isCritical = false;
     bool m_isHidden = false;
+    bool m_showReadOnly = false;
     bool m_startExpanded = false;
 };
 
diff --git a/src/modules/netinstall/README.md b/src/modules/netinstall/README.md
index a8803edd5..cda4b6c88 100644
--- a/src/modules/netinstall/README.md
+++ b/src/modules/netinstall/README.md
@@ -48,8 +48,12 @@ More keys (per group) are supported:
  - *critical*: if true, make the installation process fail if installing
    any of the packages in the group fails. Otherwise, just log a warning.
    Defaults to false.
+ - *immutable*: if true, the state of the group (and all its subgroups)
+   cannot be changed; it really only makes sense in combination
+   with *selected* set to true. This only affects the user-interface.
  - *expanded*: if true, the group is shown in an expanded form (that is,
-   not-collapsed) in the treeview on start.
+   not-collapsed) in the treeview on start. This only affects the user-
+   interface.
  - *subgroups*: if present this follows the same structure as the top level
    of the YAML file, allowing there to be sub-groups of packages to an
    arbitary depth

From 4143ad67af763c42657a660fa8bf23551dd8c371 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Sun, 22 Mar 2020 13:27:53 +0100
Subject: [PATCH 06/30] [netinstall] Remove superfluous code

- The constructor for PackageTreeItem now takes over the selected
  state from the parent.
---
 src/modules/netinstall/PackageModel.cpp | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 5fc204bdb..673e15f5e 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -230,10 +230,6 @@ PackageModel::setupModelData( const YAML::Node& data, PackageTreeItem* parent )
         {
             item->setSelected( getBool( itemDefinition, "selected" ) ? Qt::Checked : Qt::Unchecked );
         }
-        else
-        {
-            item->setSelected( parent->isSelected() );  // Inherit from it's parent
-        }
 
         if ( itemDefinition[ "packages" ] )
         {

From dc403237f29fcbb457719759764d2f5ec21fc948 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Sun, 22 Mar 2020 15:09:43 +0100
Subject: [PATCH 07/30] [netinstall] Build model from QVariantList

- As an alternative to the YAML-wranging, build the model from
  a QVariantList instead.
- Expose this as a constructor, too.
---
 src/modules/netinstall/PackageModel.cpp | 57 ++++++++++++++++++++++---
 src/modules/netinstall/PackageModel.h   |  2 +
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 673e15f5e..0ac8116b3 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -19,6 +19,7 @@
 
 #include "PackageModel.h"
 
+#include "utils/Variant.h"
 #include "utils/Yaml.h"
 
 PackageModel::PackageModel( const YAML::Node& data, QObject* parent )
@@ -28,6 +29,13 @@ PackageModel::PackageModel( const YAML::Node& data, QObject* parent )
     setupModelData( data, m_rootItem );
 }
 
+PackageModel::PackageModel( const QVariantList& data, QObject* parent )
+    : QAbstractItemModel( parent )
+{
+    m_rootItem = new PackageTreeItem();
+    setupModelData( data, m_rootItem );
+}
+
 PackageModel::~PackageModel()
 {
     delete m_rootItem;
@@ -206,18 +214,55 @@ PackageModel::getItemPackages( PackageTreeItem* item ) const
     return selectedPackages;
 }
 
-static QString
-getString( const YAML::Node& itemDefinition, const char* key )
-{
-    return itemDefinition[ key ] ? CalamaresUtils::yamlToVariant( itemDefinition[ key ] ).toString() : QString();
-}
-
 static bool
 getBool( const YAML::Node& itemDefinition, const char* key )
 {
     return itemDefinition[ key ] ? CalamaresUtils::yamlToVariant( itemDefinition[ key ] ).toBool() : false;
 }
 
+void
+PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* parent )
+{
+    for ( const auto& group : groupList )
+    {
+        QVariantMap groupMap = group.toMap();
+        if ( groupMap.isEmpty() )
+        {
+            continue;
+        }
+
+        PackageTreeItem* item = new PackageTreeItem( groupMap, parent );
+        if ( groupMap.contains( "selected" ) )
+        {
+            item->setSelected( CalamaresUtils::getBool( groupMap, "selected", false ) ? Qt::Checked : Qt::Unchecked );
+        }
+        if ( groupMap.contains( "packages" ) )
+        {
+            for ( const auto& packageName : groupMap.value( "packages" ).toStringList() )
+            {
+                item->appendChild( new PackageTreeItem( packageName, item ) );
+            }
+        }
+        if ( groupMap.contains( "subgroups" ) )
+        {
+            QVariantList subgroups = groupMap.value( "subgroups" ).toList();
+            if ( !subgroups.isEmpty() )
+            {
+                setupModelData( subgroups, item );
+            }
+        }
+        if ( item->isHidden() )
+        {
+            m_hiddenItems.append( item );
+        }
+        else
+        {
+            item->setCheckable( true );
+            parent->appendChild( item );
+        }
+    }
+}
+
 void
 PackageModel::setupModelData( const YAML::Node& data, PackageTreeItem* parent )
 {
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 09701ef7d..db41c4197 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -48,6 +48,7 @@ public:
     static constexpr const int MetaExpandRole = Qt::UserRole + 1;
 
     explicit PackageModel( const YAML::Node& data, QObject* parent = nullptr );
+    explicit PackageModel( const QVariantList& data, QObject* parent = nullptr );
     ~PackageModel() override;
 
     QVariant data( const QModelIndex& index, int role ) const override;
@@ -66,6 +67,7 @@ public:
 
 private:
     void setupModelData( const YAML::Node& data, PackageTreeItem* parent );
+    void setupModelData( const QVariantList& l, PackageTreeItem* parent );
 
     PackageTreeItem* m_rootItem;
     PackageTreeItem::List m_hiddenItems;

From bca316299e2210fafe05e9acd8267ea05d0196c1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Sun, 22 Mar 2020 21:08:32 +0100
Subject: [PATCH 08/30] [netinstall] Add tests

- Just some simple tests for the Items
- Test creation of package group from variant
- This needs Qt5::Gui to link because QStandardItem is a GUI class,
  although we can run the tests without a GUI.
---
 src/modules/netinstall/CMakeLists.txt |  11 +++
 src/modules/netinstall/Tests.cpp      | 118 ++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)
 create mode 100644 src/modules/netinstall/Tests.cpp

diff --git a/src/modules/netinstall/CMakeLists.txt b/src/modules/netinstall/CMakeLists.txt
index c5eddd32b..9d0167670 100644
--- a/src/modules/netinstall/CMakeLists.txt
+++ b/src/modules/netinstall/CMakeLists.txt
@@ -16,3 +16,14 @@ calamares_add_plugin( netinstall
         yamlcpp
     SHARED_LIB
 )
+
+calamares_add_test(
+    netinstalltest
+    SOURCES
+        Tests.cpp
+        PackageTreeItem.cpp
+        PackageModel.cpp
+    LIBRARIES
+        Qt5::Gui
+)
+
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
new file mode 100644
index 000000000..d1d73d7fe
--- /dev/null
+++ b/src/modules/netinstall/Tests.cpp
@@ -0,0 +1,118 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2020, 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
+ *   the Free Software Foundation, either version 3 of the License, or
+ *   (at your option) any later version.
+ *
+ *   Calamares is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "PackageTreeItem.h"
+
+#include "utils/Logger.h"
+#include "utils/Variant.h"
+#include "utils/Yaml.h"
+
+#include <QtTest/QtTest>
+
+class ItemTests : public QObject
+{
+    Q_OBJECT
+public:
+    ItemTests();
+    virtual ~ItemTests() {}
+
+private Q_SLOTS:
+    void initTestCase();
+
+    void testRoot();
+    void testPackage();
+    void testGroup();
+};
+
+ItemTests::ItemTests() {}
+
+void
+ItemTests::initTestCase()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+}
+
+void
+ItemTests::testRoot()
+{
+    PackageTreeItem r;
+
+    QCOMPARE( r.isSelected(), Qt::Checked );
+    QCOMPARE( r.name(), QStringLiteral( "<root>" ) );
+    QCOMPARE( r.parentItem(), nullptr );
+}
+
+void
+ItemTests::testPackage()
+{
+    PackageTreeItem p( "bash", nullptr );
+
+    QCOMPARE( p.isSelected(), Qt::Unchecked );
+    QCOMPARE( p.packageName(), QStringLiteral( "bash" ) );
+    QVERIFY( p.name().isEmpty() );  // not a group
+    QCOMPARE( p.parentItem(), nullptr );
+    QCOMPARE( p.childCount(), 0 );
+    QVERIFY( !p.isHidden() );
+    QVERIFY( !p.isCritical() );
+
+    // This doesn't happen in normal constructions,
+    // because a package can't have children.
+    PackageTreeItem c( "zsh", &p );
+    QCOMPARE( c.isSelected(), Qt::Unchecked );
+    QCOMPARE( c.packageName(), QStringLiteral( "zsh" ) );
+    QVERIFY( c.name().isEmpty() );  // not a group
+    QCOMPARE( c.parentItem(), &p );
+
+    QCOMPARE( p.childCount(), 0 );  // not noticed it has a child
+}
+
+// *INDENT-OFF*
+// clang-format off
+static const char doc[] =
+"- name: \"CCR\"\n"
+"  description: \"Tools for the Chakra Community Repository\"\n"
+"  packages:\n"
+"    - ccr\n"
+"    - base-devel\n";
+// *INDENT-ON*
+// clang-format on
+
+void
+ItemTests::testGroup()
+{
+    YAML::Node yamldoc = YAML::Load( doc );
+    QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc );
+
+    QCOMPARE( yamlContents.length(), 1 );
+
+    PackageTreeItem p( yamlContents[ 0 ].toMap(), nullptr );
+    QCOMPARE( p.name(), QStringLiteral( "CCR" ) );
+    QVERIFY( p.packageName().isEmpty() );
+    QVERIFY( p.description().startsWith( QStringLiteral( "Tools " ) ) );
+    QCOMPARE( p.parentItem(), nullptr );
+    QVERIFY( !p.isHidden() );
+    QVERIFY( !p.isCritical() );
+    // The item-constructor doesn't consider the packages: list
+    QCOMPARE( p.childCount(), 0 );
+}
+
+QTEST_GUILESS_MAIN( ItemTests )
+
+#include "utils/moc-warnings.h"
+
+#include "Tests.moc"

From 52d3f4417f5940b5b1bd42f305fcc0bda3a0985d Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 09:59:03 +0100
Subject: [PATCH 09/30] [netinstall] Add explicit isGroup()

- Previously you would either need to know where in the tree a
  PackageTreeItem was, or guess that an empty packageName() means that
  it's a group.
---
 src/modules/netinstall/PackageTreeItem.cpp | 10 ++++++----
 src/modules/netinstall/PackageTreeItem.h   | 17 ++++++++++++++++-
 src/modules/netinstall/Tests.cpp           |  7 +++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 51fa2b13e..eca62141f 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -40,6 +40,7 @@ PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* p
     : m_parentItem( parent )
     , m_packageName( packageName )
     , m_selected( parentCheckState( parent ) )
+    , m_isGroup( false )
 {
 }
 
@@ -50,6 +51,7 @@ PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem*
     , m_description( CalamaresUtils::getString( groupData, "description" ) )
     , m_preScript( CalamaresUtils::getString( groupData, "pre-install" ) )
     , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) )
+    , m_isGroup( true )
     , m_isCritical( CalamaresUtils::getBool( groupData, "critical", false ) )
     , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) )
     , m_showReadOnly( CalamaresUtils::getBool( groupData, "immutable", false ) )
@@ -57,12 +59,12 @@ PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem*
 {
 }
 
-
 PackageTreeItem::PackageTreeItem::PackageTreeItem()
-    : PackageTreeItem( QString(), nullptr )
+    : m_parentItem( nullptr )
+    , m_name( QStringLiteral( "<root>" ) )
+    , m_selected( Qt::Checked )
+    , m_isGroup( true )
 {
-    m_selected = Qt::Checked;
-    m_name = QLatin1String( "<root>" );
 }
 
 PackageTreeItem::~PackageTreeItem()
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index 89a07aadc..c35b98eea 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -53,6 +53,20 @@ public:
     QString preScript() const { return m_preScript; }
     QString postScript() const { return m_postScript; }
 
+    /** @brief Is this item a group-item?
+     *
+     * Groups have a (possibly empty) list of packages, and a
+     * (possibly empty) list of sub-groups, and can be marked
+     * critical, hidden, etc. Packages, on the other hand, only
+     * have a meaningful packageName() and selection status.
+     *
+     * Root is a group.
+     */
+    bool isGroup() const { return m_isGroup; }
+
+    /// @brief Is this item a single package?
+    bool isPackage() const { return !isGroup(); }
+
     /** @brief Is this item hidden?
      *
      * Hidden items (generally only groups) are maintained separately,
@@ -106,7 +120,7 @@ private:
     PackageTreeItem* m_parentItem;
     List m_childItems;
 
-    // An entry can be a packkage, or a group.
+    // An entry can be a package, or a group.
     QString m_name;
     QString m_packageName;
     Qt::CheckState m_selected = Qt::Unchecked;
@@ -115,6 +129,7 @@ private:
     QString m_description;
     QString m_preScript;
     QString m_postScript;
+    bool m_isGroup = false;
     bool m_isCritical = false;
     bool m_isHidden = false;
     bool m_showReadOnly = false;
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index d1d73d7fe..420b690aa 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -55,6 +55,7 @@ ItemTests::testRoot()
     QCOMPARE( r.isSelected(), Qt::Checked );
     QCOMPARE( r.name(), QStringLiteral( "<root>" ) );
     QCOMPARE( r.parentItem(), nullptr );
+    QVERIFY( r.isGroup() );
 }
 
 void
@@ -69,6 +70,8 @@ ItemTests::testPackage()
     QCOMPARE( p.childCount(), 0 );
     QVERIFY( !p.isHidden() );
     QVERIFY( !p.isCritical() );
+    QVERIFY( !p.isGroup() );
+    QVERIFY( p.isPackage() );
 
     // This doesn't happen in normal constructions,
     // because a package can't have children.
@@ -77,6 +80,8 @@ ItemTests::testPackage()
     QCOMPARE( c.packageName(), QStringLiteral( "zsh" ) );
     QVERIFY( c.name().isEmpty() );  // not a group
     QCOMPARE( c.parentItem(), &p );
+    QVERIFY( !c.isGroup() );
+    QVERIFY( c.isPackage() );
 
     QCOMPARE( p.childCount(), 0 );  // not noticed it has a child
 }
@@ -109,6 +114,8 @@ ItemTests::testGroup()
     QVERIFY( !p.isCritical() );
     // The item-constructor doesn't consider the packages: list
     QCOMPARE( p.childCount(), 0 );
+    QVERIFY( p.isGroup() );
+    QVERIFY( !p.isPackage() );
 }
 
 QTEST_GUILESS_MAIN( ItemTests )

From f7191ac29eabedd6345ec71dfc18bd349f5871cf Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 10:16:54 +0100
Subject: [PATCH 10/30] [netinstall] Compare two PackageTreeItems

- Packages and groups check different fields for equality.
- Selected-state is **not** part of equality.
- Also operator !=
---
 src/modules/netinstall/PackageTreeItem.cpp | 21 ++++++
 src/modules/netinstall/PackageTreeItem.h   | 10 +++
 src/modules/netinstall/Tests.cpp           | 78 +++++++++++++++++++++-
 3 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index eca62141f..c0f897c06 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -257,3 +257,24 @@ PackageTreeItem::toOperation() const
         return m_packageName;
     }
 }
+
+bool
+PackageTreeItem::operator==( const PackageTreeItem& rhs ) const
+{
+    if ( isGroup() != rhs.isGroup() )
+    {
+        // Different kinds
+        return false;
+    }
+
+    if ( isGroup() )
+    {
+        return name() == rhs.name() && description() == rhs.description() && preScript() == rhs.preScript()
+            && postScript() == rhs.postScript() && isCritical() == rhs.isCritical() && isHidden() == rhs.isHidden()
+            && m_showReadOnly == rhs.m_showReadOnly && expandOnStart() == rhs.expandOnStart();
+    }
+    else
+    {
+        return packageName() == rhs.packageName();
+    }
+}
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index c35b98eea..3f7dcce86 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -116,6 +116,16 @@ public:
     // QStandardItem methods
     int type() const override;
 
+    /** @brief Are two items equal
+     *
+     * This **disregards** parent-item and the child-items, and compares
+     * only the fields for the items-proper (name, .. expanded). Note
+     * also that *isSelected()* is a run-time state, and is **not**
+     * compared either.
+     */
+    bool operator==( const PackageTreeItem& rhs ) const;
+    bool operator!=( const PackageTreeItem& rhs ) const { return !( *this == rhs ); }
+
 private:
     PackageTreeItem* m_parentItem;
     List m_childItems;
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index 420b690aa..ddcf8076b 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -37,6 +37,7 @@ private Q_SLOTS:
     void testRoot();
     void testPackage();
     void testGroup();
+    void testCompare();
 };
 
 ItemTests::ItemTests() {}
@@ -56,6 +57,8 @@ ItemTests::testRoot()
     QCOMPARE( r.name(), QStringLiteral( "<root>" ) );
     QCOMPARE( r.parentItem(), nullptr );
     QVERIFY( r.isGroup() );
+
+    QVERIFY( r == r );
 }
 
 void
@@ -72,6 +75,7 @@ ItemTests::testPackage()
     QVERIFY( !p.isCritical() );
     QVERIFY( !p.isGroup() );
     QVERIFY( p.isPackage() );
+    QVERIFY( p == p );
 
     // This doesn't happen in normal constructions,
     // because a package can't have children.
@@ -82,6 +86,8 @@ ItemTests::testPackage()
     QCOMPARE( c.parentItem(), &p );
     QVERIFY( !c.isGroup() );
     QVERIFY( c.isPackage() );
+    QVERIFY( c == c );
+    QVERIFY( c != p );
 
     QCOMPARE( p.childCount(), 0 );  // not noticed it has a child
 }
@@ -93,7 +99,13 @@ static const char doc[] =
 "  description: \"Tools for the Chakra Community Repository\"\n"
 "  packages:\n"
 "    - ccr\n"
-"    - base-devel\n";
+"    - base-devel\n"
+"    - bash\n";
+
+static const char doc_no_packages[] =
+"- name: \"CCR\"\n"
+"  description: \"Tools for the Chakra Community Repository\"\n"
+"  packages: []\n";
 // *INDENT-ON*
 // clang-format on
 
@@ -116,8 +128,72 @@ ItemTests::testGroup()
     QCOMPARE( p.childCount(), 0 );
     QVERIFY( p.isGroup() );
     QVERIFY( !p.isPackage() );
+    QVERIFY( p == p );
+
+    PackageTreeItem c( "zsh", nullptr );
+    QVERIFY( p != c );
 }
 
+void
+ItemTests::testCompare()
+{
+    PackageTreeItem p0( "bash", nullptr );
+    PackageTreeItem p1( "bash", &p0 );
+    PackageTreeItem p2( "bash", nullptr );
+
+    QVERIFY( p0 == p1 );  // Parent doesn't matter
+    QVERIFY( p0 == p2 );
+
+    p2.setSelected( Qt::Checked );
+    p1.setSelected( Qt::Unchecked );
+    QVERIFY( p0 == p1 );  // Neither does selected state
+    QVERIFY( p0 == p2 );
+
+    PackageTreeItem r0( nullptr );
+    QVERIFY( p0 != r0 );
+    QVERIFY( p1 != r0 );
+    QVERIFY( r0 == r0 );
+    PackageTreeItem r1( nullptr );
+    QVERIFY( r0 == r1 );  // Different roots are still equal
+
+    PackageTreeItem r2( "<root>", nullptr );  // Fake root
+    QVERIFY( r0 != r2 );
+    QVERIFY( r1 != r2 );
+    QVERIFY( p0 != r2 );
+    PackageTreeItem r3( "<root>", nullptr );
+    QVERIFY( r3 == r2 );
+
+    YAML::Node yamldoc = YAML::Load( doc );  // See testGroup()
+    QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc );
+    QCOMPARE( yamlContents.length(), 1 );
+
+    PackageTreeItem p3( yamlContents[ 0 ].toMap(), nullptr );
+    QVERIFY( p3 == p3 );
+    QVERIFY( p3 != p1 );
+    QVERIFY( p1 != p3 );
+    QCOMPARE( p3.childCount(), 0 );  // Doesn't load the packages: list
+
+    PackageTreeItem p4( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc ) )[ 0 ].toMap(), nullptr );
+    QVERIFY( p3 == p4 );
+    PackageTreeItem p5( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc_no_packages ) )[ 0 ].toMap(), nullptr );
+    QVERIFY( p3 == p5 );
+
+#if 0
+    // Check that the sub-packages loaded correctly
+    bool found_one_bash = false;
+    for ( int i = 0; i < p3.childCount(); ++i )
+    {
+        QVERIFY( p3.child( i )->isPackage() );
+        if ( p0 == *p3.child( i ) )
+        {
+            found_one_bash = true;
+        }
+    }
+    QVERIFY( found_one_bash );
+#endif
+}
+
+
 QTEST_GUILESS_MAIN( ItemTests )
 
 #include "utils/moc-warnings.h"

From 0e2b3986b923898f6a9eb945fcede938f8199e6b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 10:46:10 +0100
Subject: [PATCH 11/30] [netinstall] Use explicit accessor for the type-of-item

---
 src/modules/netinstall/PackageTreeItem.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index c0f897c06..ff0f5d5bc 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -103,7 +103,7 @@ PackageTreeItem::row() const
 QVariant
 PackageTreeItem::data( int column ) const
 {
-    if ( !packageName().isEmpty() )  // packages have a packagename, groups don't
+    if ( isPackage() )  // packages have a packagename, groups don't
     {
         switch ( column )
         {

From 025ab8b524b1002264b2d49a3e35cbc0c76cdcc0 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 10:49:16 +0100
Subject: [PATCH 12/30] [netinstall] Be explicit about checkedness-to-bool
 conversions

---
 src/modules/netinstall/PackageTreeItem.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index ff0f5d5bc..e611da477 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -145,10 +145,10 @@ PackageTreeItem::hiddenSelected() const
 {
     if ( !m_isHidden )
     {
-        return m_selected;
+        return m_selected != Qt::Unchecked;
     }
 
-    if ( !m_selected )
+    if ( m_selected == Qt::Unchecked )
     {
         return false;
     }
@@ -163,8 +163,8 @@ PackageTreeItem::hiddenSelected() const
         currentItem = currentItem->parentItem();
     }
 
-    /* Has no non-hiddent parents */
-    return m_selected;
+    /* Has no non-hidden parents */
+    return m_selected != Qt::Unchecked;
 }
 
 
@@ -188,8 +188,8 @@ PackageTreeItem::setSelected( Qt::CheckState isSelected )
         currentItem = currentItem->parentItem();
     }
     if ( currentItem == nullptr )
-    // Reached the root .. don't bother
     {
+        // Reached the root .. don't bother
         return;
     }
 

From f592a3f3732f565c2db263b6fe6e6c135da4d54b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 12:53:35 +0100
Subject: [PATCH 13/30] [netinstall] Expand tests to include group-checking

- Check groups
- Check whole treemodels recursively (this is not in PackageTreeItem,
  because that explicitly ignores the tree structure).
- Also a stub of checking example files (from the src dir)
---
 src/modules/netinstall/PackageModel.h |  2 +
 src/modules/netinstall/Tests.cpp      | 97 +++++++++++++++++++++++++--
 2 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index db41c4197..4d127970b 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -66,6 +66,8 @@ public:
     PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
 
 private:
+    friend class ItemTests;
+
     void setupModelData( const YAML::Node& data, PackageTreeItem* parent );
     void setupModelData( const QVariantList& l, PackageTreeItem* parent );
 
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index ddcf8076b..c0658be33 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -16,6 +16,7 @@
  *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "PackageModel.h"
 #include "PackageTreeItem.h"
 
 #include "utils/Logger.h"
@@ -31,6 +32,10 @@ public:
     ItemTests();
     virtual ~ItemTests() {}
 
+private:
+    void checkAllSelected( PackageTreeItem* p );
+    void recursiveCompare( PackageTreeItem*, PackageTreeItem* );
+
 private Q_SLOTS:
     void initTestCase();
 
@@ -38,6 +43,8 @@ private Q_SLOTS:
     void testPackage();
     void testGroup();
     void testCompare();
+    void testModel();
+    void testExampleFiles();
 };
 
 ItemTests::ItemTests() {}
@@ -106,6 +113,15 @@ static const char doc_no_packages[] =
 "- name: \"CCR\"\n"
 "  description: \"Tools for the Chakra Community Repository\"\n"
 "  packages: []\n";
+
+static const char doc_with_expanded[] =
+"- name: \"CCR\"\n"
+"  description: \"Tools for the Chakra Community Repository\"\n"
+"  expanded: true\n"
+"  packages:\n"
+"    - ccr\n"
+"    - base-devel\n"
+"    - bash\n";
 // *INDENT-ON*
 // clang-format on
 
@@ -177,20 +193,91 @@ ItemTests::testCompare()
     QVERIFY( p3 == p4 );
     PackageTreeItem p5( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc_no_packages ) )[ 0 ].toMap(), nullptr );
     QVERIFY( p3 == p5 );
+}
 
-#if 0
+void
+ItemTests::checkAllSelected( PackageTreeItem* p )
+{
+    QVERIFY( p->isSelected() );
+    for ( int i = 0; i < p->childCount(); ++i )
+    {
+        checkAllSelected( p->child( i ) );
+    }
+}
+
+void
+ItemTests::recursiveCompare( PackageTreeItem* l, PackageTreeItem* r )
+{
+    QVERIFY( l && r );
+    QVERIFY( *l == *r );
+    QCOMPARE( l->childCount(), r->childCount() );
+
+    for ( int i = 0; i < l->childCount(); ++i )
+    {
+        QCOMPARE( l->childCount(), r->childCount() );
+        recursiveCompare( l->child( i ), r->child( i ) );
+    }
+}
+
+void
+ItemTests::testModel()
+{
+    YAML::Node yamldoc = YAML::Load( doc );  // See testGroup()
+    QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc );
+    QCOMPARE( yamlContents.length(), 1 );
+
+    PackageModel m0( yamlContents, nullptr );
+    PackageModel m1( yamldoc, nullptr );
+
+    QCOMPARE( m0.rowCount(), m1.rowCount() );
+    QCOMPARE( m0.m_hiddenItems.count(), 0 );  // Nothing hidden
+    QCOMPARE( m1.m_hiddenItems.count(), 0 );
+    QCOMPARE( m0.rowCount(), 1 );  // Group, the packages are invisible
+    QCOMPARE( m0.rowCount( m0.index( 0, 0 ) ), 3 );  // The packages
+    QCOMPARE( m1.rowCount( m1.index( 0, 0 ) ), 3 );  // The packages
+
+    checkAllSelected( m0.m_rootItem );
+    checkAllSelected( m1.m_rootItem );
+
+    PackageModel m2( YAML::Load( doc_with_expanded ), nullptr );
+    QCOMPARE( m2.m_hiddenItems.count(), 0 );
+    QCOMPARE( m2.rowCount(), 1 );  // Group, now the packages expanded but not counted
+    QCOMPARE( m2.rowCount( m2.index( 0, 0 ) ), 3 );  // The packages
+    checkAllSelected( m2.m_rootItem );
+
+    PackageTreeItem r;
+    QVERIFY( r == *m0.m_rootItem );
+    QVERIFY( r == *m1.m_rootItem );
+
+    QCOMPARE( m0.m_rootItem->childCount(), 1 );
+
+    PackageTreeItem* group = m0.m_rootItem->child( 0 );
+    QVERIFY( group->isGroup() );
+    QCOMPARE( group->name(), QStringLiteral( "CCR" ) );
+    QCOMPARE( group->childCount(), 3 );
+
+    PackageTreeItem bash( "bash", nullptr );
     // Check that the sub-packages loaded correctly
     bool found_one_bash = false;
-    for ( int i = 0; i < p3.childCount(); ++i )
+    for ( int i = 0; i < group->childCount(); ++i )
     {
-        QVERIFY( p3.child( i )->isPackage() );
-        if ( p0 == *p3.child( i ) )
+        QVERIFY( group->child( i )->isPackage() );
+        if ( bash == *( group->child( i ) ) )
         {
             found_one_bash = true;
         }
     }
     QVERIFY( found_one_bash );
-#endif
+
+    recursiveCompare( m0.m_rootItem, m1.m_rootItem );
+
+    // But m2 has "expanded" set which the others do no
+    QVERIFY( *( m2.m_rootItem->child( 0 ) ) != *group );
+}
+
+void
+ItemTests::testExampleFiles()
+{
 }
 
 

From ebc1db6a7b23ead9ddedb34ca287118a6a55c255 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 14:16:23 +0100
Subject: [PATCH 14/30] [netinstall] Test loading of a whole (example) file

---
 src/modules/netinstall/Tests.cpp | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index c0658be33..e85944f6c 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -35,6 +35,7 @@ public:
 private:
     void checkAllSelected( PackageTreeItem* p );
     void recursiveCompare( PackageTreeItem*, PackageTreeItem* );
+    void recursiveCompare( PackageModel&, PackageModel& );
 
 private Q_SLOTS:
     void initTestCase();
@@ -219,6 +220,13 @@ ItemTests::recursiveCompare( PackageTreeItem* l, PackageTreeItem* r )
     }
 }
 
+void
+ItemTests::recursiveCompare( PackageModel& l, PackageModel& r )
+{
+    return recursiveCompare( l.m_rootItem, r.m_rootItem );
+}
+
+
 void
 ItemTests::testModel()
 {
@@ -269,7 +277,7 @@ ItemTests::testModel()
     }
     QVERIFY( found_one_bash );
 
-    recursiveCompare( m0.m_rootItem, m1.m_rootItem );
+    recursiveCompare( m0, m1 );
 
     // But m2 has "expanded" set which the others do no
     QVERIFY( *( m2.m_rootItem->child( 0 ) ) != *group );
@@ -278,6 +286,25 @@ ItemTests::testModel()
 void
 ItemTests::testExampleFiles()
 {
+    QVERIFY( QStringLiteral( BUILD_AS_TEST ).endsWith( "/netinstall" ) );
+
+    QDir d( BUILD_AS_TEST );
+
+    for ( const QString& filename : QStringList { "netinstall.yaml" } )
+    {
+        QFile f( d.filePath( filename ) );
+        QVERIFY( f.exists() );
+        QVERIFY( f.open( QIODevice::ReadOnly ) );
+        QByteArray contents = f.readAll();
+        QVERIFY( !contents.isEmpty() );
+
+        YAML::Node yamldoc = YAML::Load( contents.constData() );
+        QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc );
+
+        PackageModel m0( yamldoc, nullptr );
+        PackageModel m1( yamlContents, nullptr );
+        recursiveCompare( m0, m1 );
+    }
 }
 
 

From fa28788f7832fd929080f2c590d51a65f1445bf1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 14:21:41 +0100
Subject: [PATCH 15/30] [netinstall] Build the model from QVariantList always

---
 src/modules/netinstall/PackageModel.cpp | 42 +------------------------
 src/modules/netinstall/PackageModel.h   |  1 -
 2 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 0ac8116b3..3a4deaf9a 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -26,7 +26,7 @@ PackageModel::PackageModel( const YAML::Node& data, QObject* parent )
     : QAbstractItemModel( parent )
 {
     m_rootItem = new PackageTreeItem();
-    setupModelData( data, m_rootItem );
+    setupModelData( CalamaresUtils::yamlSequenceToVariant( data ), m_rootItem );
 }
 
 PackageModel::PackageModel( const QVariantList& data, QObject* parent )
@@ -262,43 +262,3 @@ PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* pa
         }
     }
 }
-
-void
-PackageModel::setupModelData( const YAML::Node& data, PackageTreeItem* parent )
-{
-    for ( YAML::const_iterator it = data.begin(); it != data.end(); ++it )
-    {
-        const YAML::Node itemDefinition = *it;
-        PackageTreeItem* item = new PackageTreeItem( CalamaresUtils::yamlMapToVariant( itemDefinition ), parent );
-
-        if ( itemDefinition[ "selected" ] )
-        {
-            item->setSelected( getBool( itemDefinition, "selected" ) ? Qt::Checked : Qt::Unchecked );
-        }
-
-        if ( itemDefinition[ "packages" ] )
-        {
-            for ( YAML::const_iterator packageIt = itemDefinition[ "packages" ].begin();
-                  packageIt != itemDefinition[ "packages" ].end();
-                  ++packageIt )
-            {
-                item->appendChild(
-                    new PackageTreeItem( CalamaresUtils::yamlToVariant( *packageIt ).toString(), item ) );
-            }
-        }
-        if ( itemDefinition[ "subgroups" ] )
-        {
-            setupModelData( itemDefinition[ "subgroups" ], item );
-        }
-
-        if ( item->isHidden() )
-        {
-            m_hiddenItems.append( item );
-        }
-        else
-        {
-            item->setCheckable( true );
-            parent->appendChild( item );
-        }
-    }
-}
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 4d127970b..04004b661 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -68,7 +68,6 @@ public:
 private:
     friend class ItemTests;
 
-    void setupModelData( const YAML::Node& data, PackageTreeItem* parent );
     void setupModelData( const QVariantList& l, PackageTreeItem* parent );
 
     PackageTreeItem* m_rootItem;

From f59cae2dbb47fe1c180d3fa858e0809383f0ed61 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 14:41:00 +0100
Subject: [PATCH 16/30] [netinstall] Document `local` URL

- `local` is supposed to read from the config-file, rather than
  externally; this simplifies examples, makes it easier to have
  multiple netinstalls, and condenses the documentation.
---
 src/modules/netinstall/netinstall.conf | 39 +++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf
index 5f90fec76..96977bdd0 100644
--- a/src/modules/netinstall/netinstall.conf
+++ b/src/modules/netinstall/netinstall.conf
@@ -11,7 +11,11 @@
 #
 # The format of the groups file is documented in `README.md`.
 #
-# groupsUrl: file:///usr/share/calamares/netinstall.yaml
+# As a special case, setting *groupsUrl* to the literal string
+# `local` means that the data is obtained from **this** config
+# file, under the key *groups*.
+#
+groupsUrl: local
 
 # If the installation can proceed without netinstall (e.g. the Live CD
 # can create a working installed system, but netinstall is preferred
@@ -46,3 +50,36 @@ label:
     # sidebar[nl]: "Pakketkeuze"
     # title: "Office Package"
     # title[nl]: "Kantoorsoftware"
+
+# If, and only if, *groupsUrl* is set to the literal string `local`,
+# groups data is read from this file. The value of *groups* must be
+# a list, with the same format as the regular `netinstall.yaml` file.
+#
+# This is recommended only for small static package lists.
+groups:
+    - name: "Default"
+      description: "Default group"
+      hidden: true
+      selected: true
+      critical: false
+      packages:
+        - base
+        - chakra-live-skel
+    - name: "Shells"
+      description: "Shells"
+      hidden: false
+      selected: false
+      critical: true
+      subgroups:
+        - name: "Bash"
+          description: "Bourne Again Shell"
+          selected: true
+          packages:
+            - bash
+            - bash-completion
+        - name: "Zsh"
+          description: "Zee shell, boss"
+          packages:
+            - zsh
+            - zsh-completion
+            - zsh-extensions

From 1a5c916923d1a7e702edc3c1a2e7ba0459a23f5e Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 23 Mar 2020 23:08:31 +0100
Subject: [PATCH 17/30] [netinstall] Implement `local` loading of packages

- For a static list of selectable packages (e.g. what you might otherwise
  use file:/// for with a static file on the ISO) you can now stick the
  list in the config file itself, simplifying some setups.
- Also saves faffing about with network.

SEE #1319
---
 src/modules/netinstall/NetInstallPage.cpp     | 27 +++++++++++++++++--
 src/modules/netinstall/NetInstallPage.h       |  9 +++++++
 src/modules/netinstall/NetInstallViewStep.cpp | 10 ++++++-
 3 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 35086cb44..13e9418a4 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -154,6 +154,14 @@ NetInstallPage::dataIsHere()
     ui->groupswidget->header()->setSectionResizeMode( 0, QHeaderView::ResizeToContents );
     ui->groupswidget->header()->setSectionResizeMode( 1, QHeaderView::Stretch );
 
+    expandGroups();
+
+    emit checkReady( true );
+}
+
+void
+NetInstallPage::expandGroups()
+{
     // Go backwards because expanding a group may cause rows to appear below it
     for ( int i = m_groups->rowCount() - 1; i >= 0; --i )
     {
@@ -163,8 +171,6 @@ NetInstallPage::dataIsHere()
             ui->groupswidget->setExpanded( index, true );
         }
     }
-
-    emit checkReady( true );
 }
 
 PackageTreeItem::List
@@ -203,6 +209,23 @@ NetInstallPage::loadGroupList( const QString& confUrl )
     }
 }
 
+void
+NetInstallPage::loadGroupList( const QVariantList& l )
+{
+    // This short-cuts through loading and just uses the data,
+    // containing cruft from dataIsHere() and readGroups().
+    m_groups = new PackageModel( l );
+    retranslate();  // For changed model
+    ui->groupswidget->setModel( m_groups );
+    ui->groupswidget->header()->setSectionResizeMode( 0, QHeaderView::ResizeToContents );
+    ui->groupswidget->header()->setSectionResizeMode( 1, QHeaderView::Stretch );
+
+    expandGroups();
+
+    emit checkReady( true );
+}
+
+
 void
 NetInstallPage::setRequired( bool b )
 {
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index f9d7127e2..fdbdbac88 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -64,6 +64,8 @@ public:
      * displaying the page.
      */
     void loadGroupList( const QString& url );
+    /// @brief Retrieve pre-processed and fetched group data
+    void loadGroupList( const QVariantList& l );
 
     // Sets the "required" state of netinstall data. Influences whether
     // corrupt or unavailable data causes checkReady() to be emitted
@@ -90,6 +92,13 @@ private:
     // of this module to know the format expected of the YAML files.
     bool readGroups( const QByteArray& yamlData );
 
+    /** @brief Expand entries that should be pre-expanded
+     *
+     * Follows the *expanded* key / the startExpanded field in the
+     * group entries of the model. Call this after filling up the model.
+     */
+    void expandGroups();
+
     Ui::Page_NetInst* ui;
 
     std::unique_ptr< CalamaresUtils::Locale::TranslatedString > m_title;  // Above the treeview
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index ba16d940c..9c8b186de 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -209,7 +209,15 @@ NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
         // Keep putting groupsUrl into the global storage,
         // even though it's no longer used for in-module data-passing.
         Calamares::JobQueue::instance()->globalStorage()->insert( "groupsUrl", groupsUrl );
-        m_widget->loadGroupList( groupsUrl );
+        if ( groupsUrl == QStringLiteral( "local" ) )
+        {
+            QVariantList l = configurationMap.value( "groups" ).toList();
+            m_widget->loadGroupList( l );
+        }
+        else
+        {
+            m_widget->loadGroupList( groupsUrl );
+        }
     }
 
     bool bogus = false;

From 5e03df723ca7ecaf376cf10a7a24ae9caaa4714f Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 24 Mar 2020 12:02:16 +0100
Subject: [PATCH 18/30] [netinstall] Add a (stub) Config object

- Add initial definition of Config object, which will extract the model-
  setting and loading code from the page, and which is also prep-work
  for a QML version of this module.
- While here, remove superfluous code
---
 src/modules/netinstall/CMakeLists.txt   |  1 +
 src/modules/netinstall/Config.cpp       | 34 +++++++++++++++++
 src/modules/netinstall/Config.h         | 50 +++++++++++++++++++++++++
 src/modules/netinstall/PackageModel.cpp |  8 +---
 4 files changed, 86 insertions(+), 7 deletions(-)
 create mode 100644 src/modules/netinstall/Config.cpp
 create mode 100644 src/modules/netinstall/Config.h

diff --git a/src/modules/netinstall/CMakeLists.txt b/src/modules/netinstall/CMakeLists.txt
index 9d0167670..3e6ac3cb5 100644
--- a/src/modules/netinstall/CMakeLists.txt
+++ b/src/modules/netinstall/CMakeLists.txt
@@ -2,6 +2,7 @@ calamares_add_plugin( netinstall
     TYPE viewmodule
     EXPORT_MACRO PLUGINDLLEXPORT_PRO
     SOURCES
+        Config.cpp
         NetInstallViewStep.cpp
         NetInstallPage.cpp
         PackageTreeItem.cpp
diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
new file mode 100644
index 000000000..9218fab22
--- /dev/null
+++ b/src/modules/netinstall/Config.cpp
@@ -0,0 +1,34 @@
+/*
+ *   Copyright 2016, Luca Giambonini <almack@chakraos.org>
+ *   Copyright 2016, Lisa Vitolo     <shainer@chakraos.org>
+ *   Copyright 2017, Kyle Robbertze  <krobbertze@gmail.com>
+ *   Copyright 2017-2018, 2020, Adriaan de Groot <groot@kde.org>
+ *   Copyright 2017, Gabriel Craciunescu <crazy@frugalware.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
+ *   the Free Software Foundation, either version 3 of the License, or
+ *   (at your option) any later version.
+ *
+ *   Calamares is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "Config.h"
+
+Config::Config( QObject* parent )
+    : m_model( nullptr )
+{
+}
+
+void
+Config::setStatus( const QString& s )
+{
+    m_status = s;
+    emit statusChanged( m_status );
+}
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
new file mode 100644
index 000000000..497871633
--- /dev/null
+++ b/src/modules/netinstall/Config.h
@@ -0,0 +1,50 @@
+/*
+ *   Copyright 2016, Luca Giambonini <almack@chakraos.org>
+ *   Copyright 2016, Lisa Vitolo     <shainer@chakraos.org>
+ *   Copyright 2017, Kyle Robbertze  <krobbertze@gmail.com>
+ *   Copyright 2017-2018, 2020, 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
+ *   the Free Software Foundation, either version 3 of the License, or
+ *   (at your option) any later version.
+ *
+ *   Calamares is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef NETINSTALL_CONFIG_H
+#define NETINSTALL_CONFIG_H
+
+#include "PackageModel.h"
+
+#include <QObject>
+
+class Config : public QObject
+{
+    Q_OBJECT
+
+    Q_PROPERTY( PackageModel* packageModel MEMBER m_model FINAL )
+
+    Q_PROPERTY( QString status READ status WRITE setStatus NOTIFY statusChanged FINAL )
+
+public:
+    Config( QObject* parent = nullptr );
+
+    QString status() const { return m_status; }
+    void setStatus( const QString& s );
+
+signals:
+    void statusChanged( QString status );
+
+private:
+    QString m_status;
+    PackageModel* m_model = nullptr;
+};
+
+#endif
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 3a4deaf9a..26b5eb552 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -1,7 +1,7 @@
 /* === This file is part of Calamares - <https://github.com/calamares> ===
  *
  *   Copyright (c) 2017, Kyle Robbertze <kyle@aims.ac.za>
- *   Copyright 2017-2018, Adriaan de Groot <groot@kde.org>
+ *   Copyright 2017-2018, 2020, 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
@@ -214,12 +214,6 @@ PackageModel::getItemPackages( PackageTreeItem* item ) const
     return selectedPackages;
 }
 
-static bool
-getBool( const YAML::Node& itemDefinition, const char* key )
-{
-    return itemDefinition[ key ] ? CalamaresUtils::yamlToVariant( itemDefinition[ key ] ).toBool() : false;
-}
-
 void
 PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* parent )
 {

From 938536c0c341e81b3c3870b577f7c80f97a7ea5e Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 24 Mar 2020 12:36:31 +0100
Subject: [PATCH 19/30] [netinstall] Allow post-creation loading of model data

- Instead of loading all in the constructor, provide a public
  setupModelData().
- This allows creating the model and setting it for UI, before
  the load completes.
---
 src/modules/netinstall/NetInstallPage.cpp |  6 ++--
 src/modules/netinstall/PackageModel.cpp   | 43 ++++++++++++++---------
 src/modules/netinstall/PackageModel.h     |  7 ++--
 src/modules/netinstall/Tests.cpp          | 21 +++++------
 4 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 13e9418a4..b1e04c56a 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -87,7 +87,8 @@ NetInstallPage::readGroups( const QByteArray& yamlData )
             cWarning() << "netinstall groups data does not form a sequence.";
         }
         Q_ASSERT( groups.IsSequence() );
-        m_groups = new PackageModel( groups );
+        m_groups = new PackageModel();
+        m_groups->setupModelData( CalamaresUtils::yamlSequenceToVariant( groups ) );
         return true;
     }
     catch ( YAML::Exception& e )
@@ -214,7 +215,8 @@ NetInstallPage::loadGroupList( const QVariantList& l )
 {
     // This short-cuts through loading and just uses the data,
     // containing cruft from dataIsHere() and readGroups().
-    m_groups = new PackageModel( l );
+    m_groups = new PackageModel();
+    m_groups->setupModelData( l );
     retranslate();  // For changed model
     ui->groupswidget->setModel( m_groups );
     ui->groupswidget->header()->setSectionResizeMode( 0, QHeaderView::ResizeToContents );
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 26b5eb552..88a06a1bb 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -22,18 +22,9 @@
 #include "utils/Variant.h"
 #include "utils/Yaml.h"
 
-PackageModel::PackageModel( const YAML::Node& data, QObject* parent )
+PackageModel::PackageModel( QObject* parent )
     : QAbstractItemModel( parent )
 {
-    m_rootItem = new PackageTreeItem();
-    setupModelData( CalamaresUtils::yamlSequenceToVariant( data ), m_rootItem );
-}
-
-PackageModel::PackageModel( const QVariantList& data, QObject* parent )
-    : QAbstractItemModel( parent )
-{
-    m_rootItem = new PackageTreeItem();
-    setupModelData( data, m_rootItem );
 }
 
 PackageModel::~PackageModel()
@@ -44,7 +35,7 @@ PackageModel::~PackageModel()
 QModelIndex
 PackageModel::index( int row, int column, const QModelIndex& parent ) const
 {
-    if ( !hasIndex( row, column, parent ) )
+    if ( !m_rootItem || !hasIndex( row, column, parent ) )
     {
         return QModelIndex();
     }
@@ -74,7 +65,7 @@ PackageModel::index( int row, int column, const QModelIndex& parent ) const
 QModelIndex
 PackageModel::parent( const QModelIndex& index ) const
 {
-    if ( !index.isValid() )
+    if ( !m_rootItem || !index.isValid() )
     {
         return QModelIndex();
     }
@@ -92,7 +83,7 @@ PackageModel::parent( const QModelIndex& index ) const
 int
 PackageModel::rowCount( const QModelIndex& parent ) const
 {
-    if ( parent.column() > 0 )
+    if ( !m_rootItem || ( parent.column() > 0 ) )
     {
         return 0;
     }
@@ -119,7 +110,7 @@ PackageModel::columnCount( const QModelIndex& ) const
 QVariant
 PackageModel::data( const QModelIndex& index, int role ) const
 {
-    if ( !index.isValid() )
+    if ( !m_rootItem || !index.isValid() )
     {
         return QVariant();
     }
@@ -141,6 +132,11 @@ PackageModel::data( const QModelIndex& index, int role ) const
 bool
 PackageModel::setData( const QModelIndex& index, const QVariant& value, int role )
 {
+    if ( !m_rootItem )
+    {
+        return false;
+    }
+
     if ( role == Qt::CheckStateRole && index.isValid() )
     {
         PackageTreeItem* item = static_cast< PackageTreeItem* >( index.internalPointer() );
@@ -156,7 +152,7 @@ PackageModel::setData( const QModelIndex& index, const QVariant& value, int role
 Qt::ItemFlags
 PackageModel::flags( const QModelIndex& index ) const
 {
-    if ( !index.isValid() )
+    if ( !m_rootItem || !index.isValid() )
     {
         return Qt::ItemFlags();
     }
@@ -180,6 +176,11 @@ PackageModel::headerData( int section, Qt::Orientation orientation, int role ) c
 PackageTreeItem::List
 PackageModel::getPackages() const
 {
+    if ( !m_rootItem )
+    {
+        return PackageTreeItem::List();
+    }
+
     auto items = getItemPackages( m_rootItem );
     for ( auto package : m_hiddenItems )
     {
@@ -194,7 +195,7 @@ PackageModel::getPackages() const
 PackageTreeItem::List
 PackageModel::getItemPackages( PackageTreeItem* item ) const
 {
-    QList< PackageTreeItem* > selectedPackages;
+    PackageTreeItem::List selectedPackages;
     for ( int i = 0; i < item->childCount(); i++ )
     {
         if ( item->child( i )->isSelected() == Qt::Unchecked )
@@ -256,3 +257,13 @@ PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* pa
         }
     }
 }
+
+void
+PackageModel::setupModelData( const QVariantList& l )
+{
+    emit beginResetModel();
+    delete m_rootItem;
+    m_rootItem = new PackageTreeItem();
+    setupModelData( l, m_rootItem );
+    emit endResetModel();
+}
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 04004b661..b4e8fc102 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -47,10 +47,11 @@ public:
      */
     static constexpr const int MetaExpandRole = Qt::UserRole + 1;
 
-    explicit PackageModel( const YAML::Node& data, QObject* parent = nullptr );
-    explicit PackageModel( const QVariantList& data, QObject* parent = nullptr );
+    explicit PackageModel( QObject* parent = nullptr );
     ~PackageModel() override;
 
+    void setupModelData( const QVariantList& l );
+
     QVariant data( const QModelIndex& index, int role ) const override;
     bool setData( const QModelIndex& index, const QVariant& value, int role = Qt::EditRole ) override;
     Qt::ItemFlags flags( const QModelIndex& index ) const override;
@@ -70,7 +71,7 @@ private:
 
     void setupModelData( const QVariantList& l, PackageTreeItem* parent );
 
-    PackageTreeItem* m_rootItem;
+    PackageTreeItem* m_rootItem = nullptr;
     PackageTreeItem::List m_hiddenItems;
 };
 
diff --git a/src/modules/netinstall/Tests.cpp b/src/modules/netinstall/Tests.cpp
index e85944f6c..cfaf20efa 100644
--- a/src/modules/netinstall/Tests.cpp
+++ b/src/modules/netinstall/Tests.cpp
@@ -234,20 +234,17 @@ ItemTests::testModel()
     QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc );
     QCOMPARE( yamlContents.length(), 1 );
 
-    PackageModel m0( yamlContents, nullptr );
-    PackageModel m1( yamldoc, nullptr );
+    PackageModel m0( nullptr );
+    m0.setupModelData( yamlContents );
 
-    QCOMPARE( m0.rowCount(), m1.rowCount() );
     QCOMPARE( m0.m_hiddenItems.count(), 0 );  // Nothing hidden
-    QCOMPARE( m1.m_hiddenItems.count(), 0 );
     QCOMPARE( m0.rowCount(), 1 );  // Group, the packages are invisible
     QCOMPARE( m0.rowCount( m0.index( 0, 0 ) ), 3 );  // The packages
-    QCOMPARE( m1.rowCount( m1.index( 0, 0 ) ), 3 );  // The packages
 
     checkAllSelected( m0.m_rootItem );
-    checkAllSelected( m1.m_rootItem );
 
-    PackageModel m2( YAML::Load( doc_with_expanded ), nullptr );
+    PackageModel m2( nullptr );
+    m2.setupModelData( CalamaresUtils::yamlSequenceToVariant( YAML::Load( doc_with_expanded ) ) );
     QCOMPARE( m2.m_hiddenItems.count(), 0 );
     QCOMPARE( m2.rowCount(), 1 );  // Group, now the packages expanded but not counted
     QCOMPARE( m2.rowCount( m2.index( 0, 0 ) ), 3 );  // The packages
@@ -255,7 +252,6 @@ ItemTests::testModel()
 
     PackageTreeItem r;
     QVERIFY( r == *m0.m_rootItem );
-    QVERIFY( r == *m1.m_rootItem );
 
     QCOMPARE( m0.m_rootItem->childCount(), 1 );
 
@@ -277,8 +273,6 @@ ItemTests::testModel()
     }
     QVERIFY( found_one_bash );
 
-    recursiveCompare( m0, m1 );
-
     // But m2 has "expanded" set which the others do no
     QVERIFY( *( m2.m_rootItem->child( 0 ) ) != *group );
 }
@@ -301,9 +295,10 @@ ItemTests::testExampleFiles()
         YAML::Node yamldoc = YAML::Load( contents.constData() );
         QVariantList yamlContents = CalamaresUtils::yamlSequenceToVariant( yamldoc );
 
-        PackageModel m0( yamldoc, nullptr );
-        PackageModel m1( yamlContents, nullptr );
-        recursiveCompare( m0, m1 );
+        PackageModel m1( nullptr );
+        m1.setupModelData( yamlContents );
+
+        // TODO: should test *something* about this file :/
     }
 }
 

From f5b4e5d5e1df2261e769d78bee855c6cb4cc6650 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 24 Mar 2020 13:13:18 +0100
Subject: [PATCH 20/30] [netinstall] Add data-loading to the Config object

- Mostly copied from NetInstallPage
---
 src/modules/netinstall/Config.cpp | 103 +++++++++++++++++++++++++++++-
 src/modules/netinstall/Config.h   |  24 ++++++-
 2 files changed, 125 insertions(+), 2 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 9218fab22..1856e5a49 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -21,14 +21,115 @@
 
 #include "Config.h"
 
+#include "network/Manager.h"
+#include "utils/Logger.h"
+#include "utils/Yaml.h"
+
+#include <QNetworkReply>
+
 Config::Config( QObject* parent )
-    : m_model( nullptr )
+    : QObject( parent )
+    , m_model( new PackageModel( this ) )
 {
 }
 
+Config::~Config() {}
+
 void
 Config::setStatus( const QString& s )
 {
     m_status = s;
     emit statusChanged( m_status );
 }
+
+void
+Config::loadGroupList( const QVariantList& groupData )
+{
+    m_model->setupModelData( groupData );
+}
+
+void
+Config::loadGroupList( const QUrl& url )
+{
+    if ( !url.isValid() )
+    {
+        setStatus( tr( "Network Installation. (Disabled: Incorrect configuration)" ) );
+    }
+
+    using namespace CalamaresUtils::Network;
+
+    cDebug() << "NetInstall loading groups from" << url;
+    QNetworkReply* reply = Manager::instance().asynchronouseGet(
+        url,
+        RequestOptions( RequestOptions::FakeUserAgent | RequestOptions::FollowRedirect, std::chrono::seconds( 30 ) ) );
+
+    if ( !reply )
+    {
+        cDebug() << Logger::Continuation << "request failed immediately.";
+        setStatus( tr( "Network Installation. (Disabled: Incorrect configuration)" ) );
+    }
+    else
+    {
+        m_reply = reply;
+        connect( reply, &QNetworkReply::finished, this, &Config::receivedGroupData );
+    }
+}
+
+/// @brief Convenience to zero out and deleteLater on the reply, used in dataIsHere
+struct ReplyDeleter
+{
+    QNetworkReply*& p;
+
+    ~ReplyDeleter()
+    {
+        if ( p )
+        {
+            p->deleteLater();
+        }
+        p = nullptr;
+    }
+};
+
+void
+Config::receivedGroupData()
+{
+    if ( !m_reply || !m_reply->isFinished() )
+    {
+        cWarning() << "NetInstall data called too early.";
+        setStatus( tr( "Network Installation. (Disabled: internal error)" ) );
+        return;
+    }
+
+    cDebug() << "NetInstall group data received" << m_reply->size() << "bytes from" << m_reply->url();
+
+    ReplyDeleter d { m_reply };
+
+    // If m_required is *false* then we still say we're ready
+    // even if the reply is corrupt or missing.
+    if ( m_reply->error() != QNetworkReply::NoError )
+    {
+        cWarning() << "unable to fetch netinstall package lists.";
+        cDebug() << Logger::SubEntry << "Netinstall reply error: " << m_reply->error();
+        cDebug() << Logger::SubEntry << "Request for url: " << m_reply->url().toString()
+                 << " failed with: " << m_reply->errorString();
+        setStatus( tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" ) );
+        return;
+    }
+
+    QByteArray yamlData = m_reply->readAll();
+    try
+    {
+        YAML::Node groups = YAML::Load( yamlData.constData() );
+
+        if ( !groups.IsSequence() )
+        {
+            cWarning() << "NetInstall groups data does not form a sequence.";
+        }
+        loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
+    }
+    catch ( YAML::Exception& e )
+    {
+        CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
+        setStatus( tr( "Network Installation. (Disabled: Received invalid groups data)" ) );
+    }
+}
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 497871633..17bfab31a 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -24,6 +24,9 @@
 #include "PackageModel.h"
 
 #include <QObject>
+#include <QUrl>
+
+class QNetworkReply;
 
 class Config : public QObject
 {
@@ -35,16 +38,35 @@ class Config : public QObject
 
 public:
     Config( QObject* parent = nullptr );
+    virtual ~Config();
 
     QString status() const { return m_status; }
     void setStatus( const QString& s );
 
+    /** @brief Retrieves the groups, with name, description and packages
+     *
+     * Loads data from the given URL. Once done, the data is parsed
+     * and passed on to the other loadGroupList() method.
+     */
+    void loadGroupList( const QUrl& url );
+
+    /** @brief Fill model from parsed data.
+     *
+     * Fills the model with a list of groups -- which can contain
+     * subgroups and packages -- from @p groupData.
+     */
+    void loadGroupList( const QVariantList& groupData );
+
 signals:
     void statusChanged( QString status );
 
+private slots:
+    void receivedGroupData();  ///< From async-loading group data
+
 private:
     QString m_status;
-    PackageModel* m_model = nullptr;
+    PackageModel* m_model;
+    QNetworkReply* m_reply = nullptr;  // For fetching data
 };
 
 #endif

From 1a74a713b663485c68a35591c74c389b29d305dd Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 14:54:06 +0100
Subject: [PATCH 21/30] [netinstall] Make status an enum

- Since we might change translations after loading, display
  the message based on the status enum, rather than
  setting it once at load-time.
---
 src/modules/netinstall/Config.cpp | 35 ++++++++++++++++++++++++-------
 src/modules/netinstall/Config.h   | 18 +++++++++++-----
 2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 1856e5a49..3ed9074c6 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -29,17 +29,38 @@
 
 Config::Config( QObject* parent )
     : QObject( parent )
+    , m_status( Status::Ok )
     , m_model( new PackageModel( this ) )
 {
 }
 
 Config::~Config() {}
 
+QString
+Config::status() const
+{
+    switch ( m_status )
+    {
+    case Status::Ok:
+        return QString();
+    case Status::FailedBadConfiguration:
+        return tr( "Network Installation. (Disabled: Incorrect configuration)" );
+    case Status::FailedBadData:
+        return tr( "Network Installation. (Disabled: Received invalid groups data)" );
+    case Status::FailedInternalError:
+        return tr( "Network Installation. (Disabled: internal error)" );
+    case Status::FailedNetworkError:
+        return tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" );
+    }
+    NOTREACHED return QString();
+}
+
+
 void
-Config::setStatus( const QString& s )
+Config::setStatus( Status s )
 {
     m_status = s;
-    emit statusChanged( m_status );
+    emit statusChanged( status() );
 }
 
 void
@@ -53,7 +74,7 @@ Config::loadGroupList( const QUrl& url )
 {
     if ( !url.isValid() )
     {
-        setStatus( tr( "Network Installation. (Disabled: Incorrect configuration)" ) );
+        setStatus( Status::FailedBadConfiguration );
     }
 
     using namespace CalamaresUtils::Network;
@@ -66,7 +87,7 @@ Config::loadGroupList( const QUrl& url )
     if ( !reply )
     {
         cDebug() << Logger::Continuation << "request failed immediately.";
-        setStatus( tr( "Network Installation. (Disabled: Incorrect configuration)" ) );
+        setStatus( Status::FailedBadConfiguration );
     }
     else
     {
@@ -96,7 +117,7 @@ Config::receivedGroupData()
     if ( !m_reply || !m_reply->isFinished() )
     {
         cWarning() << "NetInstall data called too early.";
-        setStatus( tr( "Network Installation. (Disabled: internal error)" ) );
+        setStatus( Status::FailedInternalError );
         return;
     }
 
@@ -112,7 +133,7 @@ Config::receivedGroupData()
         cDebug() << Logger::SubEntry << "Netinstall reply error: " << m_reply->error();
         cDebug() << Logger::SubEntry << "Request for url: " << m_reply->url().toString()
                  << " failed with: " << m_reply->errorString();
-        setStatus( tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" ) );
+        setStatus( Status::FailedNetworkError );
         return;
     }
 
@@ -130,6 +151,6 @@ Config::receivedGroupData()
     catch ( YAML::Exception& e )
     {
         CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
-        setStatus( tr( "Network Installation. (Disabled: Received invalid groups data)" ) );
+        setStatus( Status::FailedBadData );
     }
 }
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 17bfab31a..62d43c17e 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -33,15 +33,23 @@ class Config : public QObject
     Q_OBJECT
 
     Q_PROPERTY( PackageModel* packageModel MEMBER m_model FINAL )
-
-    Q_PROPERTY( QString status READ status WRITE setStatus NOTIFY statusChanged FINAL )
+    Q_PROPERTY( QString status READ status NOTIFY statusChanged FINAL )
 
 public:
     Config( QObject* parent = nullptr );
     virtual ~Config();
 
-    QString status() const { return m_status; }
-    void setStatus( const QString& s );
+    enum class Status
+    {
+        Ok,
+        FailedBadConfiguration,
+        FailedInternalError,
+        FailedNetworkError,
+        FailedBadData
+    };
+
+    QString status() const;
+    void setStatus( Status s );
 
     /** @brief Retrieves the groups, with name, description and packages
      *
@@ -64,7 +72,7 @@ private slots:
     void receivedGroupData();  ///< From async-loading group data
 
 private:
-    QString m_status;
+    Status m_status;
     PackageModel* m_model;
     QNetworkReply* m_reply = nullptr;  // For fetching data
 };

From 9a354271135031b577b09937ef0f632365d08450 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 15:14:37 +0100
Subject: [PATCH 22/30] [netinstall] Remove unused m_jobs

- Netinstall doesn't make any jobs itself, so drop the member variable
- Use type alias, and simplify jobs()
---
 src/modules/netinstall/NetInstallViewStep.cpp | 4 ++--
 src/modules/netinstall/NetInstallViewStep.h   | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 9c8b186de..8515d537a 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -111,10 +111,10 @@ NetInstallViewStep::isAtEnd() const
 }
 
 
-QList< Calamares::job_ptr >
+Calamares::JobList
 NetInstallViewStep::jobs() const
 {
-    return m_jobs;
+    return Calamares::JobList();
 }
 
 
diff --git a/src/modules/netinstall/NetInstallViewStep.h b/src/modules/netinstall/NetInstallViewStep.h
index ad796b8b2..ffcc785c8 100644
--- a/src/modules/netinstall/NetInstallViewStep.h
+++ b/src/modules/netinstall/NetInstallViewStep.h
@@ -47,7 +47,7 @@ public:
     bool isAtBeginning() const override;
     bool isAtEnd() const override;
 
-    QList< Calamares::job_ptr > jobs() const override;
+    Calamares::JobList jobs() const override;
 
     void onActivate() override;
 
@@ -63,7 +63,6 @@ private:
     NetInstallPage* m_widget;
     bool m_nextEnabled;
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel;  // As it appears in the sidebar
-    QList< Calamares::job_ptr > m_jobs;
 };
 
 CALAMARES_PLUGIN_FACTORY_DECLARATION( NetInstallViewStepFactory )

From 4cdfe1276ae76bb42caf44e143ebe44357e6dc10 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 15:51:03 +0100
Subject: [PATCH 23/30] [netinstall] Rip loading out of the UI page

- Create a config object in the ViewStep
- Model lives in the config object and loads there
- Give model to the UI page for display
---
 src/modules/netinstall/Config.cpp             |   1 -
 src/modules/netinstall/Config.h               |  10 +-
 src/modules/netinstall/NetInstallPage.cpp     | 162 +-----------------
 src/modules/netinstall/NetInstallPage.h       |  47 +----
 src/modules/netinstall/NetInstallViewStep.cpp |  11 +-
 src/modules/netinstall/NetInstallViewStep.h   |   6 +-
 6 files changed, 34 insertions(+), 203 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 3ed9074c6..af4ae8e50 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -29,7 +29,6 @@
 
 Config::Config( QObject* parent )
     : QObject( parent )
-    , m_status( Status::Ok )
     , m_model( new PackageModel( this ) )
 {
 }
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 62d43c17e..372c82bee 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -51,6 +51,9 @@ public:
     QString status() const;
     void setStatus( Status s );
 
+    bool required() const { return m_required; }
+    void setRequired( bool r ) { m_required = r; }
+
     /** @brief Retrieves the groups, with name, description and packages
      *
      * Loads data from the given URL. Once done, the data is parsed
@@ -65,6 +68,8 @@ public:
      */
     void loadGroupList( const QVariantList& groupData );
 
+    PackageModel* model() const { return m_model; }
+
 signals:
     void statusChanged( QString status );
 
@@ -72,9 +77,10 @@ private slots:
     void receivedGroupData();  ///< From async-loading group data
 
 private:
-    Status m_status;
-    PackageModel* m_model;
+    PackageModel* m_model = nullptr;
     QNetworkReply* m_reply = nullptr;  // For fetching data
+    Status m_status = Status::Ok;
+    bool m_required = false;
 };
 
 #endif
diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index b1e04c56a..ab3e1a933 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -37,19 +37,13 @@
 NetInstallPage::NetInstallPage( QWidget* parent )
     : QWidget( parent )
     , ui( new Ui::Page_NetInst )
-    , m_reply( nullptr )
-    , m_groups( nullptr )
 {
     ui->setupUi( this );
     setPageTitle( nullptr );
     CALAMARES_RETRANSLATE_SLOT( &NetInstallPage::retranslate );
 }
 
-NetInstallPage::~NetInstallPage()
-{
-    delete m_groups;
-    delete m_reply;
-}
+NetInstallPage::~NetInstallPage() {}
 
 void
 NetInstallPage::setPageTitle( CalamaresUtils::Locale::TranslatedString* t )
@@ -75,165 +69,21 @@ NetInstallPage::retranslate()
     }
 }
 
-bool
-NetInstallPage::readGroups( const QByteArray& yamlData )
-{
-    try
-    {
-        YAML::Node groups = YAML::Load( yamlData.constData() );
-
-        if ( !groups.IsSequence() )
-        {
-            cWarning() << "netinstall groups data does not form a sequence.";
-        }
-        Q_ASSERT( groups.IsSequence() );
-        m_groups = new PackageModel();
-        m_groups->setupModelData( CalamaresUtils::yamlSequenceToVariant( groups ) );
-        return true;
-    }
-    catch ( YAML::Exception& e )
-    {
-        CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
-        return false;
-    }
-}
-
-/// @brief Convenience to zero out and deleteLater on the reply, used in dataIsHere
-struct ReplyDeleter
-{
-    QNetworkReply*& p;
-
-    ~ReplyDeleter()
-    {
-        if ( p )
-        {
-            p->deleteLater();
-        }
-        p = nullptr;
-    }
-};
-
 void
-NetInstallPage::dataIsHere()
-{
-    if ( !m_reply || !m_reply->isFinished() )
-    {
-        cWarning() << "NetInstall data called too early.";
-        return;
-    }
-
-    cDebug() << "NetInstall group data received" << m_reply->url();
-
-    ReplyDeleter d { m_reply };
-
-    // If m_required is *false* then we still say we're ready
-    // even if the reply is corrupt or missing.
-    if ( m_reply->error() != QNetworkReply::NoError )
-    {
-        cWarning() << "unable to fetch netinstall package lists.";
-        cDebug() << Logger::SubEntry << "Netinstall reply error: " << m_reply->error();
-        cDebug() << Logger::SubEntry << "Request for url: " << m_reply->url().toString()
-                 << " failed with: " << m_reply->errorString();
-        ui->netinst_status->setText(
-            tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" ) );
-        emit checkReady( !m_required );
-        return;
-    }
-
-    if ( !readGroups( m_reply->readAll() ) )
-    {
-        cWarning() << "netinstall groups data was received, but invalid.";
-        cDebug() << Logger::SubEntry << "Url:     " << m_reply->url().toString();
-        cDebug() << Logger::SubEntry << "Headers: " << m_reply->rawHeaderList();
-        ui->netinst_status->setText( tr( "Network Installation. (Disabled: Received invalid groups data)" ) );
-        emit checkReady( !m_required );
-        return;
-    }
-
-    retranslate();  // For changed model
-    ui->groupswidget->setModel( m_groups );
-    ui->groupswidget->header()->setSectionResizeMode( 0, QHeaderView::ResizeToContents );
-    ui->groupswidget->header()->setSectionResizeMode( 1, QHeaderView::Stretch );
-
-    expandGroups();
-
-    emit checkReady( true );
-}
-
-void
-NetInstallPage::expandGroups()
+NetInstallPage::setModel( QAbstractItemModel* model )
 {
+    ui->groupswidget->setModel( model );
     // Go backwards because expanding a group may cause rows to appear below it
-    for ( int i = m_groups->rowCount() - 1; i >= 0; --i )
+    for ( int i = model->rowCount() - 1; i >= 0; --i )
     {
-        auto index = m_groups->index( i, 0 );
-        if ( m_groups->data( index, PackageModel::MetaExpandRole ).toBool() )
+        auto index = model->index( i, 0 );
+        if ( model->data( index, PackageModel::MetaExpandRole ).toBool() )
         {
             ui->groupswidget->setExpanded( index, true );
         }
     }
 }
 
-PackageTreeItem::List
-NetInstallPage::selectedPackages() const
-{
-    if ( m_groups )
-    {
-        return m_groups->getPackages();
-    }
-    else
-    {
-        cWarning() << "no netinstall groups are available.";
-        return PackageTreeItem::List();
-    }
-}
-
-void
-NetInstallPage::loadGroupList( const QString& confUrl )
-{
-    using namespace CalamaresUtils::Network;
-
-    cDebug() << "NetInstall loading groups from" << confUrl;
-    QNetworkReply* reply = Manager::instance().asynchronouseGet(
-        QUrl( confUrl ),
-        RequestOptions( RequestOptions::FakeUserAgent | RequestOptions::FollowRedirect, std::chrono::seconds( 30 ) ) );
-
-    if ( !reply )
-    {
-        cDebug() << Logger::Continuation << "request failed immediately.";
-        ui->netinst_status->setText( tr( "Network Installation. (Disabled: Incorrect configuration)" ) );
-    }
-    else
-    {
-        m_reply = reply;
-        connect( reply, &QNetworkReply::finished, this, &NetInstallPage::dataIsHere );
-    }
-}
-
-void
-NetInstallPage::loadGroupList( const QVariantList& l )
-{
-    // This short-cuts through loading and just uses the data,
-    // containing cruft from dataIsHere() and readGroups().
-    m_groups = new PackageModel();
-    m_groups->setupModelData( l );
-    retranslate();  // For changed model
-    ui->groupswidget->setModel( m_groups );
-    ui->groupswidget->header()->setSectionResizeMode( 0, QHeaderView::ResizeToContents );
-    ui->groupswidget->header()->setSectionResizeMode( 1, QHeaderView::Stretch );
-
-    expandGroups();
-
-    emit checkReady( true );
-}
-
-
-void
-NetInstallPage::setRequired( bool b )
-{
-    m_required = b;
-}
-
 
 void
 NetInstallPage::onActivate()
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index fdbdbac88..4db0b2f5d 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -56,56 +56,27 @@ public:
      */
     void setPageTitle( CalamaresUtils::Locale::TranslatedString* );
 
+    /** @brief Sets the model of packages to display
+     *
+     * While setting up the UI, expand entries that should be pre-expanded.
+     *
+     * Follows the *expanded* key / the startExpanded field in the
+     * group entries of the model. Call this after filling up the model.
+     */
+    void setModel( QAbstractItemModel* );
+
     void onActivate();
 
-    /** @brief Retrieves the groups, with name, description and packages
-     *
-     * Loads data from the given URL. This should be called before
-     * displaying the page.
-     */
-    void loadGroupList( const QString& url );
-    /// @brief Retrieve pre-processed and fetched group data
-    void loadGroupList( const QVariantList& l );
-
-    // Sets the "required" state of netinstall data. Influences whether
-    // corrupt or unavailable data causes checkReady() to be emitted
-    // true (not-required) or false.
-    void setRequired( bool );
-    bool getRequired() const { return m_required; }
-
-    // Returns the list of packages belonging to groups that are
-    // selected in the view in this given moment. No data is cached here, so
-    // this function does not have constant time.
-    PackageTreeItem::List selectedPackages() const;
-
 public slots:
-    void dataIsHere();
-
     void retranslate();
 
 signals:
     void checkReady( bool );
 
 private:
-    // Takes the YAML data representing the groups and reads them into the
-    // m_groups and m_groupOrder internal structures. See the README.md
-    // of this module to know the format expected of the YAML files.
-    bool readGroups( const QByteArray& yamlData );
-
-    /** @brief Expand entries that should be pre-expanded
-     *
-     * Follows the *expanded* key / the startExpanded field in the
-     * group entries of the model. Call this after filling up the model.
-     */
-    void expandGroups();
-
     Ui::Page_NetInst* ui;
 
     std::unique_ptr< CalamaresUtils::Locale::TranslatedString > m_title;  // Above the treeview
-
-    QNetworkReply* m_reply;
-    PackageModel* m_groups;
-    bool m_required;
 };
 
 #endif  // NETINSTALLPAGE_H
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 8515d537a..0b5fb8b15 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -33,8 +33,8 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( NetInstallViewStepFactory, registerPlugin<
 NetInstallViewStep::NetInstallViewStep( QObject* parent )
     : Calamares::ViewStep( parent )
     , m_widget( new NetInstallPage() )
-    , m_nextEnabled( false )
     , m_sidebarLabel( nullptr )
+    , m_nextEnabled( false )
 {
     emit nextStatusChanged( true );
     connect( m_widget, &NetInstallPage::checkReady, this, &NetInstallViewStep::nextIsReady );
@@ -127,7 +127,7 @@ NetInstallViewStep::onActivate()
 void
 NetInstallViewStep::onLeave()
 {
-    auto packages = m_widget->selectedPackages();
+    auto packages = m_config.model()->getPackages();
     cDebug() << "Netinstall: Processing" << packages.length() << "packages.";
 
     static const char PACKAGEOP[] = "packageOperations";
@@ -201,7 +201,8 @@ NetInstallViewStep::nextIsReady( bool b )
 void
 NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
 {
-    m_widget->setRequired( CalamaresUtils::getBool( configurationMap, "required", false ) );
+    m_config.setRequired( CalamaresUtils::getBool( configurationMap, "required", false ) );
+    m_widget->setModel( m_config.model() );
 
     QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
     if ( !groupsUrl.isEmpty() )
@@ -212,11 +213,11 @@ NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
         if ( groupsUrl == QStringLiteral( "local" ) )
         {
             QVariantList l = configurationMap.value( "groups" ).toList();
-            m_widget->loadGroupList( l );
+            m_config.loadGroupList( l );
         }
         else
         {
-            m_widget->loadGroupList( groupsUrl );
+            m_config.loadGroupList( groupsUrl );
         }
     }
 
diff --git a/src/modules/netinstall/NetInstallViewStep.h b/src/modules/netinstall/NetInstallViewStep.h
index ffcc785c8..2dcf464c3 100644
--- a/src/modules/netinstall/NetInstallViewStep.h
+++ b/src/modules/netinstall/NetInstallViewStep.h
@@ -20,6 +20,8 @@
 #ifndef NETINSTALLVIEWSTEP_H
 #define NETINSTALLVIEWSTEP_H
 
+#include "Config.h"
+
 #include "DllMacro.h"
 #include "locale/TranslatableConfiguration.h"
 #include "utils/PluginFactory.h"
@@ -60,9 +62,11 @@ public slots:
     void nextIsReady( bool );
 
 private:
+    Config m_config;
+
     NetInstallPage* m_widget;
-    bool m_nextEnabled;
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel;  // As it appears in the sidebar
+    bool m_nextEnabled;
 };
 
 CALAMARES_PLUGIN_FACTORY_DECLARATION( NetInstallViewStepFactory )

From 85551f0fdbda2d4d6083fb0ef0892402a70217d4 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 16:12:48 +0100
Subject: [PATCH 24/30] [netinstall] Various refactoring

- move ready-indication to Config
- don't check pointers that can't be null
- hand the whole Config to the page
---
 src/modules/netinstall/Config.cpp             |  1 +
 src/modules/netinstall/Config.h               |  3 ++-
 src/modules/netinstall/NetInstallPage.cpp     | 19 ++++++++++++----
 src/modules/netinstall/NetInstallPage.h       | 22 +++++++++----------
 src/modules/netinstall/NetInstallViewStep.cpp | 14 +++++-------
 src/modules/netinstall/NetInstallViewStep.h   |  4 ++--
 6 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index af4ae8e50..ebcda3b8b 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -66,6 +66,7 @@ void
 Config::loadGroupList( const QVariantList& groupData )
 {
     m_model->setupModelData( groupData );
+    emit statusReady();
 }
 
 void
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 372c82bee..781c9be5d 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -71,7 +71,8 @@ public:
     PackageModel* model() const { return m_model; }
 
 signals:
-    void statusChanged( QString status );
+    void statusChanged( QString status );  ///< Something changed
+    void statusReady();  ///< Loading groups is complete
 
 private slots:
     void receivedGroupData();  ///< From async-loading group data
diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index ab3e1a933..688e99b09 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -34,11 +34,16 @@
 #include <QHeaderView>
 #include <QNetworkReply>
 
-NetInstallPage::NetInstallPage( QWidget* parent )
+NetInstallPage::NetInstallPage( Config* c, QWidget* parent )
     : QWidget( parent )
+    , m_config( c )
     , ui( new Ui::Page_NetInst )
 {
     ui->setupUi( this );
+    ui->groupswidget->setModel( c->model() );
+    connect( c, &Config::statusChanged, this, &NetInstallPage::setStatus );
+    connect( c, &Config::statusReady, this, &NetInstallPage::expandGroups );
+
     setPageTitle( nullptr );
     CALAMARES_RETRANSLATE_SLOT( &NetInstallPage::retranslate );
 }
@@ -63,16 +68,17 @@ NetInstallPage::setPageTitle( CalamaresUtils::Locale::TranslatedString* t )
 void
 NetInstallPage::retranslate()
 {
-    if ( ui && m_title )
+    if ( m_title )
     {
         ui->label->setText( m_title->get() );  // That's get() on the TranslatedString
     }
+    ui->netinst_status->setText( m_config->status() );
 }
 
 void
-NetInstallPage::setModel( QAbstractItemModel* model )
+NetInstallPage::expandGroups()
 {
-    ui->groupswidget->setModel( model );
+    auto* model = m_config->model();
     // Go backwards because expanding a group may cause rows to appear below it
     for ( int i = model->rowCount() - 1; i >= 0; --i )
     {
@@ -84,6 +90,11 @@ NetInstallPage::setModel( QAbstractItemModel* model )
     }
 }
 
+void
+NetInstallPage::setStatus( QString s )
+{
+    ui->netinst_status->setText( s );
+}
 
 void
 NetInstallPage::onActivate()
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index 4db0b2f5d..13a106eaf 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -21,6 +21,7 @@
 #ifndef NETINSTALLPAGE_H
 #define NETINSTALLPAGE_H
 
+#include "Config.h"
 #include "PackageModel.h"
 #include "PackageTreeItem.h"
 
@@ -42,7 +43,7 @@ class NetInstallPage : public QWidget
 {
     Q_OBJECT
 public:
-    NetInstallPage( QWidget* parent = nullptr );
+    NetInstallPage( Config* config, QWidget* parent = nullptr );
     virtual ~NetInstallPage();
 
     /** @brief Sets the page title
@@ -56,24 +57,21 @@ public:
      */
     void setPageTitle( CalamaresUtils::Locale::TranslatedString* );
 
-    /** @brief Sets the model of packages to display
-     *
-     * While setting up the UI, expand entries that should be pre-expanded.
-     *
-     * Follows the *expanded* key / the startExpanded field in the
-     * group entries of the model. Call this after filling up the model.
-     */
-    void setModel( QAbstractItemModel* );
-
     void onActivate();
 
 public slots:
     void retranslate();
+    void setStatus( QString s );
 
-signals:
-    void checkReady( bool );
+    /** @brief Expand entries that should be pre-expanded.
+     *
+     * Follows the *expanded* key / the startExpanded field in the
+     * group entries of the model. Call this after filling up the model.
+     */
+    void expandGroups();
 
 private:
+    Config* m_config;
     Ui::Page_NetInst* ui;
 
     std::unique_ptr< CalamaresUtils::Locale::TranslatedString > m_title;  // Above the treeview
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 0b5fb8b15..3eba788db 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -32,12 +32,11 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( NetInstallViewStepFactory, registerPlugin<
 
 NetInstallViewStep::NetInstallViewStep( QObject* parent )
     : Calamares::ViewStep( parent )
-    , m_widget( new NetInstallPage() )
+    , m_widget( new NetInstallPage( &m_config ) )
     , m_sidebarLabel( nullptr )
     , m_nextEnabled( false )
 {
-    emit nextStatusChanged( true );
-    connect( m_widget, &NetInstallPage::checkReady, this, &NetInstallViewStep::nextIsReady );
+    connect( &m_config, &Config::statusReady, this, &NetInstallViewStep::nextIsReady );
 }
 
 
@@ -86,7 +85,7 @@ NetInstallViewStep::widget()
 bool
 NetInstallViewStep::isNextEnabled() const
 {
-    return m_nextEnabled;
+    return !m_config.required() || m_nextEnabled;
 }
 
 
@@ -192,17 +191,16 @@ NetInstallViewStep::onLeave()
 }
 
 void
-NetInstallViewStep::nextIsReady( bool b )
+NetInstallViewStep::nextIsReady()
 {
-    m_nextEnabled = b;
-    emit nextStatusChanged( b );
+    m_nextEnabled = true;
+    emit nextStatusChanged( true );
 }
 
 void
 NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
 {
     m_config.setRequired( CalamaresUtils::getBool( configurationMap, "required", false ) );
-    m_widget->setModel( m_config.model() );
 
     QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
     if ( !groupsUrl.isEmpty() )
diff --git a/src/modules/netinstall/NetInstallViewStep.h b/src/modules/netinstall/NetInstallViewStep.h
index 2dcf464c3..d2114a346 100644
--- a/src/modules/netinstall/NetInstallViewStep.h
+++ b/src/modules/netinstall/NetInstallViewStep.h
@@ -59,14 +59,14 @@ public:
     void setConfigurationMap( const QVariantMap& configurationMap ) override;
 
 public slots:
-    void nextIsReady( bool );
+    void nextIsReady();
 
 private:
     Config m_config;
 
     NetInstallPage* m_widget;
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel;  // As it appears in the sidebar
-    bool m_nextEnabled;
+    bool m_nextEnabled = false;
 };
 
 CALAMARES_PLUGIN_FACTORY_DECLARATION( NetInstallViewStepFactory )

From 7a42a4d71fa33f34dac8431ae5a56e42cdd542ac Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 16:35:06 +0100
Subject: [PATCH 25/30] [netinstall] Add example section that is immutable

- The section can't be changed, but is selected (it doesn't
  make sense otherwise)
---
 src/modules/netinstall/netinstall.conf | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf
index 96977bdd0..82e12d558 100644
--- a/src/modules/netinstall/netinstall.conf
+++ b/src/modules/netinstall/netinstall.conf
@@ -83,3 +83,13 @@ groups:
             - zsh
             - zsh-completion
             - zsh-extensions
+    - name: "Kernel"
+      description: "Kernel bits"
+      hidden: false
+      selected: true
+      critical: true
+      immutable: true
+      packages:
+        - kernel
+        - kernel-debugsym
+        - kernel-nvidia

From 63b940a62365f262e050b4f1ce0e4541d504dd58 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 16:47:33 +0100
Subject: [PATCH 26/30] [netinstall] Implement immutable groups

- An immutable group doesn't show a checkbox at all
---
 src/modules/netinstall/PackageModel.cpp    | 7 ++++++-
 src/modules/netinstall/PackageTreeItem.cpp | 1 +
 src/modules/netinstall/PackageTreeItem.h   | 7 +++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 88a06a1bb..3339a7284 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -119,7 +119,7 @@ PackageModel::data( const QModelIndex& index, int role ) const
     switch ( role )
     {
     case Qt::CheckStateRole:
-        return index.column() == NameColumn ? item->isSelected() : QVariant();
+        return index.column() == NameColumn ? ( item->isImmutable() ? QVariant() : item->isSelected() ) : QVariant();
     case Qt::DisplayRole:
         return item->isHidden() ? QVariant() : item->data( index.column() );
     case MetaExpandRole:
@@ -158,6 +158,11 @@ PackageModel::flags( const QModelIndex& index ) const
     }
     if ( index.column() == NameColumn )
     {
+        PackageTreeItem* item = static_cast< PackageTreeItem* >( index.internalPointer() );
+        if ( item->isImmutable() )
+        {
+            return QAbstractItemModel::flags( index );  //Qt::NoItemFlags;
+        }
         return Qt::ItemIsUserCheckable | QAbstractItemModel::flags( index );
     }
     return QAbstractItemModel::flags( index );
diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index e611da477..3c5ed0a85 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -41,6 +41,7 @@ PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* p
     , m_packageName( packageName )
     , m_selected( parentCheckState( parent ) )
     , m_isGroup( false )
+    , m_showReadOnly( parent ? parent->isImmutable() : false )
 {
 }
 
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index 3f7dcce86..3c3aca814 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -97,6 +97,13 @@ public:
      */
     bool expandOnStart() const { return m_startExpanded; }
 
+    /** @brief Is this an immutable item?
+     *
+     * Groups can be immutable: then you can't toggle the selected
+     * state of any of its items.
+     */
+    bool isImmutable() const { return m_showReadOnly; }
+
     /** @brief is this item selected?
      *
      * Groups may be partially selected; packages are only on or off.

From 464561b420ee1f247f4939e6ae64b2b2b89318a1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 17:28:32 +0100
Subject: [PATCH 27/30] [netinstall] Update subgroup-checkedness based on
 children

- An unselected group with (some) selected subgroups was not
  displayed as (semi)checked -- it was unchecked, because
  its checked-ness was not updated based on the children.
---
 src/modules/netinstall/PackageModel.cpp    |  4 ++++
 src/modules/netinstall/PackageTreeItem.cpp | 21 ++++++++++++++-------
 src/modules/netinstall/PackageTreeItem.h   |  7 +++++++
 3 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 3339a7284..0698d4cfb 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -249,6 +249,10 @@ PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* pa
             if ( !subgroups.isEmpty() )
             {
                 setupModelData( subgroups, item );
+                // The children might be checked while the parent isn't (yet).
+                // Children are added to their parent (below) without affecting
+                // the checked-state -- do it manually.
+                item->updateSelected();
             }
         }
         if ( item->isHidden() )
diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 3c5ed0a85..d6a3531f9 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -194,34 +194,41 @@ PackageTreeItem::setSelected( Qt::CheckState isSelected )
         return;
     }
 
+    currentItem->updateSelected();
+}
+
+void
+PackageTreeItem::updateSelected()
+{
     // Figure out checked-state based on the children
     int childrenSelected = 0;
     int childrenPartiallySelected = 0;
-    for ( int i = 0; i < currentItem->childCount(); i++ )
+    for ( int i = 0; i < childCount(); i++ )
     {
-        if ( currentItem->child( i )->isSelected() == Qt::Checked )
+        if ( child( i )->isSelected() == Qt::Checked )
         {
             childrenSelected++;
         }
-        if ( currentItem->child( i )->isSelected() == Qt::PartiallyChecked )
+        if ( child( i )->isSelected() == Qt::PartiallyChecked )
         {
             childrenPartiallySelected++;
         }
     }
     if ( !childrenSelected && !childrenPartiallySelected )
     {
-        currentItem->setSelected( Qt::Unchecked );
+        setSelected( Qt::Unchecked );
     }
-    else if ( childrenSelected == currentItem->childCount() )
+    else if ( childrenSelected == childCount() )
     {
-        currentItem->setSelected( Qt::Checked );
+        setSelected( Qt::Checked );
     }
     else
     {
-        currentItem->setSelected( Qt::PartiallyChecked );
+        setSelected( Qt::PartiallyChecked );
     }
 }
 
+
 void
 PackageTreeItem::setChildrenSelected( Qt::CheckState isSelected )
 {
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index 3c3aca814..d443bcdc6 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -120,6 +120,13 @@ public:
     void setSelected( Qt::CheckState isSelected );
     void setChildrenSelected( Qt::CheckState isSelected );
 
+    /** @brief Update selectedness based on the children's states
+     *
+     * This only makes sense for groups, which might have packages
+     * or subgroups; it checks only direct children.
+     */
+    void updateSelected();
+
     // QStandardItem methods
     int type() const override;
 

From 14a3e10cc2850bd1405fcb0e7f8820c5b3868681 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 20:36:34 +0100
Subject: [PATCH 28/30] [netinstall] Simplify getItemPackages

- Use convenience predicate isPackage()
- Name child->item(i) for brevity
---
 src/modules/netinstall/PackageModel.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 0698d4cfb..dad2207b2 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -203,18 +203,19 @@ PackageModel::getItemPackages( PackageTreeItem* item ) const
     PackageTreeItem::List selectedPackages;
     for ( int i = 0; i < item->childCount(); i++ )
     {
-        if ( item->child( i )->isSelected() == Qt::Unchecked )
+        auto* child = item->child( i );
+        if ( child->isSelected() == Qt::Unchecked )
         {
             continue;
         }
 
-        if ( !item->child( i )->childCount() )  // package
+        if ( child->isPackage() )  // package
         {
-            selectedPackages.append( item->child( i ) );
+            selectedPackages.append( child );
         }
         else
         {
-            selectedPackages.append( getItemPackages( item->child( i ) ) );
+            selectedPackages.append( getItemPackages( child ) );
         }
     }
     return selectedPackages;

From 83a89c144c25f0e4b58853ae674886837705b8e8 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 20:45:04 +0100
Subject: [PATCH 29/30] [netinstall] Packages should inherit critical-ness from
 parent

---
 src/modules/netinstall/PackageTreeItem.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index d6a3531f9..04358fd43 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -41,6 +41,7 @@ PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* p
     , m_packageName( packageName )
     , m_selected( parentCheckState( parent ) )
     , m_isGroup( false )
+    , m_isCritical( parent ? parent->isCritical() : false )
     , m_showReadOnly( parent ? parent->isImmutable() : false )
 {
 }

From 433ed8384f65a523a50b983018a38cd3e52972ad Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 27 Mar 2020 20:51:52 +0100
Subject: [PATCH 30/30] [netinstall] Inherit criticalness in groups

- Groups inherit slightly differently: if a subgroup **explicitly**
  configures criticalness, use that. It would be weird, but possibly,
  to have a non-critical subgroup of a critical group.
---
 src/modules/netinstall/PackageTreeItem.cpp | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 04358fd43..edc89c536 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -22,6 +22,7 @@
 #include "utils/Logger.h"
 #include "utils/Variant.h"
 
+/** @brief Should a package be selected, given its parent's state? */
 static Qt::CheckState
 parentCheckState( PackageTreeItem* parent )
 {
@@ -36,6 +37,20 @@ parentCheckState( PackageTreeItem* parent )
     }
 }
 
+/** @brief Should a subgroup be marked critical?
+ *
+ * If set explicitly, then use that, otherwise use the parent's critical-ness.
+ */
+static bool
+parentCriticality( const QVariantMap& groupData, PackageTreeItem* parent )
+{
+    if ( groupData.contains( "critical" ) )
+    {
+        return CalamaresUtils::getBool( groupData, "critical", false );
+    }
+    return parent ? parent->isCritical() : false;
+}
+
 PackageTreeItem::PackageTreeItem( const QString& packageName, PackageTreeItem* parent )
     : m_parentItem( parent )
     , m_packageName( packageName )
@@ -54,7 +69,7 @@ PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, PackageTreeItem*
     , m_preScript( CalamaresUtils::getString( groupData, "pre-install" ) )
     , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) )
     , m_isGroup( true )
-    , m_isCritical( CalamaresUtils::getBool( groupData, "critical", false ) )
+    , m_isCritical( parentCriticality( groupData, parent ) )
     , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) )
     , m_showReadOnly( CalamaresUtils::getBool( groupData, "immutable", false ) )
     , m_startExpanded( CalamaresUtils::getBool( groupData, "expanded", false ) )