From 2eda55d3afb145e79f0c6b90d282a012cb0a9aea Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Sat, 15 Jan 2022 09:41:23 -0600
Subject: [PATCH 1/9] [packagechooser,netinstall] Add support for
 packagechooser to drive netinstall

---
 src/modules/netinstall/NetInstallPage.cpp   | 16 ++++++++++
 src/modules/netinstall/PackageModel.cpp     | 35 +++++++++++++++++++++
 src/modules/netinstall/PackageModel.h       |  5 +++
 src/modules/packagechooser/Config.cpp       | 19 +++++++++++
 src/modules/packagechooser/Config.h         |  2 ++
 src/modules/packagechooser/PackageModel.cpp | 25 ++++++++++++++-
 src/modules/packagechooser/PackageModel.h   |  9 ++++++
 7 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index a1a86294e..75d7b660c 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -15,6 +15,7 @@
 #include "PackageModel.h"
 #include "ui_page_netinst.h"
 
+#include "GlobalStorage.h"
 #include "JobQueue.h"
 
 #include "network/Manager.h"
@@ -62,4 +63,19 @@ void
 NetInstallPage::onActivate()
 {
     ui->groupswidget->setFocus();
+
+    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
+    if ( gs->contains( "NetinstallSelect" ) && gs->value( "NetinstallSelect" ).canConvert( QVariant::StringList ) )
+    {
+        const QStringList selectNames = gs->value( "NetinstallSelect" ).toStringList();
+
+        static_cast< PackageModel* >( ui->groupswidget->model() )->setSelections( selectNames );
+    }
+
+    if ( gs->contains( "NetinstallAdd" ) && gs->value( "NetinstallAdd" ).canConvert( QVariant::List ) )
+    {
+        const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
+
+        static_cast< PackageModel* >( ui->groupswidget->model() )->appendModelData( groups );
+    }
 }
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index d4887b6c2..015b05303 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -170,6 +170,29 @@ PackageModel::headerData( int section, Qt::Orientation orientation, int role ) c
     return QVariant();
 }
 
+void
+PackageModel::setSelections( QStringList selectNames )
+{
+    if ( m_rootItem )
+    {
+        setSelections( selectNames, m_rootItem );
+    }
+}
+
+void
+PackageModel::setSelections( QStringList selectNames, PackageTreeItem* item )
+{
+    for ( int i = 0; i < item->childCount(); i++ )
+    {
+        auto* child = item->child( i );
+        setSelections( selectNames, child );
+    }
+    if ( item->isGroup() && selectNames.contains( item->name() ) )
+    {
+        item->setSelected( Qt::CheckState::Checked );
+    }
+}
+
 PackageTreeItem::List
 PackageModel::getPackages() const
 {
@@ -309,3 +332,15 @@ PackageModel::setupModelData( const QVariantList& l )
     setupModelData( l, m_rootItem );
     emit endResetModel();
 }
+
+
+void
+PackageModel::appendModelData( const QVariantList& groupList )
+{
+    if ( m_rootItem )
+    {
+        emit beginResetModel();
+        setupModelData( groupList, m_rootItem );
+        emit endResetModel();
+    }
+}
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 998f42c38..fcdfe5daa 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -54,9 +54,14 @@ public:
     int rowCount( const QModelIndex& parent = QModelIndex() ) const override;
     int columnCount( const QModelIndex& parent = QModelIndex() ) const override;
 
+    void setSelections( QStringList selectNames );
+    void setSelections(QStringList selectNames, PackageTreeItem *item );
+
     PackageTreeItem::List getPackages() const;
     PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
 
+    void appendModelData( const QVariantList& groupList );
+
 private:
     friend class ItemTests;
 
diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index 5c0db5d91..abf6a640e 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -55,6 +55,8 @@ PackageChooserMethodNames()
         { "custom", PackageChooserMethod::Legacy },
         { "contextualprocess", PackageChooserMethod::Legacy },
         { "packages", PackageChooserMethod::Packages },
+        { "netinstall-add", PackageChooserMethod::NetAdd },
+        { "netinstall-select", PackageChooserMethod::NetSelect },
     };
     return names;
 }
@@ -121,6 +123,23 @@ Config::updateGlobalStorage( const QStringList& selected ) const
         CalamaresUtils::Packages::setGSPackageAdditions(
             Calamares::JobQueue::instance()->globalStorage(), m_defaultId, packageNames );
     }
+    else if ( m_method == PackageChooserMethod::NetAdd )
+    {
+        QVariantList netinstallDataList = m_model->getNetinstallDataForNames( selected );
+        if ( netinstallDataList.isEmpty() )
+        {
+            cWarning() << "No netinstall information found for " << selected;
+        }
+        else
+        {
+            Calamares::JobQueue::instance()->globalStorage()->insert( "NetinstallAdd", netinstallDataList );
+        }
+    }
+    else if ( m_method == PackageChooserMethod::NetSelect )
+    {
+        cDebug() << m_defaultId << "groups to select in netinstall" << selected;
+        Calamares::JobQueue::instance()->globalStorage()->insert( "NetinstallSelect", selected );
+    }
     else
     {
         cWarning() << "Unknown packagechooser method" << smash( m_method );
diff --git a/src/modules/packagechooser/Config.h b/src/modules/packagechooser/Config.h
index 32f1e8b57..76aa0c0f3 100644
--- a/src/modules/packagechooser/Config.h
+++ b/src/modules/packagechooser/Config.h
@@ -33,6 +33,8 @@ enum class PackageChooserMethod
 {
     Legacy,  // use contextualprocess or other custom
     Packages,  // use the packages module
+    NetAdd,  // adds packages to the netinstall module
+    NetSelect, // makes selections in the netinstall module
 };
 
 const NamedEnumTable< PackageChooserMethod >& PackageChooserMethodNames();
diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp
index 8a0b13e51..f92b6893e 100644
--- a/src/modules/packagechooser/PackageModel.cpp
+++ b/src/modules/packagechooser/PackageModel.cpp
@@ -15,6 +15,14 @@
 
 #include <QFileInfo>
 
+static QVariantMap
+getSubMap( const QVariantMap& map, const QString& key )
+{
+    bool success;
+
+    return CalamaresUtils::getSubMap( map, key, success );
+}
+
 static QPixmap
 loadScreenshot( const QString& path )
 {
@@ -51,12 +59,13 @@ PackageItem::PackageItem( const QString& a_id,
 {
 }
 
-PackageItem::PackageItem::PackageItem( const QVariantMap& item_map )
+PackageItem::PackageItem( const QVariantMap& item_map )
     : id( CalamaresUtils::getString( item_map, "id" ) )
     , name( CalamaresUtils::Locale::TranslatedString( item_map, "name" ) )
     , description( CalamaresUtils::Locale::TranslatedString( item_map, "description" ) )
     , screenshot( loadScreenshot( CalamaresUtils::getString( item_map, "screenshot" ) ) )
     , packageNames( CalamaresUtils::getStringList( item_map, "packages" ) )
+    , netinstallData( getSubMap( item_map, "netinstall" ) )
 {
     if ( name.isEmpty() && id.isEmpty() )
     {
@@ -125,6 +134,20 @@ PackageListModel::getInstallPackagesForNames( const QStringList& ids ) const
     return l;
 }
 
+QVariantList
+PackageListModel::getNetinstallDataForNames( const QStringList& ids ) const
+{
+    QVariantList l;
+    for ( const auto& p : qAsConst( m_packages ) )
+    {
+        if ( ids.contains( p.id ) )
+        {
+            l.append( p.netinstallData );
+        }
+    }
+    return l;
+}
+
 int
 PackageListModel::rowCount( const QModelIndex& index ) const
 {
diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h
index 71003197d..18682a121 100644
--- a/src/modules/packagechooser/PackageModel.h
+++ b/src/modules/packagechooser/PackageModel.h
@@ -26,6 +26,7 @@ struct PackageItem
     CalamaresUtils::Locale::TranslatedString description;
     QPixmap screenshot;
     QStringList packageNames;
+    QVariantMap netinstallData;
 
     /// @brief Create blank PackageItem
     PackageItem();
@@ -111,6 +112,14 @@ public:
      */
     QStringList getInstallPackagesForNames( const QStringList& ids ) const;
 
+    /** @brief Does a name lookup (based on id) and returns the netinstall data
+     *
+     * If there is a package with an id in @p ids, returns their netinstall data
+     *
+     * returns a list of netinstall data or an emply list if none is found
+     */
+    QVariantList getNetinstallDataForNames( const QStringList& ids ) const;
+
     enum Roles : int
     {
         NameRole = Qt::DisplayRole,

From b4ac6b73c8a188b00868bba4e0497fa2fc47fdfa Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Sat, 15 Jan 2022 11:13:17 -0600
Subject: [PATCH 2/9] [packagechooser,netinstall] Add documentation for
 packagechooser/netinstall integration

---
 src/modules/netinstall/NetInstallPage.cpp      |  2 ++
 src/modules/netinstall/PackageModel.cpp        | 17 ++++++++++++++++-
 src/modules/packagechooser/PackageModel.cpp    |  2 ++
 src/modules/packagechooser/packagechooser.conf | 17 ++++++++++++++++-
 4 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 75d7b660c..f897c0018 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -64,6 +64,7 @@ NetInstallPage::onActivate()
 {
     ui->groupswidget->setFocus();
 
+    // The NetInstallSelect global sotrage value can be used to make additional items selected by default
     Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
     if ( gs->contains( "NetinstallSelect" ) && gs->value( "NetinstallSelect" ).canConvert( QVariant::StringList ) )
     {
@@ -72,6 +73,7 @@ NetInstallPage::onActivate()
         static_cast< PackageModel* >( ui->groupswidget->model() )->setSelections( selectNames );
     }
 
+    // If NetInstallAdd is found in global storage, add those items to the tree
     if ( gs->contains( "NetinstallAdd" ) && gs->value( "NetinstallAdd" ).canConvert( QVariant::List ) )
     {
         const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 015b05303..9eafd898e 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -170,6 +170,15 @@ PackageModel::headerData( int section, Qt::Orientation orientation, int role ) c
     return QVariant();
 }
 
+/** @brief Sets the checked flag on matching groups in the tree
+ *
+ * Recursively traverses the tree pointed to by m_rootItem and
+ * checks if a group name matches any of the items in @p selectNames.
+ * If a match is found, set check the box for that group and it's children.
+ *
+ * Individual packages will not be matched.
+ *
+ */
 void
 PackageModel::setSelections( QStringList selectNames )
 {
@@ -333,7 +342,13 @@ PackageModel::setupModelData( const QVariantList& l )
     emit endResetModel();
 }
 
-
+/** @brief Appends groups to the tree
+ *
+ * Uses the data from @p groupList to add elements to the
+ * existing tree that m_rootItem points to.  If m_rootItem
+ * is not valid, it does nothing
+ *
+ */
 void
 PackageModel::appendModelData( const QVariantList& groupList )
 {
diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp
index f92b6893e..7b7cfc08f 100644
--- a/src/modules/packagechooser/PackageModel.cpp
+++ b/src/modules/packagechooser/PackageModel.cpp
@@ -15,6 +15,8 @@
 
 #include <QFileInfo>
 
+/** @brief A wrapper for CalamaresUtils::getSubMap that excludes the success param
+ */
 static QVariantMap
 getSubMap( const QVariantMap& map, const QString& key )
 {
diff --git a/src/modules/packagechooser/packagechooser.conf b/src/modules/packagechooser/packagechooser.conf
index bb982916e..ca458042b 100644
--- a/src/modules/packagechooser/packagechooser.conf
+++ b/src/modules/packagechooser/packagechooser.conf
@@ -33,6 +33,15 @@ mode: required
 #   in the `exec` section. These package settings will then be handed
 #   off to whatever package manager is configured there.
 #
+# - "netinstall-select"
+#   When this is set, the id(s) selected are passed to the netinstall module.
+#   Any id that matches a group name in that module is set to checked
+#
+# - "netinstall-add"
+#   With this method, the packagechooser module is used to add groups to the
+#   netinstall module.  For this to hav=e any effect.  You must set netinstall,
+#   which is described below.
+#
 # There is no need to put this module in the `exec` section. There
 # are no jobs that this module provides. You should put **other**
 # modules, either *contextualprocess* or *packages* or some custom
@@ -101,13 +110,19 @@ labels:
 #       an additional attempt is made to load the image from the **branding**
 #       directory.
 #
-# The following field is **optional** for an item:
+# The following fields are **optional** for an item:
 #
 #  - *packages* :
 #       List of package names for the product. If using the *method*
 #       "packages", consider this item mandatory (because otherwise
 #       selecting the item would install no packages).
 #
+#  - *netinstall* :
+#       The data in this field should follow the format of a group
+#       from the netinstall module documented in
+#       src/modules/netinstall/netinstall.conf.  This is only used
+#       when method is set to "netinstall-add"
+#
 # # AppData Items #
 #
 # For data provided by AppData XML: the item has an *appdata*

From 2aa8c2f0e0fd29edec1935d225f78d0a317e4f64 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Wed, 19 Jan 2022 16:48:56 -0600
Subject: [PATCH 3/9] [packagechooser] Ensure multiple instances don't override
 the GS values

---
 src/modules/packagechooser/Config.cpp | 37 +++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index abf6a640e..2f54442b7 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -132,13 +132,46 @@ Config::updateGlobalStorage( const QStringList& selected ) const
         }
         else
         {
-            Calamares::JobQueue::instance()->globalStorage()->insert( "NetinstallAdd", netinstallDataList );
+            auto* gs = Calamares::JobQueue::instance()->globalStorage();
+
+            // If an earlier packagechooser instance added this data to global storage, combine them
+            if ( gs->contains( "NetinstallAdd" ) )
+            {
+                auto netinstallAddOrig = gs->value( "NetinstallAdd" );
+                if ( netinstallAddOrig.canConvert( QVariant::List ) )
+                {
+                    netinstallDataList += netinstallAddOrig.toList();
+                }
+                else
+                {
+                    cWarning() << "Invalid NetinstallAdd data in global storage.  Earlier selections purged";
+                }
+                gs->remove( "NetinstallAdd" );
+            }
+            gs->insert( "NetinstallAdd", netinstallDataList );
         }
     }
     else if ( m_method == PackageChooserMethod::NetSelect )
     {
         cDebug() << m_defaultId << "groups to select in netinstall" << selected;
-        Calamares::JobQueue::instance()->globalStorage()->insert( "NetinstallSelect", selected );
+        QStringList newSelected = selected;
+        auto* gs = Calamares::JobQueue::instance()->globalStorage();
+
+        // If an earlier packagechooser instance added this data to global storage, combine them
+        if ( gs->contains( "NetinstallSelect" ) )
+        {
+            auto selectedOrig = gs->value( "NetinstallSelect" );
+            if ( selectedOrig.canConvert( QVariant::StringList ) )
+            {
+                newSelected += selectedOrig.toStringList();
+            }
+            else
+            {
+                cWarning() << "Invalid NetinstallSelect data in global storage.  Earlier selections purged";
+            }
+            gs->remove( "NetinstallSelect" );
+        }
+        gs->insert( "NetinstallSelect", newSelected );
     }
     else
     {

From f4c2db7f21214ba29cb4a9038fbfee759eee7224 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Sun, 23 Jan 2022 13:58:10 -0600
Subject: [PATCH 4/9] [packagechooser,netinstall] Fix issues where going back
 and forth between pkgchooser and netinstall produced unexpected behavior

---
 src/modules/netinstall/NetInstallPage.cpp   |  2 +-
 src/modules/netinstall/PackageModel.cpp     | 22 +++++++++++++--------
 src/modules/netinstall/PackageModel.h       | 15 ++++++++++++--
 src/modules/netinstall/PackageTreeItem.cpp  | 14 +++++++++++++
 src/modules/netinstall/PackageTreeItem.h    |  4 ++++
 src/modules/packagechooser/Config.cpp       |  2 +-
 src/modules/packagechooser/PackageModel.cpp |  9 +++++++--
 7 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index f897c0018..4468e9cc0 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -78,6 +78,6 @@ NetInstallPage::onActivate()
     {
         const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
 
-        static_cast< PackageModel* >( ui->groupswidget->model() )->appendModelData( groups );
+        static_cast< PackageModel* >( ui->groupswidget->model() )->appendModelData( groups, "packageChooser" );
     }
 }
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 9eafd898e..d139a89f6 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -342,20 +342,26 @@ PackageModel::setupModelData( const QVariantList& l )
     emit endResetModel();
 }
 
-/** @brief Appends groups to the tree
- *
- * Uses the data from @p groupList to add elements to the
- * existing tree that m_rootItem points to.  If m_rootItem
- * is not valid, it does nothing
- *
- */
 void
-PackageModel::appendModelData( const QVariantList& groupList )
+PackageModel::appendModelData( const QVariantList& groupList, const QString source )
 {
     if ( m_rootItem )
     {
         emit beginResetModel();
+
+        // Prune any existing data from the same source
+        for ( int i = 0; i < m_rootItem->childCount(); i++ )
+        {
+            PackageTreeItem* child = m_rootItem->child( i );
+            if ( child->source() == source )
+            {
+                m_rootItem->removeChild( i );
+            }
+        }
+
+        // Add the new data to the model
         setupModelData( groupList, m_rootItem );
+
         emit endResetModel();
     }
 }
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index fcdfe5daa..49a17f0b6 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -55,12 +55,23 @@ public:
     int columnCount( const QModelIndex& parent = QModelIndex() ) const override;
 
     void setSelections( QStringList selectNames );
-    void setSelections(QStringList selectNames, PackageTreeItem *item );
+    void setSelections( QStringList selectNames, PackageTreeItem* item );
 
     PackageTreeItem::List getPackages() const;
     PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
 
-    void appendModelData( const QVariantList& groupList );
+    /** @brief Appends groups to the tree
+     *
+     * Uses the data from @p groupList to add elements to the
+     * existing tree that m_rootItem points to.  If m_rootItem
+     * is not valid, it does nothing
+     *
+     * Before adding anything to the model, it ensures that there
+     * is no existing data from the same source.  If there is, that
+     * data is pruned first
+     *
+     */
+    void appendModelData( const QVariantList& groupList, const QString source );
 
 private:
     friend class ItemTests;
diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index b30cdf915..245f1ddc6 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -70,6 +70,7 @@ PackageTreeItem::PackageTreeItem( const QVariantMap& groupData, GroupTag&& paren
     , m_description( CalamaresUtils::getString( groupData, "description" ) )
     , m_preScript( CalamaresUtils::getString( groupData, "pre-install" ) )
     , m_postScript( CalamaresUtils::getString( groupData, "post-install" ) )
+    , m_source( CalamaresUtils::getString( groupData, "source" ) )
     , m_isGroup( true )
     , m_isCritical( parentCriticality( groupData, parent.parent ) )
     , m_isHidden( CalamaresUtils::getBool( groupData, "hidden", false ) )
@@ -248,6 +249,19 @@ PackageTreeItem::setChildrenSelected( Qt::CheckState isSelected )
         }
 }
 
+void
+PackageTreeItem::removeChild( int row )
+{
+    if ( row < m_childItems.count() )
+    {
+        m_childItems.removeAt( row );
+    }
+    else
+    {
+        cWarning() << "Attempt to remove invalid child in removeChild() at row " + QString::number( row );
+    }
+}
+
 int
 PackageTreeItem::type() const
 {
diff --git a/src/modules/netinstall/PackageTreeItem.h b/src/modules/netinstall/PackageTreeItem.h
index c04b9a21d..2a0fca83e 100644
--- a/src/modules/netinstall/PackageTreeItem.h
+++ b/src/modules/netinstall/PackageTreeItem.h
@@ -56,6 +56,7 @@ public:
     QString description() const { return m_description; }
     QString preScript() const { return m_preScript; }
     QString postScript() const { return m_postScript; }
+    QString source() const { return m_source; }
 
     /** @brief Is this item a group-item?
      *
@@ -124,6 +125,8 @@ public:
     void setSelected( Qt::CheckState isSelected );
     void setChildrenSelected( Qt::CheckState isSelected );
 
+    void removeChild( int row );
+
     /** @brief Update selectedness based on the children's states
      *
      * This only makes sense for groups, which might have packages
@@ -157,6 +160,7 @@ private:
     QString m_description;
     QString m_preScript;
     QString m_postScript;
+    QString m_source;
     bool m_isGroup = false;
     bool m_isCritical = false;
     bool m_isHidden = false;
diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index 2f54442b7..c573889ab 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -140,7 +140,7 @@ Config::updateGlobalStorage( const QStringList& selected ) const
                 auto netinstallAddOrig = gs->value( "NetinstallAdd" );
                 if ( netinstallAddOrig.canConvert( QVariant::List ) )
                 {
-                    netinstallDataList += netinstallAddOrig.toList();
+                    //netinstallDataList += netinstallAddOrig.toList();
                 }
                 else
                 {
diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp
index 7b7cfc08f..3d1c0c044 100644
--- a/src/modules/packagechooser/PackageModel.cpp
+++ b/src/modules/packagechooser/PackageModel.cpp
@@ -140,11 +140,16 @@ QVariantList
 PackageListModel::getNetinstallDataForNames( const QStringList& ids ) const
 {
     QVariantList l;
-    for ( const auto& p : qAsConst( m_packages ) )
+    for ( auto &p : m_packages )
     {
         if ( ids.contains( p.id ) )
         {
-            l.append( p.netinstallData );
+            if ( !p.netinstallData.isEmpty() )
+            {
+                QVariantMap newData = p.netinstallData;
+                newData["source"] = "packageChooser";
+                l.append( newData );
+            }
         }
     }
     return l;

From a657d7388cdbea6c0363034829b7eae2d7f3fbd7 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Sun, 23 Jan 2022 14:47:14 -0600
Subject: [PATCH 5/9] [packagechooser] Remove obsolete functionality from
 netinstall-add

---
 src/modules/packagechooser/Config.cpp | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index c573889ab..ffe816f17 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -132,23 +132,7 @@ Config::updateGlobalStorage( const QStringList& selected ) const
         }
         else
         {
-            auto* gs = Calamares::JobQueue::instance()->globalStorage();
-
-            // If an earlier packagechooser instance added this data to global storage, combine them
-            if ( gs->contains( "NetinstallAdd" ) )
-            {
-                auto netinstallAddOrig = gs->value( "NetinstallAdd" );
-                if ( netinstallAddOrig.canConvert( QVariant::List ) )
-                {
-                    //netinstallDataList += netinstallAddOrig.toList();
-                }
-                else
-                {
-                    cWarning() << "Invalid NetinstallAdd data in global storage.  Earlier selections purged";
-                }
-                gs->remove( "NetinstallAdd" );
-            }
-            gs->insert( "NetinstallAdd", netinstallDataList );
+            Calamares::JobQueue::instance()->globalStorage()->insert( "NetinstallAdd", netinstallDataList );
         }
     }
     else if ( m_method == PackageChooserMethod::NetSelect )

From 1db217931bf16afc858662f1a2980b88a7b36824 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Mon, 24 Jan 2022 15:39:14 -0600
Subject: [PATCH 6/9] [netinstall] Minor changes from code review

---
 src/modules/netinstall/NetInstallPage.cpp  | 16 +++----
 src/modules/netinstall/PackageModel.cpp    | 51 +++++++++-------------
 src/modules/netinstall/PackageModel.h      | 14 ++++--
 src/modules/netinstall/PackageTreeItem.cpp |  2 +-
 4 files changed, 40 insertions(+), 43 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 4468e9cc0..80e6a58f9 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -64,20 +64,18 @@ NetInstallPage::onActivate()
 {
     ui->groupswidget->setFocus();
 
-    // The NetInstallSelect global sotrage value can be used to make additional items selected by default
+    // The NetInstallSelect global storage value can be used to make additional items selected by default
     Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
-    if ( gs->contains( "NetinstallSelect" ) && gs->value( "NetinstallSelect" ).canConvert( QVariant::StringList ) )
+    const QStringList selectNames = gs->value( "NetinstallSelect" ).toStringList();
+    if ( !selectNames.isEmpty() )
     {
-        const QStringList selectNames = gs->value( "NetinstallSelect" ).toStringList();
-
-        static_cast< PackageModel* >( ui->groupswidget->model() )->setSelections( selectNames );
+        m_config->model()->setSelections( selectNames );
     }
 
     // If NetInstallAdd is found in global storage, add those items to the tree
-    if ( gs->contains( "NetinstallAdd" ) && gs->value( "NetinstallAdd" ).canConvert( QVariant::List ) )
+    const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
+    if ( !groups.isEmpty() )
     {
-        const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
-
-        static_cast< PackageModel* >( ui->groupswidget->model() )->appendModelData( groups, "packageChooser" );
+        m_config->model()->appendModelData( groups, "packageChooser" );
     }
 }
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index d139a89f6..9ffaa2650 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -14,6 +14,20 @@
 #include "utils/Variant.h"
 #include "utils/Yaml.h"
 
+static void
+setSelections2( const QStringList& selectNames, PackageTreeItem* item )
+{
+    for ( int i = 0; i < item->childCount(); i++ )
+    {
+        auto* child = item->child( i );
+        setSelections2( selectNames, child );
+    }
+    if ( item->isGroup() && selectNames.contains( item->name() ) )
+    {
+        item->setSelected( Qt::CheckState::Checked );
+    }
+}
+
 PackageModel::PackageModel( QObject* parent )
     : QAbstractItemModel( parent )
 {
@@ -170,35 +184,12 @@ PackageModel::headerData( int section, Qt::Orientation orientation, int role ) c
     return QVariant();
 }
 
-/** @brief Sets the checked flag on matching groups in the tree
- *
- * Recursively traverses the tree pointed to by m_rootItem and
- * checks if a group name matches any of the items in @p selectNames.
- * If a match is found, set check the box for that group and it's children.
- *
- * Individual packages will not be matched.
- *
- */
 void
-PackageModel::setSelections( QStringList selectNames )
+PackageModel::setSelections( const QStringList& selectNames )
 {
     if ( m_rootItem )
     {
-        setSelections( selectNames, m_rootItem );
-    }
-}
-
-void
-PackageModel::setSelections( QStringList selectNames, PackageTreeItem* item )
-{
-    for ( int i = 0; i < item->childCount(); i++ )
-    {
-        auto* child = item->child( i );
-        setSelections( selectNames, child );
-    }
-    if ( item->isGroup() && selectNames.contains( item->name() ) )
-    {
-        item->setSelected( Qt::CheckState::Checked );
+        setSelections2( selectNames, m_rootItem );
     }
 }
 
@@ -335,19 +326,19 @@ PackageModel::setupModelData( const QVariantList& groupList, PackageTreeItem* pa
 void
 PackageModel::setupModelData( const QVariantList& l )
 {
-    emit beginResetModel();
+    Q_EMIT beginResetModel();
     delete m_rootItem;
     m_rootItem = new PackageTreeItem();
     setupModelData( l, m_rootItem );
-    emit endResetModel();
+    Q_EMIT endResetModel();
 }
 
 void
-PackageModel::appendModelData( const QVariantList& groupList, const QString source )
+PackageModel::appendModelData( const QVariantList& groupList, const QString& source )
 {
     if ( m_rootItem )
     {
-        emit beginResetModel();
+        Q_EMIT beginResetModel();
 
         // Prune any existing data from the same source
         for ( int i = 0; i < m_rootItem->childCount(); i++ )
@@ -362,6 +353,6 @@ PackageModel::appendModelData( const QVariantList& groupList, const QString sour
         // Add the new data to the model
         setupModelData( groupList, m_rootItem );
 
-        emit endResetModel();
+        Q_EMIT endResetModel();
     }
 }
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 49a17f0b6..3e79faf98 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -54,8 +54,16 @@ public:
     int rowCount( const QModelIndex& parent = QModelIndex() ) const override;
     int columnCount( const QModelIndex& parent = QModelIndex() ) const override;
 
-    void setSelections( QStringList selectNames );
-    void setSelections( QStringList selectNames, PackageTreeItem* item );
+    /** @brief Sets the checked flag on matching groups in the tree
+     *
+     * Recursively traverses the tree pointed to by m_rootItem and
+     * checks if a group name matches any of the items in @p selectNames.
+     * If a match is found, set check the box for that group and it's children.
+     *
+     * Individual packages will not be matched.
+     *
+     */
+    void setSelections(const QStringList &selectNames );
 
     PackageTreeItem::List getPackages() const;
     PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
@@ -71,7 +79,7 @@ public:
      * data is pruned first
      *
      */
-    void appendModelData( const QVariantList& groupList, const QString source );
+    void appendModelData(const QVariantList& groupList, const QString &source );
 
 private:
     friend class ItemTests;
diff --git a/src/modules/netinstall/PackageTreeItem.cpp b/src/modules/netinstall/PackageTreeItem.cpp
index 245f1ddc6..36a2e7d88 100644
--- a/src/modules/netinstall/PackageTreeItem.cpp
+++ b/src/modules/netinstall/PackageTreeItem.cpp
@@ -258,7 +258,7 @@ PackageTreeItem::removeChild( int row )
     }
     else
     {
-        cWarning() << "Attempt to remove invalid child in removeChild() at row " + QString::number( row );
+        cWarning() << "Attempt to remove invalid child in removeChild() at row " << row;
     }
 }
 

From 22c9d888b475a42e17c393bb489b70501f514376 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Mon, 24 Jan 2022 17:01:16 -0600
Subject: [PATCH 7/9] [packagechooser,netinstall] Proper implementation of
 source field

---
 src/modules/netinstall/NetInstallPage.cpp   |  2 +-
 src/modules/netinstall/PackageModel.cpp     | 50 ++++++++++++++++++---
 src/modules/netinstall/PackageModel.h       | 13 +-----
 src/modules/packagechooser/PackageModel.cpp |  4 +-
 4 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 80e6a58f9..17c16a472 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -76,6 +76,6 @@ NetInstallPage::onActivate()
     const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
     if ( !groups.isEmpty() )
     {
-        m_config->model()->appendModelData( groups, "packageChooser" );
+        m_config->model()->appendModelData( groups );
     }
 }
diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index 9ffaa2650..ac7443c15 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -14,6 +14,17 @@
 #include "utils/Variant.h"
 #include "utils/Yaml.h"
 
+/** @brief Appends groups to the tree
+ *
+ * Uses the data from @p groupList to add elements to the
+ * existing tree that m_rootItem points to.  If m_rootItem
+ * is not valid, it does nothing
+ *
+ * Before adding anything to the model, it ensures that there
+ * is no existing data from the same source.  If there is, that
+ * data is pruned first
+ *
+ */
 static void
 setSelections2( const QStringList& selectNames, PackageTreeItem* item )
 {
@@ -28,6 +39,28 @@ setSelections2( const QStringList& selectNames, PackageTreeItem* item )
     }
 }
 
+/** @brief Collects all the "source" values from @p groupList
+ *
+ * Iterates over @p groupList and returns all nonempty "source"
+ * values from the maps.
+ *
+ */
+static QStringList
+collectSources( const QVariantList& groupList )
+{
+    QStringList sources;
+    for ( const QVariant& group : groupList )
+    {
+        QVariantMap groupMap = group.toMap();
+        if ( !groupMap[ "source" ].toString().isEmpty() )
+        {
+            sources.append( groupMap[ "source" ].toString() );
+        }
+    }
+
+    return sources;
+}
+
 PackageModel::PackageModel( QObject* parent )
     : QAbstractItemModel( parent )
 {
@@ -334,19 +367,24 @@ PackageModel::setupModelData( const QVariantList& l )
 }
 
 void
-PackageModel::appendModelData( const QVariantList& groupList, const QString& source )
+PackageModel::appendModelData( const QVariantList& groupList )
 {
     if ( m_rootItem )
     {
         Q_EMIT beginResetModel();
 
-        // Prune any existing data from the same source
-        for ( int i = 0; i < m_rootItem->childCount(); i++ )
+        const QStringList sources = collectSources( groupList );
+
+        if ( !sources.isEmpty() )
         {
-            PackageTreeItem* child = m_rootItem->child( i );
-            if ( child->source() == source )
+            // Prune any existing data from the same source
+            for ( int i = 0; i < m_rootItem->childCount(); i++ )
             {
-                m_rootItem->removeChild( i );
+                PackageTreeItem* child = m_rootItem->child( i );
+                if ( sources.contains( child->source() ) )
+                {
+                    m_rootItem->removeChild( i );
+                }
             }
         }
 
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 3e79faf98..5146c63dd 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -68,18 +68,7 @@ public:
     PackageTreeItem::List getPackages() const;
     PackageTreeItem::List getItemPackages( PackageTreeItem* item ) const;
 
-    /** @brief Appends groups to the tree
-     *
-     * Uses the data from @p groupList to add elements to the
-     * existing tree that m_rootItem points to.  If m_rootItem
-     * is not valid, it does nothing
-     *
-     * Before adding anything to the model, it ensures that there
-     * is no existing data from the same source.  If there is, that
-     * data is pruned first
-     *
-     */
-    void appendModelData(const QVariantList& groupList, const QString &source );
+    void appendModelData(const QVariantList& groupList);
 
 private:
     friend class ItemTests;
diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp
index 3d1c0c044..f1d1184ad 100644
--- a/src/modules/packagechooser/PackageModel.cpp
+++ b/src/modules/packagechooser/PackageModel.cpp
@@ -140,14 +140,14 @@ QVariantList
 PackageListModel::getNetinstallDataForNames( const QStringList& ids ) const
 {
     QVariantList l;
-    for ( auto &p : m_packages )
+    for ( auto& p : m_packages )
     {
         if ( ids.contains( p.id ) )
         {
             if ( !p.netinstallData.isEmpty() )
             {
                 QVariantMap newData = p.netinstallData;
-                newData["source"] = "packageChooser";
+                newData[ "source" ] = QStringLiteral( "packageChooser" );
                 l.append( newData );
             }
         }

From 63ed2e5fb8427b80677523c45f9a0bf3ed67c5d4 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Tue, 25 Jan 2022 17:43:19 -0600
Subject: [PATCH 8/9] [packagechooser,netinstall] Change globalstorage keys to
 camel case

---
 src/modules/netinstall/NetInstallPage.cpp |  4 ++--
 src/modules/packagechooser/Config.cpp     | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 17c16a472..ec350a6c6 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -66,14 +66,14 @@ NetInstallPage::onActivate()
 
     // The NetInstallSelect global storage value can be used to make additional items selected by default
     Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
-    const QStringList selectNames = gs->value( "NetinstallSelect" ).toStringList();
+    const QStringList selectNames = gs->value( "netinstallSelect" ).toStringList();
     if ( !selectNames.isEmpty() )
     {
         m_config->model()->setSelections( selectNames );
     }
 
     // If NetInstallAdd is found in global storage, add those items to the tree
-    const QVariantList groups = gs->value( "NetinstallAdd" ).toList();
+    const QVariantList groups = gs->value( "netinstallAdd" ).toList();
     if ( !groups.isEmpty() )
     {
         m_config->model()->appendModelData( groups );
diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index ffe816f17..c6f5c9658 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -132,7 +132,7 @@ Config::updateGlobalStorage( const QStringList& selected ) const
         }
         else
         {
-            Calamares::JobQueue::instance()->globalStorage()->insert( "NetinstallAdd", netinstallDataList );
+            Calamares::JobQueue::instance()->globalStorage()->insert( "netinstallAdd", netinstallDataList );
         }
     }
     else if ( m_method == PackageChooserMethod::NetSelect )
@@ -142,9 +142,9 @@ Config::updateGlobalStorage( const QStringList& selected ) const
         auto* gs = Calamares::JobQueue::instance()->globalStorage();
 
         // If an earlier packagechooser instance added this data to global storage, combine them
-        if ( gs->contains( "NetinstallSelect" ) )
+        if ( gs->contains( "netinstallSelect" ) )
         {
-            auto selectedOrig = gs->value( "NetinstallSelect" );
+            auto selectedOrig = gs->value( "netinstallSelect" );
             if ( selectedOrig.canConvert( QVariant::StringList ) )
             {
                 newSelected += selectedOrig.toStringList();
@@ -153,9 +153,9 @@ Config::updateGlobalStorage( const QStringList& selected ) const
             {
                 cWarning() << "Invalid NetinstallSelect data in global storage.  Earlier selections purged";
             }
-            gs->remove( "NetinstallSelect" );
+            gs->remove( "netinstallSelect" );
         }
-        gs->insert( "NetinstallSelect", newSelected );
+        gs->insert( "netinstallSelect", newSelected );
     }
     else
     {

From e5979980218edb7058138a6078a642b3b69971f7 Mon Sep 17 00:00:00 2001
From: dalto <dalto@fastmail.com>
Date: Thu, 27 Jan 2022 19:31:34 -0600
Subject: [PATCH 9/9] [packagechooser,netinstall] Clean up duplication and
 pruning logic for netinstall-add

---
 src/modules/netinstall/PackageModel.cpp |  7 +++++-
 src/modules/packagechooser/Config.cpp   | 32 ++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/modules/netinstall/PackageModel.cpp b/src/modules/netinstall/PackageModel.cpp
index ac7443c15..3fc2f4d43 100644
--- a/src/modules/netinstall/PackageModel.cpp
+++ b/src/modules/netinstall/PackageModel.cpp
@@ -378,14 +378,19 @@ PackageModel::appendModelData( const QVariantList& groupList )
         if ( !sources.isEmpty() )
         {
             // Prune any existing data from the same source
+            QList< int > removeList;
             for ( int i = 0; i < m_rootItem->childCount(); i++ )
             {
                 PackageTreeItem* child = m_rootItem->child( i );
                 if ( sources.contains( child->source() ) )
                 {
-                    m_rootItem->removeChild( i );
+                    removeList.insert( 0, i );
                 }
             }
+            for ( const int& item : qAsConst( removeList ) )
+            {
+                m_rootItem->removeChild( item );
+            }
         }
 
         // Add the new data to the model
diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index c6f5c9658..491fe5c25 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -27,6 +27,29 @@
 #include "utils/Logger.h"
 #include "utils/Variant.h"
 
+/** @brief This removes any values from @p groups that match @p source
+ *
+ * This is used to remove duplicates from the netinstallAdd structure
+ * It iterates over @p groups and for each map in the list, if the
+ * "source" element matches @p source, it is removed from the returned
+ * list.
+ */
+static QVariantList
+pruneNetinstallAdd( const QString& source, const QVariant& groups )
+{
+    QVariantList newGroupList;
+    const QVariantList groupList = groups.toList();
+    for ( const QVariant& group : groupList )
+    {
+        QVariantMap groupMap = group.toMap();
+        if ( groupMap.value( "source", "" ).toString() != source )
+        {
+            newGroupList.append( groupMap );
+        }
+    }
+    return newGroupList;
+}
+
 const NamedEnumTable< PackageChooserMode >&
 packageChooserModeNames()
 {
@@ -132,7 +155,14 @@ Config::updateGlobalStorage( const QStringList& selected ) const
         }
         else
         {
-            Calamares::JobQueue::instance()->globalStorage()->insert( "netinstallAdd", netinstallDataList );
+            // If an earlier packagechooser instance added this data to global storage, combine them
+            auto* gs = Calamares::JobQueue::instance()->globalStorage();
+            if ( gs->contains( "netinstallAdd" ) )
+            {
+                netinstallDataList
+                    += pruneNetinstallAdd( QStringLiteral( "packageChooser" ), gs->value( "netinstallAdd" ) );
+            }
+            gs->insert( "netinstallAdd", netinstallDataList );
         }
     }
     else if ( m_method == PackageChooserMethod::NetSelect )