Merge branch 'requirements-model'

The requirements-checker in the Welcome module was not connected
to the module-manager's idea of what the requirements are, but
the *next* button was. So you could get in a situation where the
welcome modules' requirements were met, but **other** modules failed:
no display of the problem, and a disabled *next* button.

Rip out the welcome module's requirements-checking model, move it
to the module-manager, re-do the signals between the lot.

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 <QFuture>
@@ -32,47 +33,14 @@
 namespace Calamares
 {
 
-static void
+RequirementsChecker::RequirementsChecker( QVector< Module* > modules, RequirementsModel* model, QObject* parent )
-registerMetatypes()
-{
-    static bool done = false;
-
-    if ( !done )
-    {
-        qRegisterMetaType< RequirementEntry >( "RequirementEntry" );
-        // It's sensitive to the names of types in parameters; in particular
-        // althrough QList<RequirementEntry> 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<RequirementEntry>" );
-        qRegisterMetaType< RequirementsList >( "RequirementsList" );
-        done = true;
-    }
-}
-
-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 <i>%1</i> is complete." ).arg( m->name() ) );
-}
-
-RequirementsChecker::RequirementsChecker( QVector< Module* > modules, 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();
 }
 
 RequirementsChecker::~RequirementsChecker() {}
@@ -87,7 +55,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 );
@@ -114,33 +82,23 @@ RequirementsChecker::finished()
             m_progressTimer = nullptr;
         }
 
-        bool acceptable = true;
+        m_model->describe();
-        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 );
         QTimer::singleShot( 0, this, &RequirementsChecker::done );
     }
 }
 
 void
-RequirementsChecker::addCheckedRequirements( RequirementsList l )
+RequirementsChecker::addCheckedRequirements( Module* m )
 {
-    static QMutex addMutex;
+    RequirementsList l = m->checkRequirements();
+    cDebug() << "Got" << l.count() << "requirement results from" << m->name();
+    if ( l.count() > 0 )
     {
-        QMutexLocker lock( &addMutex );
+        m_model->addRequirementsList( l );
-        m_collectedRequirements.append( l );
     }
-    cDebug() << "Added" << l.count() << "requirement results";
+
-    emit requirementsResult( l );
+    requirementsProgress(
+        tr( "Requirements checking for module <i>%1</i> is complete." ).arg( m->name() ) );
 }
 
 void

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:
@@ -48,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();
@@ -59,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();
 
@@ -75,7 +69,7 @@ private:
     using Watcher = QFutureWatcher< void >;
     QVector< Watcher* > m_watchers;
 
-    RequirementsList m_collectedRequirements;
+    RequirementsModel* m_model;
 
     QTimer* m_progressTimer;
     unsigned m_progressTimeouts;

src/libcalamares/modulesystem/RequirementsModel.cpp

@@ -18,15 +18,24 @@
 
 #include "RequirementsModel.h"
 
+#include "utils/Logger.h"
+
 namespace Calamares
 {
 
 void
-RequirementsModel::setRequirementsList( const Calamares::RequirementsList& requirements )
+RequirementsModel::addRequirementsList( const Calamares::RequirementsList& requirements )
 {
+    QMutexLocker l( &m_addLock );
     emit beginResetModel();
-    m_requirements = requirements;
+    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 +44,6 @@ RequirementsModel::setRequirementsList( const Calamares::RequirementsList& requi
 
     emit satisfiedRequirementsChanged( m_satisfiedRequirements );
     emit satisfiedMandatoryChanged( m_satisfiedMandatory );
-    emit endResetModel();
 }
 
 int
@@ -61,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();
     }
@@ -75,7 +85,24 @@ RequirementsModel::roleNames() const
     roles[ Roles::NegatedText ] = "negatedText";
     roles[ Roles::Satisfied ] = "satisfied";
     roles[ Roles::Mandatory ] = "mandatory";
+    roles[ Roles::HasDetails ] = "hasDetails";
     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

src/libcalamares/modulesystem/RequirementsModel.h

@@ -24,12 +24,25 @@
 #include "DllMacro.h"
 
 #include <QAbstractListModel>
+#include <QMutex>
 
 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
+ * 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
 {
+    friend class RequirementsChecker;
+
     Q_OBJECT
     Q_PROPERTY( bool satisfiedRequirements READ satisfiedRequirements NOTIFY satisfiedRequirementsChanged FINAL )
     Q_PROPERTY( bool satisfiedMandatory READ satisfiedMandatory NOTIFY satisfiedMandatoryChanged FINAL )
@@ -48,20 +61,18 @@ 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; }
 
-    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;
     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 );
@@ -69,11 +80,17 @@ signals:
 protected:
     QHash< int, QByteArray > roleNames() const override;
 
+    ///@brief Append some requirements; resets the model
+    void addRequirementsList( const Calamares::RequirementsList& requirements );
+
 private:
-    Calamares::RequirementsList m_requirements;
+    ///@brief Implementation for {set,add}RequirementsList
+    void changeRequirementsList();
+
+    QMutex m_addLock;
+    RequirementsList m_requirements;
     bool m_satisfiedRequirements = false;
     bool m_satisfiedMandatory = false;
-
 };
 
 }  // namespace Calamares

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,11 +357,10 @@ 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 );
     connect( rq, &RequirementsChecker::done, rq, &RequirementsChecker::deleteLater );
+    connect( rq, &RequirementsChecker::done, this, [=](){ this->requirementsComplete( m_requirementsModel->satisfiedMandatory() ); } );
 
     QTimer::singleShot( 0, rq, &RequirementsChecker::run );
 }

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.
@@ -87,17 +87,19 @@ 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
     void modulesFailed( QStringList );  /// .. or not
     // Below, see RequirementsChecker documentation
     void requirementsComplete( bool );
-    void requirementsResult( RequirementsList );
     void requirementsProgress( const QString& );
 
 private slots:
@@ -130,6 +132,7 @@ private:
     QMap< QString, QString > m_moduleDirectoriesByModuleName;
     QMap< ModuleSystem::InstanceKey, Module* > m_loadedModulesByInstanceKey;
     const QStringList m_paths;
+    RequirementsModel* m_requirementsModel;
 
     static ModuleManager* s_instance;
 };

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.<br/>"
@@ -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 )
 {

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 <QObject>
@@ -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;

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 )
 {

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();

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();

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 );
+        const auto& index = model.index( i );
-        if ( !predicate( entry ) )
+        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() );
         }
     }
 

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 "