From 59ea88f1ade3045fcdc846e64ccdb6e7c3f9cf87 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 13 Apr 2021 12:29:06 +0200 Subject: [PATCH 01/13] [packagechoose] Remove the *package* member The single-values *package* member in a PackageItem was not used, so remove it -- to show that it really isn't used. This is prep- work for putting the package name *back*, as multi-valued, and using the *packages* module. --- src/modules/packagechooser/PackageModel.cpp | 5 ----- src/modules/packagechooser/PackageModel.h | 5 +---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index 1072b8b3b..bb48ab888 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -35,23 +35,19 @@ roleNames() PackageItem::PackageItem() {} PackageItem::PackageItem( const QString& a_id, - const QString& a_package, const QString& a_name, const QString& a_description ) : id( a_id ) - , package( a_package ) , name( a_name ) , description( a_description ) { } PackageItem::PackageItem( const QString& a_id, - const QString& a_package, const QString& a_name, const QString& a_description, const QString& screenshotPath ) : id( a_id ) - , package( a_package ) , name( a_name ) , description( a_description ) , screenshot( screenshotPath ) @@ -60,7 +56,6 @@ PackageItem::PackageItem( const QString& a_id, PackageItem::PackageItem::PackageItem( const QVariantMap& item_map ) : id( CalamaresUtils::getString( item_map, "id" ) ) - , package( CalamaresUtils::getString( item_map, "package" ) ) , name( CalamaresUtils::Locale::TranslatedString( item_map, "name" ) ) , description( CalamaresUtils::Locale::TranslatedString( item_map, "description" ) ) , screenshot( CalamaresUtils::getString( item_map, "screenshot" ) ) diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index 375cf28c4..fc1d787f1 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -31,8 +31,6 @@ const NamedEnumTable< PackageChooserMode >& roleNames(); struct PackageItem { QString id; - // FIXME: unused - QString package; CalamaresUtils::Locale::TranslatedString name; CalamaresUtils::Locale::TranslatedString description; QPixmap screenshot; @@ -44,7 +42,7 @@ struct PackageItem * This constructor sets all the text members, * but leaves the screenshot blank. Set that separately. */ - PackageItem( const QString& id, const QString& package, const QString& name, const QString& description ); + PackageItem( const QString& id, const QString& name, const QString& description ); /** @brief Creates a PackageItem from given strings. * @@ -53,7 +51,6 @@ struct PackageItem * a filesystem path, whatever QPixmap understands. */ PackageItem( const QString& id, - const QString& package, const QString& name, const QString& description, const QString& screenshotPath ); From dd52e108394cbaaa3f8934ccc8168434464bf6c7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 13 Apr 2021 13:39:47 +0200 Subject: [PATCH 02/13] [packagechooser] Introduce a Config object Rip out most of the ViewStep that deals with configuration, move it to a Config object (not one that supports QML yet, though), and massage the model a little. --- src/modules/packagechooser/CMakeLists.txt | 1 + src/modules/packagechooser/Config.cpp | 170 ++++++++++++++++++ src/modules/packagechooser/Config.h | 61 +++++++ .../packagechooser/PackageChooserViewStep.cpp | 163 ++--------------- .../packagechooser/PackageChooserViewStep.h | 13 +- src/modules/packagechooser/PackageModel.cpp | 2 +- src/modules/packagechooser/PackageModel.h | 2 +- 7 files changed, 251 insertions(+), 161 deletions(-) create mode 100644 src/modules/packagechooser/Config.cpp create mode 100644 src/modules/packagechooser/Config.h diff --git a/src/modules/packagechooser/CMakeLists.txt b/src/modules/packagechooser/CMakeLists.txt index ad9cc8527..e6c2c5b1d 100644 --- a/src/modules/packagechooser/CMakeLists.txt +++ b/src/modules/packagechooser/CMakeLists.txt @@ -45,6 +45,7 @@ calamares_add_plugin( packagechooser TYPE viewmodule EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES + Config.cpp PackageChooserPage.cpp PackageChooserViewStep.cpp PackageModel.cpp diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp new file mode 100644 index 000000000..aa383d3c8 --- /dev/null +++ b/src/modules/packagechooser/Config.cpp @@ -0,0 +1,170 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2021 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +#include "Config.h" + +#ifdef HAVE_XML +#include "ItemAppData.h" +#endif + +#include "GlobalStorage.h" +#include "JobQueue.h" +#include "utils/Logger.h" +#include "utils/Variant.h" + + +Config::Config( const QString& defaultId, QObject* parent ) + : Calamares::ModuleSystem::Config( parent ) + , m_model( new PackageListModel( this ) ) + , m_mode( PackageChooserMode::Required ) + , m_defaultId( defaultId ) +{ +} + +Config::~Config() {} + +const PackageItem& +Config::introductionPackage() const +{ + for ( int i = 0; i < m_model->packageCount(); ++i ) + { + const auto& package = m_model->packageData( i ); + if ( package.isNonePackage() ) + { + return package; + } + } + + static PackageItem* defaultIntroduction = nullptr; + if ( !defaultIntroduction ) + { + defaultIntroduction = new PackageItem( + QString(), + QT_TR_NOOP( "Package Selection" ), + QT_TR_NOOP( "Please pick a product from the list. The selected product will be installed." ) ); + defaultIntroduction->screenshot = QPixmap( QStringLiteral( ":/images/no-selection.png" ) ); + // TODO: enable better translation + // defaultIntroduction->name.setContext( metaObject()->className() ); + // defaultIntroduction->description.setContext( metaObject()->className() ); + } + return *defaultIntroduction; +} + +void +Config::updateGlobalStorage( const QStringList& selected ) const +{ + QString key = QStringLiteral( "packagechooser_%1" ).arg( m_id ); + QString value = selected.join( ',' ); + Calamares::JobQueue::instance()->globalStorage()->insert( key, value ); + + cDebug() << "PackageChooser" << key << "selected" << value; +} + + +static void +fillModel( PackageListModel* model, const QVariantList& items ) +{ + if ( items.isEmpty() ) + { + cWarning() << "No *items* for PackageChooser module."; + return; + } + +#ifdef HAVE_APPSTREAM + std::unique_ptr< AppStream::Pool > pool; + bool poolOk = false; +#endif + + cDebug() << "Loading PackageChooser model items from config"; + int item_index = 0; + for ( const auto& item_it : items ) + { + ++item_index; + QVariantMap item_map = item_it.toMap(); + if ( item_map.isEmpty() ) + { + cWarning() << "PackageChooser entry" << item_index << "is not valid."; + continue; + } + + if ( item_map.contains( "appdata" ) ) + { +#ifdef HAVE_XML + model->addPackage( fromAppData( item_map ) ); +#else + cWarning() << "Loading AppData XML is not supported."; +#endif + } + else if ( item_map.contains( "appstream" ) ) + { +#ifdef HAVE_APPSTREAM + if ( !pool ) + { + pool = std::make_unique< AppStream::Pool >(); + pool->setLocale( QStringLiteral( "ALL" ) ); + poolOk = pool->load(); + } + if ( pool && poolOk ) + { + model->addPackage( fromAppStream( *pool, item_map ) ); + } +#else + cWarning() << "Loading AppStream data is not supported."; +#endif + } + else + { + model->addPackage( PackageItem( item_map ) ); + } + } +} + +void +Config::setConfigurationMap( const QVariantMap& configurationMap ) +{ + QString mode = CalamaresUtils::getString( configurationMap, "mode" ); + bool mode_ok = false; + if ( !mode.isEmpty() ) + { + m_mode = packageChooserModeNames().find( mode, mode_ok ); + } + if ( !mode_ok ) + { + m_mode = PackageChooserMode::Required; + } + + m_id = CalamaresUtils::getString( configurationMap, "id" ); + if ( m_id.isEmpty() ) + { + m_id = m_defaultId; + } + + m_defaultModelIndex = QModelIndex(); + if ( configurationMap.contains( "items" ) ) + { + fillModel( m_model, configurationMap.value( "items" ).toList() ); + } + + QString default_item_id = CalamaresUtils::getString( configurationMap, "default" ); + // find default item + if ( !default_item_id.isEmpty() ) + { + for ( int item_n = 0; item_n < m_model->packageCount(); ++item_n ) + { + 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; + } + } + } +} diff --git a/src/modules/packagechooser/Config.h b/src/modules/packagechooser/Config.h new file mode 100644 index 000000000..6a65c8788 --- /dev/null +++ b/src/modules/packagechooser/Config.h @@ -0,0 +1,61 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2021 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +#ifndef PACKAGECHOOSER_CONFIG_H +#define PACKAGECHOOSER_CONFIG_H + +#include "PackageModel.h" + +#include "modulesystem/Config.h" + +#include + +class Config : public Calamares::ModuleSystem::Config +{ + Q_OBJECT + +public: + Config( const QString& defaultId, QObject* parent = nullptr ); + ~Config() override; + + void setConfigurationMap( const QVariantMap& ) override; + + PackageChooserMode mode() const { return m_mode; } + PackageListModel* model() const { return m_model; } + QModelIndex defaultSelectionIndex() const { return m_defaultModelIndex; } + + /** @brief Returns an "introductory package" which describes packagechooser + * + * If the model contains a "none" package, returns that one on + * the assumption that it is one to describe the whole; otherwise + * returns a totally generic description. + */ + const PackageItem& introductionPackage() const; + + /** @brief Write selection to global storage + * + * Updates the GS keys for this packagechooser, marking all + * (and only) the packages in @p selected as selected. + */ + void updateGlobalStorage( const QStringList& selected ) const; + /// As updateGlobalStorage() with an empty selection list + void updateGlobalStorage() const { updateGlobalStorage( QStringList() ); } + +private: + PackageListModel* m_model = nullptr; + QModelIndex m_defaultModelIndex; + + // Configuration + PackageChooserMode m_mode = PackageChooserMode::Optional; + QString m_id; + QString m_defaultId; +}; + + +#endif diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index d576f2753..05d0d3cfd 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -9,20 +9,22 @@ #include "PackageChooserViewStep.h" +#include "Config.h" +#include "PackageChooserPage.h" +#include "PackageModel.h" + #ifdef HAVE_XML #include "ItemAppData.h" #endif + #ifdef HAVE_APPSTREAM #include "ItemAppStream.h" #include #include #endif -#include "PackageChooserPage.h" -#include "PackageModel.h" #include "GlobalStorage.h" #include "JobQueue.h" - #include "locale/TranslatableConfiguration.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" @@ -35,9 +37,8 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( PackageChooserViewStepFactory, registerPlug PackageChooserViewStep::PackageChooserViewStep( QObject* parent ) : Calamares::ViewStep( parent ) + , m_config( new Config( moduleInstanceKey().id(), this ) ) , m_widget( nullptr ) - , m_model( nullptr ) - , m_mode( PackageChooserMode::Required ) , m_stepName( nullptr ) { emit nextStatusChanged( false ); @@ -50,7 +51,6 @@ PackageChooserViewStep::~PackageChooserViewStep() { m_widget->deleteLater(); } - delete m_model; delete m_stepName; } @@ -67,19 +67,10 @@ PackageChooserViewStep::widget() { if ( !m_widget ) { - m_widget = new PackageChooserPage( m_mode, nullptr ); + m_widget = new PackageChooserPage( m_config->mode(), nullptr ); connect( m_widget, &PackageChooserPage::selectionChanged, [=]() { emit nextStatusChanged( this->isNextEnabled() ); } ); - - if ( m_model ) - { - hookupModel(); - } - else - { - cWarning() << "PackageChooser Widget created before model."; - } } return m_widget; } @@ -88,18 +79,13 @@ PackageChooserViewStep::widget() bool PackageChooserViewStep::isNextEnabled() const { - if ( !m_model ) - { - return false; - } - if ( !m_widget ) { // No way to have changed anything return true; } - switch ( m_mode ) + switch ( m_config->mode() ) { case PackageChooserMode::Optional: case PackageChooserMode::OptionalMultiple: @@ -139,22 +125,14 @@ PackageChooserViewStep::onActivate() { if ( !m_widget->hasSelection() ) { - m_widget->setSelection( m_defaultIdx ); + m_widget->setSelection( m_config->defaultSelectionIndex() ); } } void PackageChooserViewStep::onLeave() { - QString key = QStringLiteral( "packagechooser_%1" ).arg( m_id ); - QString value; - if ( m_widget->hasSelection() ) - { - value = m_widget->selectedPackageIds().join( ',' ); - } - Calamares::JobQueue::instance()->globalStorage()->insert( key, value ); - - cDebug() << "PackageChooser" << key << "selected" << value; + m_config->updateGlobalStorage( m_widget->selectedPackageIds() ); } Calamares::JobList @@ -167,23 +145,7 @@ PackageChooserViewStep::jobs() const void PackageChooserViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { - QString mode = CalamaresUtils::getString( configurationMap, "mode" ); - bool mode_ok = false; - if ( !mode.isEmpty() ) - { - m_mode = roleNames().find( mode, mode_ok ); - } - if ( !mode_ok ) - { - m_mode = PackageChooserMode::Required; - } - - m_id = CalamaresUtils::getString( configurationMap, "id" ); - if ( m_id.isEmpty() ) - { - // Not set, so use the instance id - m_id = moduleInstanceKey().id(); - } + m_config->setConfigurationMap( configurationMap ); bool labels_ok = false; auto labels = CalamaresUtils::getSubMap( configurationMap, "labels", labels_ok ); @@ -195,117 +157,22 @@ PackageChooserViewStep::setConfigurationMap( const QVariantMap& configurationMap } } - QString default_item_id = CalamaresUtils::getString( configurationMap, "default" ); - m_defaultIdx = QModelIndex(); - - bool first_time = !m_model; - if ( configurationMap.contains( "items" ) ) - { - fillModel( configurationMap.value( "items" ).toList() ); - } - - if ( first_time && m_widget && m_model ) + if ( m_widget ) { hookupModel(); } - - // find default item - if ( first_time && m_model && !default_item_id.isEmpty() ) - { - for ( int item_n = 0; item_n < m_model->packageCount(); ++item_n ) - { - 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_defaultIdx = item_idx; - break; - } - } - } } -void -PackageChooserViewStep::fillModel( const QVariantList& items ) -{ - if ( !m_model ) - { - m_model = new PackageListModel( nullptr ); - } - - if ( items.isEmpty() ) - { - cWarning() << "No *items* for PackageChooser module."; - return; - } - -#ifdef HAVE_APPSTREAM - std::unique_ptr< AppStream::Pool > pool; - bool poolOk = false; -#endif - - cDebug() << "Loading PackageChooser model items from config"; - int item_index = 0; - for ( const auto& item_it : items ) - { - ++item_index; - QVariantMap item_map = item_it.toMap(); - if ( item_map.isEmpty() ) - { - cWarning() << "PackageChooser entry" << item_index << "is not valid."; - continue; - } - - if ( item_map.contains( "appdata" ) ) - { -#ifdef HAVE_XML - m_model->addPackage( fromAppData( item_map ) ); -#else - cWarning() << "Loading AppData XML is not supported."; -#endif - } - else if ( item_map.contains( "appstream" ) ) - { -#ifdef HAVE_APPSTREAM - if ( !pool ) - { - pool = std::make_unique< AppStream::Pool >(); - pool->setLocale( QStringLiteral( "ALL" ) ); - poolOk = pool->load(); - } - if ( pool && poolOk ) - { - m_model->addPackage( fromAppStream( *pool, item_map ) ); - } -#else - cWarning() << "Loading AppStream data is not supported."; -#endif - } - else - { - m_model->addPackage( PackageItem( item_map ) ); - } - } -} void PackageChooserViewStep::hookupModel() { - if ( !m_model || !m_widget ) + if ( !m_config->model() || !m_widget ) { cError() << "Can't hook up model until widget and model both exist."; return; } - m_widget->setModel( m_model ); - for ( int i = 0; i < m_model->packageCount(); ++i ) - { - const auto& package = m_model->packageData( i ); - if ( package.id.isEmpty() ) - { - m_widget->setIntroduction( package ); - break; - } - } + m_widget->setModel( m_config->model() ); + m_widget->setIntroduction( m_config->introductionPackage() ); } diff --git a/src/modules/packagechooser/PackageChooserViewStep.h b/src/modules/packagechooser/PackageChooserViewStep.h index 9dfd2bdee..7561f2bd7 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.h +++ b/src/modules/packagechooser/PackageChooserViewStep.h @@ -15,12 +15,9 @@ #include "utils/PluginFactory.h" #include "viewpages/ViewStep.h" -#include "PackageModel.h" - -#include -#include #include +class Config; class PackageChooserPage; class PLUGINDLLEXPORT PackageChooserViewStep : public Calamares::ViewStep @@ -49,17 +46,11 @@ public: void setConfigurationMap( const QVariantMap& configurationMap ) override; private: - void fillModel( const QVariantList& items ); void hookupModel(); + Config* m_config; PackageChooserPage* m_widget; - PackageListModel* m_model; - - // Configuration - PackageChooserMode m_mode; - QString m_id; CalamaresUtils::Locale::TranslatedString* m_stepName; // As it appears in the sidebar - QModelIndex m_defaultIdx; }; CALAMARES_PLUGIN_FACTORY_DECLARATION( PackageChooserViewStepFactory ) diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index bb48ab888..f1c8b3056 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -13,7 +13,7 @@ #include "utils/Variant.h" const NamedEnumTable< PackageChooserMode >& -roleNames() +packageChooserModeNames() { static const NamedEnumTable< PackageChooserMode > names { { "optional", PackageChooserMode::Optional }, diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index fc1d787f1..0ced3ffb3 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -26,7 +26,7 @@ enum class PackageChooserMode RequiredMultiple // one or more }; -const NamedEnumTable< PackageChooserMode >& roleNames(); +const NamedEnumTable< PackageChooserMode >& packageChooserModeNames(); struct PackageItem { From a7f983db5f6575579e6aef46c33b7727ccc087bc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 16 Apr 2021 13:46:19 +0200 Subject: [PATCH 03/13] [packagechooser] Add *packageNames* to package items This is prep-work for connecting to the *packages* module by simply installing packages straight from packagechooser, rather than using a workaround. --- src/modules/packagechooser/PackageModel.cpp | 5 ++--- src/modules/packagechooser/PackageModel.h | 14 ++++++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index f1c8b3056..55300f940 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -34,9 +34,7 @@ packageChooserModeNames() PackageItem::PackageItem() {} -PackageItem::PackageItem( const QString& a_id, - const QString& a_name, - const QString& a_description ) +PackageItem::PackageItem( const QString& a_id, const QString& a_name, const QString& a_description ) : id( a_id ) , name( a_name ) , description( a_description ) @@ -59,6 +57,7 @@ PackageItem::PackageItem::PackageItem( const QVariantMap& item_map ) , name( CalamaresUtils::Locale::TranslatedString( item_map, "name" ) ) , description( CalamaresUtils::Locale::TranslatedString( item_map, "description" ) ) , screenshot( CalamaresUtils::getString( item_map, "screenshot" ) ) + , packageNames( CalamaresUtils::getStringList( item_map, "packages" ) ) { if ( name.isEmpty() && id.isEmpty() ) { diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index 0ced3ffb3..0da0e4a53 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -34,6 +34,7 @@ struct PackageItem CalamaresUtils::Locale::TranslatedString name; CalamaresUtils::Locale::TranslatedString description; QPixmap screenshot; + QStringList packageNames; /// @brief Create blank PackageItem PackageItem(); @@ -50,16 +51,21 @@ struct PackageItem * @p screenshotPath, which may be a QRC path (:/path/in/qrc) or * a filesystem path, whatever QPixmap understands. */ - PackageItem( const QString& id, - const QString& name, - const QString& description, - const QString& screenshotPath ); + PackageItem( const QString& id, const QString& name, const QString& description, const QString& screenshotPath ); /** @brief Creates a PackageItem from a QVariantMap * * This is intended for use when loading PackageItems from a * configuration map. It will look up the various keys in the map * and handle translation strings as well. + * + * The following keys are used: + * - *id*: the identifier for this item; if it is the empty string + * then this is the special "no-package". + * - *name* (and *name[lang]*): for the name and its translations + * - *description* (and *description[lang]*) + * - *screenshot*: a path to a screenshot for this package + * - *packages*: a list of package names */ PackageItem( const QVariantMap& map ); From 35f4a81768685e109aa2e822f2a93c5312b50f67 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 16 Apr 2021 14:29:39 +0200 Subject: [PATCH 04/13] [libcalamares] Extend packages service API - convenience method to install a (string) list of packages (doesn't do the installation, but adds to GS the list, so that the packages module can handle it). --- src/libcalamares/packages/Globals.cpp | 34 +++++++++++++++++++++------ src/libcalamares/packages/Globals.h | 8 +++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/packages/Globals.cpp b/src/libcalamares/packages/Globals.cpp index c5e882436..aedbc2119 100644 --- a/src/libcalamares/packages/Globals.cpp +++ b/src/libcalamares/packages/Globals.cpp @@ -12,11 +12,11 @@ #include "GlobalStorage.h" #include "utils/Logger.h" -bool -CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, - const Calamares::ModuleSystem::InstanceKey& module, - const QVariantList& installPackages, - const QVariantList& tryInstallPackages ) +static bool +additions( Calamares::GlobalStorage* gs, + const QString& key, + const QVariantList& installPackages, + const QVariantList& tryInstallPackages ) { static const char PACKAGEOP[] = "packageOperations"; @@ -25,8 +25,6 @@ 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(); - // 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. @@ -66,3 +64,25 @@ CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, } return false; } + +bool +CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, + const Calamares::ModuleSystem::InstanceKey& module, + const QVariantList& installPackages, + const QVariantList& tryInstallPackages ) +{ + return additions( gs, module.toString(), installPackages, tryInstallPackages ); +} + +bool +CalamaresUtils::Packages::setGSPackageAdditions( Calamares::GlobalStorage* gs, + const Calamares::ModuleSystem::InstanceKey& module, + const QStringList& installPackages ) +{ + QVariantList l; + for ( const auto& s : installPackages ) + { + l << s; + } + return additions( gs, module.toString(), l, QVariantList() ); +} diff --git a/src/libcalamares/packages/Globals.h b/src/libcalamares/packages/Globals.h index a47cf5ae1..a83152ff2 100644 --- a/src/libcalamares/packages/Globals.h +++ b/src/libcalamares/packages/Globals.h @@ -28,6 +28,14 @@ bool setGSPackageAdditions( Calamares::GlobalStorage* gs, const Calamares::ModuleSystem::InstanceKey& module, const QVariantList& installPackages, const QVariantList& tryInstallPackages ); +/** @brief Sets the install-packages GS keys for the given module + * + * This replaces previously-set install-packages lists. Use this with + * plain lists of package names. It does not support try-install. + */ +bool setGSPackageAdditions( Calamares::GlobalStorage* gs, + const Calamares::ModuleSystem::InstanceKey& module, + const QStringList& installPackages ); // void setGSPackageRemovals( const Calamares::ModuleSystem::InstanceKey& key, const QVariantList& removePackages ); } // namespace Packages } // namespace CalamaresUtils From ed14c49a033d649964fb4a7579809fe64a176e2b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 16 Apr 2021 14:38:39 +0200 Subject: [PATCH 05/13] [libcalamares] Extend (configuration) translated string with context Make it possible to pass in a context for strings not-from-config maps, to allow programmatically set, but translatable, strings. --- .../locale/TranslatableConfiguration.cpp | 10 ++++++++-- src/libcalamares/locale/TranslatableConfiguration.h | 12 ++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/locale/TranslatableConfiguration.cpp b/src/libcalamares/locale/TranslatableConfiguration.cpp index 1f0811c9d..c10307aee 100644 --- a/src/libcalamares/locale/TranslatableConfiguration.cpp +++ b/src/libcalamares/locale/TranslatableConfiguration.cpp @@ -23,9 +23,15 @@ namespace CalamaresUtils { namespace Locale { -TranslatedString::TranslatedString( const QString& string ) +TranslatedString::TranslatedString( const QString& key, const char* context ) + : m_context( context ) +{ + m_strings[ QString() ] = key; +} + +TranslatedString::TranslatedString( const QString& string ) + : TranslatedString( string, nullptr ) { - m_strings[ QString() ] = string; } TranslatedString::TranslatedString( const QVariantMap& map, const QString& key, const char* context ) diff --git a/src/libcalamares/locale/TranslatableConfiguration.h b/src/libcalamares/locale/TranslatableConfiguration.h index c45c8f523..04897c0a4 100644 --- a/src/libcalamares/locale/TranslatableConfiguration.h +++ b/src/libcalamares/locale/TranslatableConfiguration.h @@ -50,11 +50,23 @@ public: * metaObject()->className() as context (from a QObject based class) * to give the TranslatedString the same context as other calls * to tr() within that class. + * + * The @p context, if any, should point to static data; it is + * **not** owned by the TranslatedString. */ TranslatedString( const QVariantMap& map, const QString& key, const char* context = nullptr ); /** @brief Not-actually-translated string. */ TranslatedString( const QString& string ); + /** @brief Proxy for calling QObject::tr() + * + * This is like the two constructors above, with an empty map an a + * non-null context. It will end up calling tr() with that context. + * + * The @p context, if any, should point to static data; it is + * **not** owned by the TranslatedString. + */ + TranslatedString( const QString& key, const char* context ); /// @brief Empty string TranslatedString() : TranslatedString( QString() ) From 5e77d65424ff7038a2dfb6b8e13bb416eb252636 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 16 Apr 2021 14:53:13 +0200 Subject: [PATCH 06/13] [packagechooser] Add install-method to pick *packages* module --- src/modules/packagechooser/Config.cpp | 69 +++++++++++++++---- src/modules/packagechooser/Config.h | 29 +++++++- .../packagechooser/PackageChooserPage.h | 1 + .../packagechooser/PackageChooserViewStep.cpp | 2 +- src/modules/packagechooser/PackageModel.cpp | 20 ------ src/modules/packagechooser/PackageModel.h | 9 --- 6 files changed, 85 insertions(+), 45 deletions(-) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index aa383d3c8..78c417908 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -15,11 +15,43 @@ #include "GlobalStorage.h" #include "JobQueue.h" +#include "packages/Globals.h" #include "utils/Logger.h" #include "utils/Variant.h" +const NamedEnumTable< PackageChooserMode >& +packageChooserModeNames() +{ + static const NamedEnumTable< PackageChooserMode > names { + { "optional", PackageChooserMode::Optional }, + { "required", PackageChooserMode::Required }, + { "optionalmultiple", PackageChooserMode::OptionalMultiple }, + { "requiredmultiple", PackageChooserMode::RequiredMultiple }, + // and a bunch of aliases + { "zero-or-one", PackageChooserMode::Optional }, + { "radio", PackageChooserMode::Required }, + { "one", PackageChooserMode::Required }, + { "set", PackageChooserMode::OptionalMultiple }, + { "zero-or-more", PackageChooserMode::OptionalMultiple }, + { "multiple", PackageChooserMode::RequiredMultiple }, + { "one-or-more", PackageChooserMode::RequiredMultiple } + }; + return names; +} -Config::Config( const QString& defaultId, QObject* parent ) +const NamedEnumTable< PackageChooserMethod >& +PackageChooserMethodNames() +{ + static const NamedEnumTable< PackageChooserMethod > names { + { "legacy", PackageChooserMethod::Legacy }, + { "custom", PackageChooserMethod::Legacy }, + { "contextualprocess", PackageChooserMethod::Legacy }, + { "packages", PackageChooserMethod::Packages }, + }; + return names; +} + +Config::Config( const Calamares::ModuleSystem::InstanceKey& defaultId, QObject* parent ) : Calamares::ModuleSystem::Config( parent ) , m_model( new PackageListModel( this ) ) , m_mode( PackageChooserMode::Required ) @@ -44,14 +76,14 @@ Config::introductionPackage() const static PackageItem* defaultIntroduction = nullptr; if ( !defaultIntroduction ) { - defaultIntroduction = new PackageItem( - QString(), - QT_TR_NOOP( "Package Selection" ), - QT_TR_NOOP( "Please pick a product from the list. The selected product will be installed." ) ); + const auto name = QT_TR_NOOP( "Package Selection" ); + const auto description + = QT_TR_NOOP( "Please pick a product from the list. The selected product will be installed." ); + defaultIntroduction = new PackageItem( QString(), name, description ); defaultIntroduction->screenshot = QPixmap( QStringLiteral( ":/images/no-selection.png" ) ); - // TODO: enable better translation - // defaultIntroduction->name.setContext( metaObject()->className() ); - // defaultIntroduction->description.setContext( metaObject()->className() ); + defaultIntroduction->name = CalamaresUtils::Locale::TranslatedString( name, metaObject()->className() ); + defaultIntroduction->description + = CalamaresUtils::Locale::TranslatedString( description, metaObject()->className() ); } return *defaultIntroduction; } @@ -60,10 +92,23 @@ void Config::updateGlobalStorage( const QStringList& selected ) const { QString key = QStringLiteral( "packagechooser_%1" ).arg( m_id ); - QString value = selected.join( ',' ); - Calamares::JobQueue::instance()->globalStorage()->insert( key, value ); - cDebug() << "PackageChooser" << key << "selected" << value; + if ( m_method == PackageChooserMethod::Legacy ) + { + QString value = selected.join( ',' ); + Calamares::JobQueue::instance()->globalStorage()->insert( key, value ); + + cDebug() << "PackageChooser" << key << "selected" << value; + } + else if ( m_method == PackageChooserMethod::Packages ) + { + CalamaresUtils::Packages::setGSPackageAdditions( + Calamares::JobQueue::instance()->globalStorage(), m_defaultId, selected ); + } + else + { + cWarning() << "Unknown packagechooser method" << smash( m_method ); + } } @@ -142,7 +187,7 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) m_id = CalamaresUtils::getString( configurationMap, "id" ); if ( m_id.isEmpty() ) { - m_id = m_defaultId; + m_id = m_defaultId.id(); } m_defaultModelIndex = QModelIndex(); diff --git a/src/modules/packagechooser/Config.h b/src/modules/packagechooser/Config.h index 6a65c8788..d07b4a010 100644 --- a/src/modules/packagechooser/Config.h +++ b/src/modules/packagechooser/Config.h @@ -13,15 +13,34 @@ #include "PackageModel.h" #include "modulesystem/Config.h" +#include "modulesystem/InstanceKey.h" #include +enum class PackageChooserMode +{ + Optional, // zero or one + Required, // exactly one + OptionalMultiple, // zero or more + RequiredMultiple // one or more +}; + +const NamedEnumTable< PackageChooserMode >& packageChooserModeNames(); + +enum class PackageChooserMethod +{ + Legacy, // use contextualprocess or other custom + Packages, // use the packages module +}; + +const NamedEnumTable< PackageChooserMethod >& PackageChooserMethodNames(); + class Config : public Calamares::ModuleSystem::Config { Q_OBJECT public: - Config( const QString& defaultId, QObject* parent = nullptr ); + Config( const Calamares::ModuleSystem::InstanceKey& defaultId, QObject* parent = nullptr ); ~Config() override; void setConfigurationMap( const QVariantMap& ) override; @@ -51,10 +70,14 @@ private: PackageListModel* m_model = nullptr; QModelIndex m_defaultModelIndex; - // Configuration + /// Selection mode for this module 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; - QString m_defaultId; + /// Value to use for id if none is set in the config file + Calamares::ModuleSystem::InstanceKey m_defaultId; }; diff --git a/src/modules/packagechooser/PackageChooserPage.h b/src/modules/packagechooser/PackageChooserPage.h index 4f485c890..90c2b28a6 100644 --- a/src/modules/packagechooser/PackageChooserPage.h +++ b/src/modules/packagechooser/PackageChooserPage.h @@ -10,6 +10,7 @@ #ifndef PACKAGECHOOSERPAGE_H #define PACKAGECHOOSERPAGE_H +#include "Config.h" #include "PackageModel.h" #include diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index 05d0d3cfd..a15fd0f55 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -37,7 +37,7 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( PackageChooserViewStepFactory, registerPlug PackageChooserViewStep::PackageChooserViewStep( QObject* parent ) : Calamares::ViewStep( parent ) - , m_config( new Config( moduleInstanceKey().id(), this ) ) + , m_config( new Config( moduleInstanceKey(), this ) ) , m_widget( nullptr ) , m_stepName( nullptr ) { diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index 55300f940..05a90f220 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -12,26 +12,6 @@ #include "utils/Logger.h" #include "utils/Variant.h" -const NamedEnumTable< PackageChooserMode >& -packageChooserModeNames() -{ - static const NamedEnumTable< PackageChooserMode > names { - { "optional", PackageChooserMode::Optional }, - { "required", PackageChooserMode::Required }, - { "optionalmultiple", PackageChooserMode::OptionalMultiple }, - { "requiredmultiple", PackageChooserMode::RequiredMultiple }, - // and a bunch of aliases - { "zero-or-one", PackageChooserMode::Optional }, - { "radio", PackageChooserMode::Required }, - { "one", PackageChooserMode::Required }, - { "set", PackageChooserMode::OptionalMultiple }, - { "zero-or-more", PackageChooserMode::OptionalMultiple }, - { "multiple", PackageChooserMode::RequiredMultiple }, - { "one-or-more", PackageChooserMode::RequiredMultiple } - }; - return names; -} - PackageItem::PackageItem() {} PackageItem::PackageItem( const QString& a_id, const QString& a_name, const QString& a_description ) diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index 0da0e4a53..b27c0ed3b 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -18,15 +18,6 @@ #include #include -enum class PackageChooserMode -{ - Optional, // zero or one - Required, // exactly one - OptionalMultiple, // zero or more - RequiredMultiple // one or more -}; - -const NamedEnumTable< PackageChooserMode >& packageChooserModeNames(); struct PackageItem { From 91a29c58855b125e96d91da3a28ba59498d7ea55 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 17 Apr 2021 14:36:33 +0200 Subject: [PATCH 07/13] [packagechooser] Add getters for the *packages* members to the model --- src/modules/packagechooser/PackageModel.cpp | 27 +++++++++++++++++++++ src/modules/packagechooser/PackageModel.h | 13 ++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index 05a90f220..239705490 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -79,6 +79,33 @@ PackageListModel::addPackage( PackageItem&& p ) } } +QStringList +PackageListModel::getInstallPackagesForName( const QString& id ) const +{ + for ( const auto& p : qAsConst( m_packages ) ) + { + if ( p.id == id ) + { + return p.packageNames; + } + } + return QStringList(); +} + +QStringList +PackageListModel::getInstallPackagesForNames( const QStringList& ids ) const +{ + QStringList l; + for ( const auto& p : qAsConst( m_packages ) ) + { + if ( ids.contains( p.id ) ) + { + l.append( p.packageNames ); + } + } + return l; +} + int PackageListModel::rowCount( const QModelIndex& index ) const { diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index b27c0ed3b..71003197d 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -98,6 +98,19 @@ public: /// @brief Direct (non-abstract) count of package data int packageCount() const { return m_packages.count(); } + /** @brief Does a name lookup (based on id) and returns the packages member + * + * If there is a package with the given @p id, returns its packages + * (e.g. the names of underlying packages to install for it); returns + * an empty list if the id is not found. + */ + QStringList getInstallPackagesForName( const QString& id ) const; + /** @brief Name-lookup all the @p ids and returns the packages members + * + * Concatenates installPackagesForName() for each id in @p ids. + */ + QStringList getInstallPackagesForNames( const QStringList& ids ) const; + enum Roles : int { NameRole = Qt::DisplayRole, From 65e78e59154dd63fd77c1eecd11efad52e8ed1c7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 17 Apr 2021 14:39:24 +0200 Subject: [PATCH 08/13] [packagechooser] Use packages list instead of ids - don't pass the item IDs to packages module, use the packages lists for each item - document the item list in more detail (including the packages member and new install-method item) --- src/modules/packagechooser/Config.cpp | 3 +- .../packagechooser/packagechooser.conf | 109 ++++++++++++------ 2 files changed, 76 insertions(+), 36 deletions(-) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index 78c417908..0a0358c6b 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -102,8 +102,9 @@ Config::updateGlobalStorage( const QStringList& selected ) const } else if ( m_method == PackageChooserMethod::Packages ) { + QStringList packageNames = m_model->getInstallPackagesForNames( selected ); CalamaresUtils::Packages::setGSPackageAdditions( - Calamares::JobQueue::instance()->globalStorage(), m_defaultId, selected ); + Calamares::JobQueue::instance()->globalStorage(), m_defaultId, packageNames ); } else { diff --git a/src/modules/packagechooser/packagechooser.conf b/src/modules/packagechooser/packagechooser.conf index bb824c5e7..51032929e 100644 --- a/src/modules/packagechooser/packagechooser.conf +++ b/src/modules/packagechooser/packagechooser.conf @@ -3,26 +3,45 @@ # # Configuration for the low-density software chooser --- -# The packagechooser writes a GlobalStorage value for the choice that -# has been made. The key is *packagechooser_*. If *id* is set here, -# it is substituted into the key name. If it is not set, the module's -# instance name is used; see the *instances* section of `settings.conf`. -# If there is just one packagechooser module, and no *id* is set, -# 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. -# -# id: "" - # Software selection mode, to set whether the software packages # can be chosen singly, or multiply. # -# Possible modes are "optional", "required" (for zero or one) +# 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_*. 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. +# +# The GS value is a comma-separated list of the IDs of the selected +# packages, or an empty string if none is selected. +# +# With "legacy" installation, you should have a contextualprocess or similar +# module somewhere in the `exec` phase to process the GlobalStorage key +# and actually **do** something for the packages. +# +# - "packages" +# When set to "packages", writes GlobalStorage values suitable for +# 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. +# +# 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 +# id: "" + + # Human-visible strings in this module. These are all optional. # The following translated keys are used: # - *step*, used in the overall progress view (left-hand pane) @@ -49,27 +68,45 @@ labels: # as a source for the data. # # For data provided by the list: the item has an id, which is used in -# setting the value of *packagechooser_*. The following fields -# are mandatory: +# setting the value of *packagechooser_*. The following field +# is mandatory: # -# - *id* : ID for the product. The ID "" is special, and is used for -# "no package selected". Only include this if the mode allows -# selecting none. -# - *package* : Package name for the product. While mandatory, this is -# not actually used anywhere. -# - *name* : Human-readable name of the product. To provide translations, -# add a *[lang]* decoration as part of the key name, -# e.g. `name[nl]` for Dutch. -# The list of usable languages can be found in -# `CMakeLists.txt` or as part of the debug output of Calamares. -# - *description* : Human-readable description. These can be translated -# as well. -# - *screenshot* : Path to a single screenshot of the product. May be -# a filesystem path or a QRC path, -# e.g. ":/images/no-selection.png". +# - *id* +# ID for the product. The ID "" is special, and is used for +# "no package selected". Only include this if the mode allows +# selecting none. The name and description given for the "no package +# selected" item are displayed when the module starts. # -# Use the empty string "" as ID / key for the "no selection" item if -# you want to customize the display of that item as well. +# Each item must adhere to one of three "styles" of item. Which styles +# are supported depends on compile-time dependencies of Calamares. +# Both AppData and AppStream may **optionally** be available. +# +# # Generic Items # +# +# These items are always supported. They require the most configuration +# **in this file** and duplicate information that may be available elsewhere +# (e.g. in AppData or AppStream), but do not require any additional +# dependencies. These items have the following **mandatory** fields: +# +# - *name* +# Human-readable name of the product. To provide translations, +# add a *[lang]* decoration as part of the key name, e.g. `name[nl]` +# for Dutch. The list of usable languages can be found in +# `CMakeLists.txt` or as part of the debug output of Calamares. +# - *description* +# Human-readable description. These can be translated as well. +# - *screenshot* +# Path to a single screenshot of the product. May be a filesystem +# path or a QRC path, e.g. ":/images/no-selection.png". +# +# The following field is **optional** for an item: +# +# - *packages* : +# List of package names for the product. If using the *method* +# "packages", consider this item mandatory (because otherwise +# selecting the item would install no packages). +# +# # AppData Items # # # For data provided by AppData XML: the item has an *appdata* # key which points to an AppData XML file in the local filesystem. @@ -84,6 +121,8 @@ labels: # **may** specify an ID or screenshot path, as above. This will override # the settings from AppData. # +# # AppStream Items # +# # For data provided by AppStream cache: the item has an *appstream* # key which matches the AppStream identifier in the cache (e.g. # *org.kde.kwrite.desktop*). Data is retrieved from the AppStream @@ -93,19 +132,19 @@ labels: # key which will override the data from AppStream. items: - id: "" - package: "" + # packages: [] # This item installs no packages name: "No Desktop" name[nl]: "Geen desktop" description: "Please pick a desktop environment from the list. If you don't want to install a desktop, that's fine, your system will start up in text-only mode and you can install a desktop environment later." description[nl]: "Kies eventueel een desktop-omgeving uit deze lijst. Als u geen desktop-omgeving wenst te gebruiken, kies er dan geen. In dat geval start het systeem straks op in tekst-modus en kunt u later alsnog een desktop-omgeving installeren." screenshot: ":/images/no-selection.png" - id: kde - package: kde + packages: [ kde-frameworks, kde-plasma, kde-gear ] name: Plasma Desktop description: "KDE Plasma Desktop, simple by default, a clean work area for real-world usage which intends to stay out of your way. Plasma is powerful when needed, enabling the user to create the workflow that makes them more effective to complete their tasks." screenshot: ":/images/kde.png" - id: gnome - package: gnome + packages: [ gnome-all ] name: GNOME description: GNU Networked Object Modeling Environment Desktop screenshot: ":/images/gnome.png" From 61557cf80539f6cfd50b7867b1511dbe5929bb48 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Apr 2021 12:41:50 +0200 Subject: [PATCH 09/13] [packagechooser] Connect UI to model The model needs to be attached to the widget; because of changes in the order that widget() and setConfigurationMap() are called, the model is created earlier, but needs to be connected later. --- src/modules/packagechooser/Config.cpp | 1 + src/modules/packagechooser/PackageChooserViewStep.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index 0a0358c6b..a3467b553 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -169,6 +169,7 @@ fillModel( PackageListModel* model, const QVariantList& items ) model->addPackage( PackageItem( item_map ) ); } } + cDebug() << Logger::SubEntry << "Loaded PackageChooser with" << model->packageCount() << "entries."; } void diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index a15fd0f55..67e67495d 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -71,6 +71,7 @@ PackageChooserViewStep::widget() connect( m_widget, &PackageChooserPage::selectionChanged, [=]() { emit nextStatusChanged( this->isNextEnabled() ); } ); + hookupModel(); } return m_widget; } From 7521be3c5fca53e8318585114ec24c73aa949fd7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Apr 2021 18:03:24 +0200 Subject: [PATCH 10/13] [libcalamares] Add find() to namedenumtable that takes a default value --- src/libcalamares/utils/NamedEnum.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/libcalamares/utils/NamedEnum.h b/src/libcalamares/utils/NamedEnum.h index 1d839ddc4..1462cc0ff 100644 --- a/src/libcalamares/utils/NamedEnum.h +++ b/src/libcalamares/utils/NamedEnum.h @@ -174,6 +174,22 @@ struct NamedEnumTable return table.begin()->second; } + /** @brief Find a name @p s in the table. + * + * Searches case-insensitively. + * + * If the name @p s is not found, the value @p d is returned as + * a default. Otherwise the value corresponding to @p s is returned. + * This is a shortcut over find() using a bool to distinguish + * successful and unsuccesful lookups. + */ + enum_t find( const string_t& s, enum_t d ) const + { + bool ok = false; + enum_t e = find( s, ok ); + return ok ? e : d; + } + /** @brief Find a value @p s in the table and return its name. * * If @p s is an enum value in the table, return the corresponding From 6ce1a49f1cf9849d48065e66b3f2898c358df2d8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Apr 2021 21:46:46 +0200 Subject: [PATCH 11/13] [packagechooser] Store *method* configuration in Config object --- src/modules/packagechooser/Config.cpp | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index a3467b553..b0336fccb 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -92,17 +92,19 @@ void Config::updateGlobalStorage( const QStringList& selected ) const { QString key = QStringLiteral( "packagechooser_%1" ).arg( m_id ); + cDebug() << "Writing to GS" << key; if ( m_method == PackageChooserMethod::Legacy ) { QString value = selected.join( ',' ); Calamares::JobQueue::instance()->globalStorage()->insert( key, value ); - cDebug() << "PackageChooser" << key << "selected" << value; + cDebug() << Logger::SubEntry << "PackageChooser" << key << "selected" << value; } else if ( m_method == PackageChooserMethod::Packages ) { QStringList packageNames = m_model->getInstallPackagesForNames( selected ); + cDebug() << Logger::SubEntry << "Got packages" << packageNames; CalamaresUtils::Packages::setGSPackageAdditions( Calamares::JobQueue::instance()->globalStorage(), m_defaultId, packageNames ); } @@ -175,21 +177,14 @@ fillModel( PackageListModel* model, const QVariantList& items ) void Config::setConfigurationMap( const QVariantMap& configurationMap ) { - QString mode = CalamaresUtils::getString( configurationMap, "mode" ); - bool mode_ok = false; - if ( !mode.isEmpty() ) - { - m_mode = packageChooserModeNames().find( mode, mode_ok ); - } - if ( !mode_ok ) - { - m_mode = PackageChooserMode::Required; - } + m_mode = packageChooserModeNames().find( CalamaresUtils::getString( configurationMap, "mode" ), PackageChooserMode::Required ); + m_method = PackageChooserMethodNames().find( CalamaresUtils::getString( configurationMap, "method" ), PackageChooserMethod::Legacy ); m_id = CalamaresUtils::getString( configurationMap, "id" ); if ( m_id.isEmpty() ) { m_id = m_defaultId.id(); + cDebug() << "Using default ID" << m_id << "from" << m_defaultId.toString(); } m_defaultModelIndex = QModelIndex(); From aa3633e43a654ae3115f88b46bf783bd4ec7e17f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Apr 2021 22:04:15 +0200 Subject: [PATCH 12/13] [packagechooser] Delay initialization of default Id When the module is loaded and the viewstep created, it doesn't have a module Id **yet**. That is set after reading more of the configuration file. It **is** set by the time setConfigurationMap() is called, so pass it on to the Config object then. This means that packagechooser modules can skip the *id* config key and use the module Id. --- src/modules/packagechooser/Config.cpp | 31 ++++++++++++------- src/modules/packagechooser/Config.h | 10 +++++- .../packagechooser/PackageChooserViewStep.cpp | 3 +- 3 files changed, 31 insertions(+), 13 deletions(-) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index b0336fccb..531d285ed 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -51,11 +51,10 @@ PackageChooserMethodNames() return names; } -Config::Config( const Calamares::ModuleSystem::InstanceKey& defaultId, QObject* parent ) +Config::Config( QObject* parent ) : Calamares::ModuleSystem::Config( parent ) , m_model( new PackageListModel( this ) ) , m_mode( PackageChooserMode::Required ) - , m_defaultId( defaultId ) { } @@ -91,7 +90,7 @@ Config::introductionPackage() const void Config::updateGlobalStorage( const QStringList& selected ) const { - QString key = QStringLiteral( "packagechooser_%1" ).arg( m_id ); + const QString& key = m_id; cDebug() << "Writing to GS" << key; if ( m_method == PackageChooserMethod::Legacy ) @@ -177,24 +176,34 @@ fillModel( PackageListModel* model, const QVariantList& items ) void Config::setConfigurationMap( const QVariantMap& configurationMap ) { - m_mode = packageChooserModeNames().find( CalamaresUtils::getString( configurationMap, "mode" ), PackageChooserMode::Required ); - m_method = PackageChooserMethodNames().find( CalamaresUtils::getString( configurationMap, "method" ), PackageChooserMethod::Legacy ); + m_mode = packageChooserModeNames().find( CalamaresUtils::getString( configurationMap, "mode" ), + PackageChooserMode::Required ); + m_method = PackageChooserMethodNames().find( CalamaresUtils::getString( configurationMap, "method" ), + PackageChooserMethod::Legacy ); - m_id = CalamaresUtils::getString( configurationMap, "id" ); - if ( m_id.isEmpty() ) { - m_id = m_defaultId.id(); - cDebug() << "Using default ID" << m_id << "from" << m_defaultId.toString(); + const QString configId = CalamaresUtils::getString( configurationMap, "id" ); + if ( configId.isEmpty() ) + { + m_id = m_defaultId.toString(); + if ( m_id.isEmpty() ) + { + m_id = QString( "packagechooser" ); + } + cDebug() << "Using default ID" << m_id << "from" << m_defaultId.toString(); + } + else + { + m_id = QStringLiteral( "packagechooser_" ) + configId; + } } - m_defaultModelIndex = QModelIndex(); if ( configurationMap.contains( "items" ) ) { fillModel( m_model, configurationMap.value( "items" ).toList() ); } QString default_item_id = CalamaresUtils::getString( configurationMap, "default" ); - // find default item if ( !default_item_id.isEmpty() ) { for ( int item_n = 0; item_n < m_model->packageCount(); ++item_n ) diff --git a/src/modules/packagechooser/Config.h b/src/modules/packagechooser/Config.h index d07b4a010..4cb545cb8 100644 --- a/src/modules/packagechooser/Config.h +++ b/src/modules/packagechooser/Config.h @@ -40,9 +40,17 @@ class Config : public Calamares::ModuleSystem::Config Q_OBJECT public: - Config( const Calamares::ModuleSystem::InstanceKey& defaultId, QObject* parent = nullptr ); + Config( QObject* parent = nullptr ); ~Config() override; + /** @brief Sets the default Id for this Config + * + * The default Id is the (owning) module identifier for the config, + * and it is used when the Id read from the config file is empty. + * The **usual** configuration when using method *packages* is + * to rely on the default Id. + */ + void setDefaultId( const Calamares::ModuleSystem::InstanceKey& defaultId ) { m_defaultId = defaultId; } void setConfigurationMap( const QVariantMap& ) override; PackageChooserMode mode() const { return m_mode; } diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index 67e67495d..53912ef36 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -37,7 +37,7 @@ CALAMARES_PLUGIN_FACTORY_DEFINITION( PackageChooserViewStepFactory, registerPlug PackageChooserViewStep::PackageChooserViewStep( QObject* parent ) : Calamares::ViewStep( parent ) - , m_config( new Config( moduleInstanceKey(), this ) ) + , m_config( new Config( this ) ) , m_widget( nullptr ) , m_stepName( nullptr ) { @@ -146,6 +146,7 @@ PackageChooserViewStep::jobs() const void PackageChooserViewStep::setConfigurationMap( const QVariantMap& configurationMap ) { + m_config->setDefaultId( moduleInstanceKey() ); m_config->setConfigurationMap( configurationMap ); bool labels_ok = false; From f4fe0881b9d068e8c2b6924482e0b1350cfc32f7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Apr 2021 22:29:26 +0200 Subject: [PATCH 13/13] [packagechooser] Be more clear on the resulting GS keys - in legacy mode, *id* can have an effect and leads to "packagechooser_"; if unset, uses the the module instance id instead, still as "packagechooser_". - in packages mode, *id* is not used and only the whole module Id (generally, "packagechooser@") 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_*. 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: ""