From dd8b893ee87a8c76e9579872920b4f54084bedec Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 8 Feb 2021 15:36:27 +0100
Subject: [PATCH 01/17] Changes: mention what this branch is for

---
 CHANGES | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/CHANGES b/CHANGES
index aa5dba206..1750c1e6e 100644
--- a/CHANGES
+++ b/CHANGES
@@ -16,7 +16,11 @@ This release contains contributions from (alphabetically by first name):
  - No core changes yet
 
 ## Modules ##
- - No module changes yet
+ - *netinstall* now supports fallbacks for the groups data.
+   Instead of a single URL, multiple URLs may be specified in
+   a list and Calamares goes through them until one is successfully
+   retrieved. Older configurations with a single string are
+   treated like a one-item list. #1579
 
 
 # 3.2.36 (2021-02-03) #

From cf7391696e6f11c2a5ccba993c6b6917697cdad7 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 8 Feb 2021 22:57:38 +0100
Subject: [PATCH 02/17] [netinstall] Continue moving settings to the Config
 object

---
 src/modules/netinstall/Config.cpp             | 49 +++++++++++++++++++
 src/modules/netinstall/Config.h               | 19 ++++++-
 src/modules/netinstall/NetInstallViewStep.cpp | 27 +---------
 src/modules/netinstall/NetInstallViewStep.h   |  2 -
 4 files changed, 69 insertions(+), 28 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index e69b3c2a4..99fe28eb0 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -12,9 +12,12 @@
 
 #include "Config.h"
 
+#include "GlobalStorage.h"
+#include "JobQueue.h"
 #include "network/Manager.h"
 #include "utils/Logger.h"
 #include "utils/RAII.h"
+#include "utils/Variant.h"
 #include "utils/Yaml.h"
 
 #include <QNetworkReply>
@@ -54,6 +57,19 @@ Config::setStatus( Status s )
     emit statusChanged( status() );
 }
 
+QString
+Config::sidebarLabel() const
+{
+    return m_sidebarLabel ? m_sidebarLabel->get() : tr( "Package selection" );
+}
+
+QString
+Config::titleLabel() const
+{
+    return m_titleLabel ? m_titleLabel->get() : QString();
+}
+
+
 void
 Config::loadGroupList( const QVariantList& groupData )
 {
@@ -143,3 +159,36 @@ Config::receivedGroupData()
         setStatus( Status::FailedBadData );
     }
 }
+
+void
+Config::setConfigurationMap( const QVariantMap& configurationMap )
+{
+    setRequired( CalamaresUtils::getBool( configurationMap, "required", false ) );
+
+    // Get the translations, if any
+    bool bogus = false;
+    auto label = CalamaresUtils::getSubMap( configurationMap, "label", bogus );
+
+    if ( label.contains( "sidebar" ) )
+    {
+        m_sidebarLabel = new CalamaresUtils::Locale::TranslatedString( label, "sidebar", metaObject()->className() );
+    }
+
+    // Lastly, load the groups data
+    QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
+    if ( !groupsUrl.isEmpty() )
+    {
+        // 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 );
+        if ( groupsUrl == QStringLiteral( "local" ) )
+        {
+            QVariantList l = configurationMap.value( "groups" ).toList();
+            loadGroupList( l );
+        }
+        else
+        {
+            loadGroupList( groupsUrl );
+        }
+    }
+}
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 13eb098c6..24c5e6b68 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -14,8 +14,11 @@
 
 #include "PackageModel.h"
 
+#include "locale/TranslatableConfiguration.h"
+
 #include <QObject>
 #include <QUrl>
+#include <QVariantMap>
 
 class QNetworkReply;
 
@@ -26,10 +29,16 @@ class Config : public QObject
     Q_PROPERTY( PackageModel* packageModel MEMBER m_model FINAL )
     Q_PROPERTY( QString status READ status NOTIFY statusChanged FINAL )
 
+    // Translations, of the module name (for sidebar) and above the list
+    Q_PROPERTY( QString sidebarLabel READ sidebarLabel NOTIFY sidebarLabelChanged FINAL )
+    Q_PROPERTY( QString titleLabel READ titleLabel NOTIFY titleLabelChanged FINAL )
+
 public:
     Config( QObject* parent = nullptr );
     ~Config() override;
 
+    void setConfigurationMap( const QVariantMap& configurationMap );
+
     enum class Status
     {
         Ok,
@@ -45,6 +54,11 @@ public:
     bool required() const { return m_required; }
     void setRequired( bool r ) { m_required = r; }
 
+    PackageModel* model() const { return m_model; }
+
+    QString sidebarLabel() const;
+    QString titleLabel() const;
+
     /** @brief Retrieves the groups, with name, description and packages
      *
      * Loads data from the given URL. Once done, the data is parsed
@@ -59,16 +73,19 @@ public:
      */
     void loadGroupList( const QVariantList& groupData );
 
-    PackageModel* model() const { return m_model; }
 
 signals:
     void statusChanged( QString status );  ///< Something changed
+    void sidebarLabelChanged( QString label );
+    void titleLabelChanged( QString label );
     void statusReady();  ///< Loading groups is complete
 
 private slots:
     void receivedGroupData();  ///< From async-loading group data
 
 private:
+    CalamaresUtils::Locale::TranslatedString* m_sidebarLabel = nullptr;  // As it appears in the sidebar
+    CalamaresUtils::Locale::TranslatedString* m_titleLabel = nullptr;
     PackageModel* m_model = nullptr;
     QNetworkReply* m_reply = nullptr;  // For fetching data
     Status m_status = Status::Ok;
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index d92058e51..2e035a8aa 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -24,7 +24,6 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( NetInstallViewStepFactory, registerPlugin<
 NetInstallViewStep::NetInstallViewStep( QObject* parent )
     : Calamares::ViewStep( parent )
     , m_widget( new NetInstallPage( &m_config ) )
-    , m_sidebarLabel( nullptr )
     , m_nextEnabled( false )
 {
     connect( &m_config, &Config::statusReady, this, &NetInstallViewStep::nextIsReady );
@@ -37,14 +36,13 @@ NetInstallViewStep::~NetInstallViewStep()
     {
         m_widget->deleteLater();
     }
-    delete m_sidebarLabel;
 }
 
 
 QString
 NetInstallViewStep::prettyName() const
 {
-    return m_sidebarLabel ? m_sidebarLabel->get() : tr( "Package selection" );
+    return m_config.sidebarLabel();
 
 #if defined( TABLE_OF_TRANSLATIONS )
     __builtin_unreachable();
@@ -201,32 +199,11 @@ NetInstallViewStep::nextIsReady()
 void
 NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
 {
-    m_config.setRequired( CalamaresUtils::getBool( configurationMap, "required", false ) );
-
-    QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
-    if ( !groupsUrl.isEmpty() )
-    {
-        // 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 );
-        if ( groupsUrl == QStringLiteral( "local" ) )
-        {
-            QVariantList l = configurationMap.value( "groups" ).toList();
-            m_config.loadGroupList( l );
-        }
-        else
-        {
-            m_config.loadGroupList( groupsUrl );
-        }
-    }
+    m_config.setConfigurationMap( configurationMap );
 
     bool bogus = false;
     auto label = CalamaresUtils::getSubMap( configurationMap, "label", bogus );
 
-    if ( label.contains( "sidebar" ) )
-    {
-        m_sidebarLabel = new CalamaresUtils::Locale::TranslatedString( label, "sidebar", metaObject()->className() );
-    }
     if ( label.contains( "title" ) )
     {
         m_widget->setPageTitle(
diff --git a/src/modules/netinstall/NetInstallViewStep.h b/src/modules/netinstall/NetInstallViewStep.h
index c500cbcd9..8949632c1 100644
--- a/src/modules/netinstall/NetInstallViewStep.h
+++ b/src/modules/netinstall/NetInstallViewStep.h
@@ -14,7 +14,6 @@
 #include "Config.h"
 
 #include "DllMacro.h"
-#include "locale/TranslatableConfiguration.h"
 #include "utils/PluginFactory.h"
 #include "viewpages/ViewStep.h"
 
@@ -56,7 +55,6 @@ private:
     Config m_config;
 
     NetInstallPage* m_widget;
-    CalamaresUtils::Locale::TranslatedString* m_sidebarLabel;  // As it appears in the sidebar
     bool m_nextEnabled = false;
 };
 

From 335ccbc14913be47a33a626eef098ab90f1fa80b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 9 Feb 2021 10:58:11 +0100
Subject: [PATCH 03/17] [netinstall] Move other translation parts to Config

---
 src/modules/netinstall/Config.cpp             | 10 ++++-
 src/modules/netinstall/NetInstallPage.cpp     | 43 +++++++------------
 src/modules/netinstall/NetInstallPage.h       | 15 +------
 src/modules/netinstall/NetInstallViewStep.cpp | 12 ++----
 4 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 99fe28eb0..022e6fcd7 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -168,10 +168,18 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     // Get the translations, if any
     bool bogus = false;
     auto label = CalamaresUtils::getSubMap( configurationMap, "label", bogus );
+    // Use a different class name for translation lookup because the
+    // .. table of strings lives in NetInstallViewStep.cpp and moving them
+    // .. around is annoying for translators.
+    static const char className[] = "NetInstallViewStep";
 
     if ( label.contains( "sidebar" ) )
     {
-        m_sidebarLabel = new CalamaresUtils::Locale::TranslatedString( label, "sidebar", metaObject()->className() );
+        m_sidebarLabel = new CalamaresUtils::Locale::TranslatedString( label, "sidebar", className );
+    }
+    if ( label.contains( "title" ) )
+    {
+        m_titleLabel = new CalamaresUtils::Locale::TranslatedString( label, "title", className );
     }
 
     // Lastly, load the groups data
diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 75175a941..0dfb810b5 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -34,39 +34,12 @@ NetInstallPage::NetInstallPage( Config* c, QWidget* parent )
     ui->groupswidget->header()->setSectionResizeMode( QHeaderView::ResizeToContents );
     ui->groupswidget->setModel( c->model() );
     connect( c, &Config::statusChanged, this, &NetInstallPage::setStatus );
+    connect( c, &Config::titleLabelChanged, this, &NetInstallPage::setTitle );
     connect( c, &Config::statusReady, this, &NetInstallPage::expandGroups );
-
-    setPageTitle( nullptr );
-    CALAMARES_RETRANSLATE_SLOT( &NetInstallPage::retranslate )
 }
 
 NetInstallPage::~NetInstallPage() {}
 
-void
-NetInstallPage::setPageTitle( CalamaresUtils::Locale::TranslatedString* t )
-{
-    m_title.reset( t );
-    if ( !m_title )
-    {
-        ui->label->hide();
-    }
-    else
-    {
-        ui->label->show();
-    }
-    retranslate();
-}
-
-void
-NetInstallPage::retranslate()
-{
-    if ( m_title )
-    {
-        ui->label->setText( m_title->get() );  // That's get() on the TranslatedString
-    }
-    ui->netinst_status->setText( m_config->status() );
-}
-
 void
 NetInstallPage::expandGroups()
 {
@@ -88,6 +61,20 @@ NetInstallPage::setStatus( QString s )
     ui->netinst_status->setText( s );
 }
 
+void
+NetInstallPage::setTitle( QString title )
+{
+    if ( title.isEmpty() )
+    {
+        ui->label->hide();
+    }
+    else
+    {
+        ui->label->setText( title );  // That's get() on the TranslatedString
+        ui->label->show();
+    }
+}
+
 void
 NetInstallPage::onActivate()
 {
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index 1c97423da..e58a7dd8c 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -37,22 +37,11 @@ public:
     NetInstallPage( Config* config, QWidget* parent = nullptr );
     ~NetInstallPage() override;
 
-    /** @brief Sets the page title
-     *
-     * In situations where there is more than one netinstall page,
-     * or you want some explanatory title above the treeview,
-     * set the page title. This page takes ownership of the
-     * TranslatedString object.
-     *
-     * Set to nullptr to remove the title.
-     */
-    void setPageTitle( CalamaresUtils::Locale::TranslatedString* );
-
     void onActivate();
 
 public slots:
-    void retranslate();
     void setStatus( QString s );
+    void setTitle( QString title );
 
     /** @brief Expand entries that should be pre-expanded.
      *
@@ -64,8 +53,6 @@ public slots:
 private:
     Config* m_config;
     Ui::Page_NetInst* ui;
-
-    std::unique_ptr< CalamaresUtils::Locale::TranslatedString > m_title;  // Above the treeview
 };
 
 #endif  // NETINSTALLPAGE_H
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 2e035a8aa..a8ce1e311 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -49,6 +49,9 @@ NetInstallViewStep::prettyName() const
     // 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
     // translations can be used.
+    //
+    // These translations still live here, even though the lookup
+    // code is in the Config class.
     tr( "Package selection" );
     tr( "Office software" );
     tr( "Office package" );
@@ -200,13 +203,4 @@ void
 NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
 {
     m_config.setConfigurationMap( configurationMap );
-
-    bool bogus = false;
-    auto label = CalamaresUtils::getSubMap( configurationMap, "label", bogus );
-
-    if ( label.contains( "title" ) )
-    {
-        m_widget->setPageTitle(
-            new CalamaresUtils::Locale::TranslatedString( label, "title", metaObject()->className() ) );
-    }
 }

From ca1ae6fd1d5276a9dd44f90108053368ecaeb9a2 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 9 Feb 2021 11:06:59 +0100
Subject: [PATCH 04/17] [netinstall] Support retranslation in the Config object

---
 src/modules/netinstall/Config.cpp | 14 ++++++++++++--
 src/modules/netinstall/Config.h   |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 022e6fcd7..2327a732c 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -17,6 +17,7 @@
 #include "network/Manager.h"
 #include "utils/Logger.h"
 #include "utils/RAII.h"
+#include "utils/Retranslator.h"
 #include "utils/Variant.h"
 #include "utils/Yaml.h"
 
@@ -24,11 +25,20 @@
 
 Config::Config( QObject* parent )
     : QObject( parent )
-    , m_model( new PackageModel( this ) )
+    , m_model( new PackageModel( this ) ) { CALAMARES_RETRANSLATE_SLOT( &Config::retranslate ) }
+
+    Config::~Config()
 {
 }
 
-Config::~Config() {}
+void
+Config::retranslate()
+{
+    emit statusChanged( status() );
+    emit sidebarLabelChanged( sidebarLabel() );
+    emit titleLabelChanged( titleLabel() );
+}
+
 
 QString
 Config::status() const
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 24c5e6b68..f5f76f35f 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -82,6 +82,7 @@ signals:
 
 private slots:
     void receivedGroupData();  ///< From async-loading group data
+    void retranslate();
 
 private:
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel = nullptr;  // As it appears in the sidebar

From 04f4441182e8acad8f5cd2515f8d60c6d4804563 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 9 Feb 2021 15:06:53 +0100
Subject: [PATCH 05/17] [netinstall] Build up a list of urls, rather than just
 one

- the list is unused, and doesn't drive the loading of groups either;
  the existing one-string entry is used.
---
 src/modules/netinstall/Config.cpp | 27 +++++++++++++++++++++++++++
 src/modules/netinstall/Config.h   | 18 ++++++++++++++++++
 2 files changed, 45 insertions(+)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 2327a732c..6b1daaa07 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -170,6 +170,19 @@ Config::receivedGroupData()
     }
 }
 
+Config::SourceItem Config::SourceItem::makeSourceItem(const QVariantMap& configurationMap, const QString& groupsUrl)
+{
+    if ( groupsUrl == QStringLiteral( "local" ) )
+    {
+        return SourceItem{ QUrl(), configurationMap.value( "groups" ).toList() };
+    }
+    else
+    {
+        return SourceItem{ QUrl{ groupsUrl }, QVariantList() };
+    }
+}
+
+
 void
 Config::setConfigurationMap( const QVariantMap& configurationMap )
 {
@@ -193,6 +206,20 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     }
 
     // Lastly, load the groups data
+    const QString key = QStringLiteral( "groupsUrl" );
+    const auto& groupsUrlVariant = configurationMap.value( key );
+    if ( groupsUrlVariant.type() == QVariant::String )
+    {
+        m_urls.append( SourceItem::makeSourceItem( configurationMap, groupsUrlVariant.toString() ) );
+    }
+    else if ( groupsUrlVariant.type() == QVariant::StringList )
+    {
+        for ( const auto& s : groupsUrlVariant.toStringList() )
+        {
+            m_urls.append( SourceItem::makeSourceItem( configurationMap, s ) );
+        }
+    }
+
     QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
     if ( !groupsUrl.isEmpty() )
     {
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index f5f76f35f..554b4ce7d 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -18,6 +18,7 @@
 
 #include <QObject>
 #include <QUrl>
+#include <QQueue>
 #include <QVariantMap>
 
 class QNetworkReply;
@@ -85,6 +86,23 @@ private slots:
     void retranslate();
 
 private:
+    /** @brief Data about an entry in *groupsUrl*
+     *
+     * This can be a specific URL, or "local" which uses data stored
+     * in the configuration file itself.
+     */
+    struct SourceItem
+    {
+        QUrl url;
+        QVariantList data;
+
+        bool isUrl() const { return url.isValid(); }
+        bool isLocal() const { return !data.isEmpty(); }
+        bool isValid() const { return isUrl() || isLocal(); }
+        static SourceItem makeSourceItem( const QVariantMap& configurationMap, const QString& groupsUrl);
+    };
+
+    QQueue< SourceItem > m_urls;
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel = nullptr;  // As it appears in the sidebar
     CalamaresUtils::Locale::TranslatedString* m_titleLabel = nullptr;
     PackageModel* m_model = nullptr;

From 8e8525a941ecf660f038c582138c0bdf509f35d0 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 13:10:09 +0100
Subject: [PATCH 06/17] [netinstall] Simplify slots in the UI page

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

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 0dfb810b5..31c9d15ac 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -33,8 +33,12 @@ NetInstallPage::NetInstallPage( Config* c, QWidget* parent )
     ui->setupUi( this );
     ui->groupswidget->header()->setSectionResizeMode( QHeaderView::ResizeToContents );
     ui->groupswidget->setModel( c->model() );
-    connect( c, &Config::statusChanged, this, &NetInstallPage::setStatus );
-    connect( c, &Config::titleLabelChanged, this, &NetInstallPage::setTitle );
+    connect( c, &Config::statusChanged, ui->netinst_status, &QLabel::setText );
+    connect( c, &Config::titleLabelChanged, [ui = this->ui]( const QString title ) {
+        ui->label->setVisible( !title.isEmpty() );
+        ui->label->setText( title );
+    } );
+
     connect( c, &Config::statusReady, this, &NetInstallPage::expandGroups );
 }
 
@@ -55,26 +59,6 @@ NetInstallPage::expandGroups()
     }
 }
 
-void
-NetInstallPage::setStatus( QString s )
-{
-    ui->netinst_status->setText( s );
-}
-
-void
-NetInstallPage::setTitle( QString title )
-{
-    if ( title.isEmpty() )
-    {
-        ui->label->hide();
-    }
-    else
-    {
-        ui->label->setText( title );  // That's get() on the TranslatedString
-        ui->label->show();
-    }
-}
-
 void
 NetInstallPage::onActivate()
 {
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index e58a7dd8c..72375d0f0 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -39,10 +39,6 @@ public:
 
     void onActivate();
 
-public slots:
-    void setStatus( QString s );
-    void setTitle( QString title );
-
     /** @brief Expand entries that should be pre-expanded.
      *
      * Follows the *expanded* key / the startExpanded field in the

From 79b4f918fcc67bc1f2d154ee290a4c8548c31ddd Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 13:10:35 +0100
Subject: [PATCH 07/17] [netinstall] Apply coding style

---
 src/modules/netinstall/Config.cpp | 7 ++++---
 src/modules/netinstall/Config.h   | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 6b1daaa07..233bc65dd 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -170,15 +170,16 @@ Config::receivedGroupData()
     }
 }
 
-Config::SourceItem Config::SourceItem::makeSourceItem(const QVariantMap& configurationMap, const QString& groupsUrl)
+Config::SourceItem
+Config::SourceItem::makeSourceItem( const QVariantMap& configurationMap, const QString& groupsUrl )
 {
     if ( groupsUrl == QStringLiteral( "local" ) )
     {
-        return SourceItem{ QUrl(), configurationMap.value( "groups" ).toList() };
+        return SourceItem { QUrl(), configurationMap.value( "groups" ).toList() };
     }
     else
     {
-        return SourceItem{ QUrl{ groupsUrl }, QVariantList() };
+        return SourceItem { QUrl { groupsUrl }, QVariantList() };
     }
 }
 
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 554b4ce7d..84ab027cd 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -17,8 +17,8 @@
 #include "locale/TranslatableConfiguration.h"
 
 #include <QObject>
-#include <QUrl>
 #include <QQueue>
+#include <QUrl>
 #include <QVariantMap>
 
 class QNetworkReply;
@@ -99,7 +99,7 @@ private:
         bool isUrl() const { return url.isValid(); }
         bool isLocal() const { return !data.isEmpty(); }
         bool isValid() const { return isUrl() || isLocal(); }
-        static SourceItem makeSourceItem( const QVariantMap& configurationMap, const QString& groupsUrl);
+        static SourceItem makeSourceItem( const QVariantMap& configurationMap, const QString& groupsUrl );
     };
 
     QQueue< SourceItem > m_urls;

From 6662cb5f2d8b2d96b034f651972fc4aad1407836 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 13:17:33 +0100
Subject: [PATCH 08/17] [netinstall] Swap parameters to makeSourceItem and
 document it

---
 src/modules/netinstall/Config.cpp | 6 +++---
 src/modules/netinstall/Config.h   | 8 +++++++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 233bc65dd..017164c86 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -171,7 +171,7 @@ Config::receivedGroupData()
 }
 
 Config::SourceItem
-Config::SourceItem::makeSourceItem( const QVariantMap& configurationMap, const QString& groupsUrl )
+Config::SourceItem::makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap )
 {
     if ( groupsUrl == QStringLiteral( "local" ) )
     {
@@ -211,13 +211,13 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     const auto& groupsUrlVariant = configurationMap.value( key );
     if ( groupsUrlVariant.type() == QVariant::String )
     {
-        m_urls.append( SourceItem::makeSourceItem( configurationMap, groupsUrlVariant.toString() ) );
+        m_urls.append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
     }
     else if ( groupsUrlVariant.type() == QVariant::StringList )
     {
         for ( const auto& s : groupsUrlVariant.toStringList() )
         {
-            m_urls.append( SourceItem::makeSourceItem( configurationMap, s ) );
+            m_urls.append( SourceItem::makeSourceItem( s, configurationMap ) );
         }
     }
 
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index 84ab027cd..de96ccee4 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -99,7 +99,13 @@ private:
         bool isUrl() const { return url.isValid(); }
         bool isLocal() const { return !data.isEmpty(); }
         bool isValid() const { return isUrl() || isLocal(); }
-        static SourceItem makeSourceItem( const QVariantMap& configurationMap, const QString& groupsUrl );
+        /** @brief Create a SourceItem
+         *
+         * If the @p groupsUrl is @c "local" then the *groups* key in
+         * the @p configurationMap is used as the source; otherwise the
+         * string is used as an actual URL.
+         */
+        static SourceItem makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap );
     };
 
     QQueue< SourceItem > m_urls;

From b8688943714d1fcf583cade3557f4fa47e9d74db Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 13:50:15 +0100
Subject: [PATCH 09/17] [libcalamares] Start a packages service for netinstall
 and others

---
 src/libcalamares/CMakeLists.txt       |  9 ++++
 src/libcalamares/packages/Globals.cpp | 69 +++++++++++++++++++++++++++
 src/libcalamares/packages/Globals.h   | 32 +++++++++++++
 src/libcalamares/packages/Tests.cpp   | 44 +++++++++++++++++
 4 files changed, 154 insertions(+)
 create mode 100644 src/libcalamares/packages/Globals.cpp
 create mode 100644 src/libcalamares/packages/Globals.h
 create mode 100644 src/libcalamares/packages/Tests.cpp

diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt
index 826f0bc41..95dad0530 100644
--- a/src/libcalamares/CMakeLists.txt
+++ b/src/libcalamares/CMakeLists.txt
@@ -59,6 +59,9 @@ set( libSources
     # Network service
     network/Manager.cpp
 
+    # Packages service
+    packages/Globals.cpp
+
     # Partition service
     partition/Mount.cpp
     partition/PartitionSize.cpp
@@ -228,6 +231,12 @@ calamares_add_test(
         network/Tests.cpp
 )
 
+calamares_add_test(
+    libcalamarespackagestest
+    SOURCES
+        packages/Tests.cpp
+)
+
 calamares_add_test(
     libcalamarespartitiontest
     SOURCES
diff --git a/src/libcalamares/packages/Globals.cpp b/src/libcalamares/packages/Globals.cpp
new file mode 100644
index 000000000..a488e704b
--- /dev/null
+++ b/src/libcalamares/packages/Globals.cpp
@@ -0,0 +1,69 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2021 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#include "Globals.h"
+
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+#include "utils/Logger.h"
+
+bool
+CalamaresUtils::Packages::setGSPackageAdditions( const Calamares::ModuleSystem::InstanceKey& module,
+                                                 const QVariantList& installPackages,
+                                                 const QVariantList& tryInstallPackages )
+{
+    static const char PACKAGEOP[] = "packageOperations";
+
+    // Check if there's already a PACAKGEOP entry in GS, and if so we'll
+    // extend that one (overwriting the value in GS at the end of this method)
+    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
+    QVariantList packageOperations = gs->contains( PACKAGEOP ) ? gs->value( PACKAGEOP ).toList() : QVariantList();
+    cDebug() << "Existing package operations length" << packageOperations.length();
+
+    const QString key = module.toString() + QStringLiteral( ";add" );
+
+    // Clear out existing operations for this module, going backwards:
+    // Sometimes we remove an item, and we don't want the index to
+    // fall off the end of the list.
+    bool somethingRemoved = false;
+    for ( int index = packageOperations.length() - 1; 0 <= index; index-- )
+    {
+        const QVariantMap op = packageOperations.at( index ).toMap();
+        if ( op.contains( "source" ) && op.value( "source" ).toString() == key )
+        {
+            cDebug() << Logger::SubEntry << "Removing existing operations for" << key;
+            packageOperations.removeAt( index );
+            somethingRemoved = true;
+        }
+    }
+
+    if ( !installPackages.empty() )
+    {
+        QVariantMap op;
+        op.insert( "install", QVariant( installPackages ) );
+        op.insert( "source", key );
+        packageOperations.append( op );
+        cDebug() << Logger::SubEntry << installPackages.length() << "critical packages.";
+    }
+    if ( !tryInstallPackages.empty() )
+    {
+        QVariantMap op;
+        op.insert( "try_install", QVariant( tryInstallPackages ) );
+        op.insert( "source", key );
+        packageOperations.append( op );
+        cDebug() << Logger::SubEntry << tryInstallPackages.length() << "non-critical packages.";
+    }
+
+    if ( somethingRemoved || !packageOperations.isEmpty() )
+    {
+        gs->insert( PACKAGEOP, packageOperations );
+        return true;
+    }
+    return false;
+}
diff --git a/src/libcalamares/packages/Globals.h b/src/libcalamares/packages/Globals.h
new file mode 100644
index 000000000..ef9fa3801
--- /dev/null
+++ b/src/libcalamares/packages/Globals.h
@@ -0,0 +1,32 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2021 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#ifndef LIBCALAMARES_PACKAGES_GLOBALS_H
+#define LIBCALAMARES_PACKAGES_GLOBALS_H
+
+#include "modulesystem/InstanceKey.h"
+
+namespace CalamaresUtils
+{
+namespace Packages
+{
+/** @brief Sets the install-packages GS keys for the given module
+ *
+ * This replaces previously-set install-packages lists for the
+ * given module by the two new lists.
+ */
+bool setGSPackageAdditions( const Calamares::ModuleSystem::InstanceKey& module,
+                            const QVariantList& installPackages,
+                            const QVariantList& tryInstallPackages );
+// void setGSPackageRemovals( const Calamares::ModuleSystem::InstanceKey& key, const QVariantList& removePackages );
+}  // namespace Packages
+}  // namespace CalamaresUtils
+
+
+#endif
diff --git a/src/libcalamares/packages/Tests.cpp b/src/libcalamares/packages/Tests.cpp
new file mode 100644
index 000000000..307fb3c41
--- /dev/null
+++ b/src/libcalamares/packages/Tests.cpp
@@ -0,0 +1,44 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2021 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#include "utils/Logger.h"
+
+#include "GlobalStorage.h"
+
+#include <QtTest/QtTest>
+
+class PackagesTests : public QObject
+{
+    Q_OBJECT
+public:
+    PackagesTests() {}
+    ~PackagesTests() override {}
+private Q_SLOTS:
+    void initTestCase();
+
+    void testEmpty();
+};
+
+void
+PackagesTests::initTestCase()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+}
+
+void
+PackagesTests::testEmpty()
+{
+}
+
+
+QTEST_GUILESS_MAIN( PackagesTests )
+
+#include "utils/moc-warnings.h"
+
+#include "Tests.moc"

From 5b609565e2d7b3adefeede2688f2e44cb8495d2d Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 14:14:02 +0100
Subject: [PATCH 10/17] [libcalamares] Make Packages API more flexible

- pass in the GS object; this makes mostly **testing** much easier
---
 src/libcalamares/packages/Globals.cpp |  5 ++---
 src/libcalamares/packages/Globals.h   |  6 +++++-
 src/libcalamares/packages/Tests.cpp   | 13 ++++++++++++-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/libcalamares/packages/Globals.cpp b/src/libcalamares/packages/Globals.cpp
index a488e704b..6cd07bc4a 100644
--- a/src/libcalamares/packages/Globals.cpp
+++ b/src/libcalamares/packages/Globals.cpp
@@ -10,11 +10,11 @@
 #include "Globals.h"
 
 #include "GlobalStorage.h"
-#include "JobQueue.h"
 #include "utils/Logger.h"
 
 bool
-CalamaresUtils::Packages::setGSPackageAdditions( const Calamares::ModuleSystem::InstanceKey& module,
+CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs,
+                                                 const Calamares::ModuleSystem::InstanceKey& module,
                                                  const QVariantList& installPackages,
                                                  const QVariantList& tryInstallPackages )
 {
@@ -22,7 +22,6 @@ CalamaresUtils::Packages::setGSPackageAdditions( const Calamares::ModuleSystem::
 
     // Check if there's already a PACAKGEOP entry in GS, and if so we'll
     // extend that one (overwriting the value in GS at the end of this method)
-    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
     QVariantList packageOperations = gs->contains( PACKAGEOP ) ? gs->value( PACKAGEOP ).toList() : QVariantList();
     cDebug() << "Existing package operations length" << packageOperations.length();
 
diff --git a/src/libcalamares/packages/Globals.h b/src/libcalamares/packages/Globals.h
index ef9fa3801..a47cf5ae1 100644
--- a/src/libcalamares/packages/Globals.h
+++ b/src/libcalamares/packages/Globals.h
@@ -10,6 +10,7 @@
 #ifndef LIBCALAMARES_PACKAGES_GLOBALS_H
 #define LIBCALAMARES_PACKAGES_GLOBALS_H
 
+#include "GlobalStorage.h"
 #include "modulesystem/InstanceKey.h"
 
 namespace CalamaresUtils
@@ -20,8 +21,11 @@ namespace Packages
  *
  * This replaces previously-set install-packages lists for the
  * given module by the two new lists.
+ *
+ * Returns @c true if anything was changed, @c false otherwise.
  */
-bool setGSPackageAdditions( const Calamares::ModuleSystem::InstanceKey& module,
+bool setGSPackageAdditions( Calamares::GlobalStorage* gs,
+                            const Calamares::ModuleSystem::InstanceKey& module,
                             const QVariantList& installPackages,
                             const QVariantList& tryInstallPackages );
 // void setGSPackageRemovals( const Calamares::ModuleSystem::InstanceKey& key, const QVariantList& removePackages );
diff --git a/src/libcalamares/packages/Tests.cpp b/src/libcalamares/packages/Tests.cpp
index 307fb3c41..aaff227a9 100644
--- a/src/libcalamares/packages/Tests.cpp
+++ b/src/libcalamares/packages/Tests.cpp
@@ -7,9 +7,10 @@
  *
  */
 
-#include "utils/Logger.h"
+#include "Globals.h"
 
 #include "GlobalStorage.h"
+#include "utils/Logger.h"
 
 #include <QtTest/QtTest>
 
@@ -34,6 +35,16 @@ PackagesTests::initTestCase()
 void
 PackagesTests::testEmpty()
 {
+    Calamares::GlobalStorage gs;
+    const QString topKey( "packageOperations" );
+    Calamares::ModuleSystem::InstanceKey k( "this", "that" );
+
+    QVERIFY( !gs.contains( topKey ) );
+    QCOMPARE( k.toString(), "this@that" );
+
+    // Adding nothing at all does nothing
+    QVERIFY( !CalamaresUtils::Packages::setGSPackageAdditions( &gs, k, QVariantList(), QVariantList() ) );
+    QVERIFY( !gs.contains( topKey ) );
 }
 
 

From f1446736f817a482ff26562aa968cb370a1e85b9 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 14:37:13 +0100
Subject: [PATCH 11/17] [libcalamares] Expand tests a little

- do some additions and check they work
- drop the ";add" annotation on the source, this is not
  needed in the current situation with only adds available.
---
 src/libcalamares/packages/Globals.cpp |  2 +-
 src/libcalamares/packages/Tests.cpp   | 33 +++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/libcalamares/packages/Globals.cpp b/src/libcalamares/packages/Globals.cpp
index 6cd07bc4a..c5e882436 100644
--- a/src/libcalamares/packages/Globals.cpp
+++ b/src/libcalamares/packages/Globals.cpp
@@ -25,7 +25,7 @@ CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs,
     QVariantList packageOperations = gs->contains( PACKAGEOP ) ? gs->value( PACKAGEOP ).toList() : QVariantList();
     cDebug() << "Existing package operations length" << packageOperations.length();
 
-    const QString key = module.toString() + QStringLiteral( ";add" );
+    const QString key = module.toString();
 
     // Clear out existing operations for this module, going backwards:
     // Sometimes we remove an item, and we don't want the index to
diff --git a/src/libcalamares/packages/Tests.cpp b/src/libcalamares/packages/Tests.cpp
index aaff227a9..0a9be3a20 100644
--- a/src/libcalamares/packages/Tests.cpp
+++ b/src/libcalamares/packages/Tests.cpp
@@ -24,6 +24,7 @@ private Q_SLOTS:
     void initTestCase();
 
     void testEmpty();
+    void testAdd();
 };
 
 void
@@ -47,6 +48,38 @@ PackagesTests::testEmpty()
     QVERIFY( !gs.contains( topKey ) );
 }
 
+void
+PackagesTests::testAdd()
+{
+    Calamares::GlobalStorage gs;
+    const QString topKey( "packageOperations" );
+    Calamares::ModuleSystem::InstanceKey k( "this", "that" );
+
+    QVERIFY( !gs.contains( topKey ) );
+    QVERIFY(
+        CalamaresUtils::Packages::setGSPackageAdditions( &gs, k, QVariantList { QString( "vim" ) }, QVariantList() ) );
+    QVERIFY( gs.contains( topKey ) );
+    auto actionList = gs.value( topKey ).toList();
+    QCOMPARE( actionList.length(), 1 );
+    auto action = actionList[ 0 ].toMap();
+    QVERIFY( action.contains( "install" ) );
+    auto op = action[ "install" ].toList();
+    QCOMPARE( op.length(), 1 );
+    cDebug() << op;
+
+    QVERIFY( CalamaresUtils::Packages::setGSPackageAdditions(
+        &gs, k, QVariantList { QString( "vim" ), QString( "emacs" ) }, QVariantList() ) );
+    QVERIFY( gs.contains( topKey ) );
+    actionList = gs.value( topKey ).toList();
+    QCOMPARE( actionList.length(), 1 );
+    action = actionList[ 0 ].toMap();
+    QVERIFY( action.contains( "install" ) );
+    op = action[ "install" ].toList();
+    QCOMPARE( op.length(), 2 );
+    QCOMPARE( action[ "source" ].toString(), k.toString() );
+    cDebug() << op;
+}
+
 
 QTEST_GUILESS_MAIN( PackagesTests )
 

From 9acd2fe458407f2259daa68ab64ac690cc78fcf5 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 14:38:52 +0100
Subject: [PATCH 12/17] [netinstall] Use the packages service

---
 src/modules/netinstall/NetInstallViewStep.cpp | 50 ++-----------------
 1 file changed, 3 insertions(+), 47 deletions(-)

diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index a8ce1e311..ae8d2f95f 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -11,9 +11,8 @@
 
 #include "NetInstallViewStep.h"
 
-#include "GlobalStorage.h"
 #include "JobQueue.h"
-
+#include "packages/Globals.h"
 #include "utils/Logger.h"
 #include "utils/Variant.h"
 
@@ -127,30 +126,6 @@ void
 NetInstallViewStep::onLeave()
 {
     auto packages = m_config.model()->getPackages();
-    cDebug() << "Netinstall: Processing" << packages.length() << "packages.";
-
-    static const char PACKAGEOP[] = "packageOperations";
-
-    // Check if there's already a PACAKGEOP entry in GS, and if so we'll
-    // extend that one (overwriting the value in GS at the end of this method)
-    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
-    QVariantList packageOperations = gs->contains( PACKAGEOP ) ? gs->value( PACKAGEOP ).toList() : QVariantList();
-    cDebug() << Logger::SubEntry << "Existing package operations length" << packageOperations.length();
-
-    // Clear out existing operations for this module, going backwards:
-    // Sometimes we remove an item, and we don't want the index to
-    // fall off the end of the list.
-    bool somethingRemoved = false;
-    for ( int index = packageOperations.length() - 1; 0 <= index; index-- )
-    {
-        const QVariantMap op = packageOperations.at( index ).toMap();
-        if ( op.contains( "source" ) && op.value( "source" ).toString() == moduleInstanceKey().toString() )
-        {
-            cDebug() << Logger::SubEntry << "Removing existing operations for" << moduleInstanceKey();
-            packageOperations.removeAt( index );
-            somethingRemoved = true;
-        }
-    }
 
     // This netinstall module may add two sub-steps to the packageOperations,
     // one for installing and one for try-installing.
@@ -169,27 +144,8 @@ NetInstallViewStep::onLeave()
         }
     }
 
-    if ( !installPackages.empty() )
-    {
-        QVariantMap op;
-        op.insert( "install", QVariant( installPackages ) );
-        op.insert( "source", moduleInstanceKey().toString() );
-        packageOperations.append( op );
-        cDebug() << Logger::SubEntry << installPackages.length() << "critical packages.";
-    }
-    if ( !tryInstallPackages.empty() )
-    {
-        QVariantMap op;
-        op.insert( "try_install", QVariant( tryInstallPackages ) );
-        op.insert( "source", moduleInstanceKey().toString() );
-        packageOperations.append( op );
-        cDebug() << Logger::SubEntry << tryInstallPackages.length() << "non-critical packages.";
-    }
-
-    if ( somethingRemoved || !packageOperations.isEmpty() )
-    {
-        gs->insert( PACKAGEOP, packageOperations );
-    }
+    CalamaresUtils::Packages::setGSPackageAdditions(
+        Calamares::JobQueue::instance()->globalStorage(), moduleInstanceKey(), installPackages, tryInstallPackages );
 }
 
 void

From 603a7106b3db9c78f7b0e7da79a606b4ecc4160b Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Tue, 16 Mar 2021 14:47:12 +0100
Subject: [PATCH 13/17] [netinstall] Move package-listing wrangling to the
 Config object

Now all the business logic is in Config, the door is open to
building a QML-ified netinstall module. I'm not sure that
would be worth it: packagechooser offers more space for a
nice UI and should be QML'ed first.
---
 src/modules/netinstall/Config.cpp             | 34 +++++++++++++++++--
 src/modules/netinstall/Config.h               |  7 ++++
 src/modules/netinstall/NetInstallViewStep.cpp | 27 +--------------
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 017164c86..927eb8330 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -15,6 +15,7 @@
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 #include "network/Manager.h"
+#include "packages/Globals.h"
 #include "utils/Logger.h"
 #include "utils/RAII.h"
 #include "utils/Retranslator.h"
@@ -25,12 +26,13 @@
 
 Config::Config( QObject* parent )
     : QObject( parent )
-    , m_model( new PackageModel( this ) ) { CALAMARES_RETRANSLATE_SLOT( &Config::retranslate ) }
-
-    Config::~Config()
+    , m_model( new PackageModel( this ) )
 {
+    CALAMARES_RETRANSLATE_SLOT( &Config::retranslate );
 }
 
+Config::~Config() {}
+
 void
 Config::retranslate()
 {
@@ -238,3 +240,29 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
         }
     }
 }
+
+void
+Config::finalizeGlobalStorage( const Calamares::ModuleSystem::InstanceKey& key )
+{
+    auto packages = model()->getPackages();
+
+    // This netinstall module may add two sub-steps to the packageOperations,
+    // one for installing and one for try-installing.
+    QVariantList installPackages;
+    QVariantList tryInstallPackages;
+
+    for ( const auto& package : packages )
+    {
+        if ( package->isCritical() )
+        {
+            installPackages.append( package->toOperation() );
+        }
+        else
+        {
+            tryInstallPackages.append( package->toOperation() );
+        }
+    }
+
+    CalamaresUtils::Packages::setGSPackageAdditions(
+        Calamares::JobQueue::instance()->globalStorage(), key, installPackages, tryInstallPackages );
+}
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index de96ccee4..e748449a3 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -15,6 +15,7 @@
 #include "PackageModel.h"
 
 #include "locale/TranslatableConfiguration.h"
+#include "modulesystem/InstanceKey.h"
 
 #include <QObject>
 #include <QQueue>
@@ -74,6 +75,12 @@ public:
      */
     void loadGroupList( const QVariantList& groupData );
 
+    /** @brief Write the selected package lists to global storage
+     *
+     * Since the config doesn't know what module it is for,
+     * pass in an instance key.
+     */
+    void finalizeGlobalStorage( const Calamares::ModuleSystem::InstanceKey& key );
 
 signals:
     void statusChanged( QString status );  ///< Something changed
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index ae8d2f95f..2ac0e73c9 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -11,11 +11,6 @@
 
 #include "NetInstallViewStep.h"
 
-#include "JobQueue.h"
-#include "packages/Globals.h"
-#include "utils/Logger.h"
-#include "utils/Variant.h"
-
 #include "NetInstallPage.h"
 
 CALAMARES_PLUGIN_FACTORY_DEFINITION( NetInstallViewStepFactory, registerPlugin< NetInstallViewStep >(); )
@@ -125,27 +120,7 @@ NetInstallViewStep::onActivate()
 void
 NetInstallViewStep::onLeave()
 {
-    auto packages = m_config.model()->getPackages();
-
-    // This netinstall module may add two sub-steps to the packageOperations,
-    // one for installing and one for try-installing.
-    QVariantList installPackages;
-    QVariantList tryInstallPackages;
-
-    for ( const auto& package : packages )
-    {
-        if ( package->isCritical() )
-        {
-            installPackages.append( package->toOperation() );
-        }
-        else
-        {
-            tryInstallPackages.append( package->toOperation() );
-        }
-    }
-
-    CalamaresUtils::Packages::setGSPackageAdditions(
-        Calamares::JobQueue::instance()->globalStorage(), moduleInstanceKey(), installPackages, tryInstallPackages );
+    m_config.finalizeGlobalStorage( moduleInstanceKey() );
 }
 
 void

From 404a9ef98ace44ace1e3cc3bf6ee04143a4a5559 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Wed, 17 Mar 2021 00:09:15 +0100
Subject: [PATCH 14/17] [netinstall] Split off requesting netinstall data into
 a queue-manager

This is the actual "meat" of the branch, which makes the
netinstall module request one URL at a time until one succeeds.
---
 src/modules/netinstall/CMakeLists.txt  |   1 +
 src/modules/netinstall/Config.cpp      | 132 +++----------------
 src/modules/netinstall/Config.h        |  43 +-----
 src/modules/netinstall/LoaderQueue.cpp | 174 +++++++++++++++++++++++++
 src/modules/netinstall/LoaderQueue.h   |  67 ++++++++++
 5 files changed, 264 insertions(+), 153 deletions(-)
 create mode 100644 src/modules/netinstall/LoaderQueue.cpp
 create mode 100644 src/modules/netinstall/LoaderQueue.h

diff --git a/src/modules/netinstall/CMakeLists.txt b/src/modules/netinstall/CMakeLists.txt
index 4500a314f..ec926c9d3 100644
--- a/src/modules/netinstall/CMakeLists.txt
+++ b/src/modules/netinstall/CMakeLists.txt
@@ -8,6 +8,7 @@ calamares_add_plugin( netinstall
     EXPORT_MACRO PLUGINDLLEXPORT_PRO
     SOURCES
         Config.cpp
+        LoaderQueue.cpp
         NetInstallViewStep.cpp
         NetInstallPage.cpp
         PackageTreeItem.cpp
diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 927eb8330..491443654 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -12,15 +12,15 @@
 
 #include "Config.h"
 
+#include "LoaderQueue.h"
+
 #include "GlobalStorage.h"
 #include "JobQueue.h"
 #include "network/Manager.h"
 #include "packages/Globals.h"
 #include "utils/Logger.h"
-#include "utils/RAII.h"
 #include "utils/Retranslator.h"
 #include "utils/Variant.h"
-#include "utils/Yaml.h"
 
 #include <QNetworkReply>
 
@@ -86,106 +86,13 @@ void
 Config::loadGroupList( const QVariantList& groupData )
 {
     m_model->setupModelData( groupData );
+    if ( m_model->rowCount() < 1 )
+    {
+        cWarning() << "NetInstall groups data was empty.";
+    }
     emit statusReady();
 }
 
-void
-Config::loadGroupList( const QUrl& url )
-{
-    if ( !url.isValid() )
-    {
-        setStatus( Status::FailedBadConfiguration );
-    }
-
-    using namespace CalamaresUtils::Network;
-
-    cDebug() << "NetInstall loading groups from" << url;
-    QNetworkReply* reply = Manager::instance().asynchronousGet(
-        url,
-        RequestOptions( RequestOptions::FakeUserAgent | RequestOptions::FollowRedirect, std::chrono::seconds( 30 ) ) );
-
-    if ( !reply )
-    {
-        cDebug() << Logger::Continuation << "request failed immediately.";
-        setStatus( Status::FailedBadConfiguration );
-    }
-    else
-    {
-        m_reply = reply;
-        connect( reply, &QNetworkReply::finished, this, &Config::receivedGroupData );
-    }
-}
-
-void
-Config::receivedGroupData()
-{
-    if ( !m_reply || !m_reply->isFinished() )
-    {
-        cWarning() << "NetInstall data called too early.";
-        setStatus( Status::FailedInternalError );
-        return;
-    }
-
-    cDebug() << "NetInstall group data received" << m_reply->size() << "bytes from" << m_reply->url();
-
-    cqDeleter< QNetworkReply > 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( Status::FailedNetworkError );
-        return;
-    }
-
-    QByteArray yamlData = m_reply->readAll();
-    try
-    {
-        YAML::Node groups = YAML::Load( yamlData.constData() );
-
-        if ( groups.IsSequence() )
-        {
-            loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
-        }
-        else if ( groups.IsMap() )
-        {
-            auto map = CalamaresUtils::yamlMapToVariant( groups );
-            loadGroupList( map.value( "groups" ).toList() );
-        }
-        else
-        {
-            cWarning() << "NetInstall groups data does not form a sequence.";
-        }
-        if ( m_model->rowCount() < 1 )
-        {
-            cWarning() << "NetInstall groups data was empty.";
-        }
-    }
-    catch ( YAML::Exception& e )
-    {
-        CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
-        setStatus( Status::FailedBadData );
-    }
-}
-
-Config::SourceItem
-Config::SourceItem::makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap )
-{
-    if ( groupsUrl == QStringLiteral( "local" ) )
-    {
-        return SourceItem { QUrl(), configurationMap.value( "groups" ).toList() };
-    }
-    else
-    {
-        return SourceItem { QUrl { groupsUrl }, QVariantList() };
-    }
-}
-
-
 void
 Config::setConfigurationMap( const QVariantMap& configurationMap )
 {
@@ -213,31 +120,24 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     const auto& groupsUrlVariant = configurationMap.value( key );
     if ( groupsUrlVariant.type() == QVariant::String )
     {
-        m_urls.append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
+        m_queue = new LoaderQueue( this );
+        m_queue->append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
     }
     else if ( groupsUrlVariant.type() == QVariant::StringList )
     {
+        m_queue = new LoaderQueue( this );
         for ( const auto& s : groupsUrlVariant.toStringList() )
         {
-            m_urls.append( SourceItem::makeSourceItem( s, configurationMap ) );
+            m_queue->append( SourceItem::makeSourceItem( s, configurationMap ) );
         }
     }
-
-    QString groupsUrl = CalamaresUtils::getString( configurationMap, "groupsUrl" );
-    if ( !groupsUrl.isEmpty() )
+    if ( m_queue )
     {
-        // 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 );
-        if ( groupsUrl == QStringLiteral( "local" ) )
-        {
-            QVariantList l = configurationMap.value( "groups" ).toList();
-            loadGroupList( l );
-        }
-        else
-        {
-            loadGroupList( groupsUrl );
-        }
+        connect( m_queue, &LoaderQueue::done, [this]() {
+            m_queue->deleteLater();
+            m_queue = nullptr;
+        } );
+        m_queue->fetchNext();
     }
 }
 
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index e748449a3..b8e258eff 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -18,11 +18,11 @@
 #include "modulesystem/InstanceKey.h"
 
 #include <QObject>
-#include <QQueue>
-#include <QUrl>
 #include <QVariantMap>
 
-class QNetworkReply;
+#include <memory>
+
+class LoaderQueue;
 
 class Config : public QObject
 {
@@ -61,13 +61,6 @@ public:
     QString sidebarLabel() const;
     QString titleLabel() const;
 
-    /** @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
@@ -82,44 +75,20 @@ public:
      */
     void finalizeGlobalStorage( const Calamares::ModuleSystem::InstanceKey& key );
 
-signals:
+Q_SIGNALS:
     void statusChanged( QString status );  ///< Something changed
     void sidebarLabelChanged( QString label );
     void titleLabelChanged( QString label );
     void statusReady();  ///< Loading groups is complete
 
-private slots:
-    void receivedGroupData();  ///< From async-loading group data
+private Q_SLOTS:
     void retranslate();
 
 private:
-    /** @brief Data about an entry in *groupsUrl*
-     *
-     * This can be a specific URL, or "local" which uses data stored
-     * in the configuration file itself.
-     */
-    struct SourceItem
-    {
-        QUrl url;
-        QVariantList data;
-
-        bool isUrl() const { return url.isValid(); }
-        bool isLocal() const { return !data.isEmpty(); }
-        bool isValid() const { return isUrl() || isLocal(); }
-        /** @brief Create a SourceItem
-         *
-         * If the @p groupsUrl is @c "local" then the *groups* key in
-         * the @p configurationMap is used as the source; otherwise the
-         * string is used as an actual URL.
-         */
-        static SourceItem makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap );
-    };
-
-    QQueue< SourceItem > m_urls;
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel = nullptr;  // As it appears in the sidebar
     CalamaresUtils::Locale::TranslatedString* m_titleLabel = nullptr;
     PackageModel* m_model = nullptr;
-    QNetworkReply* m_reply = nullptr;  // For fetching data
+    LoaderQueue* m_queue;
     Status m_status = Status::Ok;
     bool m_required = false;
 };
diff --git a/src/modules/netinstall/LoaderQueue.cpp b/src/modules/netinstall/LoaderQueue.cpp
new file mode 100644
index 000000000..7236b4096
--- /dev/null
+++ b/src/modules/netinstall/LoaderQueue.cpp
@@ -0,0 +1,174 @@
+/*
+ *   SPDX-FileCopyrightText: 2016 Luca Giambonini <almack@chakraos.org>
+ *   SPDX-FileCopyrightText: 2016 Lisa Vitolo     <shainer@chakraos.org>
+ *   SPDX-FileCopyrightText: 2017 Kyle Robbertze  <krobbertze@gmail.com>
+ *   SPDX-FileCopyrightText: 2017-2018 2020, Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#include "LoaderQueue.h"
+
+#include "Config.h"
+#include "network/Manager.h"
+#include "utils/Logger.h"
+#include "utils/RAII.h"
+#include "utils/Yaml.h"
+
+#include <QNetworkReply>
+#include <QTimer>
+
+SourceItem
+SourceItem::makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap )
+{
+    if ( groupsUrl == QStringLiteral( "local" ) )
+    {
+        return SourceItem { QUrl(), configurationMap.value( "groups" ).toList() };
+    }
+    else
+    {
+        return SourceItem { QUrl { groupsUrl }, QVariantList() };
+    }
+}
+
+LoaderQueue::LoaderQueue( Config* parent )
+    : QObject( parent )
+    , m_config( parent )
+{
+}
+
+void
+LoaderQueue::append( SourceItem&& i )
+{
+    m_queue.append( std::move( i ) );
+}
+
+void
+LoaderQueue::fetchNext()
+{
+    if ( m_queue.isEmpty() )
+    {
+        m_config->setStatus( Config::Status::FailedBadData );
+        emit done();
+        return;
+    }
+
+    auto source = m_queue.takeFirst();
+    if ( source.isLocal() )
+    {
+        m_config->loadGroupList( source.data );
+        emit done();
+    }
+    else
+    {
+        fetch( source.url );
+    }
+}
+
+
+void
+LoaderQueue::fetch( const QUrl& url )
+{
+    if ( !url.isValid() )
+    {
+        m_config->setStatus( Config::Status::FailedBadConfiguration );
+    }
+
+    using namespace CalamaresUtils::Network;
+
+    cDebug() << "NetInstall loading groups from" << url;
+    QNetworkReply* reply = Manager::instance().asynchronousGet(
+        url,
+        RequestOptions( RequestOptions::FakeUserAgent | RequestOptions::FollowRedirect, std::chrono::seconds( 30 ) ) );
+
+    if ( !reply )
+    {
+        cDebug() << Logger::Continuation << "request failed immediately.";
+        m_config->setStatus( Config::Status::FailedBadConfiguration );
+    }
+    else
+    {
+        m_reply = reply;
+        connect( reply, &QNetworkReply::finished, this, &LoaderQueue::dataArrived );
+    }
+}
+
+class FetchNextUnless
+{
+public:
+    FetchNextUnless( LoaderQueue* q )
+        : m_q( q )
+    {
+    }
+    ~FetchNextUnless()
+    {
+        if ( m_q )
+        {
+            QMetaObject::invokeMethod( m_q, "fetchNext", Qt::QueuedConnection );
+        }
+    }
+    void release() { m_q = nullptr; }
+
+private:
+    LoaderQueue* m_q = nullptr;
+};
+
+void
+LoaderQueue::dataArrived()
+{
+    FetchNextUnless finished( this );
+
+    if ( !m_reply || !m_reply->isFinished() )
+    {
+        cWarning() << "NetInstall data called too early.";
+        m_config->setStatus( Config::Status::FailedInternalError );
+        return;
+    }
+
+    cDebug() << "NetInstall group data received" << m_reply->size() << "bytes from" << m_reply->url();
+
+    cqDeleter< QNetworkReply > 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();
+        m_config->setStatus( Config::Status::FailedNetworkError );
+        return;
+    }
+
+    QByteArray yamlData = m_reply->readAll();
+    try
+    {
+        YAML::Node groups = YAML::Load( yamlData.constData() );
+
+        if ( groups.IsSequence() )
+        {
+            finished.release();
+            m_config->loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
+            emit done();
+        }
+        else if ( groups.IsMap() )
+        {
+            finished.release();
+            auto map = CalamaresUtils::yamlMapToVariant( groups );
+            m_config->loadGroupList( map.value( "groups" ).toList() );
+            emit done();
+        }
+        else
+        {
+            cWarning() << "NetInstall groups data does not form a sequence.";
+        }
+    }
+    catch ( YAML::Exception& e )
+    {
+        CalamaresUtils::explainYamlException( e, yamlData, "netinstall groups data" );
+        m_config->setStatus( Config::Status::FailedBadData );
+    }
+}
diff --git a/src/modules/netinstall/LoaderQueue.h b/src/modules/netinstall/LoaderQueue.h
new file mode 100644
index 000000000..de6c7276f
--- /dev/null
+++ b/src/modules/netinstall/LoaderQueue.h
@@ -0,0 +1,67 @@
+/*
+ *   SPDX-FileCopyrightText: 2016 Luca Giambonini <almack@chakraos.org>
+ *   SPDX-FileCopyrightText: 2016 Lisa Vitolo     <shainer@chakraos.org>
+ *   SPDX-FileCopyrightText: 2017 Kyle Robbertze  <krobbertze@gmail.com>
+ *   SPDX-FileCopyrightText: 2017-2018 2020, Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ */
+
+#ifndef NETINSTALL_LOADERQUEUE_H
+#define NETINSTALL_LOADERQUEUE_H
+
+#include <QQueue>
+#include <QUrl>
+#include <QVariantList>
+
+class Config;
+class QNetworkReply;
+
+/** @brief Data about an entry in *groupsUrl*
+ *
+ * This can be a specific URL, or "local" which uses data stored
+ * in the configuration file itself.
+ */
+struct SourceItem
+{
+    QUrl url;
+    QVariantList data;
+
+    bool isUrl() const { return url.isValid(); }
+    bool isLocal() const { return !data.isEmpty(); }
+    bool isValid() const { return isUrl() || isLocal(); }
+    /** @brief Create a SourceItem
+     *
+     * If the @p groupsUrl is @c "local" then the *groups* key in
+     * the @p configurationMap is used as the source; otherwise the
+     * string is used as an actual URL.
+     */
+    static SourceItem makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap );
+};
+
+
+class LoaderQueue : public QObject
+{
+    Q_OBJECT
+public:
+    LoaderQueue( Config* parent );
+
+    void append( SourceItem&& i );
+    void fetchNext();
+
+public Q_SLOTS:
+    void fetch( const QUrl& url );
+    void dataArrived();
+
+Q_SIGNALS:
+    void done();
+
+private:
+    QQueue< SourceItem > m_queue;
+    Config* m_config = nullptr;
+    QNetworkReply* m_reply = nullptr;
+};
+
+#endif

From 03d086a2333ea8a2858662debcc4cee9d89b3d24 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 19 Mar 2021 11:41:47 +0100
Subject: [PATCH 15/17] [netinstall] Missing initialisations, split out slot

- m_queue was not initialized to nullptr, crashes
- split queue-is-done to a separate slot rather than a lambda
- prefer queueing calls to fetchNext(), for responsiveness
---
 src/modules/netinstall/Config.cpp      | 18 +++++++++++++-----
 src/modules/netinstall/Config.h        |  3 ++-
 src/modules/netinstall/LoaderQueue.cpp |  6 ++++++
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 491443654..5f703f8d4 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -93,6 +93,17 @@ Config::loadGroupList( const QVariantList& groupData )
     emit statusReady();
 }
 
+void
+Config::loadingDone()
+{
+    if ( m_queue )
+    {
+        m_queue->deleteLater();
+        m_queue = nullptr;
+    }
+}
+
+
 void
 Config::setConfigurationMap( const QVariantMap& configurationMap )
 {
@@ -133,11 +144,8 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     }
     if ( m_queue )
     {
-        connect( m_queue, &LoaderQueue::done, [this]() {
-            m_queue->deleteLater();
-            m_queue = nullptr;
-        } );
-        m_queue->fetchNext();
+        connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
+        QMetaObject::invokeMethod( m_queue, "fetchNext", Qt::QueuedConnection );
     }
 }
 
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index b8e258eff..d4c3c6f19 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -83,12 +83,13 @@ Q_SIGNALS:
 
 private Q_SLOTS:
     void retranslate();
+    void loadingDone();
 
 private:
     CalamaresUtils::Locale::TranslatedString* m_sidebarLabel = nullptr;  // As it appears in the sidebar
     CalamaresUtils::Locale::TranslatedString* m_titleLabel = nullptr;
     PackageModel* m_model = nullptr;
-    LoaderQueue* m_queue;
+    LoaderQueue* m_queue = nullptr;
     Status m_status = Status::Ok;
     bool m_required = false;
 };
diff --git a/src/modules/netinstall/LoaderQueue.cpp b/src/modules/netinstall/LoaderQueue.cpp
index 7236b4096..98245dead 100644
--- a/src/modules/netinstall/LoaderQueue.cpp
+++ b/src/modules/netinstall/LoaderQueue.cpp
@@ -95,6 +95,12 @@ LoaderQueue::fetch( const QUrl& url )
     }
 }
 
+/** @brief Call fetchNext() on the queue if it can
+ *
+ * On destruction, a new call to fetchNext() is queued, so that
+ * the queue continues loading. Calling release() before the
+ * destructor skips the fetchNext(), ending the queue-loading.
+ */
 class FetchNextUnless
 {
 public:

From fdfe52efe29df1b0f26841feaf920beaa3f24765 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 19 Mar 2021 12:30:09 +0100
Subject: [PATCH 16/17] [netinstall] Improve loader queue API a bit

- use load() to start loading
- the FetchNextUnless class is useful in more spots in
  the loading process
- set status explicitly on success (otherwise, a failure in a
  previous URL would leave a failure message lying around even
  when the module shows something useful)
---
 src/modules/netinstall/Config.cpp      | 16 ++++--
 src/modules/netinstall/Config.h        |  4 +-
 src/modules/netinstall/LoaderQueue.cpp | 70 +++++++++++++++-----------
 src/modules/netinstall/LoaderQueue.h   | 14 +++++-
 4 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/src/modules/netinstall/Config.cpp b/src/modules/netinstall/Config.cpp
index 5f703f8d4..2d663829c 100644
--- a/src/modules/netinstall/Config.cpp
+++ b/src/modules/netinstall/Config.cpp
@@ -54,9 +54,11 @@ Config::status() const
     case Status::FailedBadData:
         return tr( "Network Installation. (Disabled: Received invalid groups data)" );
     case Status::FailedInternalError:
-        return tr( "Network Installation. (Disabled: internal error)" );
+        return tr( "Network Installation. (Disabled: Internal error)" );
     case Status::FailedNetworkError:
         return tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" );
+    case Status::FailedNoData:
+        return tr( "Network Installation. (Disabled: No package list)" );
     }
     __builtin_unreachable();
 }
@@ -89,6 +91,11 @@ Config::loadGroupList( const QVariantList& groupData )
     if ( m_model->rowCount() < 1 )
     {
         cWarning() << "NetInstall groups data was empty.";
+        setStatus( Status::FailedNoData );
+    }
+    else
+    {
+        setStatus( Status::Ok );
     }
     emit statusReady();
 }
@@ -134,7 +141,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
         m_queue = new LoaderQueue( this );
         m_queue->append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
     }
-    else if ( groupsUrlVariant.type() == QVariant::StringList )
+    else if ( groupsUrlVariant.type() == QVariant::List )
     {
         m_queue = new LoaderQueue( this );
         for ( const auto& s : groupsUrlVariant.toStringList() )
@@ -142,10 +149,11 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
             m_queue->append( SourceItem::makeSourceItem( s, configurationMap ) );
         }
     }
-    if ( m_queue )
+    if ( m_queue && m_queue->count() > 0 )
     {
+        cDebug() << "Loading netinstall from" << m_queue->count() << "alternate sources.";
         connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
-        QMetaObject::invokeMethod( m_queue, "fetchNext", Qt::QueuedConnection );
+        m_queue->load();
     }
 }
 
diff --git a/src/modules/netinstall/Config.h b/src/modules/netinstall/Config.h
index d4c3c6f19..b676a7d39 100644
--- a/src/modules/netinstall/Config.h
+++ b/src/modules/netinstall/Config.h
@@ -47,7 +47,9 @@ public:
         FailedBadConfiguration,
         FailedInternalError,
         FailedNetworkError,
-        FailedBadData
+        FailedBadData,
+        FailedNoData
+
     };
 
     QString status() const;
diff --git a/src/modules/netinstall/LoaderQueue.cpp b/src/modules/netinstall/LoaderQueue.cpp
index 98245dead..f8ba17cff 100644
--- a/src/modules/netinstall/LoaderQueue.cpp
+++ b/src/modules/netinstall/LoaderQueue.cpp
@@ -20,6 +20,32 @@
 #include <QNetworkReply>
 #include <QTimer>
 
+/** @brief Call fetchNext() on the queue if it can
+ *
+ * On destruction, a new call to fetchNext() is queued, so that
+ * the queue continues loading. Calling release() before the
+ * destructor skips the fetchNext(), ending the queue-loading.
+ */
+class FetchNextUnless
+{
+public:
+    FetchNextUnless( LoaderQueue* q )
+        : m_q( q )
+    {
+    }
+    ~FetchNextUnless()
+    {
+        if ( m_q )
+        {
+            QMetaObject::invokeMethod( m_q, "fetchNext", Qt::QueuedConnection );
+        }
+    }
+    void release() { m_q = nullptr; }
+
+private:
+    LoaderQueue* m_q = nullptr;
+};
+
 SourceItem
 SourceItem::makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap )
 {
@@ -45,6 +71,13 @@ LoaderQueue::append( SourceItem&& i )
     m_queue.append( std::move( i ) );
 }
 
+void
+LoaderQueue::load()
+{
+    QMetaObject::invokeMethod( this, "fetchNext", Qt::QueuedConnection );
+}
+
+
 void
 LoaderQueue::fetchNext()
 {
@@ -67,13 +100,16 @@ LoaderQueue::fetchNext()
     }
 }
 
-
 void
 LoaderQueue::fetch( const QUrl& url )
 {
+    FetchNextUnless next( this );
+
     if ( !url.isValid() )
     {
         m_config->setStatus( Config::Status::FailedBadConfiguration );
+        cDebug() << "Invalid URL" << url;
+        return;
     }
 
     using namespace CalamaresUtils::Network;
@@ -85,42 +121,20 @@ LoaderQueue::fetch( const QUrl& url )
 
     if ( !reply )
     {
-        cDebug() << Logger::Continuation << "request failed immediately.";
+        cDebug() << Logger::SubEntry << "Request failed immediately.";
+        // If nobody sets a different status, this will remain
         m_config->setStatus( Config::Status::FailedBadConfiguration );
     }
     else
     {
+        // When the network request is done, **then** we might
+        // do the next item from the queue, so don't call fetchNext() now.
+        next.release();
         m_reply = reply;
         connect( reply, &QNetworkReply::finished, this, &LoaderQueue::dataArrived );
     }
 }
 
-/** @brief Call fetchNext() on the queue if it can
- *
- * On destruction, a new call to fetchNext() is queued, so that
- * the queue continues loading. Calling release() before the
- * destructor skips the fetchNext(), ending the queue-loading.
- */
-class FetchNextUnless
-{
-public:
-    FetchNextUnless( LoaderQueue* q )
-        : m_q( q )
-    {
-    }
-    ~FetchNextUnless()
-    {
-        if ( m_q )
-        {
-            QMetaObject::invokeMethod( m_q, "fetchNext", Qt::QueuedConnection );
-        }
-    }
-    void release() { m_q = nullptr; }
-
-private:
-    LoaderQueue* m_q = nullptr;
-};
-
 void
 LoaderQueue::dataArrived()
 {
diff --git a/src/modules/netinstall/LoaderQueue.h b/src/modules/netinstall/LoaderQueue.h
index de6c7276f..d7baf58d4 100644
--- a/src/modules/netinstall/LoaderQueue.h
+++ b/src/modules/netinstall/LoaderQueue.h
@@ -41,7 +41,14 @@ struct SourceItem
     static SourceItem makeSourceItem( const QString& groupsUrl, const QVariantMap& configurationMap );
 };
 
-
+/** @brief Queue of source items to load
+ *
+ * Queue things up by calling append() and then kick things off
+ * by calling load(). This will try to load the items, in order;
+ * the first one that succeeds will end the loading process.
+ *
+ * Signal done() is emitted when done (also when all of the items fail).
+ */
 class LoaderQueue : public QObject
 {
     Q_OBJECT
@@ -49,9 +56,12 @@ public:
     LoaderQueue( Config* parent );
 
     void append( SourceItem&& i );
-    void fetchNext();
+    int count() const { return m_queue.count(); }
 
 public Q_SLOTS:
+    void load();
+
+    void fetchNext();
     void fetch( const QUrl& url );
     void dataArrived();
 

From 3588f06767c664ec740b4c6c71e1245e8d1158e1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 19 Mar 2021 12:49:37 +0100
Subject: [PATCH 17/17] [netinstall] Document groupsUrl with multiple entries

---
 src/modules/netinstall/netinstall.conf | 48 +++++++++++++++++---------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf
index c377b526e..d38c0b040 100644
--- a/src/modules/netinstall/netinstall.conf
+++ b/src/modules/netinstall/netinstall.conf
@@ -32,20 +32,21 @@
 # This module supports multiple instances through the *label* key,
 # which allows you to distinguish them in the UI.
 ---
-# This is the URL that is retrieved to get the netinstall groups-and-packages
-# data (which should be in the format described in netinstall.yaml), e.g.:
-#    ```
-#    groupsUrl: http://example.org/netinstall.php
-#    ```
-# or it can be a locally installed file:
-#    ```
-#    groupsUrl: file:///usr/share/calamares/netinstall.yaml
-#    ```
-# or it can be the special-case literal string "local":
-#    ```
-#    groupsUrl: local
-#    ```
+# The *groupsUrl* determines where the data for the netinstall groups-and-
+# packages comes from. The value of the key may be:
 #
+# - a single string (this is treated as a list with just that string in it)
+# - a list of strings
+#
+# Each string is treated as a URL (see below for special cases. The
+# list is examined **in order** and each URL is tried in turn. The
+# first URL to load successfully -- even if it yields 0 packages --
+# ends the process. This allows using a network URL and a (fallback)
+# local URL for package lists, or for using multiple mirrors of
+# netinstall data.
+#
+# The URL must point to a YAML file that follows the format described
+# below at the key *groups* -- except for the special case URL "local".
 # Note that the contents of the groups file is the **important**
 # part of the configuration of this module. It specifies what
 # groups and packages the user may select (and so what commands are to
@@ -59,12 +60,27 @@
 # must have a list-of-groups as value; if the file does not have
 # a top-level key *groups*, then the file must contain only a list of groups.
 #
-# 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*.
+# Each item in the list *groupsUrl* may be:
+# - A remote URL like `http://example.org/netinstall.php`
+# - A local file URL like `file:///usr/share/calamares/netinstall.yaml`
+# - The special-case literal string `local`
+#
+# Non-special case URLs are loaded as YAML; if the load succeeds, then
+# they are interpreted like the *groups* key below. The special case
+# `local` loads the data directly from **this** file.
 #
 groupsUrl: local
 
+# Alternate form:
+# groupsUrl: [ local ]
+
+# Net-based package list, with fallback to local file
+# groupsUrl:
+#   - http://example.com/calamares/netinstall.yaml
+#   - file:///etc/calamares/modules/netinstall.yaml
+
+
+
 # If the installation can proceed without netinstall (e.g. the Live CD
 # can create a working installed system, but netinstall is preferred
 # to bring it up-to-date or extend functionality) leave this set to