From b8dd6e9ae73eb348fe786871cedfdb55610de156 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 30 Jul 2019 13:38:21 +0200 Subject: [PATCH 01/10] [libcalamaresui] Introduce a module-instance-key class - This replaces rather ad-hoc use of a QString as key. --- .../modulesystem/ModuleManager.h | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index 8412e51d5..3a7535de4 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -32,6 +32,41 @@ namespace Calamares class Module; struct RequirementEntry; // from Requirement.h +/** @brief A module instance's key (`module@id`) + * + * A module instance is identified by both the module's name + * (a Calamares module, e.g. `users`) and an instance id. + * Usually, the instance id is the same as the module name + * and the whole module instance key is `users@users`, but + * it is possible to use the same module more than once + * and then you distinguish those module instances by their + * secondary id (e.g. `users@one`). + * + * This is supported by the *instances* configuration entry + * in `settings.conf`. + */ +class ModuleInstanceKey : protected QPair< QString, QString > +{ +public: + /// @brief Create an instance key from explicit module and id. + ModuleInstanceKey( const QString& module, const QString& id ) + : QPair( module, id ) + { + } + + /// @brief Create "usual" instances keys `module@module` + ModuleInstanceKey( const QString& module ) + : QPair( module, module ) + { + } + + QString module() const { return first; } + QString id() const { return second; } + + explicit operator QString() const { return module() + '@' + id(); } +}; + + /** * @brief The ModuleManager class is a singleton which manages Calamares modules. * From 78de6776af621fae4a69ab1f7f5ed7c95f06e779 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 30 Jul 2019 14:32:33 +0200 Subject: [PATCH 02/10] [libcalamaresui] Swap out use of QString for ModuleInstanceKey - The strings `module@id` are used internally, make that type explicit. --- .../modulesystem/ModuleManager.cpp | 68 ++++++++++--------- .../modulesystem/ModuleManager.h | 35 +++++++++- 2 files changed, 67 insertions(+), 36 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index a65f64108..487cb47c4 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -137,7 +137,12 @@ ModuleManager::doInit() QStringList ModuleManager::loadedInstanceKeys() { - return m_loadedModulesByInstanceKey.keys(); + QStringList l; + for ( const auto& m : m_loadedModulesByInstanceKey.keys() ) + { + l << QString( m ); + } + return l; } @@ -150,7 +155,7 @@ ModuleManager::moduleDescriptor( const QString& name ) Module* ModuleManager::moduleInstance( const QString& instanceKey ) { - return m_loadedModulesByInstanceKey.value( instanceKey ); + return m_loadedModulesByInstanceKey.value( ModuleInstanceKey::fromString( instanceKey ) ); } @@ -160,12 +165,12 @@ ModuleManager::moduleInstance( const QString& instanceKey ) * @return -1 on failure, otherwise index of the instance that matches. */ static int -findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const QString& module, const QString& id ) +findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const ModuleInstanceKey& m ) { for ( int i = 0; i < customInstances.count(); ++i ) { const auto& thisInstance = customInstances[ i ]; - if ( thisInstance.value( "module" ) == module && thisInstance.value( "id" ) == id ) + if ( thisInstance.value( "module" ) == m.module() && thisInstance.value( "id" ) == m.id() ) { return i; } @@ -189,32 +194,28 @@ ModuleManager::loadModules() foreach ( const QString& moduleEntry, modulePhase.second ) { - QStringList moduleEntrySplit = moduleEntry.split( '@' ); - QString moduleName; - QString instanceId; - QString configFileName; - if ( moduleEntrySplit.length() < 1 || moduleEntrySplit.length() > 2 ) + auto instanceKey = ModuleInstanceKey::fromString( moduleEntry ); + if ( !instanceKey.isValid() ) { cError() << "Wrong module entry format for module" << moduleEntry; failedModules.append( moduleEntry ); continue; } - moduleName = moduleEntrySplit.first(); - instanceId = moduleEntrySplit.last(); - configFileName = QString( "%1.conf" ).arg( moduleName ); - if ( !m_availableDescriptorsByModuleName.contains( moduleName ) - || m_availableDescriptorsByModuleName.value( moduleName ).isEmpty() ) + + if ( !m_availableDescriptorsByModuleName.contains( instanceKey.module() ) + || m_availableDescriptorsByModuleName.value( instanceKey.module() ).isEmpty() ) { - cError() << "Module" << moduleName << "not found in module search paths." + cError() << "Module" << QString( instanceKey ) << "not found in module search paths." << Logger::DebugList( m_paths ); - failedModules.append( moduleName ); + failedModules.append( QString( instanceKey ) ); continue; } - if ( moduleName != instanceId ) //means this is a custom instance + QString configFileName; + if ( instanceKey.isCustom() ) { - int found = findCustomInstance( customInstances, moduleName, instanceId ); + int found = findCustomInstance( customInstances, instanceKey ); if ( found > -1 ) { @@ -227,6 +228,10 @@ ModuleManager::loadModules() continue; } } + else + { + configFileName = QString( "%1.conf" ).arg( instanceKey.module() ); + } // So now we can assume that the module entry is at least valid, // that we have a descriptor on hand (and therefore that the @@ -235,38 +240,35 @@ ModuleManager::loadModules() // We still don't know whether the config file for the entry // exists and is valid, but that's the only thing that could fail // from this point on. -- Teo 8/2015 - - QString instanceKey = QString( "%1@%2" ).arg( moduleName ).arg( instanceId ); - Module* thisModule = m_loadedModulesByInstanceKey.value( instanceKey, nullptr ); if ( thisModule && !thisModule->isLoaded() ) { - cError() << "Module" << instanceKey << "exists but not loaded."; - failedModules.append( instanceKey ); + cError() << "Module" << QString( instanceKey ) << "exists but not loaded."; + failedModules.append( QString( instanceKey ) ); continue; } if ( thisModule && thisModule->isLoaded() ) { - cDebug() << "Module" << instanceKey << "already loaded."; + cDebug() << "Module" << QString( instanceKey ) << "already loaded."; } else { - thisModule = Module::fromDescriptor( m_availableDescriptorsByModuleName.value( moduleName ), - instanceId, + thisModule = Module::fromDescriptor( m_availableDescriptorsByModuleName.value( instanceKey.module() ), + instanceKey.id(), configFileName, - m_moduleDirectoriesByModuleName.value( moduleName ) ); + m_moduleDirectoriesByModuleName.value( instanceKey.module() ) ); if ( !thisModule ) { - cError() << "Module" << instanceKey << "cannot be created from descriptor" << configFileName; - failedModules.append( instanceKey ); + cError() << "Module" << QString( instanceKey ) << "cannot be created from descriptor" << configFileName; + failedModules.append( QString( instanceKey ) ); continue; } if ( !checkDependencies( *thisModule ) ) { // Error message is already printed - failedModules.append( instanceKey ); + failedModules.append( QString( instanceKey ) ); continue; } @@ -275,8 +277,8 @@ ModuleManager::loadModules() m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); if ( !thisModule->isLoaded() ) { - cError() << "Module" << instanceKey << "loading FAILED."; - failedModules.append( instanceKey ); + cError() << "Module" << QString( instanceKey ) << "loading FAILED."; + failedModules.append( QString( instanceKey ) ); continue; } } @@ -293,7 +295,7 @@ ModuleManager::loadModules() ViewManager::instance()->addViewStep( evs ); } - evs->appendJobModuleInstanceKey( instanceKey ); + evs->appendJobModuleInstanceKey( QString( instanceKey ) ); } } } diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index 3a7535de4..ed314c7e0 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -45,25 +45,54 @@ struct RequirementEntry; // from Requirement.h * This is supported by the *instances* configuration entry * in `settings.conf`. */ -class ModuleInstanceKey : protected QPair< QString, QString > +class ModuleInstanceKey : public QPair< QString, QString > { public: /// @brief Create an instance key from explicit module and id. ModuleInstanceKey( const QString& module, const QString& id ) : QPair( module, id ) { + if ( second.isEmpty() ) + { + second = first; + } } /// @brief Create "usual" instances keys `module@module` - ModuleInstanceKey( const QString& module ) + explicit ModuleInstanceKey( const QString& module ) : QPair( module, module ) { } + /// @brief Create unusual, invalid instance key + ModuleInstanceKey() + : QPair( QString(), QString() ) + { + } + + /// @brief A valid module has both name and id + bool isValid() const { return !first.isEmpty() && !second.isEmpty(); } + + /// @brief A custom module has a non-default id + bool isCustom() const { return first != second; } + QString module() const { return first; } QString id() const { return second; } explicit operator QString() const { return module() + '@' + id(); } + + /// @brief Create instance key from stringified version + static ModuleInstanceKey fromString( const QString& s ) + { + QStringList moduleEntrySplit = s.split( '@' ); + if ( moduleEntrySplit.length() < 1 || moduleEntrySplit.length() > 2 ) + { + return ModuleInstanceKey(); + } + // For length 1, first == last + return ModuleInstanceKey( moduleEntrySplit.first(), moduleEntrySplit.last() ); + } + }; @@ -158,7 +187,7 @@ private: QMap< QString, QVariantMap > m_availableDescriptorsByModuleName; QMap< QString, QString > m_moduleDirectoriesByModuleName; - QMap< QString, Module* > m_loadedModulesByInstanceKey; + QMap< ModuleInstanceKey, Module* > m_loadedModulesByInstanceKey; const QStringList m_paths; static ModuleManager* s_instance; From 666462651b74c26f3ebf0efe470b5d77c4745573 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 08:18:56 -0400 Subject: [PATCH 03/10] [libcalamares] Namespace consistently - Things in libcalamares/ subdirectories are namespaced according to that subdirectory (sometimes in namespace Calamares, sometimes CalamaresUtils). Do that in modulesystem/ too. --- src/libcalamares/Settings.cpp | 6 +++--- src/libcalamares/Settings.h | 2 +- src/libcalamares/modulesystem/Actions.h | 5 ++++- src/libcalamaresui/modulesystem/ModuleManager.cpp | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index b55292bd4..456956430 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -169,14 +169,14 @@ interpretSequence( const YAML::Node& node, Settings::ModuleSequence& moduleSeque continue; } QString thisActionS = sequenceVListItem.toMap().firstKey(); - ModuleAction thisAction; + ModuleSystem::Action thisAction; if ( thisActionS == "show" ) { - thisAction = ModuleAction::Show; + thisAction = ModuleSystem::Action::Show; } else if ( thisActionS == "exec" ) { - thisAction = ModuleAction::Exec; + thisAction = ModuleSystem::Action::Exec; } else { diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index a52eac82e..4c2f2ed9d 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -46,7 +46,7 @@ public: using InstanceDescriptionList = QList< InstanceDescription >; InstanceDescriptionList customModuleInstances() const; - using ModuleSequence = QList< QPair< ModuleAction, QStringList > >; + using ModuleSequence = QList< QPair< ModuleSystem::Action, QStringList > >; ModuleSequence modulesSequence() const; QString brandingComponentName() const; diff --git a/src/libcalamares/modulesystem/Actions.h b/src/libcalamares/modulesystem/Actions.h index 06b5589c4..e1be0b867 100644 --- a/src/libcalamares/modulesystem/Actions.h +++ b/src/libcalamares/modulesystem/Actions.h @@ -22,13 +22,16 @@ namespace Calamares { +namespace ModuleSystem +{ -enum class ModuleAction : char +enum class Action : char { Show, Exec }; +} // namespace ModuleSystem } // namespace Calamares #endif diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 487cb47c4..5bbfbe50a 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -190,7 +190,7 @@ ModuleManager::loadModules() = failedModules.isEmpty() ? Settings::instance()->modulesSequence() : Settings::ModuleSequence(); for ( const auto& modulePhase : modulesSequence ) { - ModuleAction currentAction = modulePhase.first; + ModuleSystem::Action currentAction = modulePhase.first; foreach ( const QString& moduleEntry, modulePhase.second ) { @@ -285,7 +285,7 @@ ModuleManager::loadModules() // At this point we most certainly have a pointer to a loaded module in // thisModule. We now need to enqueue jobs info into an EVS. - if ( currentAction == ModuleAction::Exec ) + if ( currentAction == ModuleSystem::Action::Exec ) { ExecutionViewStep* evs = qobject_cast< ExecutionViewStep* >( Calamares::ViewManager::instance()->viewSteps().last() ); From 2f99004041c1f71f0f5ef110e977208bf061b41b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 08:50:00 -0400 Subject: [PATCH 04/10] [libcalamares] Move the module instance-key - Split out of the UI library and into (header-only) libcalamares. --- src/libcalamares/modulesystem/InstanceKey.h | 96 +++++++++++++++++++ .../modulesystem/ModuleManager.cpp | 6 +- .../modulesystem/ModuleManager.h | 68 +------------ 3 files changed, 102 insertions(+), 68 deletions(-) create mode 100644 src/libcalamares/modulesystem/InstanceKey.h diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h new file mode 100644 index 000000000..8e04dbaef --- /dev/null +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -0,0 +1,96 @@ +/* === This file is part of Calamares - === + * + * Copyright 2014-2015, Teo Mrnjavac + * Copyright 2018-2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ +#ifndef MODULESYSTEM_INSTANCEKEY_H +#define MODULESYSTEM_INSTANCEKEY_H + +#include +#include +#include + +namespace Calamares +{ +namespace ModuleSystem +{ + +/** @brief A module instance's key (`module@id`) + * + * A module instance is identified by both the module's name + * (a Calamares module, e.g. `users`) and an instance id. + * Usually, the instance id is the same as the module name + * and the whole module instance key is `users@users`, but + * it is possible to use the same module more than once + * and then you distinguish those module instances by their + * secondary id (e.g. `users@one`). + * + * This is supported by the *instances* configuration entry + * in `settings.conf`. + */ +class InstanceKey : public QPair< QString, QString > +{ +public: + /// @brief Create an instance key from explicit module and id. + InstanceKey( const QString& module, const QString& id ) + : QPair( module, id ) + { + if ( second.isEmpty() ) + { + second = first; + } + } + + /// @brief Create "usual" instances keys `module@module` + explicit InstanceKey( const QString& module ) + : QPair( module, module ) + { + } + + /// @brief Create unusual, invalid instance key + InstanceKey() + : QPair( QString(), QString() ) + { + } + + /// @brief A valid module has both name and id + bool isValid() const { return !first.isEmpty() && !second.isEmpty(); } + + /// @brief A custom module has a non-default id + bool isCustom() const { return first != second; } + + QString module() const { return first; } + QString id() const { return second; } + + explicit operator QString() const { return module() + '@' + id(); } + + /// @brief Create instance key from stringified version + static InstanceKey fromString( const QString& s ) + { + QStringList moduleEntrySplit = s.split( '@' ); + if ( moduleEntrySplit.length() < 1 || moduleEntrySplit.length() > 2 ) + { + return InstanceKey(); + } + // For length 1, first == last + return InstanceKey( moduleEntrySplit.first(), moduleEntrySplit.last() ); + } + +}; + +} +} +#endif diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 5bbfbe50a..79ad25325 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -155,7 +155,7 @@ ModuleManager::moduleDescriptor( const QString& name ) Module* ModuleManager::moduleInstance( const QString& instanceKey ) { - return m_loadedModulesByInstanceKey.value( ModuleInstanceKey::fromString( instanceKey ) ); + return m_loadedModulesByInstanceKey.value( ModuleSystem::InstanceKey::fromString( instanceKey ) ); } @@ -165,7 +165,7 @@ ModuleManager::moduleInstance( const QString& instanceKey ) * @return -1 on failure, otherwise index of the instance that matches. */ static int -findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const ModuleInstanceKey& m ) +findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const ModuleSystem::InstanceKey& m ) { for ( int i = 0; i < customInstances.count(); ++i ) { @@ -194,7 +194,7 @@ ModuleManager::loadModules() foreach ( const QString& moduleEntry, modulePhase.second ) { - auto instanceKey = ModuleInstanceKey::fromString( moduleEntry ); + auto instanceKey = ModuleSystem::InstanceKey::fromString( moduleEntry ); if ( !instanceKey.isValid() ) { cError() << "Wrong module entry format for module" << moduleEntry; diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index ed314c7e0..5fe239ae1 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -20,6 +20,8 @@ #ifndef MODULELOADER_H #define MODULELOADER_H +#include "modulesystem/InstanceKey.h" + #include "Requirement.h" #include @@ -32,70 +34,6 @@ namespace Calamares class Module; struct RequirementEntry; // from Requirement.h -/** @brief A module instance's key (`module@id`) - * - * A module instance is identified by both the module's name - * (a Calamares module, e.g. `users`) and an instance id. - * Usually, the instance id is the same as the module name - * and the whole module instance key is `users@users`, but - * it is possible to use the same module more than once - * and then you distinguish those module instances by their - * secondary id (e.g. `users@one`). - * - * This is supported by the *instances* configuration entry - * in `settings.conf`. - */ -class ModuleInstanceKey : public QPair< QString, QString > -{ -public: - /// @brief Create an instance key from explicit module and id. - ModuleInstanceKey( const QString& module, const QString& id ) - : QPair( module, id ) - { - if ( second.isEmpty() ) - { - second = first; - } - } - - /// @brief Create "usual" instances keys `module@module` - explicit ModuleInstanceKey( const QString& module ) - : QPair( module, module ) - { - } - - /// @brief Create unusual, invalid instance key - ModuleInstanceKey() - : QPair( QString(), QString() ) - { - } - - /// @brief A valid module has both name and id - bool isValid() const { return !first.isEmpty() && !second.isEmpty(); } - - /// @brief A custom module has a non-default id - bool isCustom() const { return first != second; } - - QString module() const { return first; } - QString id() const { return second; } - - explicit operator QString() const { return module() + '@' + id(); } - - /// @brief Create instance key from stringified version - static ModuleInstanceKey fromString( const QString& s ) - { - QStringList moduleEntrySplit = s.split( '@' ); - if ( moduleEntrySplit.length() < 1 || moduleEntrySplit.length() > 2 ) - { - return ModuleInstanceKey(); - } - // For length 1, first == last - return ModuleInstanceKey( moduleEntrySplit.first(), moduleEntrySplit.last() ); - } - -}; - - /** * @brief The ModuleManager class is a singleton which manages Calamares modules. * @@ -187,7 +125,7 @@ private: QMap< QString, QVariantMap > m_availableDescriptorsByModuleName; QMap< QString, QString > m_moduleDirectoriesByModuleName; - QMap< ModuleInstanceKey, Module* > m_loadedModulesByInstanceKey; + QMap< ModuleSystem::InstanceKey, Module* > m_loadedModulesByInstanceKey; const QStringList m_paths; static ModuleManager* s_instance; From ba7e96c5e1ba8da852a7455cdeb58cfd51e04353 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 09:21:37 -0400 Subject: [PATCH 05/10] [libcalamares] Add test for InstanceKey - cover all the constructors - Start with some tests that fail, showing bugs in the implementation - Fix bug that "derp@derp" was creating a valid instance-key with a bad module and id (need to use ::fromString() to get that functionality). - Extend tests with more bad cases. - Refactor tests to simplify "this is bad" assertions. --- src/libcalamares/CMakeLists.txt | 10 ++ src/libcalamares/modulesystem/InstanceKey.h | 12 ++ src/libcalamares/modulesystem/Tests.cpp | 129 ++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 src/libcalamares/modulesystem/Tests.cpp diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 87338ae6c..f942dbde1 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -207,6 +207,16 @@ if ( ECM_FOUND AND BUILD_TESTING ) Qt5::Test ) calamares_automoc( libcalamaresnetworktest ) + + ecm_add_test( + modulesystem/Tests.cpp + TEST_NAME + libcalamaresmodulesystemtest + LINK_LIBRARIES + calamares + Qt5::Test + ) + calamares_automoc( libcalamaresmodulesystemtest ) endif() if( BUILD_TESTING ) diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h index 8e04dbaef..80be23941 100644 --- a/src/libcalamares/modulesystem/InstanceKey.h +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -52,12 +52,14 @@ public: { second = first; } + validate(); } /// @brief Create "usual" instances keys `module@module` explicit InstanceKey( const QString& module ) : QPair( module, module ) { + validate(); } /// @brief Create unusual, invalid instance key @@ -89,6 +91,16 @@ public: return InstanceKey( moduleEntrySplit.first(), moduleEntrySplit.last() ); } +private: + /** @brief Check validity and reset module and id if needed. */ + void validate() + { + if ( first.contains( '@' ) || second.contains( '@' ) ) + { + first = QString(); + second = QString(); + } + } }; } diff --git a/src/libcalamares/modulesystem/Tests.cpp b/src/libcalamares/modulesystem/Tests.cpp new file mode 100644 index 000000000..b54d020c5 --- /dev/null +++ b/src/libcalamares/modulesystem/Tests.cpp @@ -0,0 +1,129 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "modulesystem/InstanceKey.h" + +#include + +using Calamares::ModuleSystem::InstanceKey; + +class ModuleSystemTests : public QObject +{ + Q_OBJECT +public: + ModuleSystemTests() {} + virtual ~ModuleSystemTests() {} + +private Q_SLOTS: + void initTestCase(); + + void testEmptyInstanceKey(); + void testSimpleInstanceKey(); + void testCustomInstanceKey(); + void testFromStringInstanceKey(); + + void testBadSimpleCases(); + void testBadFromStringCases(); +}; + +void ModuleSystemTests::initTestCase() +{ +} + +void assert_is_invalid( const InstanceKey& k ) +{ + QVERIFY( !k.isValid() ); + QVERIFY( !k.isCustom() ); + QVERIFY( k.module().isEmpty() ); + QVERIFY( k.id().isEmpty() ); +} + +void ModuleSystemTests::testEmptyInstanceKey() +{ + InstanceKey k0; + assert_is_invalid( k0 ); +} + +void ModuleSystemTests::testSimpleInstanceKey() +{ + InstanceKey k1( "derp" ); + QVERIFY( k1.isValid() ); + QVERIFY( !k1.isCustom() ); + QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k1.id(), QStringLiteral( "derp" ) ); +} + +void ModuleSystemTests::testCustomInstanceKey() +{ + InstanceKey k0("derp", "derp"); + QVERIFY( k0.isValid() ); + QVERIFY( !k0.isCustom() ); + QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); + + InstanceKey k1("derp", "horse"); + QVERIFY( k1.isValid() ); + QVERIFY( k1.isCustom() ); + QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); +} + +void ModuleSystemTests::testFromStringInstanceKey() +{ + InstanceKey k0 = InstanceKey::fromString( "derp@derp" ); + QVERIFY( k0.isValid() ); + QVERIFY( !k0.isCustom() ); + QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); + + InstanceKey k1 = InstanceKey::fromString( "derp@horse" ); + QVERIFY( k1.isValid() ); + QVERIFY( k1.isCustom() ); + QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); + + InstanceKey k2 = InstanceKey::fromString( "derp" ); + QVERIFY( k2.isValid() ); + QVERIFY( !k2.isCustom() ); + QCOMPARE( k2.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k2.id(), QStringLiteral( "derp" ) ); +} + +/// @brief These are expected to fail since they show bugs in the code +void ModuleSystemTests::testBadSimpleCases() +{ + InstanceKey k2( "derp@derp" ); + assert_is_invalid( k2 ); + + InstanceKey k3( "derp@horse" ); + assert_is_invalid( k3 ); +} + +void ModuleSystemTests::testBadFromStringCases() +{ + InstanceKey k0 = InstanceKey::fromString( QString() ); + assert_is_invalid( k0 ); + + k0 = InstanceKey::fromString( "derp@derp@derp" ); + assert_is_invalid( k0 ); +} + + +QTEST_GUILESS_MAIN( ModuleSystemTests ) + +#include "Tests.moc" From 57e4b66af23e5c140e5f71f7e52f4de71cf7bcdf Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 11:14:54 -0400 Subject: [PATCH 06/10] [libcalamares] Test QString() operator - An invalid InstanceKey should give an empty string - Test remaining QString() cases - Edge cases for 2-string constructor --- src/libcalamares/modulesystem/Tests.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/libcalamares/modulesystem/Tests.cpp b/src/libcalamares/modulesystem/Tests.cpp index b54d020c5..8c067a090 100644 --- a/src/libcalamares/modulesystem/Tests.cpp +++ b/src/libcalamares/modulesystem/Tests.cpp @@ -51,6 +51,7 @@ void assert_is_invalid( const InstanceKey& k ) QVERIFY( !k.isCustom() ); QVERIFY( k.module().isEmpty() ); QVERIFY( k.id().isEmpty() ); + QVERIFY( QString( k ).isEmpty() ); } void ModuleSystemTests::testEmptyInstanceKey() @@ -66,6 +67,7 @@ void ModuleSystemTests::testSimpleInstanceKey() QVERIFY( !k1.isCustom() ); QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); QCOMPARE( k1.id(), QStringLiteral( "derp" ) ); + QCOMPARE( QString( k1 ), QStringLiteral( "derp@derp" ) ); } void ModuleSystemTests::testCustomInstanceKey() @@ -75,12 +77,21 @@ void ModuleSystemTests::testCustomInstanceKey() QVERIFY( !k0.isCustom() ); QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); + QCOMPARE( QString( k0 ), QStringLiteral( "derp@derp" ) ); InstanceKey k1("derp", "horse"); QVERIFY( k1.isValid() ); QVERIFY( k1.isCustom() ); QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); + QCOMPARE( QString( k1 ), QStringLiteral( "derp@horse" ) ); + + InstanceKey k4( "derp", QString() ); + QVERIFY( k4.isValid() ); + QVERIFY( !k4.isCustom() ); + QCOMPARE( k4.module(), QStringLiteral( "derp" ) ); + QCOMPARE( k4.id(), QStringLiteral( "derp" ) ); + QCOMPARE( QString( k4 ), QStringLiteral( "derp@derp" ) ); } void ModuleSystemTests::testFromStringInstanceKey() @@ -90,18 +101,21 @@ void ModuleSystemTests::testFromStringInstanceKey() QVERIFY( !k0.isCustom() ); QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); + QCOMPARE( QString( k0 ), QStringLiteral( "derp@derp" ) ); InstanceKey k1 = InstanceKey::fromString( "derp@horse" ); QVERIFY( k1.isValid() ); QVERIFY( k1.isCustom() ); QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); + QCOMPARE( QString( k1 ), QStringLiteral( "derp@horse" ) ); InstanceKey k2 = InstanceKey::fromString( "derp" ); QVERIFY( k2.isValid() ); QVERIFY( !k2.isCustom() ); QCOMPARE( k2.module(), QStringLiteral( "derp" ) ); QCOMPARE( k2.id(), QStringLiteral( "derp" ) ); + QCOMPARE( QString( k2 ), QStringLiteral( "derp@derp" ) ); } /// @brief These are expected to fail since they show bugs in the code @@ -112,6 +126,9 @@ void ModuleSystemTests::testBadSimpleCases() InstanceKey k3( "derp@horse" ); assert_is_invalid( k3 ); + + InstanceKey k4( "derp", "derp@derp" ); + assert_is_invalid( k4 ); } void ModuleSystemTests::testBadFromStringCases() From 7dcc6e8e07151007f1cc420933ce68485a769ebd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 11:16:01 -0400 Subject: [PATCH 07/10] [libcalamares] Fix bug in InstanceKey::QString --- src/libcalamares/modulesystem/InstanceKey.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h index 80be23941..d5d4d26ff 100644 --- a/src/libcalamares/modulesystem/InstanceKey.h +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -77,7 +77,7 @@ public: QString module() const { return first; } QString id() const { return second; } - explicit operator QString() const { return module() + '@' + id(); } + explicit operator QString() const { return isValid() ? module() + '@' + id() : QString(); } /// @brief Create instance key from stringified version static InstanceKey fromString( const QString& s ) From ce6f6592d4d1acf6ec58092cdfc44542c2b2cbe4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 13:03:25 -0400 Subject: [PATCH 08/10] [libcalamares] Drop parts of InstanceKey API - Drop the 1-argument QString constructor, it is suprising - Drop the conversion to QString - Add a toString() instead - Drop tests for the removed API - While here, apply code formatting to the tests This is done to force consumers to update to strongly-typed InstanceKeys. --- src/libcalamares/modulesystem/InstanceKey.h | 27 ++++++----- src/libcalamares/modulesystem/Tests.cpp | 53 ++++++++------------- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h index d5d4d26ff..35ad27c40 100644 --- a/src/libcalamares/modulesystem/InstanceKey.h +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -19,10 +19,16 @@ #ifndef MODULESYSTEM_INSTANCEKEY_H #define MODULESYSTEM_INSTANCEKEY_H -#include +#include #include +#include #include +namespace Logger +{ +class CLog; +} + namespace Calamares { namespace ModuleSystem @@ -55,13 +61,6 @@ public: validate(); } - /// @brief Create "usual" instances keys `module@module` - explicit InstanceKey( const QString& module ) - : QPair( module, module ) - { - validate(); - } - /// @brief Create unusual, invalid instance key InstanceKey() : QPair( QString(), QString() ) @@ -77,8 +76,6 @@ public: QString module() const { return first; } QString id() const { return second; } - explicit operator QString() const { return isValid() ? module() + '@' + id() : QString(); } - /// @brief Create instance key from stringified version static InstanceKey fromString( const QString& s ) { @@ -91,6 +88,11 @@ public: return InstanceKey( moduleEntrySplit.first(), moduleEntrySplit.last() ); } + QString toString() const + { + return first + '@' + second; + } + private: /** @brief Check validity and reset module and id if needed. */ void validate() @@ -103,6 +105,7 @@ private: } }; -} -} +} // namespace ModuleSystem +} // namespace Calamares + #endif diff --git a/src/libcalamares/modulesystem/Tests.cpp b/src/libcalamares/modulesystem/Tests.cpp index 8c067a090..8ef62a098 100644 --- a/src/libcalamares/modulesystem/Tests.cpp +++ b/src/libcalamares/modulesystem/Tests.cpp @@ -33,7 +33,6 @@ private Q_SLOTS: void initTestCase(); void testEmptyInstanceKey(); - void testSimpleInstanceKey(); void testCustomInstanceKey(); void testFromStringInstanceKey(); @@ -41,97 +40,85 @@ private Q_SLOTS: void testBadFromStringCases(); }; -void ModuleSystemTests::initTestCase() +void +ModuleSystemTests::initTestCase() { } -void assert_is_invalid( const InstanceKey& k ) +void +assert_is_invalid( const InstanceKey& k ) { QVERIFY( !k.isValid() ); QVERIFY( !k.isCustom() ); QVERIFY( k.module().isEmpty() ); QVERIFY( k.id().isEmpty() ); - QVERIFY( QString( k ).isEmpty() ); + QVERIFY( k.toString().isEmpty() ); } -void ModuleSystemTests::testEmptyInstanceKey() +void +ModuleSystemTests::testEmptyInstanceKey() { InstanceKey k0; assert_is_invalid( k0 ); } -void ModuleSystemTests::testSimpleInstanceKey() +void +ModuleSystemTests::testCustomInstanceKey() { - InstanceKey k1( "derp" ); - QVERIFY( k1.isValid() ); - QVERIFY( !k1.isCustom() ); - QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); - QCOMPARE( k1.id(), QStringLiteral( "derp" ) ); - QCOMPARE( QString( k1 ), QStringLiteral( "derp@derp" ) ); -} - -void ModuleSystemTests::testCustomInstanceKey() -{ - InstanceKey k0("derp", "derp"); + InstanceKey k0( "derp", "derp" ); QVERIFY( k0.isValid() ); QVERIFY( !k0.isCustom() ); QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); - QCOMPARE( QString( k0 ), QStringLiteral( "derp@derp" ) ); + QCOMPARE( k0.toString(), QStringLiteral( "derp@derp" ) ); - InstanceKey k1("derp", "horse"); + InstanceKey k1( "derp", "horse" ); QVERIFY( k1.isValid() ); QVERIFY( k1.isCustom() ); QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); - QCOMPARE( QString( k1 ), QStringLiteral( "derp@horse" ) ); + QCOMPARE( k1.toString(), QStringLiteral( "derp@horse" ) ); InstanceKey k4( "derp", QString() ); QVERIFY( k4.isValid() ); QVERIFY( !k4.isCustom() ); QCOMPARE( k4.module(), QStringLiteral( "derp" ) ); QCOMPARE( k4.id(), QStringLiteral( "derp" ) ); - QCOMPARE( QString( k4 ), QStringLiteral( "derp@derp" ) ); + QCOMPARE( k4.toString(), QStringLiteral( "derp@derp" ) ); } -void ModuleSystemTests::testFromStringInstanceKey() +void +ModuleSystemTests::testFromStringInstanceKey() { InstanceKey k0 = InstanceKey::fromString( "derp@derp" ); QVERIFY( k0.isValid() ); QVERIFY( !k0.isCustom() ); QCOMPARE( k0.module(), QStringLiteral( "derp" ) ); QCOMPARE( k0.id(), QStringLiteral( "derp" ) ); - QCOMPARE( QString( k0 ), QStringLiteral( "derp@derp" ) ); InstanceKey k1 = InstanceKey::fromString( "derp@horse" ); QVERIFY( k1.isValid() ); QVERIFY( k1.isCustom() ); QCOMPARE( k1.module(), QStringLiteral( "derp" ) ); QCOMPARE( k1.id(), QStringLiteral( "horse" ) ); - QCOMPARE( QString( k1 ), QStringLiteral( "derp@horse" ) ); InstanceKey k2 = InstanceKey::fromString( "derp" ); QVERIFY( k2.isValid() ); QVERIFY( !k2.isCustom() ); QCOMPARE( k2.module(), QStringLiteral( "derp" ) ); QCOMPARE( k2.id(), QStringLiteral( "derp" ) ); - QCOMPARE( QString( k2 ), QStringLiteral( "derp@derp" ) ); } /// @brief These are expected to fail since they show bugs in the code -void ModuleSystemTests::testBadSimpleCases() +void +ModuleSystemTests::testBadSimpleCases() { - InstanceKey k2( "derp@derp" ); - assert_is_invalid( k2 ); - - InstanceKey k3( "derp@horse" ); - assert_is_invalid( k3 ); - InstanceKey k4( "derp", "derp@derp" ); assert_is_invalid( k4 ); } -void ModuleSystemTests::testBadFromStringCases() +void +ModuleSystemTests::testBadFromStringCases() { InstanceKey k0 = InstanceKey::fromString( QString() ); assert_is_invalid( k0 ); From d6ed04649564102c32cf49ecbac42d1886de32aa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 14 Sep 2019 14:15:00 -0400 Subject: [PATCH 09/10] [libcalamaresui] Replace a superfluous lambda - The whole method body can be a lot on its own, and since loadModules() does nothing but single-shot the lambda, call it from outside instead. --- src/calamares/CalamaresApplication.cpp | 3 +- .../modulesystem/ModuleManager.cpp | 206 +++++++++--------- .../modulesystem/ModuleManager.h | 3 +- 3 files changed, 106 insertions(+), 106 deletions(-) diff --git a/src/calamares/CalamaresApplication.cpp b/src/calamares/CalamaresApplication.cpp index 755808f5f..959e41fb1 100644 --- a/src/calamares/CalamaresApplication.cpp +++ b/src/calamares/CalamaresApplication.cpp @@ -39,6 +39,7 @@ #include #include #include +#include CalamaresApplication::CalamaresApplication( int& argc, char* argv[] ) @@ -354,7 +355,7 @@ CalamaresApplication::initView() connect( m_moduleManager, &Calamares::ModuleManager::modulesLoaded, this, &CalamaresApplication::initViewSteps ); connect( m_moduleManager, &Calamares::ModuleManager::modulesFailed, this, &CalamaresApplication::initFailed ); - m_moduleManager->loadModules(); + QTimer::singleShot( 0, m_moduleManager, &Calamares::ModuleManager::loadModules ); m_mainwindow->move( this->desktop()->availableGeometry().center() - m_mainwindow->rect().center() ); diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 79ad25325..f32663808 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -182,133 +182,131 @@ findCustomInstance( const Settings::InstanceDescriptionList& customInstances, co void ModuleManager::loadModules() { - QTimer::singleShot( 0, this, [this]() { - QStringList failedModules = checkDependencies(); - Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); + QStringList failedModules = checkDependencies(); + Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); - const auto modulesSequence - = failedModules.isEmpty() ? Settings::instance()->modulesSequence() : Settings::ModuleSequence(); - for ( const auto& modulePhase : modulesSequence ) + const auto modulesSequence + = failedModules.isEmpty() ? Settings::instance()->modulesSequence() : Settings::ModuleSequence(); + for ( const auto& modulePhase : modulesSequence ) + { + ModuleSystem::Action currentAction = modulePhase.first; + + foreach ( const QString& moduleEntry, modulePhase.second ) { - ModuleSystem::Action currentAction = modulePhase.first; - - foreach ( const QString& moduleEntry, modulePhase.second ) + auto instanceKey = ModuleSystem::InstanceKey::fromString( moduleEntry ); + if ( !instanceKey.isValid() ) { - auto instanceKey = ModuleSystem::InstanceKey::fromString( moduleEntry ); - if ( !instanceKey.isValid() ) + cError() << "Wrong module entry format for module" << moduleEntry; + failedModules.append( moduleEntry ); + continue; + } + + + if ( !m_availableDescriptorsByModuleName.contains( instanceKey.module() ) + || m_availableDescriptorsByModuleName.value( instanceKey.module() ).isEmpty() ) + { + cError() << "Module" << QString( instanceKey ) << "not found in module search paths." + << Logger::DebugList( m_paths ); + failedModules.append( QString( instanceKey ) ); + continue; + } + + QString configFileName; + if ( instanceKey.isCustom() ) + { + int found = findCustomInstance( customInstances, instanceKey ); + + if ( found > -1 ) { - cError() << "Wrong module entry format for module" << moduleEntry; + configFileName = customInstances[ found ].value( "config" ); + } + else //ought to be a custom instance, but cannot find instance entry + { + cError() << "Custom instance" << moduleEntry << "not found in custom instances section."; failedModules.append( moduleEntry ); continue; } + } + else + { + configFileName = QString( "%1.conf" ).arg( instanceKey.module() ); + } + // So now we can assume that the module entry is at least valid, + // that we have a descriptor on hand (and therefore that the + // module exists), and that the instance is either default or + // defined in the custom instances section. + // We still don't know whether the config file for the entry + // exists and is valid, but that's the only thing that could fail + // from this point on. -- Teo 8/2015 + Module* thisModule = m_loadedModulesByInstanceKey.value( instanceKey, nullptr ); + if ( thisModule && !thisModule->isLoaded() ) + { + cError() << "Module" << QString( instanceKey ) << "exists but not loaded."; + failedModules.append( QString( instanceKey ) ); + continue; + } - if ( !m_availableDescriptorsByModuleName.contains( instanceKey.module() ) - || m_availableDescriptorsByModuleName.value( instanceKey.module() ).isEmpty() ) + if ( thisModule && thisModule->isLoaded() ) + { + cDebug() << "Module" << QString( instanceKey ) << "already loaded."; + } + else + { + thisModule = Module::fromDescriptor( m_availableDescriptorsByModuleName.value( instanceKey.module() ), + instanceKey.id(), + configFileName, + m_moduleDirectoriesByModuleName.value( instanceKey.module() ) ); + if ( !thisModule ) { - cError() << "Module" << QString( instanceKey ) << "not found in module search paths." - << Logger::DebugList( m_paths ); + cError() << "Module" << QString( instanceKey ) << "cannot be created from descriptor" << configFileName; failedModules.append( QString( instanceKey ) ); continue; } - QString configFileName; - if ( instanceKey.isCustom() ) + if ( !checkDependencies( *thisModule ) ) { - int found = findCustomInstance( customInstances, instanceKey ); - - if ( found > -1 ) - { - configFileName = customInstances[ found ].value( "config" ); - } - else //ought to be a custom instance, but cannot find instance entry - { - cError() << "Custom instance" << moduleEntry << "not found in custom instances section."; - failedModules.append( moduleEntry ); - continue; - } - } - else - { - configFileName = QString( "%1.conf" ).arg( instanceKey.module() ); - } - - // So now we can assume that the module entry is at least valid, - // that we have a descriptor on hand (and therefore that the - // module exists), and that the instance is either default or - // defined in the custom instances section. - // We still don't know whether the config file for the entry - // exists and is valid, but that's the only thing that could fail - // from this point on. -- Teo 8/2015 - Module* thisModule = m_loadedModulesByInstanceKey.value( instanceKey, nullptr ); - if ( thisModule && !thisModule->isLoaded() ) - { - cError() << "Module" << QString( instanceKey ) << "exists but not loaded."; + // Error message is already printed failedModules.append( QString( instanceKey ) ); continue; } - if ( thisModule && thisModule->isLoaded() ) + // If it's a ViewModule, it also appends the ViewStep to the ViewManager. + thisModule->loadSelf(); + m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); + if ( !thisModule->isLoaded() ) { - cDebug() << "Module" << QString( instanceKey ) << "already loaded."; - } - else - { - thisModule = Module::fromDescriptor( m_availableDescriptorsByModuleName.value( instanceKey.module() ), - instanceKey.id(), - configFileName, - m_moduleDirectoriesByModuleName.value( instanceKey.module() ) ); - if ( !thisModule ) - { - cError() << "Module" << QString( instanceKey ) << "cannot be created from descriptor" << configFileName; - failedModules.append( QString( instanceKey ) ); - continue; - } - - if ( !checkDependencies( *thisModule ) ) - { - // Error message is already printed - failedModules.append( QString( instanceKey ) ); - continue; - } - - // If it's a ViewModule, it also appends the ViewStep to the ViewManager. - thisModule->loadSelf(); - m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); - if ( !thisModule->isLoaded() ) - { - cError() << "Module" << QString( instanceKey ) << "loading FAILED."; - failedModules.append( QString( instanceKey ) ); - continue; - } - } - - // At this point we most certainly have a pointer to a loaded module in - // thisModule. We now need to enqueue jobs info into an EVS. - if ( currentAction == ModuleSystem::Action::Exec ) - { - ExecutionViewStep* evs - = qobject_cast< ExecutionViewStep* >( Calamares::ViewManager::instance()->viewSteps().last() ); - if ( !evs ) // If the last step is not an EVS, we must create it. - { - evs = new ExecutionViewStep( ViewManager::instance() ); - ViewManager::instance()->addViewStep( evs ); - } - - evs->appendJobModuleInstanceKey( QString( instanceKey ) ); + cError() << "Module" << QString( instanceKey ) << "loading FAILED."; + failedModules.append( QString( instanceKey ) ); + continue; } } + + // At this point we most certainly have a pointer to a loaded module in + // thisModule. We now need to enqueue jobs info into an EVS. + if ( currentAction == ModuleSystem::Action::Exec ) + { + ExecutionViewStep* evs + = qobject_cast< ExecutionViewStep* >( Calamares::ViewManager::instance()->viewSteps().last() ); + if ( !evs ) // If the last step is not an EVS, we must create it. + { + evs = new ExecutionViewStep( ViewManager::instance() ); + ViewManager::instance()->addViewStep( evs ); + } + + evs->appendJobModuleInstanceKey( QString( instanceKey ) ); + } } - if ( !failedModules.isEmpty() ) - { - ViewManager::instance()->onInitFailed( failedModules ); - emit modulesFailed( failedModules ); - } - else - { - emit modulesLoaded(); - } - } ); + } + if ( !failedModules.isEmpty() ) + { + ViewManager::instance()->onInitFailed( failedModules ); + emit modulesFailed( failedModules ); + } + else + { + emit modulesLoaded(); + } } void diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index 5fe239ae1..a4f28fab8 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -79,8 +79,9 @@ public: Module* moduleInstance( const QString& instanceKey ); /** - * @brief loadModules initiates the asynchronous module loading operation. + * @brief loadModules does all of the module loading operation. * When this is done, the signal modulesLoaded is emitted. + * It is recommended to call this from a single-shot QTimer. */ void loadModules(); From 7a5ac63f92ff5737eefcdb32b82f736a1d192e0b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 15 Sep 2019 14:36:27 -0400 Subject: [PATCH 10/10] [libcalamares] Chase change of API - Replace QString( x ) by x.toString() where x is an InstanceKey --- .../modulesystem/ModuleManager.cpp | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index f32663808..78164ae18 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -140,7 +140,7 @@ ModuleManager::loadedInstanceKeys() QStringList l; for ( const auto& m : m_loadedModulesByInstanceKey.keys() ) { - l << QString( m ); + l << m.toString(); } return l; } @@ -205,9 +205,9 @@ ModuleManager::loadModules() if ( !m_availableDescriptorsByModuleName.contains( instanceKey.module() ) || m_availableDescriptorsByModuleName.value( instanceKey.module() ).isEmpty() ) { - cError() << "Module" << QString( instanceKey ) << "not found in module search paths." + cError() << "Module" << instanceKey.toString() << "not found in module search paths." << Logger::DebugList( m_paths ); - failedModules.append( QString( instanceKey ) ); + failedModules.append( instanceKey.toString() ); continue; } @@ -242,14 +242,14 @@ ModuleManager::loadModules() Module* thisModule = m_loadedModulesByInstanceKey.value( instanceKey, nullptr ); if ( thisModule && !thisModule->isLoaded() ) { - cError() << "Module" << QString( instanceKey ) << "exists but not loaded."; - failedModules.append( QString( instanceKey ) ); + cError() << "Module" << instanceKey.toString() << "exists but not loaded."; + failedModules.append( instanceKey.toString() ); continue; } if ( thisModule && thisModule->isLoaded() ) { - cDebug() << "Module" << QString( instanceKey ) << "already loaded."; + cDebug() << "Module" << instanceKey.toString() << "already loaded."; } else { @@ -259,15 +259,15 @@ ModuleManager::loadModules() m_moduleDirectoriesByModuleName.value( instanceKey.module() ) ); if ( !thisModule ) { - cError() << "Module" << QString( instanceKey ) << "cannot be created from descriptor" << configFileName; - failedModules.append( QString( instanceKey ) ); + cError() << "Module" << instanceKey.toString() << "cannot be created from descriptor" << configFileName; + failedModules.append( instanceKey.toString() ); continue; } if ( !checkDependencies( *thisModule ) ) { // Error message is already printed - failedModules.append( QString( instanceKey ) ); + failedModules.append( instanceKey.toString() ); continue; } @@ -276,8 +276,8 @@ ModuleManager::loadModules() m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); if ( !thisModule->isLoaded() ) { - cError() << "Module" << QString( instanceKey ) << "loading FAILED."; - failedModules.append( QString( instanceKey ) ); + cError() << "Module" << instanceKey.toString() << "loading FAILED."; + failedModules.append( instanceKey.toString() ); continue; } } @@ -294,7 +294,7 @@ ModuleManager::loadModules() ViewManager::instance()->addViewStep( evs ); } - evs->appendJobModuleInstanceKey( QString( instanceKey ) ); + evs->appendJobModuleInstanceKey( instanceKey.toString() ); } } }