From f4fe0881b9d068e8c2b6924482e0b1350cfc32f7 Mon Sep 17 00:00:00 2001
From: Adriaan de Groot <groot@kde.org>
Date: Fri, 23 Apr 2021 22:29:26 +0200
Subject: [PATCH] [packagechooser] Be more clear on the resulting GS keys

- in legacy mode, *id* can have an effect and leads to
  "packagechooser_<id>"; if unset, uses the the module
  instance id instead, still as "packagechooser_<instanceid>".
- in packages mode, *id* is not used and only the whole
  module Id (generally, "packagechooser@<instanceid>")
  is used, but in packages mode there's no need for other
  packages to mess with GS settings for this packagechooser.
---
 src/modules/packagechooser/Config.cpp         | 25 +++++++++++--------
 .../packagechooser/packagechooser.conf        |  6 +++--
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp
index 531d285ed..de5cb0813 100644
--- a/src/modules/packagechooser/Config.cpp
+++ b/src/modules/packagechooser/Config.cpp
@@ -90,20 +90,16 @@ Config::introductionPackage() const
 void
 Config::updateGlobalStorage( const QStringList& selected ) const
 {
-    const QString& key = m_id;
-    cDebug() << "Writing to GS" << key;
-
     if ( m_method == PackageChooserMethod::Legacy )
     {
         QString value = selected.join( ',' );
-        Calamares::JobQueue::instance()->globalStorage()->insert( key, value );
-
-        cDebug() << Logger::SubEntry << "PackageChooser" << key << "selected" << value;
+        Calamares::JobQueue::instance()->globalStorage()->insert( m_id, value );
+        cDebug() << m_id<< "selected" << value;
     }
     else if ( m_method == PackageChooserMethod::Packages )
     {
         QStringList packageNames = m_model->getInstallPackagesForNames( selected );
-        cDebug() << Logger::SubEntry << "Got packages" << packageNames;
+        cDebug() << m_defaultId << "packages to install" << packageNames;
         CalamaresUtils::Packages::setGSPackageAdditions(
             Calamares::JobQueue::instance()->globalStorage(), m_defaultId, packageNames );
     }
@@ -181,20 +177,27 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     m_method = PackageChooserMethodNames().find( CalamaresUtils::getString( configurationMap, "method" ),
                                                  PackageChooserMethod::Legacy );
 
+    if ( m_method == PackageChooserMethod::Legacy )
     {
         const QString configId = CalamaresUtils::getString( configurationMap, "id" );
+        const QString base = QStringLiteral( "packagechooser_" );
         if ( configId.isEmpty() )
         {
-            m_id = m_defaultId.toString();
-            if ( m_id.isEmpty() )
+            if ( m_defaultId.id().isEmpty() )
             {
-                m_id = QString( "packagechooser" );
+                // 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 = QStringLiteral( "packagechooser_" ) + configId;
+            m_id = base + configId;
+            cDebug() << "Using configured ID" << m_id;
         }
     }
 
diff --git a/src/modules/packagechooser/packagechooser.conf b/src/modules/packagechooser/packagechooser.conf
index 51032929e..2bde1369c 100644
--- a/src/modules/packagechooser/packagechooser.conf
+++ b/src/modules/packagechooser/packagechooser.conf
@@ -18,8 +18,8 @@ mode: required
 #   has been made. The key is *packagechooser_<id>*. Normally, 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*.
+#   You can set *id* to change that, but it is not recommended.
 #
 #   The GS value is a comma-separated list of the IDs of the selected
 #   packages, or an empty string if none is selected.
@@ -33,12 +33,14 @@ 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: ""