From 616fbb08f3098fb06432de4d580652d71bc560a3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 11:19:08 +0200 Subject: [PATCH 01/11] [libcalamares] Improve docs of RequirementsModel --- src/libcalamares/modulesystem/RequirementsModel.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/modulesystem/RequirementsModel.h b/src/libcalamares/modulesystem/RequirementsModel.h index 2acf785e7..b288b9d49 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.h +++ b/src/libcalamares/modulesystem/RequirementsModel.h @@ -27,7 +27,15 @@ namespace Calamares { - +/** @brief System requirements from each module and their checked-status + * + * A Calamares module can have system requirements (e.g. check for + * internet, or amount of RAM, or an available disk) which can + * be stated and checked. + * + * This model collects those requirements, can run the checks, and + * reports on the overall status of those checks. + */ class DLLEXPORT RequirementsModel : public QAbstractListModel { Q_OBJECT @@ -70,10 +78,9 @@ protected: QHash< int, QByteArray > roleNames() const override; private: - Calamares::RequirementsList m_requirements; + RequirementsList m_requirements; bool m_satisfiedRequirements = false; bool m_satisfiedMandatory = false; - }; } // namespace Calamares From 0f5db0ba5ed475404cd6ddd9d89c06cabbd76d96 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 11:34:02 +0200 Subject: [PATCH 02/11] [libcalamares] Remove direct access to model internals - This was just for the ResultsListWidget, which can also use normal role-based model access. --- .../modulesystem/RequirementsModel.h | 5 ---- .../welcome/checker/ResultsListWidget.cpp | 30 +++++++++++-------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/libcalamares/modulesystem/RequirementsModel.h b/src/libcalamares/modulesystem/RequirementsModel.h index b288b9d49..7722707a3 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.h +++ b/src/libcalamares/modulesystem/RequirementsModel.h @@ -59,11 +59,6 @@ public: bool satisfiedRequirements() const { return m_satisfiedRequirements; } bool satisfiedMandatory() const { return m_satisfiedMandatory; } - const Calamares::RequirementEntry& getEntry( int index ) const - { - return m_requirements.at( index ); - } - void setRequirementsList( const Calamares::RequirementsList& requirements ); QVariant data( const QModelIndex& index, int role ) const override; diff --git a/src/modules/welcome/checker/ResultsListWidget.cpp b/src/modules/welcome/checker/ResultsListWidget.cpp index 2554f907b..afde9f08d 100644 --- a/src/modules/welcome/checker/ResultsListWidget.cpp +++ b/src/modules/welcome/checker/ResultsListWidget.cpp @@ -48,27 +48,29 @@ static void createResultWidgets( QLayout* layout, QList< ResultWidget* >& resultWidgets, const Calamares::RequirementsModel& model, - std::function< bool( const Calamares::RequirementEntry& ) > predicate ) + std::function< bool( const Calamares::RequirementsModel&, QModelIndex ) > predicate ) { resultWidgets.clear(); resultWidgets.reserve( model.count() ); for ( auto i = 0; i < model.count(); i++ ) { - const auto& entry = model.getEntry( i ); - if ( !predicate( entry ) ) + const auto& index = model.index( i ); + if ( !predicate( model, index ) ) { resultWidgets.append( nullptr ); continue; } - ResultWidget* ciw = new ResultWidget( entry.satisfied, entry.mandatory ); + const bool is_satisfied = model.data( index, Calamares::RequirementsModel::Satisfied ).toBool(); + const bool is_mandatory = model.data( index, Calamares::RequirementsModel::Mandatory ).toBool(); + ResultWidget* ciw = new ResultWidget( is_satisfied, is_mandatory ); layout->addWidget( ciw ); ciw->setSizePolicy( QSizePolicy::Expanding, QSizePolicy::Preferred ); ciw->setAutoFillBackground( true ); QPalette pal( ciw->palette() ); QColor bgColor = pal.window().color(); - int bgHue = ( entry.satisfied ) ? bgColor.hue() : ( entry.mandatory ) ? 0 : 60; + int bgHue = ( is_satisfied ) ? bgColor.hue() : ( is_mandatory ) ? 0 : 60; bgColor.setHsv( bgHue, 64, bgColor.value() ); pal.setColor( QPalette::Window, bgColor ); ciw->setPalette( pal ); @@ -114,7 +116,9 @@ ResultsListDialog::ResultsListDialog( const Calamares::RequirementsModel& model, m_title = new QLabel( this ); createResultWidgets( - entriesLayout, m_resultWidgets, model, []( const Calamares::RequirementEntry& e ) { return e.hasDetails(); } ); + entriesLayout, m_resultWidgets, model, []( const Calamares::RequirementsModel& m, QModelIndex i ) { + return m.data( i, Calamares::RequirementsModel::HasDetails ).toBool(); + } ); QDialogButtonBox* buttonBox = new QDialogButtonBox( QDialogButtonBox::Close, Qt::Horizontal, this ); @@ -130,7 +134,7 @@ ResultsListDialog::ResultsListDialog( const Calamares::RequirementsModel& model, retranslate(); // Do it now to fill in the texts } -ResultsListDialog::~ResultsListDialog() { } +ResultsListDialog::~ResultsListDialog() {} void ResultsListDialog::retranslate() @@ -140,10 +144,10 @@ ResultsListDialog::retranslate() for ( auto i = 0; i < m_model.count(); i++ ) { - const auto& entry = m_model.getEntry( i ); if ( m_resultWidgets[ i ] ) { - m_resultWidgets[ i ]->setText( entry.enumerationText() ); + m_resultWidgets[ i ]->setText( + m_model.data( m_model.index( i ), Calamares::RequirementsModel::Details ).toString() ); } } } @@ -180,7 +184,9 @@ ResultsListWidget::ResultsListWidget( const Calamares::RequirementsModel& model, // all *mandatory* entries are satisfied (gives errors if not). const bool requirementsSatisfied = m_model.satisfiedRequirements(); - auto isUnSatisfied = []( const Calamares::RequirementEntry& e ) { return !e.satisfied; }; + auto isUnSatisfied = []( const Calamares::RequirementsModel& m, QModelIndex i ) { + return !m.data( i, Calamares::RequirementsModel::Satisfied ).toBool(); + }; createResultWidgets( entriesLayout, m_resultWidgets, model, isUnSatisfied ); @@ -240,10 +246,10 @@ ResultsListWidget::retranslate() { for ( auto i = 0; i < m_model.count(); i++ ) { - const auto& entry = m_model.getEntry( i ); if ( m_resultWidgets[ i ] ) { - m_resultWidgets[ i ]->setText( entry.negatedText() ); + m_resultWidgets[ i ]->setText( + m_model.data( m_model.index( i ), Calamares::RequirementsModel::NegatedText ).toString() ); } } From d87d714b8dc6e9d9ffc61ebf0462888e38a57ed2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 11:41:42 +0200 Subject: [PATCH 03/11] [libcalamares] Make the requirements model more adaptable - Either replace the list of results, or add to them - Lock model while adding rows --- .../modulesystem/RequirementsModel.cpp | 18 +++++++++++++++++- .../modulesystem/RequirementsModel.h | 10 ++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/modulesystem/RequirementsModel.cpp b/src/libcalamares/modulesystem/RequirementsModel.cpp index 4001d2d81..4a776b3eb 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.cpp +++ b/src/libcalamares/modulesystem/RequirementsModel.cpp @@ -24,9 +24,26 @@ namespace Calamares void RequirementsModel::setRequirementsList( const Calamares::RequirementsList& requirements ) { + QMutexLocker l( &m_addLock ); emit beginResetModel(); m_requirements = requirements; + changeRequirementsList(); + emit endResetModel(); +} +void +RequirementsModel::addRequirementsList( const Calamares::RequirementsList& requirements ) +{ + QMutexLocker l( &m_addLock ); + emit beginResetModel(); + m_requirements.append( requirements ); + changeRequirementsList(); + emit endResetModel(); +} + +void +RequirementsModel::changeRequirementsList() +{ auto isUnSatisfied = []( const Calamares::RequirementEntry& e ) { return !e.satisfied; }; auto isMandatoryAndUnSatisfied = []( const Calamares::RequirementEntry& e ) { return e.mandatory && !e.satisfied; }; @@ -35,7 +52,6 @@ RequirementsModel::setRequirementsList( const Calamares::RequirementsList& requi emit satisfiedRequirementsChanged( m_satisfiedRequirements ); emit satisfiedMandatoryChanged( m_satisfiedMandatory ); - emit endResetModel(); } int diff --git a/src/libcalamares/modulesystem/RequirementsModel.h b/src/libcalamares/modulesystem/RequirementsModel.h index 7722707a3..e34dd02c2 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.h +++ b/src/libcalamares/modulesystem/RequirementsModel.h @@ -24,6 +24,7 @@ #include "DllMacro.h" #include +#include namespace Calamares { @@ -56,10 +57,15 @@ public: }; // No Q_ENUM because these are exposed through roleNames() + ///@brief Are all the requirements satisfied? bool satisfiedRequirements() const { return m_satisfiedRequirements; } + ///@brief Are all the **mandatory** requirements satisfied? bool satisfiedMandatory() const { return m_satisfiedMandatory; } + ///@brief Replace the entire list of requirements; resets the model void setRequirementsList( const Calamares::RequirementsList& requirements ); + ///@brief Append some requirements; resets the model + void addRequirementsList( const Calamares::RequirementsList& requirements ); QVariant data( const QModelIndex& index, int role ) const override; int rowCount( const QModelIndex& ) const override; @@ -73,6 +79,10 @@ protected: QHash< int, QByteArray > roleNames() const override; private: + ///@brief Implementation for {set,add}RequirementsList + void changeRequirementsList(); + + QMutex m_addLock; RequirementsList m_requirements; bool m_satisfiedRequirements = false; bool m_satisfiedMandatory = false; From 8306de731a84b44c0f3aac420957bba782e53393 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 14:21:57 +0200 Subject: [PATCH 04/11] [welcome] Setting requirements from own reqs is totally wrong - The requirements are collected by ModuleManager, checked by an internal RequirementsChecker and changes to the requirements state are all signalled from ModuleManager. By connecting the requirements in the welcome modules' Config only to their own configs -- and immediately checking them, which is bad on its own -- we end up with a disconnect between what the ModuleManager says about requirements, and what the welcome modules report on. --- src/modules/welcome/WelcomeViewStep.cpp | 4 ++-- src/modules/welcomeq/WelcomeQmlViewStep.cpp | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/modules/welcome/WelcomeViewStep.cpp b/src/modules/welcome/WelcomeViewStep.cpp index e4e56c44c..0ed887fa9 100644 --- a/src/modules/welcome/WelcomeViewStep.cpp +++ b/src/modules/welcome/WelcomeViewStep.cpp @@ -111,12 +111,12 @@ WelcomeViewStep::setConfigurationMap( const QVariantMap& configurationMap ) && configurationMap.value( "requirements" ).type() == QVariant::Map ) { m_requirementsChecker->setConfigurationMap( configurationMap.value( "requirements" ).toMap() ); - - m_conf->requirementsModel().setRequirementsList( checkRequirements() ); } else + { cWarning() << "no valid requirements map found in welcome " "module configuration."; + } //here init the qml or qwidgets needed bits m_widget->init(); diff --git a/src/modules/welcomeq/WelcomeQmlViewStep.cpp b/src/modules/welcomeq/WelcomeQmlViewStep.cpp index 4869673bb..f520a9953 100644 --- a/src/modules/welcomeq/WelcomeQmlViewStep.cpp +++ b/src/modules/welcomeq/WelcomeQmlViewStep.cpp @@ -102,8 +102,6 @@ WelcomeQmlViewStep::setConfigurationMap( const QVariantMap& configurationMap ) && configurationMap.value( "requirements" ).type() == QVariant::Map ) { m_requirementsChecker->setConfigurationMap( configurationMap.value( "requirements" ).toMap() ); - - m_config->requirementsModel().setRequirementsList( checkRequirements() ); } else cWarning() << "no valid requirements map found in welcome " From 039065ee4ae788fdc01997cb56a972100b4053f1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 15:21:07 +0200 Subject: [PATCH 05/11] [libcalamares] Minor debugging support in RequirementsModel --- .../modulesystem/RequirementsModel.cpp | 18 ++++++++++++++++++ .../modulesystem/RequirementsModel.h | 3 +++ 2 files changed, 21 insertions(+) diff --git a/src/libcalamares/modulesystem/RequirementsModel.cpp b/src/libcalamares/modulesystem/RequirementsModel.cpp index 4a776b3eb..50d70b150 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.cpp +++ b/src/libcalamares/modulesystem/RequirementsModel.cpp @@ -18,6 +18,8 @@ #include "RequirementsModel.h" +#include "utils/Logger.h" + namespace Calamares { @@ -94,4 +96,20 @@ RequirementsModel::roleNames() const return roles; } +void +RequirementsModel::describe() const +{ + bool acceptable = true; + int count = 0; + for ( const auto& r : m_requirements ) + { + if ( r.mandatory && !r.satisfied ) + { + cDebug() << Logger::SubEntry << "requirement" << count << r.name << "is not satisfied."; + acceptable = false; + } + ++count; + } +} + } // namespace Calamares diff --git a/src/libcalamares/modulesystem/RequirementsModel.h b/src/libcalamares/modulesystem/RequirementsModel.h index e34dd02c2..eaf597509 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.h +++ b/src/libcalamares/modulesystem/RequirementsModel.h @@ -71,6 +71,9 @@ public: int rowCount( const QModelIndex& ) const override; int count() const { return m_requirements.count(); } + ///@brief Debugging tool, describe the checking-state + void describe() const; + signals: void satisfiedRequirementsChanged( bool value ); void satisfiedMandatoryChanged( bool value ); From b7c60cec66c290f615599eaad5cace0397e24a11 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 15:35:03 +0200 Subject: [PATCH 06/11] [libcalamares] Re-vamp RequirementsChecker - Give the ModuleManager a RequirementsModel -- that is the source of truth about the module-requirements of the modules managed by that particular ModuleManager. - Let the RequirementsChecker operate on a given RequirementsModel. --- .../modulesystem/RequirementsChecker.cpp | 26 +++++-------------- .../modulesystem/RequirementsChecker.h | 5 ++-- .../modulesystem/RequirementsModel.cpp | 10 ------- .../modulesystem/RequirementsModel.h | 12 +++++---- .../modulesystem/ModuleManager.cpp | 4 ++- .../modulesystem/ModuleManager.h | 3 ++- 6 files changed, 21 insertions(+), 39 deletions(-) diff --git a/src/libcalamares/modulesystem/RequirementsChecker.cpp b/src/libcalamares/modulesystem/RequirementsChecker.cpp index 97a4c912f..6617fa6d5 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.cpp +++ b/src/libcalamares/modulesystem/RequirementsChecker.cpp @@ -20,6 +20,7 @@ #include "modulesystem/Module.h" #include "modulesystem/Requirement.h" +#include "modulesystem/RequirementsModel.h" #include "utils/Logger.h" #include @@ -63,14 +64,14 @@ check( Module* const& m, RequirementsChecker* c ) QObject::tr( "Requirements checking for module %1 is complete." ).arg( m->name() ) ); } -RequirementsChecker::RequirementsChecker( QVector< Module* > modules, QObject* parent ) +RequirementsChecker::RequirementsChecker( QVector< Module* > modules, RequirementsModel* model, QObject* parent ) : QObject( parent ) , m_modules( std::move( modules ) ) + , m_model( model ) , m_progressTimer( nullptr ) , m_progressTimeouts( 0 ) { m_watchers.reserve( m_modules.count() ); - m_collectedRequirements.reserve( m_modules.count() ); registerMetatypes(); } @@ -114,19 +115,8 @@ RequirementsChecker::finished() m_progressTimer = nullptr; } - bool acceptable = true; - int count = 0; - for ( const auto& r : m_collectedRequirements ) - { - if ( r.mandatory && !r.satisfied ) - { - cDebug() << Logger::SubEntry << "requirement" << count << r.name << "is not satisfied."; - acceptable = false; - } - ++count; - } - - emit requirementsComplete( acceptable ); + m_model->describe(); + emit requirementsComplete( m_model->satisfiedMandatory() ); QTimer::singleShot( 0, this, &RequirementsChecker::done ); } } @@ -134,11 +124,7 @@ RequirementsChecker::finished() void RequirementsChecker::addCheckedRequirements( RequirementsList l ) { - static QMutex addMutex; - { - QMutexLocker lock( &addMutex ); - m_collectedRequirements.append( l ); - } + m_model->addRequirementsList( l ); cDebug() << "Added" << l.count() << "requirement results"; emit requirementsResult( l ); } diff --git a/src/libcalamares/modulesystem/RequirementsChecker.h b/src/libcalamares/modulesystem/RequirementsChecker.h index 450495dc1..b4651be21 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.h +++ b/src/libcalamares/modulesystem/RequirementsChecker.h @@ -29,6 +29,7 @@ namespace Calamares { class Module; +class RequirementsModel; /** @brief A manager-class that checks all the module requirements * @@ -40,7 +41,7 @@ class RequirementsChecker : public QObject Q_OBJECT public: - RequirementsChecker( QVector< Module* > modules, QObject* parent = nullptr ); + RequirementsChecker( QVector< Module* > modules, RequirementsModel* model, QObject* parent = nullptr ); virtual ~RequirementsChecker() override; public Q_SLOTS: @@ -75,7 +76,7 @@ private: using Watcher = QFutureWatcher< void >; QVector< Watcher* > m_watchers; - RequirementsList m_collectedRequirements; + RequirementsModel* m_model; QTimer* m_progressTimer; unsigned m_progressTimeouts; diff --git a/src/libcalamares/modulesystem/RequirementsModel.cpp b/src/libcalamares/modulesystem/RequirementsModel.cpp index 50d70b150..dc1c3c3cb 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.cpp +++ b/src/libcalamares/modulesystem/RequirementsModel.cpp @@ -23,16 +23,6 @@ namespace Calamares { -void -RequirementsModel::setRequirementsList( const Calamares::RequirementsList& requirements ) -{ - QMutexLocker l( &m_addLock ); - emit beginResetModel(); - m_requirements = requirements; - changeRequirementsList(); - emit endResetModel(); -} - void RequirementsModel::addRequirementsList( const Calamares::RequirementsList& requirements ) { diff --git a/src/libcalamares/modulesystem/RequirementsModel.h b/src/libcalamares/modulesystem/RequirementsModel.h index eaf597509..ce9f23168 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.h +++ b/src/libcalamares/modulesystem/RequirementsModel.h @@ -28,6 +28,8 @@ namespace Calamares { +class RequirementsChecker; + /** @brief System requirements from each module and their checked-status * * A Calamares module can have system requirements (e.g. check for @@ -39,6 +41,8 @@ namespace Calamares */ class DLLEXPORT RequirementsModel : public QAbstractListModel { + friend class RequirementsChecker; + Q_OBJECT Q_PROPERTY( bool satisfiedRequirements READ satisfiedRequirements NOTIFY satisfiedRequirementsChanged FINAL ) Q_PROPERTY( bool satisfiedMandatory READ satisfiedMandatory NOTIFY satisfiedMandatoryChanged FINAL ) @@ -62,11 +66,6 @@ public: ///@brief Are all the **mandatory** requirements satisfied? bool satisfiedMandatory() const { return m_satisfiedMandatory; } - ///@brief Replace the entire list of requirements; resets the model - void setRequirementsList( const Calamares::RequirementsList& requirements ); - ///@brief Append some requirements; resets the model - void addRequirementsList( const Calamares::RequirementsList& requirements ); - QVariant data( const QModelIndex& index, int role ) const override; int rowCount( const QModelIndex& ) const override; int count() const { return m_requirements.count(); } @@ -81,6 +80,9 @@ signals: protected: QHash< int, QByteArray > roleNames() const override; + ///@brief Append some requirements; resets the model + void addRequirementsList( const Calamares::RequirementsList& requirements ); + private: ///@brief Implementation for {set,add}RequirementsList void changeRequirementsList(); diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 8d4b2342f..4734838ca 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -24,6 +24,7 @@ #include "Settings.h" #include "modulesystem/Module.h" #include "modulesystem/RequirementsChecker.h" +#include "modulesystem/RequirementsModel.h" #include "utils/Logger.h" #include "utils/Yaml.h" #include "viewpages/ExecutionViewStep.h" @@ -46,6 +47,7 @@ ModuleManager::instance() ModuleManager::ModuleManager( const QStringList& paths, QObject* parent ) : QObject( parent ) , m_paths( paths ) + , m_requirementsModel( new RequirementsModel( this ) ) { Q_ASSERT( !s_instance ); s_instance = this; @@ -355,7 +357,7 @@ ModuleManager::checkRequirements() modules[ count++ ] = module; } - RequirementsChecker* rq = new RequirementsChecker( modules, this ); + RequirementsChecker* rq = new RequirementsChecker( modules, m_requirementsModel, this ); connect( rq, &RequirementsChecker::requirementsResult, this, &ModuleManager::requirementsResult ); connect( rq, &RequirementsChecker::requirementsComplete, this, &ModuleManager::requirementsComplete ); connect( rq, &RequirementsChecker::requirementsProgress, this, &ModuleManager::requirementsProgress ); diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index fdb63cd87..b0e537d9c 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -32,7 +32,7 @@ namespace Calamares { class Module; -struct RequirementEntry; // from Requirement.h +class RequirementsModel; /** * @brief The ModuleManager class is a singleton which manages Calamares modules. @@ -130,6 +130,7 @@ private: QMap< QString, QString > m_moduleDirectoriesByModuleName; QMap< ModuleSystem::InstanceKey, Module* > m_loadedModulesByInstanceKey; const QStringList m_paths; + RequirementsModel* m_requirementsModel; static ModuleManager* s_instance; }; From 9b0ea3f63dcedff7754850d632a4a7e889849697 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 15:42:27 +0200 Subject: [PATCH 07/11] [libcalamares] Remove runaround through free function - Call into a method directly to do the work of adding results from a single module. --- .../modulesystem/RequirementsChecker.cpp | 27 ++++++++----------- .../modulesystem/RequirementsChecker.h | 2 +- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/libcalamares/modulesystem/RequirementsChecker.cpp b/src/libcalamares/modulesystem/RequirementsChecker.cpp index 6617fa6d5..9fa9a72f8 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.cpp +++ b/src/libcalamares/modulesystem/RequirementsChecker.cpp @@ -52,18 +52,6 @@ registerMetatypes() } } -static void -check( Module* const& m, RequirementsChecker* c ) -{ - RequirementsList l = m->checkRequirements(); - if ( l.count() > 0 ) - { - c->addCheckedRequirements( l ); - } - c->requirementsProgress( - QObject::tr( "Requirements checking for module %1 is complete." ).arg( m->name() ) ); -} - RequirementsChecker::RequirementsChecker( QVector< Module* > modules, RequirementsModel* model, QObject* parent ) : QObject( parent ) , m_modules( std::move( modules ) ) @@ -88,7 +76,7 @@ RequirementsChecker::run() for ( const auto& module : m_modules ) { Watcher* watcher = new Watcher( this ); - watcher->setFuture( QtConcurrent::run( check, module, this ) ); + watcher->setFuture( QtConcurrent::run( this, &RequirementsChecker::addCheckedRequirements, module ) ); watcher->setObjectName( module->name() ); m_watchers.append( watcher ); connect( watcher, &Watcher::finished, this, &RequirementsChecker::finished ); @@ -122,10 +110,17 @@ RequirementsChecker::finished() } void -RequirementsChecker::addCheckedRequirements( RequirementsList l ) +RequirementsChecker::addCheckedRequirements( Module* m ) { - m_model->addRequirementsList( l ); - cDebug() << "Added" << l.count() << "requirement results"; + RequirementsList l = m->checkRequirements(); + cDebug() << "Got" << l.count() << "requirement results from" << m->name(); + if ( l.count() > 0 ) + { + m_model->addRequirementsList( l ); + } + + requirementsProgress( + tr( "Requirements checking for module %1 is complete." ).arg( m->name() ) ); emit requirementsResult( l ); } diff --git a/src/libcalamares/modulesystem/RequirementsChecker.h b/src/libcalamares/modulesystem/RequirementsChecker.h index b4651be21..45aa4fc4f 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.h +++ b/src/libcalamares/modulesystem/RequirementsChecker.h @@ -49,7 +49,7 @@ public Q_SLOTS: void run(); /// @brief Called when requirements are reported by a module - void addCheckedRequirements( RequirementsList ); + void addCheckedRequirements( Module* ); /// @brief Called when all requirements have been checked void finished(); From 153757933a0b21dc305696bfdbf560a830c40ed3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 15:53:29 +0200 Subject: [PATCH 08/11] [libcalamares] Stop emitting signals with RequirementsList - The architecture of letting someone build up a list of requirements from data emitted by the ModuleManager is broken: if it gets loaded later, it will miss data; passing around complicated objects is no fun anyway. Get rid of it, on the way to "ModuleManager has its own model of requirements". --- .../modulesystem/RequirementsChecker.cpp | 23 ------------------- .../modulesystem/RequirementsChecker.h | 7 ------ .../modulesystem/ModuleManager.cpp | 3 +-- .../modulesystem/ModuleManager.h | 1 - 4 files changed, 1 insertion(+), 33 deletions(-) diff --git a/src/libcalamares/modulesystem/RequirementsChecker.cpp b/src/libcalamares/modulesystem/RequirementsChecker.cpp index 9fa9a72f8..92e194818 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.cpp +++ b/src/libcalamares/modulesystem/RequirementsChecker.cpp @@ -33,25 +33,6 @@ namespace Calamares { -static void -registerMetatypes() -{ - static bool done = false; - - if ( !done ) - { - qRegisterMetaType< RequirementEntry >( "RequirementEntry" ); - // It's sensitive to the names of types in parameters; in particular - // althrough QList is the same as RequirementsList, - // because we *name* the type as RequirementsList in the parameters, - // we need to register that (as well). Here, be safe and register - // both names. - qRegisterMetaType< QList< RequirementEntry > >( "QList" ); - qRegisterMetaType< RequirementsList >( "RequirementsList" ); - done = true; - } -} - RequirementsChecker::RequirementsChecker( QVector< Module* > modules, RequirementsModel* model, QObject* parent ) : QObject( parent ) , m_modules( std::move( modules ) ) @@ -60,8 +41,6 @@ RequirementsChecker::RequirementsChecker( QVector< Module* > modules, Requiremen , m_progressTimeouts( 0 ) { m_watchers.reserve( m_modules.count() ); - - registerMetatypes(); } RequirementsChecker::~RequirementsChecker() {} @@ -104,7 +83,6 @@ RequirementsChecker::finished() } m_model->describe(); - emit requirementsComplete( m_model->satisfiedMandatory() ); QTimer::singleShot( 0, this, &RequirementsChecker::done ); } } @@ -121,7 +99,6 @@ RequirementsChecker::addCheckedRequirements( Module* m ) requirementsProgress( tr( "Requirements checking for module %1 is complete." ).arg( m->name() ) ); - emit requirementsResult( l ); } void diff --git a/src/libcalamares/modulesystem/RequirementsChecker.h b/src/libcalamares/modulesystem/RequirementsChecker.h index 45aa4fc4f..21c86f0a6 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.h +++ b/src/libcalamares/modulesystem/RequirementsChecker.h @@ -60,13 +60,6 @@ public Q_SLOTS: signals: /// @brief Human-readable progress message void requirementsProgress( const QString& ); - /// @brief Requirements from a single module - void requirementsResult( RequirementsList ); - /** @brief When all requirements are collected - * - * The argument indicates if all mandatory requirements are satisfied. - */ - void requirementsComplete( bool ); /// @brief Emitted after requirementsComplete void done(); diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 4734838ca..b6040371b 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -358,10 +358,9 @@ ModuleManager::checkRequirements() } RequirementsChecker* rq = new RequirementsChecker( modules, m_requirementsModel, this ); - connect( rq, &RequirementsChecker::requirementsResult, this, &ModuleManager::requirementsResult ); - connect( rq, &RequirementsChecker::requirementsComplete, this, &ModuleManager::requirementsComplete ); connect( rq, &RequirementsChecker::requirementsProgress, this, &ModuleManager::requirementsProgress ); connect( rq, &RequirementsChecker::done, rq, &RequirementsChecker::deleteLater ); + connect( rq, &RequirementsChecker::done, this, [=](){ this->requirementsComplete( m_requirementsModel->satisfiedMandatory() ); } ); QTimer::singleShot( 0, rq, &RequirementsChecker::run ); } diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index b0e537d9c..871a03a2a 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -97,7 +97,6 @@ signals: void modulesFailed( QStringList ); /// .. or not // Below, see RequirementsChecker documentation void requirementsComplete( bool ); - void requirementsResult( RequirementsList ); void requirementsProgress( const QString& ); private slots: From fabe5ec43917e32a5bbaacc4a5dba2ee7758ba7d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 16:18:55 +0200 Subject: [PATCH 09/11] [welcome] Config should not have its own RequirementsModel - Use the one from ModuleManager --- .../modulesystem/ModuleManager.h | 5 +++- src/modules/welcome/Config.cpp | 25 ++++++++----------- src/modules/welcome/Config.h | 9 +++---- src/modules/welcome/WelcomePage.cpp | 2 +- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index 871a03a2a..2c2685a3a 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -87,10 +87,13 @@ public: /** * @brief Starts asynchronous requirements checking for each module. - * When this is done, the signal modulesChecked is emitted. + * When this is done, the signal requirementsComplete is emitted. */ void checkRequirements(); + ///@brief Gets the model that requirements-checking works on. + RequirementsModel* requirementsModel() { return m_requirementsModel; } + signals: void initDone(); void modulesLoaded(); /// All of the modules were loaded successfully diff --git a/src/modules/welcome/Config.cpp b/src/modules/welcome/Config.cpp index 8ca20af7e..71d6de9bb 100644 --- a/src/modules/welcome/Config.cpp +++ b/src/modules/welcome/Config.cpp @@ -22,6 +22,7 @@ #include "Settings.h" #include "geoip/Handler.h" #include "locale/Lookup.h" +#include "modulesystem/ModuleManager.h" #include "utils/Logger.h" #include "utils/Retranslator.h" #include "utils/Variant.h" @@ -30,14 +31,8 @@ Config::Config( QObject* parent ) : QObject( parent ) - , m_requirementsModel( new Calamares::RequirementsModel( this ) ) , m_languages( CalamaresUtils::Locale::availableTranslations() ) { - connect( m_requirementsModel, - &Calamares::RequirementsModel::satisfiedRequirementsChanged, - this, - &Config::setIsNextEnabled ); - initLanguages(); CALAMARES_RETRANSLATE_SLOT( &Config::retranslate ) @@ -49,12 +44,13 @@ Config::retranslate() m_genericWelcomeMessage = genericWelcomeMessage().arg( Calamares::Branding::instance()->versionedName() ); emit genericWelcomeMessageChanged( m_genericWelcomeMessage ); - if ( !m_requirementsModel->satisfiedRequirements() ) + const auto* r = requirementsModel(); + if ( !r->satisfiedRequirements() ) { QString message; const bool setup = Calamares::Settings::instance()->isSetupMode(); - if ( !m_requirementsModel->satisfiedMandatory() ) + if ( !r->satisfiedMandatory() ) { message = setup ? tr( "This computer does not satisfy the minimum " "requirements for setting up %1.
" @@ -95,6 +91,13 @@ Config::languagesModel() const return m_languages; } +Calamares::RequirementsModel* +Config::requirementsModel() const +{ + return Calamares::ModuleManager::instance()->requirementsModel(); +} + + QString Config::languageIcon() const { @@ -183,12 +186,6 @@ Config::setLocaleIndex( int index ) emit localeIndexChanged( m_localeIndex ); } -Calamares::RequirementsModel& -Config::requirementsModel() const -{ - return *m_requirementsModel; -} - void Config::setIsNextEnabled( bool isNextEnabled ) { diff --git a/src/modules/welcome/Config.h b/src/modules/welcome/Config.h index 0123482f3..7fe6fa04e 100644 --- a/src/modules/welcome/Config.h +++ b/src/modules/welcome/Config.h @@ -20,7 +20,6 @@ #define WELCOME_CONFIG_H #include "locale/LabelModel.h" -#include "modulesystem/Requirement.h" #include "modulesystem/RequirementsModel.h" #include @@ -30,7 +29,7 @@ class Config : public QObject { Q_OBJECT Q_PROPERTY( CalamaresUtils::Locale::LabelModel* languagesModel READ languagesModel CONSTANT FINAL ) - Q_PROPERTY( Calamares::RequirementsModel* requirementsModel MEMBER m_requirementsModel CONSTANT FINAL ) + Q_PROPERTY( Calamares::RequirementsModel* requirementsModel READ requirementsModel CONSTANT FINAL ) Q_PROPERTY( QString languageIcon READ languageIcon CONSTANT FINAL ) @@ -52,8 +51,6 @@ public: void setConfigurationMap( const QVariantMap& ); - Calamares::RequirementsModel& requirementsModel() const; - void setCountryCode( const QString& countryCode ); QString languageIcon() const; @@ -83,6 +80,9 @@ public slots: CalamaresUtils::Locale::LabelModel* languagesModel() const; void retranslate(); + ///@brief The **global** requirements model, from ModuleManager + Calamares::RequirementsModel* requirementsModel() const; + signals: void countryCodeChanged( QString countryCode ); void localeIndexChanged( int localeIndex ); @@ -99,7 +99,6 @@ signals: private: void initLanguages(); - Calamares::RequirementsModel* m_requirementsModel; CalamaresUtils::Locale::LabelModel* m_languages; QString m_languageIcon; diff --git a/src/modules/welcome/WelcomePage.cpp b/src/modules/welcome/WelcomePage.cpp index dc955613c..2c6b4cc0e 100644 --- a/src/modules/welcome/WelcomePage.cpp +++ b/src/modules/welcome/WelcomePage.cpp @@ -47,7 +47,7 @@ WelcomePage::WelcomePage( Config* conf, QWidget* parent ) : QWidget( parent ) , ui( new Ui::WelcomePage ) - , m_checkingWidget( new CheckerContainer( conf->requirementsModel(), this ) ) + , m_checkingWidget( new CheckerContainer( *(conf->requirementsModel()), this ) ) , m_languages( nullptr ) , m_conf( conf ) { From 7d00f7e0dc2cc2e156e74b86520b45398d4bc1e9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 16:26:38 +0200 Subject: [PATCH 10/11] [welcome] Explain in the debug log what failed --- src/modules/welcome/checker/CheckerContainer.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/modules/welcome/checker/CheckerContainer.cpp b/src/modules/welcome/checker/CheckerContainer.cpp index 53e65fa04..f98b5b3d3 100644 --- a/src/modules/welcome/checker/CheckerContainer.cpp +++ b/src/modules/welcome/checker/CheckerContainer.cpp @@ -55,6 +55,16 @@ CheckerContainer::~CheckerContainer() void CheckerContainer::requirementsComplete( bool ok ) { + if ( !ok ) + { + cDebug() << "Requirements not satisfied" << m_model.count() << "entries:"; + for ( int i = 0; i < m_model.count(); ++i ) + { + auto index = m_model.index( i ); + cDebug() << Logger::SubEntry << i << m_model.data( index, Calamares::RequirementsModel::Name ).toString() + << m_model.data( index, Calamares::RequirementsModel::Satisfied ).toBool(); + } + } layout()->removeWidget( m_waitingWidget ); m_waitingWidget->deleteLater(); From 09b73dce0640760bfe8df088546c6932b48d00c0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 11 May 2020 17:10:03 +0200 Subject: [PATCH 11/11] [libcalamares] Implement the HasDetails role --- src/libcalamares/modulesystem/RequirementsModel.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcalamares/modulesystem/RequirementsModel.cpp b/src/libcalamares/modulesystem/RequirementsModel.cpp index dc1c3c3cb..eb2f892be 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.cpp +++ b/src/libcalamares/modulesystem/RequirementsModel.cpp @@ -69,6 +69,8 @@ RequirementsModel::data( const QModelIndex& index, int role ) const return requirement.satisfied; case Roles::Mandatory: return requirement.mandatory; + case Roles::HasDetails: + return requirement.hasDetails(); default: return QVariant(); } @@ -83,6 +85,7 @@ RequirementsModel::roleNames() const roles[ Roles::NegatedText ] = "negatedText"; roles[ Roles::Satisfied ] = "satisfied"; roles[ Roles::Mandatory ] = "mandatory"; + roles[ Roles::HasDetails ] = "hasDetails"; return roles; }