Merge branch 'fix-pckq' into calamares

src/modules/packagechooser/Config.cpp

@@ -95,15 +95,24 @@ Config::introductionPackage() const
     return *defaultIntroduction;
 }
 
+static inline QString
+make_gs_key( const Calamares::ModuleSystem::InstanceKey& key )
+{
+    return QStringLiteral( "packagechooser_" ) + key.id();
+}
+
 void
 Config::updateGlobalStorage( const QStringList& selected ) const
 {
+    if ( m_packageChoice.has_value() )
+    {
+        cWarning() << "Inconsistent package choices -- both model and single-selection QML";
+    }
     if ( m_method == PackageChooserMethod::Legacy )
     {
-        //QString value = selected.join( ',' );
-        QString value = ( m_pkgc );
-        Calamares::JobQueue::instance()->globalStorage()->insert( m_id, value );
-        cDebug() << m_id<< "selected" << value;
+        QString value = selected.join( ',' );
+        Calamares::JobQueue::instance()->globalStorage()->insert( make_gs_key( m_defaultId ), value );
+        cDebug() << m_defaultId << "selected" << value;
     }
     else if ( m_method == PackageChooserMethod::Packages )
     {
@@ -119,16 +128,53 @@ Config::updateGlobalStorage( const QStringList& selected ) const
 }
 
 void
-Config::setPkgc( const QString& pkgc )
+Config::updateGlobalStorage() const
+{
+    if ( m_model->packageCount() > 0 )
+    {
+        cWarning() << "Inconsistent package choices -- both model and single-selection QML";
+    }
+    if ( m_method == PackageChooserMethod::Legacy )
+    {
+        auto* gs = Calamares::JobQueue::instance()->globalStorage();
+        if ( m_packageChoice.has_value() )
+        {
+            gs->insert( make_gs_key( m_defaultId ), m_packageChoice.value() );
+        }
+        else
+        {
+            gs->remove( make_gs_key( m_defaultId ) );
+        }
+    }
+    else if ( m_method == PackageChooserMethod::Packages )
+    {
+        cWarning() << "Unsupported single-selection packagechooser method 'Packages'";
+    }
+    else
+    {
+        cWarning() << "Unknown packagechooser method" << smash( m_method );
+    }
+}
+
+
+void
+Config::setPackageChoice( const QString& packageChoice )
 {
-    m_pkgc = pkgc;
-    emit pkgcChanged( m_pkgc );
+    if ( packageChoice.isEmpty() )
+    {
+        m_packageChoice.reset();
+    }
+    else
+    {
+        m_packageChoice = packageChoice;
+    }
+    emit packageChoiceChanged( m_packageChoice.value_or( QString() ) );
 }
 
 QString
 Config::prettyStatus() const
 {
-    return tr( "Install option: <strong>%1</strong>" ).arg( m_pkgc );
+    return tr( "Install option: <strong>%1</strong>" ).arg( m_packageChoice.value_or( tr( "None" ) ) );
 }
 
 static void
@@ -197,50 +243,38 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
                                              PackageChooserMode::Required );
     m_method = PackageChooserMethodNames().find( CalamaresUtils::getString( configurationMap, "method" ),
                                                  PackageChooserMethod::Legacy );
-    m_pkgc = CalamaresUtils::getString( configurationMap, "pkgc" );
 
     if ( m_method == PackageChooserMethod::Legacy )
     {
-        const QString configId = CalamaresUtils::getString( configurationMap, "id" );
-        const QString base = QStringLiteral( "packagechooser_" );
-        if ( configId.isEmpty() )
-        {
-            if ( m_defaultId.id().isEmpty() )
-            {
-                // We got nothing to work with
-                m_id = base;
-            }
-            else
-            {
-                m_id = base + m_defaultId.id();
-            }
-            cDebug() << "Using default ID" << m_id << "from" << m_defaultId.toString();
-        }
-        else
-        {
-            m_id = base + configId;
-            cDebug() << "Using configured ID" << m_id;
-        }
+        cDebug() << "Using module ID" << m_defaultId;
     }
 
     if ( configurationMap.contains( "items" ) )
     {
         fillModel( m_model, configurationMap.value( "items" ).toList() );
-    }
 
-    QString default_item_id = CalamaresUtils::getString( configurationMap, "default" );
-    if ( !default_item_id.isEmpty() )
-    {
-        for ( int item_n = 0; item_n < m_model->packageCount(); ++item_n )
+        QString default_item_id = CalamaresUtils::getString( configurationMap, "default" );
+        if ( !default_item_id.isEmpty() )
         {
-            QModelIndex item_idx = m_model->index( item_n, 0 );
-            QVariant item_id = m_model->data( item_idx, PackageListModel::IdRole );
-
-            if ( item_id.toString() == default_item_id )
+            for ( int item_n = 0; item_n < m_model->packageCount(); ++item_n )
             {
-                m_defaultModelIndex = item_idx;
-                break;
+                QModelIndex item_idx = m_model->index( item_n, 0 );
+                QVariant item_id = m_model->data( item_idx, PackageListModel::IdRole );
+
+                if ( item_id.toString() == default_item_id )
+                {
+                    m_defaultModelIndex = item_idx;
+                    break;
+                }
             }
         }
     }
+    else
+    {
+        setPackageChoice( CalamaresUtils::getString( configurationMap, "packageChoice" ) );
+        if ( m_method != PackageChooserMethod::Legacy )
+        {
+            cWarning() << "Single-selection QML module must use 'Legacy' method.";
+        }
+    }
 }

src/modules/packagechooser/Config.h

@@ -17,6 +17,7 @@
 #include "modulesystem/InstanceKey.h"
 
 #include <memory>
+#include <optional>
 
 enum class PackageChooserMode
 {
@@ -40,7 +41,16 @@ class Config : public Calamares::ModuleSystem::Config
 {
     Q_OBJECT
 
-    Q_PROPERTY( QString pkgc READ pkgc WRITE setPkgc NOTIFY pkgcChanged )
+    /** @brief This is the single-select package-choice
+     *
+     * For (QML) modules that support only a single selection and
+     * just want to do things in a straightforward pick-this-one
+     * way, the packageChoice property is a (the) way to go.
+     *
+     * Writing to this property means that any other form of package-
+     * choice or selection is ignored.
+     */
+    Q_PROPERTY( QString packageChoice READ packageChoice WRITE setPackageChoice NOTIFY packageChoiceChanged )
     Q_PROPERTY( QString prettyStatus READ prettyStatus NOTIFY prettyStatusChanged FINAL )
 
 public:
@@ -75,16 +85,21 @@ public:
      * (and only) the packages in @p selected as selected.
      */
     void updateGlobalStorage( const QStringList& selected ) const;
-    /// As updateGlobalStorage() with an empty selection list
-    void fillGSSecondaryConfiguration() const { updateGlobalStorage( QStringList() ); }
+    /** @brief Write selection to global storage
+     *
+     * Updates the GS keys for this packagechooser, marking **only**
+     * the package choice as selected. This assumes that the single-
+     * selection QML code is in use.
+     */
+    void updateGlobalStorage() const;
 
-    QString pkgc() const { return m_pkgc; }
-    void setPkgc( const QString& pkgc );
+    QString packageChoice() const { return m_packageChoice.value_or( QString() ); }
+    void setPackageChoice( const QString& packageChoice );
 
     QString prettyStatus() const;
 
 signals:
-    void pkgcChanged( QString pkgc );
+    void packageChoiceChanged( QString packageChoice );
     void prettyStatusChanged();
 
 private:
@@ -95,12 +110,14 @@ private:
     PackageChooserMode m_mode = PackageChooserMode::Optional;
     /// How this module stores to GS
     PackageChooserMethod m_method = PackageChooserMethod::Legacy;
-    /// Id (used to identify settings from this module in GS)
-    QString m_id;
     /// Value to use for id if none is set in the config file
     Calamares::ModuleSystem::InstanceKey m_defaultId;
-    /// QML selection
-    QString m_pkgc;
+    /** @brief QML selection (for single-selection approaches)
+     *
+     * If there is no value, then there has been no selection.
+     * Reading the property will return an empty QString.
+     */
+    std::optional< QString > m_packageChoice;
 };
 
 

src/modules/packagechooser/packagechooser.conf

@@ -15,11 +15,10 @@ mode: required
 #
 # - "legacy" or "custom" or "contextualprocess"
 #   When set to "legacy", writes a GlobalStorage value for the choice that
-#   has been made. The key is *packagechooser_<id>*. Normally, the module's
+#   has been made. The key is *packagechooser_<id>*. The module's
 #   instance name is used; see the *instances* section of `settings.conf`.
 #   If there is just one packagechooser module, and no special instance is set,
-#   resulting GS key is probably *packagechooser@packagechooser*.
-#   You can set *id* to change that, but it is not recommended.
+#   resulting GS key is probably *packagechooser_packagechooser*.
 #
 #   The GS value is a comma-separated list of the IDs of the selected
 #   packages, or an empty string if none is selected.
@@ -33,15 +32,12 @@ mode: required
 #   consumption by the *packages* module (which should appear later
 #   in the `exec` section. These package settings will then be handed
 #   off to whatever package manager is configured there.
-#   The *id* key is not used.
 #
 # There is no need to put this module in the `exec` section. There
 # are no jobs that this module provides. You should put **other**
 # modules, either *contextualprocess* or *packages* or some custom
 # module, in the `exec` section to do the actual work.
 method: legacy
-# The *id* key is used only in "legacy" mode
-# id: ""
 
 
 # Human-visible strings in this module. These are all optional.
@@ -51,13 +47,13 @@ method: legacy
 # Each key can have a [locale] added to it, which is used as
 # the translated string for that locale. For the strings
 # associated with the "no-selection" item, see *items*, below
-# with the explicit id "".
+# with the explicit item-*id* "".
 #
 labels:
     step: "Packages"
     step[nl]: "Pakketten"
 
-# (Optional) 'id' of pre-selected list-view item.
+# (Optional) item-*id* of pre-selected list-view item.
 # Pre-selects one of the items below.
 # default: kde
 

src/modules/packagechooserq/PackageChooserQmlViewStep.cpp

@@ -74,7 +74,7 @@ PackageChooserQmlViewStep::jobs() const
 void
 PackageChooserQmlViewStep::onLeave()
 {
-    m_config->fillGSSecondaryConfiguration();
+    m_config->updateGlobalStorage();
 }
 
 void

src/modules/packagechooserq/packagechooserq.conf

@@ -1,25 +1,26 @@
 # SPDX-FileCopyrightText: no
 # SPDX-License-Identifier: CC0-1.0
 #
-# Configuration for the low-density software chooser
----
-# Software selection mode, to set whether the software packages
-# can be chosen singly, or multiply.
+# Configuration for the low-density software chooser, QML implementation
+#
+# The example QML implementation uses single-selection, rather than
+# a model for the available packages. That makes it simpler: the
+# QML itself codes the available options, descriptions and images
+# -- after all, this is **low density** selection, so a custom UI
+# can make sense for the few choices that need to be made.
+#
 #
-# Possible modes are "optional", "required" (for zero-or-one or exactly-one)
-# or "optionalmultiple", "requiredmultiple" (for zero-or-more
-# or one-or-more).
-mode: required
 
+---
 # Software installation method:
 #
 # - "legacy" or "custom" or "contextualprocess"
 #   When set to "legacy", writes a GlobalStorage value for the choice that
-#   has been made. The key is *packagechooser_<id>*. Normally, the module's
+#   has been made. The key is *packagechooser_<id>*. The module's
 #   instance name is used; see the *instances* section of `settings.conf`.
-#   If there is just one packagechooser module, and no special instance is set,
-#   resulting GS key is probably *packagechooser@packagechooser*.
-#   You can set *id* to change that, but it is not recommended.
+#   If there is just one packagechooserq module, and no special instance is set,
+#   resulting GS key is probably *packagechooser_packagechooserq*.
+#   (Do note that the prefix of the GS key remains "packagechooser_")
 #
 #   The GS value is a comma-separated list of the IDs of the selected
 #   packages, or an empty string if none is selected.
@@ -33,16 +34,20 @@ mode: required
 #   consumption by the *packages* module (which should appear later
 #   in the `exec` section. These package settings will then be handed
 #   off to whatever package manager is configured there.
-#   The *id* key is not used.
 #
 # There is no need to put this module in the `exec` section. There
 # are no jobs that this module provides. You should put **other**
 # modules, either *contextualprocess* or *packages* or some custom
 # module, in the `exec` section to do the actual work.
+#
 method: legacy
-# The *id* key is used only in "legacy" mode
-# id: ""
 
-# The *pkgc* is used for setting the default selection in the QML view
-pkgc: libreoffice
+# The *packageChoice* value is used for setting the default selection
+# in the QML view; this should match one of the keys used in the QML
+# module for package names.
+#
+# (e.g. the sample QML uses "no_office_suite", "minimal_install" and
+# "libreoffice" as possible choices).
+#
+packageChoice: libreoffice
 

src/modules/packagechooserq/packagechooserq.qml

@@ -80,12 +80,8 @@ Item {
                     }
 
                     onCheckedChanged: {
-                        if ( ! checked ) {
-                            print("L not used")
-                        }
-                        else {
-                            config.pkgc = "libreoffice"
-                            print( config.pkgc )
+                        if ( checked ) {
+                            config.packageChoice = "libreoffice"
                         }
                     }
                 }
@@ -146,13 +142,8 @@ Item {
                     }
 
                     onCheckedChanged: {
-                        if ( ! checked ) {
-                            print("not used")
-                            //console.log("removed")
-                        }
-                        else {
-                            print("No Office Suite")
-                            config.pkgc = "no_office_suite"
+                        if ( checked ) {
+                            config.packageChoice = "no_office_suite"
                         }
                     }
                 }
@@ -215,12 +206,8 @@ Item {
                     }
 
                     onCheckedChanged: {
-                        if ( ! checked ) {
-                            print("M not used")
-                        }
-                        else {
-                            print("minimal")
-                            config.pkgc = "minimal_install"
+                        if ( checked ) {
+                            config.packageChoice = "minimal_install"
                         }
                     }
                 }

src/modules/summaryq/CMakeLists.txt

@@ -1,3 +1,8 @@
+# === This file is part of Calamares - <https://calamares.io> ===
+#
+#   SPDX-FileCopyrightText: 2021 Anke Boersma <demm@kaosx.us>
+#   SPDX-License-Identifier: BSD-2-Clause
+#
 if( NOT WITH_QML )
     calamares_skip_module( "summaryq (QML is not supported in this build)" )
     return()

src/modules/welcome/tests/1a-checkinternet.conf

@@ -1,3 +1,5 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
 ---
 requirements:
     internetCheckUrl:   http://example.com

src/modules/welcome/tests/1b-checkinternet.conf

@@ -1,3 +1,6 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
 # Nothing at all
 ---
 bogus:   1

src/modules/welcome/tests/1c-checkinternet.conf

@@ -1,3 +1,6 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
 # Set to blank
 ---
 requirements:

src/modules/welcome/tests/1d-checkinternet.conf

@@ -1,3 +1,6 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
 # Set to something broken
 ---
 requirements:

src/modules/welcome/tests/1e-checkinternet.conf

@@ -1,3 +1,6 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
 # Empty list
 ---
 requirements:

src/modules/welcome/tests/1f-checkinternet.conf

@@ -1,6 +1,10 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
+# Multiple, all valid
 ---
 requirements:
-    internetCheckUrl:   
+    internetCheckUrl:
         - http://example.com
         - http://bogus.example.com
         - http://nonexistent.example.com

src/modules/welcome/tests/1g-checkinternet.conf

@@ -1,3 +1,7 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
+# Multiple, all valid, in short-list form
 ---
 requirements:
     internetCheckUrl: [ http://example.com, http://bogus.example.com, http://nonexistent.example.com ]

src/modules/welcome/tests/1h-checkinternet.conf

@@ -1,7 +1,10 @@
+#   SPDX-FileCopyrightText: no
+#   SPDX-License-Identifier: CC0-1.0
+#
 # "0" is a valid URL (?) but "" is not
 ---
 requirements:
-    internetCheckUrl:   
+    internetCheckUrl:
         - http://example.com
         - 0
         - ""