From ee0b3b85dc2f1f4991d82152b2e15343940b31c3 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 6 Nov 2017 05:14:42 -0500
Subject: [PATCH 1/4] [netinstall] Improve 'next' button handling

 - Document netinstall.conf a little,
 - Add setting *required* which influences whether next is enabled or not
   in case of missing or corrupt data,
 - Enable *next* button only once some (any!) data is received.

This can be used to disallow stepping past the netinstall step when
there is no data (e.g. internet has failed between the welcome page
and the netinstall page).
---
 src/modules/netinstall/NetInstallPage.cpp     | 20 ++++++++++---------
 src/modules/netinstall/NetInstallPage.h       |  9 +++++++--
 src/modules/netinstall/NetInstallViewStep.cpp | 17 ++++++++++++++--
 src/modules/netinstall/NetInstallViewStep.h   |  3 +++
 src/modules/netinstall/netinstall.conf        | 11 ++++++++++
 5 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 030537732..13c0da336 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -57,14 +57,6 @@ NetInstallPage::NetInstallPage( QWidget* parent )
     ui->setupUi( this );
 }
 
-bool
-NetInstallPage::isReady()
-{
-    // nothing to wait for, the data are immediately ready
-    // if the user does not select any group nothing is installed
-    return true;
-}
-
 bool
 NetInstallPage::readGroups( const QByteArray& yamlData )
 {
@@ -92,10 +84,13 @@ NetInstallPage::readGroups( const QByteArray& yamlData )
 void
 NetInstallPage::dataIsHere( QNetworkReply* reply )
 {
+    // If m_required is *false* then we still say we're ready
+    // even if the reply is corrupt or missing.
     if ( reply->error() != QNetworkReply::NoError )
     {
         cDebug() << reply->errorString();
         ui->netinst_status->setText( tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" ) );
+        emit checkReady( !m_required );
         return;
     }
 
@@ -104,6 +99,7 @@ NetInstallPage::dataIsHere( QNetworkReply* reply )
         cDebug() << "Netinstall groups data was received, but invalid.";
         ui->netinst_status->setText( tr( "Network Installation. (Disabled: Unable to fetch package lists, check your network connection)" ) );
         reply->deleteLater();
+        emit checkReady( !m_required );
         return;
     }
 
@@ -112,7 +108,7 @@ NetInstallPage::dataIsHere( QNetworkReply* reply )
     ui->groupswidget->header()->setSectionResizeMode( 1, QHeaderView::Stretch );
 
     reply->deleteLater();
-    emit checkReady( isReady() );
+    emit checkReady( true );
 }
 
 QList<PackageTreeItem::ItemData> NetInstallPage::selectedPackages() const
@@ -139,6 +135,12 @@ void NetInstallPage::loadGroupList()
     m_networkManager.get( request );
 }
 
+void NetInstallPage::setRequired(bool b)
+{
+    m_required = b;
+}
+
+
 void NetInstallPage::onActivate()
 {
     ui->groupswidget->setFocus();
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index 423c16b8e..f6939cea6 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -46,13 +46,17 @@ public:
 
     void onActivate();
 
-    bool isReady();
-
     // Retrieves the groups, with name, description and packages, from
     // the remote URL configured in the settings. Assumes the URL is already
     // in the global storage. This should be called before displaying the page.
     void loadGroupList();
 
+    // Sets the "required" state of netinstall data. Influences whether
+    // corrupt or unavailable data causes checkReady() to be emitted
+    // true (not-required) or false.
+    void setRequired( bool );
+    bool getRequired() const { return m_required; }
+
     // Returns the list of packages belonging to groups that are
     // selected in the view in this given moment. No data is cached here, so
     // this function does not have constant time.
@@ -76,6 +80,7 @@ private:
     QNetworkAccessManager m_networkManager;
 
     PackageModel* m_groups;
+    bool m_required;
 };
 
 #endif // NETINSTALLPAGE_H
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 347b2bf27..20505cc34 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -2,6 +2,7 @@
  *   Copyright 2016, Luca Giambonini <almack@chakraos.org>
  *   Copyright 2016, Lisa Vitolo <shainer@chakraos.org>
  *   Copyright 2017, Kyle Robbertze  <krobbertze@gmail.com>
+ *   Copyright 2017, Adriaan de Groot <groot@kde.org>
  *
  *   Calamares is free software: you can redistribute it and/or modify
  *   it under the terms of the GNU General Public License as published by
@@ -30,11 +31,11 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( NetInstallViewStepFactory, registerPlugin<N
 NetInstallViewStep::NetInstallViewStep( QObject* parent )
     : Calamares::ViewStep( parent )
     , m_widget( new NetInstallPage() )
-    , m_nextEnabled( true )
+    , m_nextEnabled( false )
 {
     emit nextStatusChanged( true );
     connect( m_widget, &NetInstallPage::checkReady,
-             this, &NetInstallViewStep::nextStatusChanged );
+             this, &NetInstallViewStep::nextIsReady );
 }
 
 
@@ -178,6 +179,11 @@ NetInstallViewStep::onLeave()
 void
 NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
 {
+    m_widget->setRequired(
+        configurationMap.contains( "required" ) &&
+        configurationMap.value( "required" ).type() == QVariant::Bool &&
+        configurationMap.value( "required" ).toBool() );
+
     if ( configurationMap.contains( "groupsUrl" ) &&
             configurationMap.value( "groupsUrl" ).type() == QVariant::String )
     {
@@ -186,3 +192,10 @@ NetInstallViewStep::setConfigurationMap( const QVariantMap& configurationMap )
         m_widget->loadGroupList();
     }
 }
+
+void
+NetInstallViewStep::nextIsReady( bool b )
+{
+    m_nextEnabled = b;
+    emit nextStatusChanged( b );
+}
diff --git a/src/modules/netinstall/NetInstallViewStep.h b/src/modules/netinstall/NetInstallViewStep.h
index d9853f26f..ee53f61ce 100644
--- a/src/modules/netinstall/NetInstallViewStep.h
+++ b/src/modules/netinstall/NetInstallViewStep.h
@@ -60,6 +60,9 @@ public:
 
     void setConfigurationMap( const QVariantMap& configurationMap ) override;
 
+public slots:
+    void nextIsReady( bool );
+
 private:
     NetInstallPage* m_widget;
     bool m_nextEnabled;
diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf
index b87aef43e..f5977a267 100644
--- a/src/modules/netinstall/netinstall.conf
+++ b/src/modules/netinstall/netinstall.conf
@@ -1,2 +1,13 @@
 ---
+# 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).
 groupsUrl: http://chakraos.org/netinstall.php
+
+# 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
+# false (the default). If set to true, the netinstall data is required.
+#
+# This only has an effect if the netinstall data cannot be retrieved,
+# or is corrupt: having "required" set, means the install cannot proceed.
+required: false

From f424af36d3f79997a3e630d875e1221e52161805 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 6 Nov 2017 05:25:14 -0500
Subject: [PATCH 2/4] [netinstall] Avoid crash when do groups are available

 - m_groups is only set to a non-nullptr value when data is received
   and fully processed,
 - avoid nullptr dereference when paging *back* from a netinstall
   page that hasn't loaded groups data.

FIXES #859
---
 src/modules/netinstall/NetInstallPage.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 13c0da336..0d5bd4049 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -113,7 +113,13 @@ NetInstallPage::dataIsHere( QNetworkReply* reply )
 
 QList<PackageTreeItem::ItemData> NetInstallPage::selectedPackages() const
 {
-    return m_groups->getPackages();
+    if ( m_groups )
+        return m_groups->getPackages();
+    else
+    {
+        cDebug() << "WARNING: no netinstall groups are available.";
+        return QList<PackageTreeItem::ItemData>();
+    }
 }
 
 void NetInstallPage::loadGroupList()

From 91e949f8fc623592ef1c50cabc453d9c217bf0f1 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 6 Nov 2017 05:34:57 -0500
Subject: [PATCH 3/4] [netinstall] Apply Calamares C++ style

---
 src/modules/netinstall/NetInstallPage.cpp     | 12 ++++++++----
 src/modules/netinstall/NetInstallPage.h       |  5 ++++-
 src/modules/netinstall/NetInstallViewStep.cpp |  6 +++---
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 0d5bd4049..0e70bde15 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -111,7 +111,8 @@ NetInstallPage::dataIsHere( QNetworkReply* reply )
     emit checkReady( true );
 }
 
-QList<PackageTreeItem::ItemData> NetInstallPage::selectedPackages() const
+QList<PackageTreeItem::ItemData>
+NetInstallPage::selectedPackages() const
 {
     if ( m_groups )
         return m_groups->getPackages();
@@ -122,7 +123,8 @@ QList<PackageTreeItem::ItemData> NetInstallPage::selectedPackages() const
     }
 }
 
-void NetInstallPage::loadGroupList()
+void
+NetInstallPage::loadGroupList()
 {
     QString confUrl(
         Calamares::JobQueue::instance()->globalStorage()->value(
@@ -141,13 +143,15 @@ void NetInstallPage::loadGroupList()
     m_networkManager.get( request );
 }
 
-void NetInstallPage::setRequired(bool b)
+void
+NetInstallPage::setRequired( bool b )
 {
     m_required = b;
 }
 
 
-void NetInstallPage::onActivate()
+void
+NetInstallPage::onActivate()
 {
     ui->groupswidget->setFocus();
 }
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index f6939cea6..5671ac93e 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -55,7 +55,10 @@ public:
     // corrupt or unavailable data causes checkReady() to be emitted
     // true (not-required) or false.
     void setRequired( bool );
-    bool getRequired() const { return m_required; }
+    bool getRequired() const
+    {
+        return m_required;
+    }
 
     // Returns the list of packages belonging to groups that are
     // selected in the view in this given moment. No data is cached here, so
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index 20505cc34..bb1f014cd 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -139,7 +139,7 @@ NetInstallViewStep::onLeave()
         QVariant details( package.packageName );
         // If it's a package with a pre- or post-script, replace
         // with the more complicated datastructure.
-        if (!package.preScript.isEmpty() || !package.postScript.isEmpty())
+        if ( !package.preScript.isEmpty() || !package.postScript.isEmpty() )
         {
             QMap<QString, QVariant> sdetails;
             sdetails.insert( "pre-script", package.preScript );
@@ -157,14 +157,14 @@ NetInstallViewStep::onLeave()
     {
         QMap<QString, QVariant> op;
         op.insert( "install", QVariant( installPackages ) );
-        packageOperations.append(op);
+        packageOperations.append( op );
         cDebug() << "  .." << installPackages.length() << "critical packages.";
     }
     if ( !tryInstallPackages.empty() )
     {
         QMap<QString, QVariant> op;
         op.insert( "try_install", QVariant( tryInstallPackages ) );
-        packageOperations.append(op);
+        packageOperations.append( op );
         cDebug() << "  .." << tryInstallPackages.length() << "non-critical packages.";
     }
 

From 51c74c6abb4ef2f17c1127bf443f68b3c433c4a9 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Mon, 6 Nov 2017 05:42:13 -0500
Subject: [PATCH 4/4] [netinstall] Convenience typedefs

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

diff --git a/src/modules/netinstall/NetInstallPage.cpp b/src/modules/netinstall/NetInstallPage.cpp
index 0e70bde15..07e0d2397 100644
--- a/src/modules/netinstall/NetInstallPage.cpp
+++ b/src/modules/netinstall/NetInstallPage.cpp
@@ -111,7 +111,7 @@ NetInstallPage::dataIsHere( QNetworkReply* reply )
     emit checkReady( true );
 }
 
-QList<PackageTreeItem::ItemData>
+PackageModel::PackageItemDataList
 NetInstallPage::selectedPackages() const
 {
     if ( m_groups )
@@ -119,7 +119,7 @@ NetInstallPage::selectedPackages() const
     else
     {
         cDebug() << "WARNING: no netinstall groups are available.";
-        return QList<PackageTreeItem::ItemData>();
+        return PackageModel::PackageItemDataList();
     }
 }
 
diff --git a/src/modules/netinstall/NetInstallPage.h b/src/modules/netinstall/NetInstallPage.h
index 5671ac93e..58308412d 100644
--- a/src/modules/netinstall/NetInstallPage.h
+++ b/src/modules/netinstall/NetInstallPage.h
@@ -63,7 +63,7 @@ public:
     // Returns the list of packages belonging to groups that are
     // selected in the view in this given moment. No data is cached here, so
     // this function does not have constant time.
-    QList<PackageTreeItem::ItemData> selectedPackages() const;
+    PackageModel::PackageItemDataList selectedPackages() const;
 
 public slots:
     void dataIsHere( QNetworkReply* );
diff --git a/src/modules/netinstall/NetInstallViewStep.cpp b/src/modules/netinstall/NetInstallViewStep.cpp
index bb1f014cd..50e08486b 100644
--- a/src/modules/netinstall/NetInstallViewStep.cpp
+++ b/src/modules/netinstall/NetInstallViewStep.cpp
@@ -127,7 +127,7 @@ NetInstallViewStep::onLeave()
     cDebug() << "Leaving netinstall, adding packages to be installed"
              << "to global storage";
 
-    QList<PackageTreeItem::ItemData> packages = m_widget->selectedPackages();
+    PackageModel::PackageItemDataList packages = m_widget->selectedPackages();
     QVariantList installPackages;
     QVariantList tryInstallPackages;
     QVariantList packageOperations;
diff --git a/src/modules/netinstall/PackageModel.h b/src/modules/netinstall/PackageModel.h
index 148bd99ab..06d6c0ca1 100644
--- a/src/modules/netinstall/PackageModel.h
+++ b/src/modules/netinstall/PackageModel.h
@@ -1,3 +1,4 @@
+
 /* === This file is part of Calamares - <http://github.com/calamares> ===
  *
  *   Copyright (c) 2017, Kyle Robbertze <kyle@aims.ac.za>
@@ -28,14 +29,13 @@
 
 #include <yaml-cpp/yaml.h>
 
-// Required forward declarations
-class PackageTreeItem;
-
 class PackageModel : public QAbstractItemModel
 {
     Q_OBJECT
 
 public:
+    using PackageItemDataList = QList< PackageTreeItem::ItemData >;
+
     explicit PackageModel( const YAML::Node& data, QObject* parent = nullptr );
     ~PackageModel() override;
 
@@ -52,7 +52,7 @@ public:
     QModelIndex parent( const QModelIndex& index ) const override;
     int rowCount( const QModelIndex& parent = QModelIndex() ) const override;
     int columnCount( const QModelIndex& parent = QModelIndex() ) const override;
-    QList<PackageTreeItem::ItemData> getPackages() const;
+    PackageItemDataList getPackages() const;
     QList<PackageTreeItem*> getItemPackages( PackageTreeItem* item ) const;
 
 private: