From ea709aab59f51a784b735299e1f0808fd30d9fad Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 01:31:22 +0200 Subject: [PATCH 01/34] [libcalamaresui] Swap out unstructured string for structured data --- src/calamares/DebugWindow.cpp | 2 +- src/calamares/testmain.cpp | 2 +- src/libcalamaresui/modulesystem/ModuleManager.cpp | 6 +++--- src/libcalamaresui/modulesystem/ModuleManager.h | 2 +- src/libcalamaresui/viewpages/ExecutionViewStep.cpp | 4 ++-- src/libcalamaresui/viewpages/ExecutionViewStep.h | 5 +++-- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/calamares/DebugWindow.cpp b/src/calamares/DebugWindow.cpp index 82533a648..9e2bdc501 100644 --- a/src/calamares/DebugWindow.cpp +++ b/src/calamares/DebugWindow.cpp @@ -193,7 +193,7 @@ DebugWindow::DebugWindow() #endif ] { QString moduleName = m_ui->modulesListView->currentIndex().data().toString(); - Module* module = ModuleManager::instance()->moduleInstance( moduleName ); + Module* module = ModuleManager::instance()->moduleInstance( ModuleSystem::InstanceKey::fromString( moduleName ) ); if ( module ) { m_module = module->configurationMap(); diff --git a/src/calamares/testmain.cpp b/src/calamares/testmain.cpp index a29c8ce91..c9ac00f2b 100644 --- a/src/calamares/testmain.cpp +++ b/src/calamares/testmain.cpp @@ -217,7 +217,7 @@ ExecViewModule::loadSelf() auto* viewStep = new Calamares::ExecutionViewStep(); viewStep->setModuleInstanceKey( instanceKey() ); viewStep->setConfigurationMap( m_configurationMap ); - viewStep->appendJobModuleInstanceKey( instanceKey().toString() ); + viewStep->appendJobModuleInstanceKey( instanceKey() ); Calamares::ViewManager::instance()->addViewStep( viewStep ); m_loaded = true; } diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 23df7ce8a..c872bdaa4 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -151,9 +151,9 @@ ModuleManager::moduleDescriptor( const QString& name ) } Module* -ModuleManager::moduleInstance( const QString& instanceKey ) +ModuleManager::moduleInstance( const ModuleSystem::InstanceKey& instanceKey ) { - return m_loadedModulesByInstanceKey.value( ModuleSystem::InstanceKey::fromString( instanceKey ) ); + return m_loadedModulesByInstanceKey.value( instanceKey ); } @@ -320,7 +320,7 @@ ModuleManager::loadModules() ViewManager::instance()->addViewStep( evs ); } - evs->appendJobModuleInstanceKey( instanceKey.toString() ); + evs->appendJobModuleInstanceKey( instanceKey ); } } } diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index bea8acf41..d079baee7 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -76,7 +76,7 @@ public: * @param instanceKey the instance key for a module instance. * @return a pointer to an object of a subtype of Module. */ - Module* moduleInstance( const QString& instanceKey ); + Module* moduleInstance( const ModuleSystem::InstanceKey& instanceKey ); /** * @brief loadModules does all of the module loading operation. diff --git a/src/libcalamaresui/viewpages/ExecutionViewStep.cpp b/src/libcalamaresui/viewpages/ExecutionViewStep.cpp index 2dd4b79df..b2205d70e 100644 --- a/src/libcalamaresui/viewpages/ExecutionViewStep.cpp +++ b/src/libcalamaresui/viewpages/ExecutionViewStep.cpp @@ -147,7 +147,7 @@ ExecutionViewStep::onActivate() m_slideshow->changeSlideShowState( Slideshow::Start ); JobQueue* queue = JobQueue::instance(); - foreach ( const QString& instanceKey, m_jobInstanceKeys ) + for( const auto& instanceKey : m_jobInstanceKeys ) { Calamares::Module* module = Calamares::ModuleManager::instance()->moduleInstance( instanceKey ); if ( module ) @@ -176,7 +176,7 @@ ExecutionViewStep::jobs() const void -ExecutionViewStep::appendJobModuleInstanceKey( const QString& instanceKey ) +ExecutionViewStep::appendJobModuleInstanceKey( const ModuleSystem::InstanceKey& instanceKey ) { m_jobInstanceKeys.append( instanceKey ); } diff --git a/src/libcalamaresui/viewpages/ExecutionViewStep.h b/src/libcalamaresui/viewpages/ExecutionViewStep.h index 48604fe93..0edb965a1 100644 --- a/src/libcalamaresui/viewpages/ExecutionViewStep.h +++ b/src/libcalamaresui/viewpages/ExecutionViewStep.h @@ -21,6 +21,7 @@ #define EXECUTIONVIEWSTEP_H #include "ViewStep.h" +#include "modulesystem/InstanceKey.h" #include @@ -57,7 +58,7 @@ public: JobList jobs() const override; - void appendJobModuleInstanceKey( const QString& instanceKey ); + void appendJobModuleInstanceKey( const ModuleSystem::InstanceKey& instanceKey ); private: QWidget* m_widget; @@ -65,7 +66,7 @@ private: QLabel* m_label; Slideshow* m_slideshow; - QStringList m_jobInstanceKeys; + QList< ModuleSystem::InstanceKey > m_jobInstanceKeys; void updateFromJobQueue( qreal percent, const QString& message ); }; From 88e5e98d29207f6d011001b4aa25c36d1deec1e4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 10:19:40 +0200 Subject: [PATCH 02/34] [libcalamares] Use consistent type alias (Descriptor) --- src/libcalamares/modulesystem/Module.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/modulesystem/Module.h b/src/libcalamares/modulesystem/Module.h index 81737cf8f..44e89fd75 100644 --- a/src/libcalamares/modulesystem/Module.h +++ b/src/libcalamares/modulesystem/Module.h @@ -1,7 +1,8 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2017 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef CALAMARES_MODULE_H @@ -176,9 +174,9 @@ protected: explicit Module(); /// @brief For subclasses to read their part of the descriptor - virtual void initFrom( const QVariantMap& moduleDescriptor ) = 0; + virtual void initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) = 0; /// @brief Generic part of descriptor reading (and instance id) - void initFrom( const QVariantMap& moduleDescriptor, const QString& id ); + void initFrom( const ModuleSystem::Descriptor& moduleDescriptor, const QString& id ); QVariantMap m_configurationMap; From 320779ccbe187e07535aab2617854ac37efff13c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 4 Aug 2020 11:19:17 +0200 Subject: [PATCH 03/34] [libcalamares] Document Job::prettyDescription The TODO said it was unused: it **is** used, but only in a very limited scope. Drop it from jobs where it wasn't useful (e.g. those that just return prettyName(), outside of the partition module). --- src/libcalamares/Job.h | 16 +++++++++++----- src/modules/plasmalnf/PlasmaLnfJob.cpp | 6 ------ src/modules/plasmalnf/PlasmaLnfJob.h | 1 - src/modules/tracking/TrackingJobs.cpp | 18 ------------------ src/modules/tracking/TrackingJobs.h | 3 --- 5 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/libcalamares/Job.h b/src/libcalamares/Job.h index 18f11158f..41674cfff 100644 --- a/src/libcalamares/Job.h +++ b/src/libcalamares/Job.h @@ -1,6 +1,8 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac + * SPDX-FileCopyrightText: 2017 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef CALAMARES_JOB_H #define CALAMARES_JOB_H @@ -114,7 +113,14 @@ public: * For status and state information, see prettyStatusMessage(). */ virtual QString prettyName() const = 0; - // TODO: Unused + /** @brief a longer human-readable description of what the job will do + * + * This **may** be used by view steps to fill in the summary + * messages for the summary page; at present, only the *partition* + * module does so. + * + * The default implementation returns an empty string. + */ virtual QString prettyDescription() const; /** @brief A human-readable status for progress reporting * diff --git a/src/modules/plasmalnf/PlasmaLnfJob.cpp b/src/modules/plasmalnf/PlasmaLnfJob.cpp index d5db8ae4c..44bdb5cc7 100644 --- a/src/modules/plasmalnf/PlasmaLnfJob.cpp +++ b/src/modules/plasmalnf/PlasmaLnfJob.cpp @@ -41,12 +41,6 @@ PlasmaLnfJob::prettyName() const return tr( "Plasma Look-and-Feel Job" ); } -QString -PlasmaLnfJob::prettyDescription() const -{ - return prettyName(); -} - QString PlasmaLnfJob::prettyStatusMessage() const { return prettyName(); diff --git a/src/modules/plasmalnf/PlasmaLnfJob.h b/src/modules/plasmalnf/PlasmaLnfJob.h index ef9ee1257..9c52f2c8d 100644 --- a/src/modules/plasmalnf/PlasmaLnfJob.h +++ b/src/modules/plasmalnf/PlasmaLnfJob.h @@ -33,7 +33,6 @@ public: virtual ~PlasmaLnfJob() override; QString prettyName() const override; - QString prettyDescription() const override; QString prettyStatusMessage() const override; Calamares::JobResult exec() override; diff --git a/src/modules/tracking/TrackingJobs.cpp b/src/modules/tracking/TrackingJobs.cpp index 00ef06e10..2df3a66a0 100644 --- a/src/modules/tracking/TrackingJobs.cpp +++ b/src/modules/tracking/TrackingJobs.cpp @@ -46,12 +46,6 @@ TrackingInstallJob::prettyName() const return tr( "Installation feedback" ); } -QString -TrackingInstallJob::prettyDescription() const -{ - return prettyName(); -} - QString TrackingInstallJob::prettyStatusMessage() const { @@ -86,12 +80,6 @@ TrackingMachineUpdateManagerJob::prettyName() const return tr( "Machine feedback" ); } -QString -TrackingMachineUpdateManagerJob::prettyDescription() const -{ - return prettyName(); -} - QString TrackingMachineUpdateManagerJob::prettyStatusMessage() const { @@ -143,12 +131,6 @@ TrackingKUserFeedbackJob::prettyName() const return tr( "KDE user feedback" ); } -QString -TrackingKUserFeedbackJob::prettyDescription() const -{ - return prettyName(); -} - QString TrackingKUserFeedbackJob::prettyStatusMessage() const { diff --git a/src/modules/tracking/TrackingJobs.h b/src/modules/tracking/TrackingJobs.h index 38349515a..33b8eceb1 100644 --- a/src/modules/tracking/TrackingJobs.h +++ b/src/modules/tracking/TrackingJobs.h @@ -54,7 +54,6 @@ public: ~TrackingInstallJob() override; QString prettyName() const override; - QString prettyDescription() const override; QString prettyStatusMessage() const override; Calamares::JobResult exec() override; @@ -75,7 +74,6 @@ public: ~TrackingMachineUpdateManagerJob() override; QString prettyName() const override; - QString prettyDescription() const override; QString prettyStatusMessage() const override; Calamares::JobResult exec() override; }; @@ -93,7 +91,6 @@ public: ~TrackingKUserFeedbackJob() override; QString prettyName() const override; - QString prettyDescription() const override; QString prettyStatusMessage() const override; Calamares::JobResult exec() override; From 21b4a36a912277237e30229f0060dc597474a8d7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 10 Aug 2020 09:50:27 +0200 Subject: [PATCH 04/34] [libcalamares] Remove empty .cpp file - Requirement.cpp was there "just in case" the header grew functions that need an implementation, but that seems unlikely (the header is just a struct of POD). --- src/libcalamares/CMakeLists.txt | 1 - src/libcalamares/modulesystem/Requirement.cpp | 22 ------------------- 2 files changed, 23 deletions(-) delete mode 100644 src/libcalamares/modulesystem/Requirement.cpp diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index dc05270af..3bfc8e479 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -62,7 +62,6 @@ set( libSources # Modules modulesystem/InstanceKey.cpp modulesystem/Module.cpp - modulesystem/Requirement.cpp modulesystem/RequirementsChecker.cpp modulesystem/RequirementsModel.cpp diff --git a/src/libcalamares/modulesystem/Requirement.cpp b/src/libcalamares/modulesystem/Requirement.cpp deleted file mode 100644 index d5ba23282..000000000 --- a/src/libcalamares/modulesystem/Requirement.cpp +++ /dev/null @@ -1,22 +0,0 @@ -/* === This file is part of Calamares - === - * - * SPDX-FileCopyrightText: 2017 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 . - * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * - */ -#include "Requirement.h" From 483c0a84f8729088e279fe7f11795826eec4cbdb Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 10 Aug 2020 09:53:05 +0200 Subject: [PATCH 05/34] [libcalamares] Update SPDX in modulesystem/ --- src/libcalamares/modulesystem/Actions.h | 6 ++---- src/libcalamares/modulesystem/Descriptor.h | 6 ++---- src/libcalamares/modulesystem/InstanceKey.cpp | 6 ++---- src/libcalamares/modulesystem/InstanceKey.h | 6 ++---- src/libcalamares/modulesystem/Module.cpp | 6 ++---- src/libcalamares/modulesystem/Requirement.h | 6 ++---- src/libcalamares/modulesystem/RequirementsChecker.cpp | 6 ++---- src/libcalamares/modulesystem/RequirementsChecker.h | 6 ++---- src/libcalamares/modulesystem/RequirementsModel.cpp | 4 +--- src/libcalamares/modulesystem/RequirementsModel.h | 6 ++---- src/libcalamares/modulesystem/Tests.cpp | 6 ++---- 11 files changed, 21 insertions(+), 43 deletions(-) diff --git a/src/libcalamares/modulesystem/Actions.h b/src/libcalamares/modulesystem/Actions.h index 0b7133f78..4376733a1 100644 --- a/src/libcalamares/modulesystem/Actions.h +++ b/src/libcalamares/modulesystem/Actions.h @@ -1,7 +1,8 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014 Teo Mrnjavac * SPDX-FileCopyrightText: 2019 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef MODULESYSTEM_ACTIONS_H diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index b34f163ad..c6b5ab5cf 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2020 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef MODULESYSTEM_DESCRIPTOR_H diff --git a/src/libcalamares/modulesystem/InstanceKey.cpp b/src/libcalamares/modulesystem/InstanceKey.cpp index 82ccf7800..982b5f532 100644 --- a/src/libcalamares/modulesystem/InstanceKey.cpp +++ b/src/libcalamares/modulesystem/InstanceKey.cpp @@ -1,7 +1,8 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2018-2019 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "InstanceKey.h" diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h index c81d83a75..0bfa636f1 100644 --- a/src/libcalamares/modulesystem/InstanceKey.h +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -1,7 +1,8 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2018-2019 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef MODULESYSTEM_INSTANCEKEY_H #define MODULESYSTEM_INSTANCEKEY_H diff --git a/src/libcalamares/modulesystem/Module.cpp b/src/libcalamares/modulesystem/Module.cpp index 407a10205..dae3c84aa 100644 --- a/src/libcalamares/modulesystem/Module.cpp +++ b/src/libcalamares/modulesystem/Module.cpp @@ -1,7 +1,8 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -16,9 +17,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "Module.h" diff --git a/src/libcalamares/modulesystem/Requirement.h b/src/libcalamares/modulesystem/Requirement.h index eb664d2a9..02e0a009a 100644 --- a/src/libcalamares/modulesystem/Requirement.h +++ b/src/libcalamares/modulesystem/Requirement.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2017 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef CALAMARES_REQUIREMENT_H #define CALAMARES_REQUIREMENT_H diff --git a/src/libcalamares/modulesystem/RequirementsChecker.cpp b/src/libcalamares/modulesystem/RequirementsChecker.cpp index 2dbaea8cf..3256c460d 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.cpp +++ b/src/libcalamares/modulesystem/RequirementsChecker.cpp @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2019 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "RequirementsChecker.h" diff --git a/src/libcalamares/modulesystem/RequirementsChecker.h b/src/libcalamares/modulesystem/RequirementsChecker.h index 910e34dfc..523bc2bc7 100644 --- a/src/libcalamares/modulesystem/RequirementsChecker.h +++ b/src/libcalamares/modulesystem/RequirementsChecker.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2019 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef CALAMARES_REQUIREMENTSCHECKER_H #define CALAMARES_REQUIREMENTSCHECKER_H diff --git a/src/libcalamares/modulesystem/RequirementsModel.cpp b/src/libcalamares/modulesystem/RequirementsModel.cpp index 41a5616c1..2ff184f3b 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.cpp +++ b/src/libcalamares/modulesystem/RequirementsModel.cpp @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * SPDX-FileCopyrightText: 2019-2020 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "RequirementsModel.h" diff --git a/src/libcalamares/modulesystem/RequirementsModel.h b/src/libcalamares/modulesystem/RequirementsModel.h index df56ccd7b..904e0b84e 100644 --- a/src/libcalamares/modulesystem/RequirementsModel.h +++ b/src/libcalamares/modulesystem/RequirementsModel.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2019-2020 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef CALAMARES_REQUIREMENTSMODEL_H diff --git a/src/libcalamares/modulesystem/Tests.cpp b/src/libcalamares/modulesystem/Tests.cpp index 7973cc0ad..282300eec 100644 --- a/src/libcalamares/modulesystem/Tests.cpp +++ b/src/libcalamares/modulesystem/Tests.cpp @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2019 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -15,9 +16,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "modulesystem/InstanceKey.h" From 3227658475de2b36c71f064ffcbcac5a7a62b6f6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 10 Aug 2020 16:10:16 +0200 Subject: [PATCH 06/34] [calamares] Fix up multiple-moc of KDSAG - was getting multiple definitions of moc-related code due to automoc combined with KDSAG having its own #include moc, comment-out the include. - while here, simplify the CMake bits for building KDSAG --- .../kdsingleapplicationguard.cpp | 2 +- src/calamares/CMakeLists.txt | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/3rdparty/kdsingleapplicationguard/kdsingleapplicationguard.cpp b/3rdparty/kdsingleapplicationguard/kdsingleapplicationguard.cpp index cd8fadcce..8ddf7aad6 100644 --- a/3rdparty/kdsingleapplicationguard/kdsingleapplicationguard.cpp +++ b/3rdparty/kdsingleapplicationguard/kdsingleapplicationguard.cpp @@ -1119,7 +1119,7 @@ void KDSingleApplicationGuard::Private::poll() { } } -#include "moc_kdsingleapplicationguard.cpp" +// #include "moc_kdsingleapplicationguard.cpp" #ifdef KDTOOLSCORE_UNITTESTS diff --git a/src/calamares/CMakeLists.txt b/src/calamares/CMakeLists.txt index ff91904e2..5480dd1f8 100644 --- a/src/calamares/CMakeLists.txt +++ b/src/calamares/CMakeLists.txt @@ -11,15 +11,12 @@ set( calamaresSources ) if( NOT WITH_KF5DBus ) - set( kdsagSources "" ) - foreach( _s - kdsingleapplicationguard/kdsingleapplicationguard.cpp - kdsingleapplicationguard/kdsharedmemorylocker.cpp - kdsingleapplicationguard/kdtoolsglobal.cpp - kdsingleapplicationguard/kdlockedsharedmemorypointer.cpp + set( kdsagSources + ${CMAKE_SOURCE_DIR}/3rdparty/kdsingleapplicationguard/kdsingleapplicationguard.cpp + ${CMAKE_SOURCE_DIR}/3rdparty/kdsingleapplicationguard/kdsharedmemorylocker.cpp + ${CMAKE_SOURCE_DIR}/3rdparty/kdsingleapplicationguard/kdtoolsglobal.cpp + ${CMAKE_SOURCE_DIR}/3rdparty/kdsingleapplicationguard/kdlockedsharedmemorypointer.cpp ) - list( APPEND kdsagSources ${CMAKE_SOURCE_DIR}/3rdparty/${_s} ) - endforeach() mark_thirdparty_code( ${kdsagSources} ) list( APPEND calamaresSources ${kdsagSources} ) endif() @@ -46,6 +43,10 @@ calamares_automoc( calamares_bin ) calamares_autouic( calamares_bin ) calamares_autorcc( calamares_bin ) +if( kdsagSources ) + set_source_files_properties( ${kdsagSources} PROPERTIES AUTOMOC OFF ) +endif() + target_link_libraries( calamares_bin PRIVATE calamares From 9c382e3555c55f475539d995e7b609e41c4441ab Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 10:16:00 +0200 Subject: [PATCH 07/34] [libcalamares] Support switching public/private during tests --- src/libcalamares/DllMacro.h | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/DllMacro.h b/src/libcalamares/DllMacro.h index 2eeea331e..8613fa584 100644 --- a/src/libcalamares/DllMacro.h +++ b/src/libcalamares/DllMacro.h @@ -1,5 +1,5 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014 Teo Mrnjavac * SPDX-FileCopyrightText: 2020 Adriaan de Groot * @@ -76,4 +76,16 @@ #endif #endif +/* + * For private functions that should be public for testing purposes, + * use PRIVATETEST, which is private except when building tests. + */ +#ifndef PRIVATETEST +#if defined( BUILD_AS_TEST ) +#define PRIVATETEST public +#else +#define PRIVATETEST private +#endif +#endif + #endif From 53eb6c614aea3fdcc4c2d99f7de9ef8700845ee9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 10:31:12 +0200 Subject: [PATCH 08/34] [libcalamares] Make InstanceDescription a class - switch from dumb struct to a class; use a structured InstanceKey - expand testing of InstanceKey and InstanceDescription --- src/libcalamares/Settings.cpp | 41 ++++--- src/libcalamares/Settings.h | 45 ++++++-- src/libcalamares/Tests.cpp | 197 ++++++++++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 27 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 3c45c2e43..c4e7a3b4c 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -1,9 +1,10 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2019 Gabriel Craciunescu * SPDX-FileCopyrightText: 2019 Dominic Hayes * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -18,9 +19,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #include "Settings.h" @@ -75,22 +73,31 @@ requireBool( const YAML::Node& config, const char* key, bool d ) namespace Calamares { -InstanceDescription::InstanceDescription( const QVariantMap& m ) - : module( m.value( "module" ).toString() ) - , id( m.value( "id" ).toString() ) - , config( m.value( "config" ).toString() ) - , weight( m.value( "weight" ).toInt() ) +InstanceDescription::InstanceDescription( Calamares::ModuleSystem::InstanceKey&& key, int weight ) + : m_instanceKey( key ) + , m_weight( qBound( 1, weight, 100 ) ) { - if ( id.isEmpty() ) + if ( !isValid() ) { - id = module; - } - if ( config.isEmpty() ) - { - config = module + QStringLiteral( ".conf" ); + m_weight = 0; } +} - weight = qBound( 1, weight, 100 ); +InstanceDescription +InstanceDescription::fromSettings( const QVariantMap& m ) +{ + InstanceDescription r( + Calamares::ModuleSystem::InstanceKey( m.value( "module" ).toString(), m.value( "id" ).toString() ), + m.value( "weight" ).toInt() ); + if ( r.isValid() ) + { + r.m_configFileName = m.value( "config" ).toString(); + if ( r.m_configFileName.isEmpty() ) + { + r.m_configFileName = r.key().module() + QStringLiteral( ".conf" ); + } + } + return r; } Settings* Settings::s_instance = nullptr; @@ -156,7 +163,7 @@ interpretInstances( const YAML::Node& node, Settings::InstanceDescriptionList& c { continue; } - customInstances.append( InstanceDescription( instancesVListItem.toMap() ) ); + customInstances.append( InstanceDescription::fromSettings( instancesVListItem.toMap() ) ); } } } diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 098e010e5..6aa308dc5 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -1,9 +1,10 @@ /* === This file is part of Calamares - === - * + * * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2019 Gabriel Craciunescu * SPDX-FileCopyrightText: 2019 Dominic Hayes * SPDX-FileCopyrightText: 2017-2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -18,9 +19,6 @@ * You should have received a copy of the GNU General Public License * along with Calamares. If not, see . * - * SPDX-License-Identifier: GPL-3.0-or-later - * License-Filename: LICENSE - * */ #ifndef SETTINGS_H @@ -28,6 +26,7 @@ #include "DllMacro.h" #include "modulesystem/Actions.h" +#include "modulesystem/InstanceKey.h" #include #include @@ -36,14 +35,40 @@ namespace Calamares { -struct DLLEXPORT InstanceDescription +/** @brief Description of an instance as named in `settings.conf` + * + * An instance is an intended-step-in-sequence; it is not yet + * a loaded module. The instances have config-files and weights + * which are used by the module manager when loading modules + * and creating jobs. + */ +class DLLEXPORT InstanceDescription { - InstanceDescription( const QVariantMap& ); + using InstanceKey = Calamares::ModuleSystem::InstanceKey; - QString module; ///< Module name (e.g. "welcome") - QString id; ///< Id, to distinguish multiple instances (e.g. "one", for "welcome@one") - QString config; ///< Config-file name (for multiple instances) - int weight; + PRIVATETEST : InstanceDescription( InstanceKey&& key, int weight ); + +public: + /** @brief An invalid InstanceDescription + * + * Use `fromSettings()` to populate an InstanceDescription and + * check its validity. + */ + InstanceDescription() = default; + + static InstanceDescription fromSettings( const QVariantMap& ); + + bool isValid() const { return m_instanceKey.isValid(); } + + const InstanceKey& key() const { return m_instanceKey; } + QString configFileName() const { return m_configFileName; } + bool isCustom() const { return m_instanceKey.isCustom(); } + int weight() const { return m_weight; } + +private: + InstanceKey m_instanceKey; + QString m_configFileName; + int m_weight = 0; }; class DLLEXPORT Settings : public QObject diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 53bedc544..a9f53dc56 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -20,6 +20,8 @@ */ #include "GlobalStorage.h" +#include "Settings.h" +#include "modulesystem/InstanceKey.h" #include "utils/Logger.h" @@ -39,6 +41,9 @@ private Q_SLOTS: void testGSLoadSave(); void testGSLoadSave2(); void testGSLoadSaveYAMLStringList(); + + void testInstanceKey(); + void testInstanceDescription(); }; void @@ -177,6 +182,198 @@ TestLibCalamares::testGSLoadSaveYAMLStringList() QCOMPARE( gs2.value( "dwarfs" ).toString(), QStringLiteral( "" ) ); // .. they're gone } +void +TestLibCalamares::testInstanceKey() +{ + using InstanceKey = Calamares::ModuleSystem::InstanceKey; + { + InstanceKey k; + QVERIFY( !k.isValid() ); + QVERIFY( !k.isCustom() ); + QVERIFY( k.module().isEmpty() ); + } + { + InstanceKey k( QStringLiteral( "welcome" ), QString() ); + QVERIFY( k.isValid() ); + QVERIFY( !k.isCustom() ); + QCOMPARE( k.module(), QStringLiteral( "welcome" ) ); + QCOMPARE( k.id(), QStringLiteral( "welcome" ) ); + } + { + InstanceKey k( QStringLiteral( "shellprocess" ), QStringLiteral( "zfssetup" ) ); + QVERIFY( k.isValid() ); + QVERIFY( k.isCustom() ); + QCOMPARE( k.module(), QStringLiteral( "shellprocess" ) ); + QCOMPARE( k.id(), QStringLiteral( "zfssetup" ) ); + } + + { + // This is a bad idea, names and ids with odd punctuation + InstanceKey k( QStringLiteral( " o__O " ), QString() ); + QVERIFY( k.isValid() ); + QVERIFY( !k.isCustom() ); + QCOMPARE( k.module(), QStringLiteral( " o__O " ) ); + } + { + // .. but @ is disallowed + InstanceKey k( QStringLiteral( "welcome@hi" ), QString() ); + QVERIFY( !k.isValid() ); + QVERIFY( !k.isCustom() ); + QVERIFY( k.module().isEmpty() ); + } + + { + InstanceKey k = InstanceKey::fromString( "welcome" ); + QVERIFY( k.isValid() ); + QVERIFY( !k.isCustom() ); + QCOMPARE( k.module(), QStringLiteral( "welcome" ) ); + QCOMPARE( k.id(), QStringLiteral( "welcome" ) ); + } + { + InstanceKey k = InstanceKey::fromString( "welcome@welcome" ); + QVERIFY( k.isValid() ); + QVERIFY( !k.isCustom() ); + QCOMPARE( k.module(), QStringLiteral( "welcome" ) ); + QCOMPARE( k.id(), QStringLiteral( "welcome" ) ); + } + + { + InstanceKey k = InstanceKey::fromString( "welcome@hi" ); + QVERIFY( k.isValid() ); + QVERIFY( k.isCustom() ); + QCOMPARE( k.module(), QStringLiteral( "welcome" ) ); + QCOMPARE( k.id(), QStringLiteral( "hi" ) ); + } + { + InstanceKey k = InstanceKey::fromString( "welcome@hi@hi" ); + QVERIFY( !k.isValid() ); + QVERIFY( !k.isCustom() ); + QVERIFY( k.module().isEmpty() ); + QVERIFY( k.id().isEmpty() ); + } +} + +void +TestLibCalamares::testInstanceDescription() +{ + using InstanceDescription = Calamares::InstanceDescription; + using InstanceKey = Calamares::ModuleSystem::InstanceKey; + + // With invalid keys + // + // + { + InstanceDescription d; + QVERIFY( !d.isValid() ); + QVERIFY( !d.isCustom() ); + QCOMPARE( d.weight(), 0 ); + QVERIFY( d.configFileName().isEmpty() ); + } + + { + InstanceDescription d( InstanceKey(), 0 ); + QVERIFY( !d.isValid() ); + QVERIFY( !d.isCustom() ); + QCOMPARE( d.weight(), 0 ); + QVERIFY( d.configFileName().isEmpty() ); + } + + { + InstanceDescription d( InstanceKey(), 100 ); + QVERIFY( !d.isValid() ); + QVERIFY( !d.isCustom() ); + QCOMPARE( d.weight(), 0 ); + QVERIFY( d.configFileName().isEmpty() ); + } + + // Private constructor + // + // This doesn't set up the config file yet. + { + InstanceDescription d( InstanceKey::fromString( "welcome" ), 0 ); + QVERIFY( d.isValid() ); + QVERIFY( !d.isCustom() ); + QCOMPARE( d.weight(), 1 ); // **now** the constraints kick in + QVERIFY( d.configFileName().isEmpty() ); + } + + { + InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 0 ); + QVERIFY( d.isValid() ); + QVERIFY( d.isCustom() ); + QCOMPARE( d.weight(), 1 ); // **now** the constraints kick in + QVERIFY( d.configFileName().isEmpty() ); + } + + { + InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 75 ); + QCOMPARE( d.weight(), 75 ); + } + { + InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 105 ); + QCOMPARE( d.weight(), 100 ); + } + + + // From settings, normal program flow + // + // + { + QVariantMap m; + + InstanceDescription d = InstanceDescription::fromSettings( m ); + QVERIFY( !d.isValid() ); + } + { + QVariantMap m; + m.insert( "name", "welcome" ); + + InstanceDescription d = InstanceDescription::fromSettings( m ); + QVERIFY( !d.isValid() ); + } + { + QVariantMap m; + m.insert( "module", "welcome" ); + + InstanceDescription d = InstanceDescription::fromSettings( m ); + QVERIFY( d.isValid() ); + QVERIFY( !d.isCustom() ); + QCOMPARE( d.weight(), 1 ); + QCOMPARE( d.key().module(), QString( "welcome" ) ); + QCOMPARE( d.key().id(), QString( "welcome" ) ); + QCOMPARE( d.configFileName(), QString( "welcome.conf" ) ); + } + { + QVariantMap m; + m.insert( "module", "welcome" ); + m.insert( "id", "hi" ); + m.insert( "weight", "17" ); // String, that's kind of bogus + + InstanceDescription d = InstanceDescription::fromSettings( m ); + QVERIFY( d.isValid() ); + QVERIFY( d.isCustom() ); + QCOMPARE( d.weight(), 17 ); + QCOMPARE( d.key().module(), QString( "welcome" ) ); + QCOMPARE( d.key().id(), QString( "hi" ) ); + QCOMPARE( d.configFileName(), QString( "welcome.conf" ) ); + } + { + QVariantMap m; + m.insert( "module", "welcome" ); + m.insert( "id", "hi" ); + m.insert( "weight", 134 ); + m.insert( "config", "hi.conf" ); + + InstanceDescription d = InstanceDescription::fromSettings( m ); + QVERIFY( d.isValid() ); + QVERIFY( d.isCustom() ); + QCOMPARE( d.weight(), 100 ); + QCOMPARE( d.key().module(), QString( "welcome" ) ); + QCOMPARE( d.key().id(), QString( "hi" ) ); + QCOMPARE( d.configFileName(), QString( "hi.conf" ) ); + } +} + QTEST_GUILESS_MAIN( TestLibCalamares ) From 34e31d433108092130c48648939de51288961428 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 11:09:07 +0200 Subject: [PATCH 09/34] [libcalamares] Revert PRIVATETEST - looks funny - is hard to get clang-format to respect this; it's intended as an access-modifier, but those are baked into the code rather than being configurable. - is probably rare enough that #ifdef is acceptable --- src/libcalamares/DllMacro.h | 12 ------------ src/libcalamares/Settings.h | 5 ++++- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/libcalamares/DllMacro.h b/src/libcalamares/DllMacro.h index 8613fa584..5677e928b 100644 --- a/src/libcalamares/DllMacro.h +++ b/src/libcalamares/DllMacro.h @@ -76,16 +76,4 @@ #endif #endif -/* - * For private functions that should be public for testing purposes, - * use PRIVATETEST, which is private except when building tests. - */ -#ifndef PRIVATETEST -#if defined( BUILD_AS_TEST ) -#define PRIVATETEST public -#else -#define PRIVATETEST private -#endif -#endif - #endif diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 6aa308dc5..2ad489a15 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -46,7 +46,10 @@ class DLLEXPORT InstanceDescription { using InstanceKey = Calamares::ModuleSystem::InstanceKey; - PRIVATETEST : InstanceDescription( InstanceKey&& key, int weight ); +#ifdef BUILD_AS_TEST +public: +#endif + InstanceDescription( InstanceKey&& key, int weight ); public: /** @brief An invalid InstanceDescription From f157d9c4590f9b31303fb127d7241b223de030eb Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 11:45:21 +0200 Subject: [PATCH 10/34] [libcalamares] Refactor data-loading in Settings - expose, for testing purposes, the load-from-YAML-data part alongside the public constructor that reads a YAML file - add test for building the list of instances --- src/libcalamares/Settings.cpp | 61 ++++++++++++++++++------------ src/libcalamares/Settings.h | 6 +++ src/libcalamares/Tests.cpp | 71 +++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 24 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index c4e7a3b4c..0a475be00 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -213,6 +213,16 @@ interpretSequence( const YAML::Node& node, Settings::ModuleSequence& moduleSeque } } +Settings::Settings( bool debugMode ) + : QObject() + , m_debug( debugMode ) + , m_doChroot( true ) + , m_promptInstall( false ) + , m_disableCancel( false ) + , m_disableCancelDuringExec( false ) +{ +} + Settings::Settings( const QString& settingsFilePath, bool debugMode ) : QObject() , m_debug( debugMode ) @@ -225,30 +235,7 @@ Settings::Settings( const QString& settingsFilePath, bool debugMode ) QFile file( settingsFilePath ); if ( file.exists() && file.open( QFile::ReadOnly | QFile::Text ) ) { - QByteArray ba = file.readAll(); - - try - { - YAML::Node config = YAML::Load( ba.constData() ); - Q_ASSERT( config.IsMap() ); - - interpretModulesSearch( - debugMode, CalamaresUtils::yamlToStringList( config[ "modules-search" ] ), m_modulesSearchPaths ); - interpretInstances( config[ "instances" ], m_customModuleInstances ); - interpretSequence( config[ "sequence" ], m_modulesSequence ); - - m_brandingComponentName = requireString( config, "branding" ); - m_promptInstall = requireBool( config, "prompt-install", false ); - m_doChroot = !requireBool( config, "dont-chroot", false ); - m_isSetupMode = requireBool( config, "oem-setup", !m_doChroot ); - m_disableCancel = requireBool( config, "disable-cancel", false ); - m_disableCancelDuringExec = requireBool( config, "disable-cancel-during-exec", false ); - m_quitAtEnd = requireBool( config, "quit-at-end", false ); - } - catch ( YAML::Exception& e ) - { - CalamaresUtils::explainYamlException( e, ba, file.fileName() ); - } + setConfiguration( file.readAll(), file.fileName() ); } else { @@ -258,6 +245,32 @@ Settings::Settings( const QString& settingsFilePath, bool debugMode ) s_instance = this; } +void +Settings::setConfiguration( const QByteArray& ba, const QString& explainName ) +{ + try + { + YAML::Node config = YAML::Load( ba.constData() ); + Q_ASSERT( config.IsMap() ); + + interpretModulesSearch( + debugMode(), CalamaresUtils::yamlToStringList( config[ "modules-search" ] ), m_modulesSearchPaths ); + interpretInstances( config[ "instances" ], m_customModuleInstances ); + interpretSequence( config[ "sequence" ], m_modulesSequence ); + + m_brandingComponentName = requireString( config, "branding" ); + m_promptInstall = requireBool( config, "prompt-install", false ); + m_doChroot = !requireBool( config, "dont-chroot", false ); + m_isSetupMode = requireBool( config, "oem-setup", !m_doChroot ); + m_disableCancel = requireBool( config, "disable-cancel", false ); + m_disableCancelDuringExec = requireBool( config, "disable-cancel-during-exec", false ); + m_quitAtEnd = requireBool( config, "quit-at-end", false ); + } + catch ( YAML::Exception& e ) + { + CalamaresUtils::explainYamlException( e, ba, explainName ); + } +} QStringList Settings::modulesSearchPaths() const diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 2ad489a15..de12ab57a 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -77,8 +77,14 @@ private: class DLLEXPORT Settings : public QObject { Q_OBJECT +#ifdef BUILD_AS_TEST +public: +#endif + explicit Settings( bool debugMode ); explicit Settings( const QString& settingsFilePath, bool debugMode ); + void setConfiguration( const QByteArray& configData, const QString& explainName ); + public: static Settings* instance(); /// @brief Find a settings.conf, following @p debugMode diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index a9f53dc56..9fe61e727 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -44,6 +44,8 @@ private Q_SLOTS: void testInstanceKey(); void testInstanceDescription(); + + void testSettings(); }; void @@ -374,6 +376,75 @@ TestLibCalamares::testInstanceDescription() } } +void +TestLibCalamares::testSettings() +{ + { + Calamares::Settings s( false ); + QVERIFY( !s.debugMode() ); + } + { + Calamares::Settings s( true ); + QVERIFY( s.debugMode() ); + QVERIFY( s.customModuleInstances().isEmpty() ); + QVERIFY( s.modulesSequence().isEmpty() ); + QVERIFY( s.brandingComponentName().isEmpty() ); + + s.setConfiguration( R"(--- +instances: + - module: welcome + id: hi + weight: 75 + - module: welcome + id: yo + config: yolo.conf +sequence: + - show: + - welcome@hi + - welcome@yo + - dummycpp + - summary + - exec: + - welcome@hi +)", + QStringLiteral( "" ) ); + + QVERIFY( s.debugMode() ); + QCOMPARE( s.customModuleInstances().count(), 2 ); + QCOMPARE( s.modulesSequence().count(), 2 ); // 2 steps (show, exec) + QVERIFY( s.brandingComponentName().isEmpty() ); + + // Make a lambda where we can adjust what it looks for from the outside, + // by capturing a reference. + QString moduleKey = QString( "welcome" ); + auto moduleFinder = [&moduleKey]( const Calamares::InstanceDescription& d ) { + return d.isValid() && d.key().module() == moduleKey; + }; + + const auto it0 = std::find_if( + s.customModuleInstances().constBegin(), s.customModuleInstances().constEnd(), moduleFinder ); + QVERIFY( it0 != s.customModuleInstances().constEnd() ); + + moduleKey = QString( "derp" ); + const auto it1 = std::find_if( + s.customModuleInstances().constBegin(), s.customModuleInstances().constEnd(), moduleFinder ); + QVERIFY( it1 == s.customModuleInstances().constEnd() ); + + int validCount = 0; + int customCount = 0; + for ( const auto& d : s.customModuleInstances() ) + { + if ( d.isValid() ) + validCount++; + if ( d.isCustom() ) + customCount++; + QVERIFY( d.isCustom() ? d.isValid() : true ); // All custom entries are valid + } + QCOMPARE( customCount, 2 ); + QCOMPARE( validCount, 2 ); + } +} + QTEST_GUILESS_MAIN( TestLibCalamares ) From 6f7234e4ac2d9ae75adb0cd586c20c01308f7887 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 12:43:59 +0200 Subject: [PATCH 11/34] [libcalamares] Add all mentioned instances to the instanceList - "custom" instances is now a misnomer, since all the instances go into it; they are distinguished by `isCustom()` on the descriptor --- src/libcalamares/Settings.cpp | 56 +++++++++++++++++++++++++++++++++++ src/libcalamares/Settings.h | 11 +++++++ 2 files changed, 67 insertions(+) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 0a475be00..e5bc71c97 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -73,6 +73,16 @@ requireBool( const YAML::Node& config, const char* key, bool d ) namespace Calamares { +InstanceDescription::InstanceDescription( const Calamares::ModuleSystem::InstanceKey& key ) + : m_instanceKey( key ) + , m_weight( 1 ) +{ + if ( !isValid() ) + { + m_weight = 0; + } +} + InstanceDescription::InstanceDescription( Calamares::ModuleSystem::InstanceKey&& key, int weight ) : m_instanceKey( key ) , m_weight( qBound( 1, weight, 100 ) ) @@ -245,6 +255,50 @@ Settings::Settings( const QString& settingsFilePath, bool debugMode ) s_instance = this; } +void +Settings::validateSequence() +{ + // Since moduleFinder captures targetKey by reference, we can + // update targetKey to change what the finder lambda looks for. + Calamares::ModuleSystem::InstanceKey targetKey; + auto moduleFinder = [&targetKey]( const InstanceDescription& d ) { return d.isValid() && d.key() == targetKey; }; + + // Check the sequence against the existing instances (which so far are only custom) + for ( const auto& step : m_modulesSequence ) + { + for ( const auto& instance : step.second ) + { + Calamares::ModuleSystem::InstanceKey k = Calamares::ModuleSystem::InstanceKey::fromString( instance ); + if ( !k.isValid() ) + { + cWarning() << "Invalid instance key in *sequence*," << instance; + } + if ( k.isValid() && k.isCustom() ) + { + targetKey = k; + const auto it = std::find_if( + m_customModuleInstances.constBegin(), m_customModuleInstances.constEnd(), moduleFinder ); + if ( it == m_customModuleInstances.constEnd() ) + { + cWarning() << "Custom instance key" << instance << "is not listed in the *instances*"; + // don't add it, let this fail later. + } + } + if ( k.isValid() && !k.isCustom() ) + { + targetKey = k; + const auto it = std::find_if( + m_customModuleInstances.constBegin(), m_customModuleInstances.constEnd(), moduleFinder ); + if ( it == m_customModuleInstances.constEnd() ) + { + // Non-custom instance, just mentioned in *sequence* + m_customModuleInstances.append( InstanceDescription( k ) ); + } + } + } + } +} + void Settings::setConfiguration( const QByteArray& ba, const QString& explainName ) { @@ -265,6 +319,8 @@ Settings::setConfiguration( const QByteArray& ba, const QString& explainName ) m_disableCancel = requireBool( config, "disable-cancel", false ); m_disableCancelDuringExec = requireBool( config, "disable-cancel-during-exec", false ); m_quitAtEnd = requireBool( config, "quit-at-end", false ); + + validateSequence(); } catch ( YAML::Exception& e ) { diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index de12ab57a..97ee77bc4 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -59,6 +59,16 @@ public: */ InstanceDescription() = default; + /** @brief An InstanceDescription with no special settings. + * + * Regardless of @p key being custom, sets weight to 1 and + * the configuration file to @c key.module() (plus the ".conf" + * extension). + * + * To InstanceDescription is custom if the key is. + */ + InstanceDescription( const InstanceKey& key ); + static InstanceDescription fromSettings( const QVariantMap& ); bool isValid() const { return m_instanceKey.isValid(); } @@ -84,6 +94,7 @@ public: explicit Settings( const QString& settingsFilePath, bool debugMode ); void setConfiguration( const QByteArray& configData, const QString& explainName ); + void validateSequence(); public: static Settings* instance(); From 1f57a99ff24545afe0f4c6b2625336fa47410c66 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 12:54:29 +0200 Subject: [PATCH 12/34] [libcalamares] Rename moduleInstances() and fix tests - "custom" is a misnomer, so drop that from the name - tests adjusted: all instances are returned, not just the "custom" ones. --- src/libcalamares/Settings.cpp | 16 ++++++++-------- src/libcalamares/Settings.h | 4 ++-- src/libcalamares/Tests.cpp | 16 ++++++++-------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index e5bc71c97..42da7e1f8 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -277,8 +277,8 @@ Settings::validateSequence() { targetKey = k; const auto it = std::find_if( - m_customModuleInstances.constBegin(), m_customModuleInstances.constEnd(), moduleFinder ); - if ( it == m_customModuleInstances.constEnd() ) + m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); + if ( it == m_moduleInstances.constEnd() ) { cWarning() << "Custom instance key" << instance << "is not listed in the *instances*"; // don't add it, let this fail later. @@ -288,11 +288,11 @@ Settings::validateSequence() { targetKey = k; const auto it = std::find_if( - m_customModuleInstances.constBegin(), m_customModuleInstances.constEnd(), moduleFinder ); - if ( it == m_customModuleInstances.constEnd() ) + m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); + if ( it == m_moduleInstances.constEnd() ) { // Non-custom instance, just mentioned in *sequence* - m_customModuleInstances.append( InstanceDescription( k ) ); + m_moduleInstances.append( InstanceDescription( k ) ); } } } @@ -309,7 +309,7 @@ Settings::setConfiguration( const QByteArray& ba, const QString& explainName ) interpretModulesSearch( debugMode(), CalamaresUtils::yamlToStringList( config[ "modules-search" ] ), m_modulesSearchPaths ); - interpretInstances( config[ "instances" ], m_customModuleInstances ); + interpretInstances( config[ "instances" ], m_moduleInstances ); interpretSequence( config[ "sequence" ], m_modulesSequence ); m_brandingComponentName = requireString( config, "branding" ); @@ -336,9 +336,9 @@ Settings::modulesSearchPaths() const Settings::InstanceDescriptionList -Settings::customModuleInstances() const +Settings::moduleInstances() const { - return m_customModuleInstances; + return m_moduleInstances; } diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 97ee77bc4..5c2908b4c 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -106,7 +106,7 @@ public: QStringList modulesSearchPaths() const; using InstanceDescriptionList = QList< InstanceDescription >; - InstanceDescriptionList customModuleInstances() const; + InstanceDescriptionList moduleInstances() const; using ModuleSequence = QList< QPair< ModuleSystem::Action, QStringList > >; ModuleSequence modulesSequence() const; @@ -158,7 +158,7 @@ private: QStringList m_modulesSearchPaths; - InstanceDescriptionList m_customModuleInstances; + InstanceDescriptionList m_moduleInstances; ModuleSequence m_modulesSequence; QString m_brandingComponentName; diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 9fe61e727..e2060789f 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -386,7 +386,7 @@ TestLibCalamares::testSettings() { Calamares::Settings s( true ); QVERIFY( s.debugMode() ); - QVERIFY( s.customModuleInstances().isEmpty() ); + QVERIFY( s.moduleInstances().isEmpty() ); QVERIFY( s.modulesSequence().isEmpty() ); QVERIFY( s.brandingComponentName().isEmpty() ); @@ -410,7 +410,7 @@ sequence: QStringLiteral( "" ) ); QVERIFY( s.debugMode() ); - QCOMPARE( s.customModuleInstances().count(), 2 ); + QCOMPARE( s.moduleInstances().count(), 4 ); // there are 4 module instances mentioned QCOMPARE( s.modulesSequence().count(), 2 ); // 2 steps (show, exec) QVERIFY( s.brandingComponentName().isEmpty() ); @@ -422,17 +422,17 @@ sequence: }; const auto it0 = std::find_if( - s.customModuleInstances().constBegin(), s.customModuleInstances().constEnd(), moduleFinder ); - QVERIFY( it0 != s.customModuleInstances().constEnd() ); + s.moduleInstances().constBegin(), s.moduleInstances().constEnd(), moduleFinder ); + QVERIFY( it0 != s.moduleInstances().constEnd() ); moduleKey = QString( "derp" ); const auto it1 = std::find_if( - s.customModuleInstances().constBegin(), s.customModuleInstances().constEnd(), moduleFinder ); - QVERIFY( it1 == s.customModuleInstances().constEnd() ); + s.moduleInstances().constBegin(), s.moduleInstances().constEnd(), moduleFinder ); + QVERIFY( it1 == s.moduleInstances().constEnd() ); int validCount = 0; int customCount = 0; - for ( const auto& d : s.customModuleInstances() ) + for ( const auto& d : s.moduleInstances() ) { if ( d.isValid() ) validCount++; @@ -441,7 +441,7 @@ sequence: QVERIFY( d.isCustom() ? d.isValid() : true ); // All custom entries are valid } QCOMPARE( customCount, 2 ); - QCOMPARE( validCount, 2 ); + QCOMPARE( validCount, 4 ); // welcome@hi is listed twice, in *show* and *exec* } } From e507338f4cebec516dd5d83fec95c5de65e114fc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 13:27:15 +0200 Subject: [PATCH 13/34] [libcalamares] Test config filenames as well (custom vs standard) --- src/libcalamares/Tests.cpp | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index e2060789f..1da333365 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -421,13 +421,11 @@ sequence: return d.isValid() && d.key().module() == moduleKey; }; - const auto it0 = std::find_if( - s.moduleInstances().constBegin(), s.moduleInstances().constEnd(), moduleFinder ); + const auto it0 = std::find_if( s.moduleInstances().constBegin(), s.moduleInstances().constEnd(), moduleFinder ); QVERIFY( it0 != s.moduleInstances().constEnd() ); moduleKey = QString( "derp" ); - const auto it1 = std::find_if( - s.moduleInstances().constBegin(), s.moduleInstances().constEnd(), moduleFinder ); + const auto it1 = std::find_if( s.moduleInstances().constBegin(), s.moduleInstances().constEnd(), moduleFinder ); QVERIFY( it1 == s.moduleInstances().constEnd() ); int validCount = 0; @@ -435,10 +433,31 @@ sequence: for ( const auto& d : s.moduleInstances() ) { if ( d.isValid() ) + { validCount++; + } if ( d.isCustom() ) + { customCount++; + } QVERIFY( d.isCustom() ? d.isValid() : true ); // All custom entries are valid + + if ( !d.isCustom() ) + { + QCOMPARE( d.configFileName(), QString( "%1.conf" ).arg( d.key().module() ) ); + } + else + { + // Specific cases from this config file + if ( d.key().id() == QString( "yo" ) ) + { + QCOMPARE( d.configFileName(), QString( "yolo.conf" ) ); + } + else + { + QCOMPARE( d.configFileName(), QString( "welcome.conf" ) ); // Not set in the settings data + } + } } QCOMPARE( customCount, 2 ); QCOMPARE( validCount, 4 ); // welcome@hi is listed twice, in *show* and *exec* From 4968efdaa75069c16c5666a23243df811c78a3bf Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 13:31:38 +0200 Subject: [PATCH 14/34] [libcalamares] Simplify constructors of InstanceDescription - no more weights in constructors; do that in fromSettings() only. - simplify test to drop those constructors - set config file also for "normal" descriptors; fix test --- src/libcalamares/Settings.cpp | 30 +++++++++++++----------------- src/libcalamares/Settings.h | 5 ----- src/libcalamares/Tests.cpp | 31 ++++++++----------------------- 3 files changed, 21 insertions(+), 45 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 42da7e1f8..ae9b6506c 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -81,15 +81,9 @@ InstanceDescription::InstanceDescription( const Calamares::ModuleSystem::Instanc { m_weight = 0; } -} - -InstanceDescription::InstanceDescription( Calamares::ModuleSystem::InstanceKey&& key, int weight ) - : m_instanceKey( key ) - , m_weight( qBound( 1, weight, 100 ) ) -{ - if ( !isValid() ) + else { - m_weight = 0; + m_configFileName = key.module() + QStringLiteral( ".conf" ); } } @@ -97,14 +91,16 @@ InstanceDescription InstanceDescription::fromSettings( const QVariantMap& m ) { InstanceDescription r( - Calamares::ModuleSystem::InstanceKey( m.value( "module" ).toString(), m.value( "id" ).toString() ), - m.value( "weight" ).toInt() ); + Calamares::ModuleSystem::InstanceKey( m.value( "module" ).toString(), m.value( "id" ).toString() ) ); if ( r.isValid() ) { - r.m_configFileName = m.value( "config" ).toString(); - if ( r.m_configFileName.isEmpty() ) + int w = qBound( 1, m.value( "weight" ).toInt(), 100 ); + r.m_weight = w; + + QString c = m.value( "config" ).toString(); + if ( !c.isEmpty() ) { - r.m_configFileName = r.key().module() + QStringLiteral( ".conf" ); + r.m_configFileName = c; } } return r; @@ -276,8 +272,8 @@ Settings::validateSequence() if ( k.isValid() && k.isCustom() ) { targetKey = k; - const auto it = std::find_if( - m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); + const auto it + = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); if ( it == m_moduleInstances.constEnd() ) { cWarning() << "Custom instance key" << instance << "is not listed in the *instances*"; @@ -287,8 +283,8 @@ Settings::validateSequence() if ( k.isValid() && !k.isCustom() ) { targetKey = k; - const auto it = std::find_if( - m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); + const auto it + = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); if ( it == m_moduleInstances.constEnd() ) { // Non-custom instance, just mentioned in *sequence* diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 5c2908b4c..0b8a548ab 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -46,11 +46,6 @@ class DLLEXPORT InstanceDescription { using InstanceKey = Calamares::ModuleSystem::InstanceKey; -#ifdef BUILD_AS_TEST -public: -#endif - InstanceDescription( InstanceKey&& key, int weight ); - public: /** @brief An invalid InstanceDescription * diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 1da333365..61da7639e 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -273,15 +273,7 @@ TestLibCalamares::testInstanceDescription() } { - InstanceDescription d( InstanceKey(), 0 ); - QVERIFY( !d.isValid() ); - QVERIFY( !d.isCustom() ); - QCOMPARE( d.weight(), 0 ); - QVERIFY( d.configFileName().isEmpty() ); - } - - { - InstanceDescription d( InstanceKey(), 100 ); + InstanceDescription d( InstanceKey {} ); // most-vexing, use brace-init instead QVERIFY( !d.isValid() ); QVERIFY( !d.isCustom() ); QCOMPARE( d.weight(), 0 ); @@ -290,30 +282,23 @@ TestLibCalamares::testInstanceDescription() // Private constructor // - // This doesn't set up the config file yet. + // This does set up the config file, to default values { - InstanceDescription d( InstanceKey::fromString( "welcome" ), 0 ); + InstanceDescription d( InstanceKey::fromString( "welcome" ) ); QVERIFY( d.isValid() ); QVERIFY( !d.isCustom() ); QCOMPARE( d.weight(), 1 ); // **now** the constraints kick in - QVERIFY( d.configFileName().isEmpty() ); + QVERIFY( !d.configFileName().isEmpty() ); + QCOMPARE( d.configFileName(), QStringLiteral( "welcome.conf" ) ); } { - InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 0 ); + InstanceDescription d( InstanceKey::fromString( "welcome@hi" ) ); QVERIFY( d.isValid() ); QVERIFY( d.isCustom() ); QCOMPARE( d.weight(), 1 ); // **now** the constraints kick in - QVERIFY( d.configFileName().isEmpty() ); - } - - { - InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 75 ); - QCOMPARE( d.weight(), 75 ); - } - { - InstanceDescription d( InstanceKey::fromString( "welcome@hi" ), 105 ); - QCOMPARE( d.weight(), 100 ); + QVERIFY( !d.configFileName().isEmpty() ); + QCOMPARE( d.configFileName(), QStringLiteral( "welcome.conf" ) ); } From 57f5a92d962540efc27853f2233406df9eb15cfc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 14:47:59 +0200 Subject: [PATCH 15/34] [libcalamares] Build complete instanceDescriptor list - there's no reason to ignore custom instances that are **not** mentioned in the *instances* section: it may be useful to name more that one even without distinct config files. --- src/libcalamares/Settings.cpp | 24 +++++++----------------- src/libcalamares/Settings.h | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index ae9b6506c..a05474216 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -268,28 +268,18 @@ Settings::validateSequence() if ( !k.isValid() ) { cWarning() << "Invalid instance key in *sequence*," << instance; + continue; } - if ( k.isValid() && k.isCustom() ) + + targetKey = k; + const auto it = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); + if ( it == m_moduleInstances.constEnd() ) { - targetKey = k; - const auto it - = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); - if ( it == m_moduleInstances.constEnd() ) + if ( k.isCustom() ) { cWarning() << "Custom instance key" << instance << "is not listed in the *instances*"; - // don't add it, let this fail later. - } - } - if ( k.isValid() && !k.isCustom() ) - { - targetKey = k; - const auto it - = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); - if ( it == m_moduleInstances.constEnd() ) - { - // Non-custom instance, just mentioned in *sequence* - m_moduleInstances.append( InstanceDescription( k ) ); } + m_moduleInstances.append( InstanceDescription( k ) ); } } } diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 0b8a548ab..d06a97716 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -101,9 +101,23 @@ public: QStringList modulesSearchPaths() const; using InstanceDescriptionList = QList< InstanceDescription >; + /** @brief All the module instances used + * + * Each module-instance mentioned in `settings.conf` has an entry + * in the moduleInstances list -- both custom entries that are + * in the *instances* section, and each module mentioned in the + * *sequence*. + */ InstanceDescriptionList moduleInstances() const; using ModuleSequence = QList< QPair< ModuleSystem::Action, QStringList > >; + /** @brief Representation of *sequence* of execution + * + * Each "section" of the *sequence* key in `settings.conf` gets an + * entry here, stating what kind of action is taken and which modules + * take part (in order). Each entry in the list is an instance key + * which can be found in the moduleInstances() list. + */ ModuleSequence modulesSequence() const; QString brandingComponentName() const; From b23dbd47c730f77486d39d7aa9109bff00570dfe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 14:50:15 +0200 Subject: [PATCH 16/34] [libcalamaresui] Chase changes in instanceDescriptor - this is mostly about deleting code, since the special-cases now live in libcalamares where `settings.conf` is interpreted. --- .../modulesystem/ModuleManager.cpp | 80 ++++++------------- 1 file changed, 25 insertions(+), 55 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index c872bdaa4..523e6ec24 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -1,7 +1,8 @@ /* === This file is part of Calamares - === * - * Copyright 2014-2015, Teo Mrnjavac - * Copyright 2018, Adriaan de Groot + * SPDX-FileCopyrightText: 2014-2015 Teo Mrnjavac + * SPDX-FileCopyrightText: 2018 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -157,60 +158,37 @@ ModuleManager::moduleInstance( const ModuleSystem::InstanceKey& instanceKey ) } -/** - * @brief Search a list of instance descriptions for one matching @p module and @p id - * - * @return -1 on failure, otherwise index of the instance that matches. - */ -static int -findCustomInstance( const Settings::InstanceDescriptionList& customInstances, const ModuleSystem::InstanceKey& m ) -{ - for ( int i = 0; i < customInstances.count(); ++i ) - { - const auto& thisInstance = customInstances[ i ]; - if ( thisInstance.module == m.module() && thisInstance.id == m.id() ) - { - return i; - } - } - return -1; -} - -/** @brief Returns the config file name for the fiven @p instanceKey +/** @brief Returns the config file name for the given @p instanceKey * * Custom instances have custom config files, non-custom ones * have a .conf file. Returns an empty QString on * errors. */ static QString -getConfigFileName( const Settings::InstanceDescriptionList& customInstances, +getConfigFileName( const Settings::InstanceDescriptionList& descriptorList, const ModuleSystem::InstanceKey& instanceKey, const ModuleSystem::Descriptor& descriptor ) { - if ( instanceKey.isCustom() ) + if ( descriptor.value( "noconfig", false ).toBool() ) { - int found = findCustomInstance( customInstances, instanceKey ); - - if ( found < 0 ) - { - // This should already have been checked and failed the module already - return QString(); - } - - return customInstances[ found ].config; + // Explicitly set to no-configuration. This doesn't apply + // to custom instances (above) since the only reason to + // **have** a custom instance is to specify a different + // config file for more than one module. + return QString(); } - else + + for ( const auto& descriptor : descriptorList ) { - if ( descriptor.value( "noconfig", false ).toBool() ) + if ( descriptor.key() == instanceKey ) { - // Explicitly set to no-configuration. This doesn't apply - // to custom instances (above) since the only reason to - // **have** a custom instance is to specify a different - // config file for more than one module. - return QString(); + return descriptor.configFileName(); } - return QString( "%1.conf" ).arg( instanceKey.module() ); } + + + // This should already have been checked and failed the module already + return QString(); } void @@ -220,7 +198,7 @@ ModuleManager::loadModules() { cWarning() << "Some installed modules have unmet dependencies."; } - Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); + Settings::InstanceDescriptionList customInstances = Settings::instance()->moduleInstances(); QStringList failedModules; const auto modulesSequence = Settings::instance()->modulesSequence(); @@ -237,16 +215,6 @@ ModuleManager::loadModules() failedModules.append( moduleEntry ); continue; } - if ( instanceKey.isCustom() ) - { - int found = findCustomInstance( customInstances, instanceKey ); - if ( found < 0 ) - { - cError() << "Custom instance" << moduleEntry << "not found in custom instances section."; - failedModules.append( moduleEntry ); - continue; - } - } ModuleSystem::Descriptor descriptor = m_availableDescriptorsByModuleName.value( instanceKey.module(), ModuleSystem::Descriptor() ); @@ -336,7 +304,7 @@ ModuleManager::loadModules() } bool -ModuleManager::addModule( Module *module ) +ModuleManager::addModule( Module* module ) { if ( !module ) { @@ -344,7 +312,7 @@ ModuleManager::addModule( Module *module ) } if ( !module->instanceKey().isValid() ) { - cWarning() << "Module" << module->location() << Logger::Pointer(module) << "has invalid instance key."; + cWarning() << "Module" << module->location() << Logger::Pointer( module ) << "has invalid instance key."; return false; } if ( !checkModuleDependencies( *module ) ) @@ -383,7 +351,9 @@ ModuleManager::checkRequirements() RequirementsChecker* rq = new RequirementsChecker( modules, m_requirementsModel, this ); connect( rq, &RequirementsChecker::done, rq, &RequirementsChecker::deleteLater ); - connect( rq, &RequirementsChecker::done, this, [=](){ this->requirementsComplete( m_requirementsModel->satisfiedMandatory() ); } ); + connect( rq, &RequirementsChecker::done, this, [=]() { + this->requirementsComplete( m_requirementsModel->satisfiedMandatory() ); + } ); QTimer::singleShot( 0, rq, &RequirementsChecker::run ); } From 253e5610afd1098ae6102ae2f6071662a715a0fc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 15:27:30 +0200 Subject: [PATCH 17/34] [libcalamares] Better type for the list of InstanceKeys - *sequence* lists module instance keys; make the stored type of those keys InstanceKey instead of QString --- src/libcalamares/Settings.cpp | 23 +++++++++++++-------- src/libcalamares/Settings.h | 2 +- src/libcalamares/modulesystem/InstanceKey.h | 3 +++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index a05474216..3252e706f 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -210,7 +210,13 @@ interpretSequence( const YAML::Node& node, Settings::ModuleSequence& moduleSeque } QStringList thisActionRoster = sequenceVListItem.toMap().value( thisActionS ).toStringList(); - moduleSequence.append( qMakePair( thisAction, thisActionRoster ) ); + Calamares::ModuleSystem::InstanceKeyList roster; + roster.reserve( thisActionRoster.count() ); + std::transform( thisActionRoster.constBegin(), + thisActionRoster.constEnd(), + std::back_inserter( roster ), + []( const QString& s ) { return Calamares::ModuleSystem::InstanceKey::fromString( s ); } ); + moduleSequence.append( qMakePair( thisAction, roster ) ); } } else @@ -262,24 +268,23 @@ Settings::validateSequence() // Check the sequence against the existing instances (which so far are only custom) for ( const auto& step : m_modulesSequence ) { - for ( const auto& instance : step.second ) + for ( const auto& instanceKey : step.second ) { - Calamares::ModuleSystem::InstanceKey k = Calamares::ModuleSystem::InstanceKey::fromString( instance ); - if ( !k.isValid() ) + if ( !instanceKey.isValid() ) { - cWarning() << "Invalid instance key in *sequence*," << instance; + cWarning() << "Invalid instance key in *sequence*," << instanceKey; continue; } - targetKey = k; + targetKey = instanceKey; const auto it = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); if ( it == m_moduleInstances.constEnd() ) { - if ( k.isCustom() ) + if ( instanceKey.isCustom() ) { - cWarning() << "Custom instance key" << instance << "is not listed in the *instances*"; + cWarning() << "Custom instance key" << instanceKey << "is not listed in the *instances*"; } - m_moduleInstances.append( InstanceDescription( k ) ); + m_moduleInstances.append( InstanceDescription( instanceKey ) ); } } } diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index d06a97716..6ebccfb43 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -110,7 +110,7 @@ public: */ InstanceDescriptionList moduleInstances() const; - using ModuleSequence = QList< QPair< ModuleSystem::Action, QStringList > >; + using ModuleSequence = QList< QPair< ModuleSystem::Action, Calamares::ModuleSystem::InstanceKeyList > >; /** @brief Representation of *sequence* of execution * * Each "section" of the *sequence* key in `settings.conf` gets an diff --git a/src/libcalamares/modulesystem/InstanceKey.h b/src/libcalamares/modulesystem/InstanceKey.h index 0bfa636f1..074743bf3 100644 --- a/src/libcalamares/modulesystem/InstanceKey.h +++ b/src/libcalamares/modulesystem/InstanceKey.h @@ -22,6 +22,7 @@ #define MODULESYSTEM_INSTANCEKEY_H #include +#include #include #include @@ -96,6 +97,8 @@ private: } }; +using InstanceKeyList = QList< InstanceKey >; + QDebug& operator<<( QDebug& s, const Calamares::ModuleSystem::InstanceKey& i ); } // namespace ModuleSystem From a8075fba5f2fd9cab3d2c2c1f9c8f9e0046a2a53 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 15:28:25 +0200 Subject: [PATCH 18/34] [libcalamares] Chase API change in settings --- src/libcalamaresui/modulesystem/ModuleManager.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 523e6ec24..8c15fc1eb 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -206,13 +206,12 @@ ModuleManager::loadModules() { ModuleSystem::Action currentAction = modulePhase.first; - foreach ( const QString& moduleEntry, modulePhase.second ) + for ( const auto& instanceKey : modulePhase.second ) { - auto instanceKey = ModuleSystem::InstanceKey::fromString( moduleEntry ); if ( !instanceKey.isValid() ) { - cError() << "Wrong module entry format for module" << moduleEntry; - failedModules.append( moduleEntry ); + cError() << "Wrong module entry format for module" << instanceKey; + failedModules.append( instanceKey.toString() ); continue; } From 4cd2a4ae91e3899e8e029a77d720e621d1ff4757 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 15:49:14 +0200 Subject: [PATCH 19/34] [libcalamares] Produce warnings while reading settings - any invalid instance key will cause a complaint - "new" custom instances in sequence get a complaint, but the instance description added to the list is valid --- src/libcalamares/Settings.cpp | 26 +++++++++++++++++++------- src/libcalamares/Settings.h | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 3252e706f..813da5c35 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -169,7 +169,13 @@ interpretInstances( const YAML::Node& node, Settings::InstanceDescriptionList& c { continue; } - customInstances.append( InstanceDescription::fromSettings( instancesVListItem.toMap() ) ); + auto description = InstanceDescription::fromSettings( instancesVListItem.toMap() ); + if ( !description.isValid() ) + { + cWarning() << "Invalid entry in *instances*" << instancesVListItem; + } + // Append it **anyway**, since this will bail out after Settings is constructed + customInstances.append( description ); } } } @@ -206,16 +212,22 @@ interpretSequence( const YAML::Node& node, Settings::ModuleSequence& moduleSeque } else { + cDebug() << "Unknown action in *sequence*" << thisActionS; continue; } QStringList thisActionRoster = sequenceVListItem.toMap().value( thisActionS ).toStringList(); Calamares::ModuleSystem::InstanceKeyList roster; roster.reserve( thisActionRoster.count() ); - std::transform( thisActionRoster.constBegin(), - thisActionRoster.constEnd(), - std::back_inserter( roster ), - []( const QString& s ) { return Calamares::ModuleSystem::InstanceKey::fromString( s ); } ); + for ( const auto& s : thisActionRoster ) + { + auto instanceKey = Calamares::ModuleSystem::InstanceKey::fromString( s ); + if ( !instanceKey.isValid() ) + { + cWarning() << "Invalid instance in *sequence*" << s; + } + roster.append( instanceKey ); + } moduleSequence.append( qMakePair( thisAction, roster ) ); } } @@ -258,7 +270,7 @@ Settings::Settings( const QString& settingsFilePath, bool debugMode ) } void -Settings::validateSequence() +Settings::reconcileInstancesAndSequence() { // Since moduleFinder captures targetKey by reference, we can // update targetKey to change what the finder lambda looks for. @@ -311,7 +323,7 @@ Settings::setConfiguration( const QByteArray& ba, const QString& explainName ) m_disableCancelDuringExec = requireBool( config, "disable-cancel-during-exec", false ); m_quitAtEnd = requireBool( config, "quit-at-end", false ); - validateSequence(); + reconcileInstancesAndSequence(); } catch ( YAML::Exception& e ) { diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 6ebccfb43..5507e2322 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -89,7 +89,7 @@ public: explicit Settings( const QString& settingsFilePath, bool debugMode ); void setConfiguration( const QByteArray& configData, const QString& explainName ); - void validateSequence(); + void reconcileInstancesAndSequence(); public: static Settings* instance(); From d81d585c3235d8f9feed9d30bd554b7648f02194 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 16:15:37 +0200 Subject: [PATCH 20/34] [libcalamares] Add isValid() to Settings - settings can be invalid (missing data, whatever) and that can be used to shut things down early. Validity must be checked explicitly, though. --- src/libcalamares/Settings.cpp | 27 +++++++++++++++++++++------ src/libcalamares/Settings.h | 7 +++++++ src/libcalamares/Tests.cpp | 6 +++++- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 813da5c35..7e10de0e8 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -282,12 +282,6 @@ Settings::reconcileInstancesAndSequence() { for ( const auto& instanceKey : step.second ) { - if ( !instanceKey.isValid() ) - { - cWarning() << "Invalid instance key in *sequence*," << instanceKey; - continue; - } - targetKey = instanceKey; const auto it = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), moduleFinder ); if ( it == m_moduleInstances.constEnd() ) @@ -447,4 +441,25 @@ Settings::init( const QString& path ) return new Calamares::Settings( path, true ); } +bool +Settings::isValid() const +{ + if ( brandingComponentName().isEmpty() ) + { + cWarning() << "No branding component is set"; + return false; + } + + const auto invalidDescriptor = []( const InstanceDescription& d ) { return !d.isValid(); }; + const auto invalidDescriptorIt + = std::find_if( m_moduleInstances.constBegin(), m_moduleInstances.constEnd(), invalidDescriptor ); + if ( invalidDescriptorIt != m_moduleInstances.constEnd() ) + { + cWarning() << "Invalid module instance in *instances* or *sequence*"; + return false; + } + + return true; +} + } // namespace Calamares diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 5507e2322..8e463fc70 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -122,6 +122,13 @@ public: QString brandingComponentName() const; + /** @brief Are the settings consistent and valid? + * + * Checks that at least branding is set, and that the instances + * and sequence are valid. + */ + bool isValid() const; + /** @brief Is this a debugging run? * * Returns true if Calamares is in debug mode. In debug mode, diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 61da7639e..1dc297dd4 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -367,6 +367,7 @@ TestLibCalamares::testSettings() { Calamares::Settings s( false ); QVERIFY( !s.debugMode() ); + QVERIFY( !s.isValid() ); } { Calamares::Settings s( true ); @@ -374,8 +375,10 @@ TestLibCalamares::testSettings() QVERIFY( s.moduleInstances().isEmpty() ); QVERIFY( s.modulesSequence().isEmpty() ); QVERIFY( s.brandingComponentName().isEmpty() ); + QVERIFY( !s.isValid() ); s.setConfiguration( R"(--- +branding: default # needed for it to be considered valid instances: - module: welcome id: hi @@ -397,7 +400,8 @@ sequence: QVERIFY( s.debugMode() ); QCOMPARE( s.moduleInstances().count(), 4 ); // there are 4 module instances mentioned QCOMPARE( s.modulesSequence().count(), 2 ); // 2 steps (show, exec) - QVERIFY( s.brandingComponentName().isEmpty() ); + QVERIFY( !s.brandingComponentName().isEmpty() ); + QVERIFY( s.isValid() ); // Make a lambda where we can adjust what it looks for from the outside, // by capturing a reference. From c8964717c77500feb8a00e5d90677c22666df87e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 16:16:43 +0200 Subject: [PATCH 21/34] [calamares] Bail out on invalid settings --- src/calamares/main.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/calamares/main.cpp b/src/calamares/main.cpp index 030ccc239..4ca089102 100644 --- a/src/calamares/main.cpp +++ b/src/calamares/main.cpp @@ -166,6 +166,11 @@ main( int argc, char* argv[] ) #endif Calamares::Settings::init( is_debug ); + if ( !Calamares::Settings::instance() || !Calamares::Settings::instance()->isValid() ) + { + qCritical() << "Calamares has invalid settings, shutting down."; + return 78; // EX_CONFIG on FreeBSD + } a.init(); return a.exec(); } From 7cef99605f482b43306c7f22bd6e29ced868aca6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 16:45:51 +0200 Subject: [PATCH 22/34] [libcalamares] Distinguish instances with an explicit weight - setting the weight in *instances* should be different from letting the default weight (of 1) stand; explicitly saying 1 should carry some weight (ha!) --- src/libcalamares/Settings.cpp | 9 ++++++--- src/libcalamares/Settings.h | 3 ++- src/libcalamares/Tests.cpp | 27 +++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 7e10de0e8..592ffd02d 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -75,7 +75,7 @@ namespace Calamares InstanceDescription::InstanceDescription( const Calamares::ModuleSystem::InstanceKey& key ) : m_instanceKey( key ) - , m_weight( 1 ) + , m_weight( -1 ) { if ( !isValid() ) { @@ -94,8 +94,11 @@ InstanceDescription::fromSettings( const QVariantMap& m ) Calamares::ModuleSystem::InstanceKey( m.value( "module" ).toString(), m.value( "id" ).toString() ) ); if ( r.isValid() ) { - int w = qBound( 1, m.value( "weight" ).toInt(), 100 ); - r.m_weight = w; + if ( m.value( "weight" ).isValid() ) + { + int w = qBound( 1, m.value( "weight" ).toInt(), 100 ); + r.m_weight = w; + } QString c = m.value( "config" ).toString(); if ( !c.isEmpty() ) diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 8e463fc70..1cff4b34a 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -71,7 +71,8 @@ public: const InstanceKey& key() const { return m_instanceKey; } QString configFileName() const { return m_configFileName; } bool isCustom() const { return m_instanceKey.isCustom(); } - int weight() const { return m_weight; } + int weight() const { return m_weight < 0 ? 1 : m_weight; } + bool explicitWeight() const { return m_weight > 0; } private: InstanceKey m_instanceKey; diff --git a/src/libcalamares/Tests.cpp b/src/libcalamares/Tests.cpp index 1dc297dd4..5903dfde6 100644 --- a/src/libcalamares/Tests.cpp +++ b/src/libcalamares/Tests.cpp @@ -270,6 +270,7 @@ TestLibCalamares::testInstanceDescription() QVERIFY( !d.isCustom() ); QCOMPARE( d.weight(), 0 ); QVERIFY( d.configFileName().isEmpty() ); + QVERIFY( !d.explicitWeight() ); } { @@ -278,6 +279,7 @@ TestLibCalamares::testInstanceDescription() QVERIFY( !d.isCustom() ); QCOMPARE( d.weight(), 0 ); QVERIFY( d.configFileName().isEmpty() ); + QVERIFY( !d.explicitWeight() ); } // Private constructor @@ -290,6 +292,7 @@ TestLibCalamares::testInstanceDescription() QCOMPARE( d.weight(), 1 ); // **now** the constraints kick in QVERIFY( !d.configFileName().isEmpty() ); QCOMPARE( d.configFileName(), QStringLiteral( "welcome.conf" ) ); + QVERIFY( !d.explicitWeight() ); } { @@ -299,6 +302,7 @@ TestLibCalamares::testInstanceDescription() QCOMPARE( d.weight(), 1 ); // **now** the constraints kick in QVERIFY( !d.configFileName().isEmpty() ); QCOMPARE( d.configFileName(), QStringLiteral( "welcome.conf" ) ); + QVERIFY( !d.explicitWeight() ); } @@ -317,6 +321,7 @@ TestLibCalamares::testInstanceDescription() InstanceDescription d = InstanceDescription::fromSettings( m ); QVERIFY( !d.isValid() ); + QVERIFY( !d.explicitWeight() ); } { QVariantMap m; @@ -325,7 +330,27 @@ TestLibCalamares::testInstanceDescription() InstanceDescription d = InstanceDescription::fromSettings( m ); QVERIFY( d.isValid() ); QVERIFY( !d.isCustom() ); + // Valid, but no weight set by settings QCOMPARE( d.weight(), 1 ); + QVERIFY( !d.explicitWeight() ); + + QCOMPARE( d.key().module(), QString( "welcome" ) ); + QCOMPARE( d.key().id(), QString( "welcome" ) ); + QCOMPARE( d.configFileName(), QString( "welcome.conf" ) ); + } + { + QVariantMap m; + m.insert( "module", "welcome" ); + m.insert( "weight", 1); + + InstanceDescription d = InstanceDescription::fromSettings( m ); + QVERIFY( d.isValid() ); + QVERIFY( !d.isCustom() ); + + //Valid, set explicitly + QCOMPARE( d.weight(), 1 ); + QVERIFY( d.explicitWeight() ); + QCOMPARE( d.key().module(), QString( "welcome" ) ); QCOMPARE( d.key().id(), QString( "welcome" ) ); QCOMPARE( d.configFileName(), QString( "welcome.conf" ) ); @@ -343,6 +368,7 @@ TestLibCalamares::testInstanceDescription() QCOMPARE( d.key().module(), QString( "welcome" ) ); QCOMPARE( d.key().id(), QString( "hi" ) ); QCOMPARE( d.configFileName(), QString( "welcome.conf" ) ); + QVERIFY( d.explicitWeight() ); } { QVariantMap m; @@ -358,6 +384,7 @@ TestLibCalamares::testInstanceDescription() QCOMPARE( d.key().module(), QString( "welcome" ) ); QCOMPARE( d.key().id(), QString( "hi" ) ); QCOMPARE( d.configFileName(), QString( "hi.conf" ) ); + QVERIFY( d.explicitWeight() ); } } From bdd6bdc3b251b86edd1de3df5e4aae88e1234df8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 20:15:14 +0200 Subject: [PATCH 23/34] [libcalamares] Migrate module type and interface to descriptor - move the enums - expose the named-enum functions for them - **start** replacing Descriptor with something stronger; this fails zero tests so it obviously wasn't tested at all --- src/libcalamares/CMakeLists.txt | 1 + src/libcalamares/modulesystem/Descriptor.cpp | 48 ++++++++++++++++++++ src/libcalamares/modulesystem/Descriptor.h | 44 +++++++++++++++++- src/libcalamares/modulesystem/Module.cpp | 44 ++---------------- src/libcalamares/modulesystem/Module.h | 29 +----------- 5 files changed, 99 insertions(+), 67 deletions(-) create mode 100644 src/libcalamares/modulesystem/Descriptor.cpp diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 3bfc8e479..3eab21a99 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -60,6 +60,7 @@ set( libSources locale/TranslatableString.cpp # Modules + modulesystem/Descriptor.cpp modulesystem/InstanceKey.cpp modulesystem/Module.cpp modulesystem/RequirementsChecker.cpp diff --git a/src/libcalamares/modulesystem/Descriptor.cpp b/src/libcalamares/modulesystem/Descriptor.cpp new file mode 100644 index 000000000..c8a3c3118 --- /dev/null +++ b/src/libcalamares/modulesystem/Descriptor.cpp @@ -0,0 +1,48 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2020 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + */ + +#include "Descriptor.h" + +namespace Calamares +{ +namespace ModuleSystem +{ + +const NamedEnumTable< Type >& +typeNames() +{ + // *INDENT-OFF* + // clang-format off + static const NamedEnumTable< Type > table{ + { QStringLiteral( "job" ), Type::Job }, + { QStringLiteral( "view" ), Type::View }, + { QStringLiteral( "viewmodule" ), Type::View }, + { QStringLiteral( "jobmodule" ), Type::Job } + }; + // *INDENT-ON* + // clang-format on + return table; +} + +const NamedEnumTable< Interface >& +interfaceNames() +{ + // *INDENT-OFF* + // clang-format off + static const NamedEnumTable< Interface > table { + { QStringLiteral("process"), Interface::Process }, + { QStringLiteral("qtplugin"), Interface::QtPlugin }, + { QStringLiteral("python"), Interface::Python }, + { QStringLiteral("pythonqt"), Interface::PythonQt } + }; + // *INDENT-ON* + // clang-format on + return table; +} + +} // namespace ModuleSystem +} // namespace Calamares diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index c6b5ab5cf..1d1a24b8b 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -21,17 +21,59 @@ #ifndef MODULESYSTEM_DESCRIPTOR_H #define MODULESYSTEM_DESCRIPTOR_H +#include "utils/NamedEnum.h" + #include namespace Calamares { namespace ModuleSystem { +/** + * @brief The Type enum represents the intended functionality of the module + * Every module is either a job module or a view module. + * A job module is a single Calamares job. + * A view module has a UI (one or more view pages) and zero-to-many jobs. + */ +enum class Type +{ + Job, + View +}; +const NamedEnumTable< Type >& typeNames(); + +/** + * @brief The Interface enum represents the interface through which the module + * talks to Calamares. + * Not all Type-Interface associations are valid. + */ +enum class Interface +{ + QtPlugin, // Jobs or Views + Python, // Jobs only + Process, // Deprecated interface + PythonQt // Views only, available as enum even if PythonQt isn't used +}; +const NamedEnumTable< Interface >& interfaceNames(); + + /* While this isn't a useful *using* right now, the intention is * to create a more strongly-typed Module Descriptor that carries * only the necessary information and no variants. */ -using Descriptor = QVariantMap; +// using Descriptor = QVariantMap; + +class Descriptor +{ +public: + Descriptor(); + + bool isValid() const { return false; } + + QString name() const { return QString(); } + bool isEmergency() const { return false; } +}; + } // namespace ModuleSystem } // namespace Calamares diff --git a/src/libcalamares/modulesystem/Module.cpp b/src/libcalamares/modulesystem/Module.cpp index dae3c84aa..18c33bde8 100644 --- a/src/libcalamares/modulesystem/Module.cpp +++ b/src/libcalamares/modulesystem/Module.cpp @@ -49,10 +49,10 @@ Module::~Module() {} void Module::initFrom( const Calamares::ModuleSystem::Descriptor& moduleDescriptor, const QString& id ) { - m_key = ModuleSystem::InstanceKey( moduleDescriptor.value( "name" ).toString(), id ); - if ( moduleDescriptor.contains( EMERGENCY ) ) + m_key = ModuleSystem::InstanceKey( moduleDescriptor.name(), id ); + if ( moduleDescriptor.isEmergency() ) { - m_maybe_emergency = moduleDescriptor[ EMERGENCY ].toBool(); + m_maybe_emergency = true; } } @@ -133,54 +133,20 @@ Module::loadConfigurationFile( const QString& configFileName ) //throws YAML::E } -static const NamedEnumTable< Module::Type >& -typeNames() -{ - using Type = Module::Type; - // *INDENT-OFF* - // clang-format off - static const NamedEnumTable< Type > table{ - { QStringLiteral( "job" ), Type::Job }, - { QStringLiteral( "view" ), Type::View }, - { QStringLiteral( "viewmodule" ), Type::View }, - { QStringLiteral( "jobmodule" ), Type::Job } - }; - // *INDENT-ON* - // clang-format on - return table; -} - QString Module::typeString() const { bool ok = false; - QString v = typeNames().find( type(), ok ); + QString v = Calamares::ModuleSystem::typeNames().find( type(), ok ); return ok ? v : QString(); } -static const NamedEnumTable< Module::Interface >& -interfaceNames() -{ - using Interface = Module::Interface; - // *INDENT-OFF* - // clang-format off - static const NamedEnumTable< Interface > table { - { QStringLiteral("process"), Interface::Process }, - { QStringLiteral("qtplugin"), Interface::QtPlugin }, - { QStringLiteral("python"), Interface::Python }, - { QStringLiteral("pythonqt"), Interface::PythonQt } - }; - // *INDENT-ON* - // clang-format on - return table; -} - QString Module::interfaceString() const { bool ok = false; - QString v = interfaceNames().find( interface(), ok ); + QString v = Calamares::ModuleSystem::interfaceNames().find( interface(), ok ); return ok ? v : QString(); } diff --git a/src/libcalamares/modulesystem/Module.h b/src/libcalamares/modulesystem/Module.h index 44e89fd75..a9492c21c 100644 --- a/src/libcalamares/modulesystem/Module.h +++ b/src/libcalamares/modulesystem/Module.h @@ -51,31 +51,6 @@ Module* moduleFromDescriptor( const ModuleSystem::Descriptor& moduleDescriptor, class DLLEXPORT Module { public: - /** - * @brief The Type enum represents the intended functionality of the module - * Every module is either a job module or a view module. - * A job module is a single Calamares job. - * A view module has a UI (one or more view pages) and zero-to-many jobs. - */ - enum class Type - { - Job, - View - }; - - /** - * @brief The Interface enum represents the interface through which the module - * talks to Calamares. - * Not all Type-Interface associations are valid. - */ - enum class Interface - { - QtPlugin, // Jobs or Views - Python, // Jobs only - Process, // Deprecated interface - PythonQt // Views only, available as enum even if PythonQt isn't used - }; - virtual ~Module(); /** @@ -157,13 +132,13 @@ public: * @brief type returns the Type of this module object. * @return the type enum value. */ - virtual Type type() const = 0; + virtual ModuleSystem::Type type() const = 0; /** * @brief interface the Interface used by this module. * @return the interface enum value. */ - virtual Interface interface() const = 0; + virtual ModuleSystem::Interface interface() const = 0; /** * @brief Check the requirements of this module. From 197cb9982c33ab83325386479226fd8349a6b77a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 20:19:00 +0200 Subject: [PATCH 24/34] [libcalamares] Sort the tests by subdir --- src/libcalamares/CMakeLists.txt | 43 +++++++++++++++++---------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 3eab21a99..284acd1d2 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -223,18 +223,6 @@ calamares_add_test( Tests.cpp ) -calamares_add_test( - libcalamaresutilstest - SOURCES - utils/Tests.cpp -) - -calamares_add_test( - libcalamaresutilspathstest - SOURCES - utils/TestPaths.cpp -) - calamares_add_test( libcalamaresgeoiptest SOURCES @@ -242,6 +230,24 @@ calamares_add_test( ${geoip_src} ) +calamares_add_test( + libcalamareslocaletest + SOURCES + locale/Tests.cpp +) + +calamares_add_test( + libcalamaresmodulesystemtest + SOURCES + modulesystem/Tests.cpp +) + +calamares_add_test( + libcalamaresnetworktest + SOURCES + network/Tests.cpp +) + calamares_add_test( libcalamarespartitiontest SOURCES @@ -259,22 +265,17 @@ if( KPMcore_FOUND ) endif() calamares_add_test( - libcalamareslocaletest + libcalamaresutilstest SOURCES - locale/Tests.cpp + utils/Tests.cpp ) calamares_add_test( - libcalamaresnetworktest + libcalamaresutilspathstest SOURCES - network/Tests.cpp + utils/TestPaths.cpp ) -calamares_add_test( - libcalamaresmodulesystemtest - SOURCES - modulesystem/Tests.cpp -) # This is not an actual test, it's a test / demo application # for experimenting with GeoIP. From f0c41645153d0cedacb93ed53c0543849cfd9dc4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 20:49:57 +0200 Subject: [PATCH 25/34] [libcalamares] Add a more convenient find() to NamedEnum - In most cases, you **know** the table covers all the enum values, and the extra parameter *ok* is just annoying. Provide a convenience that doesn't distinguish empty from empty-but-valid. --- src/libcalamares/utils/NamedEnum.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libcalamares/utils/NamedEnum.h b/src/libcalamares/utils/NamedEnum.h index bf8ff5d18..d962b3c76 100644 --- a/src/libcalamares/utils/NamedEnum.h +++ b/src/libcalamares/utils/NamedEnum.h @@ -105,6 +105,16 @@ struct NamedEnumTable // ok is still false return string_t(); } + + /** @brief Find a value @p s in the table and return its name. + * + * Returns emptry string if the value is not found. + */ + string_t find( enum_t s ) const + { + bool ok = false; + return find( s, ok ); + } }; /** @brief Smashes an enum value to its underlying type. */ From ee834a7abb346bf8fd6c8cdd397d2ffaa8435423 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 22:08:34 +0200 Subject: [PATCH 26/34] [libcalamares] Define interface for Module descriptor - add fields -- all const, all bogus -- to the descriptor, introduce a stub method to load the descriptor from YAML data (e.g. read from module.desc) - lighten the type-naming in Module a little, with usings --- src/libcalamares/modulesystem/Descriptor.cpp | 14 ++++++ src/libcalamares/modulesystem/Descriptor.h | 52 +++++++++++++++++++- src/libcalamares/modulesystem/Module.h | 7 ++- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/modulesystem/Descriptor.cpp b/src/libcalamares/modulesystem/Descriptor.cpp index c8a3c3118..6927e74a6 100644 --- a/src/libcalamares/modulesystem/Descriptor.cpp +++ b/src/libcalamares/modulesystem/Descriptor.cpp @@ -7,6 +7,8 @@ #include "Descriptor.h" +#include "utils/Logger.h" + namespace Calamares { namespace ModuleSystem @@ -44,5 +46,17 @@ interfaceNames() return table; } +Descriptor::Descriptor() {} + +Descriptor +Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) +{ + Descriptor d; + + cDebug() << moduleDesc; + + return d; +} + } // namespace ModuleSystem } // namespace Calamares diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index 1d1a24b8b..a1a04472e 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -61,17 +61,65 @@ const NamedEnumTable< Interface >& interfaceNames(); * to create a more strongly-typed Module Descriptor that carries * only the necessary information and no variants. */ -// using Descriptor = QVariantMap; - class Descriptor { public: + ///@brief an invalid, and empty, descriptor Descriptor(); + /** @brief Fills a descriptor from the loaded (YAML) data. + * + */ + static Descriptor fromDescriptorData( const QVariantMap& moduleDesc ); + bool isValid() const { return false; } QString name() const { return QString(); } + Type type() const { return Type::Job; } + Interface interface() const { return Interface::QtPlugin; } + bool isEmergency() const { return false; } + bool hasConfig() const { return true; } + + /// @brief The directory where the module.desc lives + QString directory() const { return m_directory; } + void setDirectory( const QString& d ) { m_directory = d; } + + QStringList requiredModules() const { return QStringList {}; } + + /** @section C++ Modules + * + * The C++ modules are the most general, and are loaded as + * a shared library after which a suitable factory creates + * objects from them. + */ + + /// @brief Short path to the shared-library; no extension. + QString load() const { return QString(); } + + /** @section Process Job modules + * + * Process Jobs are somewhat deprecated in favor of shellprocess + * and contextualprocess jobs, since those handle multiple configuration + * much more gracefully. + * + * Process Jobs execute one command. + */ + /// @brief The command to execute; passed to the shell + QString command() const { return QString(); } + /// @brief Timeout in seconds + int timeout() const { return 30; } + /// @brief Run command in chroot? + bool chroot() const { return false; } + + /** @section Python Job modules + * + * Python job modules have one specific script to load and run. + */ + QString script() const { return QString(); } + +private: + QString m_directory; }; } // namespace ModuleSystem diff --git a/src/libcalamares/modulesystem/Module.h b/src/libcalamares/modulesystem/Module.h index a9492c21c..e9719d756 100644 --- a/src/libcalamares/modulesystem/Module.h +++ b/src/libcalamares/modulesystem/Module.h @@ -51,6 +51,9 @@ Module* moduleFromDescriptor( const ModuleSystem::Descriptor& moduleDescriptor, class DLLEXPORT Module { public: + using Type = ModuleSystem::Type; + using Interface = ModuleSystem::Interface; + virtual ~Module(); /** @@ -132,13 +135,13 @@ public: * @brief type returns the Type of this module object. * @return the type enum value. */ - virtual ModuleSystem::Type type() const = 0; + virtual Type type() const = 0; /** * @brief interface the Interface used by this module. * @return the interface enum value. */ - virtual ModuleSystem::Interface interface() const = 0; + virtual Interface interface() const = 0; /** * @brief Check the requirements of this module. From 3c6e53ecb979ea8c3d1515d612b4787178e9f8f0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 22:12:16 +0200 Subject: [PATCH 27/34] [libcalamaresui] Chase the change in Module descriptor - most of the code becomes **simpler** because the requirement to handle unstructured data is now in the descriptor itself, rather than in consumers. --- .../modulesystem/CppJobModule.cpp | 7 ++--- .../modulesystem/CppJobModule.h | 2 +- .../modulesystem/ModuleFactory.cpp | 31 +++++++++---------- .../modulesystem/ModuleManager.cpp | 23 +++++++------- .../modulesystem/ModuleManager.h | 1 - .../modulesystem/ProcessJobModule.cpp | 26 +++------------- .../modulesystem/ProcessJobModule.h | 2 +- .../modulesystem/PythonJobModule.cpp | 8 ++--- .../modulesystem/PythonJobModule.h | 2 +- .../modulesystem/ViewModule.cpp | 7 ++--- src/libcalamaresui/modulesystem/ViewModule.h | 2 +- 11 files changed, 42 insertions(+), 69 deletions(-) diff --git a/src/libcalamaresui/modulesystem/CppJobModule.cpp b/src/libcalamaresui/modulesystem/CppJobModule.cpp index 632a9dcb8..6050c6955 100644 --- a/src/libcalamaresui/modulesystem/CppJobModule.cpp +++ b/src/libcalamaresui/modulesystem/CppJobModule.cpp @@ -84,13 +84,12 @@ CppJobModule::jobs() const void -CppJobModule::initFrom( const QVariantMap& moduleDescriptor ) +CppJobModule::initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) { QDir directory( location() ); - QString load; - if ( !moduleDescriptor.value( "load" ).toString().isEmpty() ) + QString load = moduleDescriptor.load(); + if ( !load.isEmpty() ) { - load = moduleDescriptor.value( "load" ).toString(); load = directory.absoluteFilePath( load ); } // If a load path is not specified, we look for a plugin to load in the directory. diff --git a/src/libcalamaresui/modulesystem/CppJobModule.h b/src/libcalamaresui/modulesystem/CppJobModule.h index 2fd82433c..8774a67e5 100644 --- a/src/libcalamaresui/modulesystem/CppJobModule.h +++ b/src/libcalamaresui/modulesystem/CppJobModule.h @@ -39,7 +39,7 @@ public: JobList jobs() const override; protected: - void initFrom( const QVariantMap& moduleDescriptor ) override; + void initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) override; private: explicit CppJobModule(); diff --git a/src/libcalamaresui/modulesystem/ModuleFactory.cpp b/src/libcalamaresui/modulesystem/ModuleFactory.cpp index f3b46eab7..050854613 100644 --- a/src/libcalamaresui/modulesystem/ModuleFactory.cpp +++ b/src/libcalamaresui/modulesystem/ModuleFactory.cpp @@ -52,23 +52,22 @@ moduleFromDescriptor( const Calamares::ModuleSystem::Descriptor& moduleDescripto const QString& configFileName, const QString& moduleDirectory ) { + using Type = Calamares::ModuleSystem::Type; + using Interface = Calamares::ModuleSystem::Interface; + std::unique_ptr< Module > m; - QString typeString = moduleDescriptor.value( "type" ).toString(); - QString intfString = moduleDescriptor.value( "interface" ).toString(); - - if ( typeString.isEmpty() || intfString.isEmpty() ) - { + if ( !moduleDescriptor.isValid() ) { cError() << "Bad module descriptor format" << instanceId; return nullptr; } - if ( ( typeString == "view" ) || ( typeString == "viewmodule" ) ) + if ( moduleDescriptor.type() == Type::View ) { - if ( intfString == "qtplugin" ) + if ( moduleDescriptor.interface() == Interface::QtPlugin ) { m.reset( new ViewModule() ); } - else if ( intfString == "pythonqt" ) + else if ( moduleDescriptor.interface() == Interface::PythonQt ) { #ifdef WITH_PYTHONQT m.reset( new PythonQtViewModule() ); @@ -78,20 +77,20 @@ moduleFromDescriptor( const Calamares::ModuleSystem::Descriptor& moduleDescripto } else { - cError() << "Bad interface" << intfString << "for module type" << typeString; + cError() << "Bad interface" << Calamares::ModuleSystem::interfaceNames().find( moduleDescriptor.interface() ) << "for module type" << Calamares::ModuleSystem::typeNames().find( moduleDescriptor.type() ); } } - else if ( typeString == "job" ) + else if ( moduleDescriptor.type() == Type::Job ) { - if ( intfString == "qtplugin" ) + if ( moduleDescriptor.interface() == Interface::QtPlugin ) { m.reset( new CppJobModule() ); } - else if ( intfString == "process" ) + else if ( moduleDescriptor.interface() == Interface::Process ) { m.reset( new ProcessJobModule() ); } - else if ( intfString == "python" ) + else if ( moduleDescriptor.interface() == Interface::Python ) { #ifdef WITH_PYTHON m.reset( new PythonJobModule() ); @@ -101,17 +100,17 @@ moduleFromDescriptor( const Calamares::ModuleSystem::Descriptor& moduleDescripto } else { - cError() << "Bad interface" << intfString << "for module type" << typeString; + cError() << "Bad interface" << Calamares::ModuleSystem::interfaceNames().find( moduleDescriptor.interface() ) << "for module type" << Calamares::ModuleSystem::typeNames().find( moduleDescriptor.type() ); } } else { - cError() << "Bad module type" << typeString; + cError() << "Bad module type" << Calamares::ModuleSystem::typeNames().find( moduleDescriptor.type() ); } if ( !m ) { - cError() << "Bad module type (" << typeString << ") or interface string (" << intfString << ") for module " + cError() << "Bad module type (" << Calamares::ModuleSystem::typeNames().find( moduleDescriptor.type() ) << ") or interface string (" << Calamares::ModuleSystem::interfaceNames().find( moduleDescriptor.interface() ) << ") for module " << instanceId; return nullptr; } diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 8c15fc1eb..56888b1b4 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -114,9 +114,9 @@ ModuleManager::doInit() if ( ok && !moduleName.isEmpty() && ( moduleName == currentDir.dirName() ) && !m_availableDescriptorsByModuleName.contains( moduleName ) ) { - m_availableDescriptorsByModuleName.insert( moduleName, moduleDescriptorMap ); - m_moduleDirectoriesByModuleName.insert( moduleName, - descriptorFileInfo.absoluteDir().absolutePath() ); + auto descriptor = Calamares::ModuleSystem::Descriptor::fromDescriptorData( moduleDescriptorMap ); + descriptor.setDirectory(descriptorFileInfo.absoluteDir().absolutePath() ); + m_availableDescriptorsByModuleName.insert( moduleName, descriptor ); } } else @@ -132,8 +132,7 @@ ModuleManager::doInit() } // At this point m_availableDescriptorsByModuleName is filled with // the modules that were found in the search paths. - cDebug() << "Found" << m_availableDescriptorsByModuleName.count() << "modules" - << m_moduleDirectoriesByModuleName.count() << "names"; + cDebug() << "Found" << m_availableDescriptorsByModuleName.count() << "modules"; emit initDone(); } @@ -169,7 +168,7 @@ getConfigFileName( const Settings::InstanceDescriptionList& descriptorList, const ModuleSystem::InstanceKey& instanceKey, const ModuleSystem::Descriptor& descriptor ) { - if ( descriptor.value( "noconfig", false ).toBool() ) + if ( !descriptor.hasConfig() ) { // Explicitly set to no-configuration. This doesn't apply // to custom instances (above) since the only reason to @@ -217,7 +216,7 @@ ModuleManager::loadModules() ModuleSystem::Descriptor descriptor = m_availableDescriptorsByModuleName.value( instanceKey.module(), ModuleSystem::Descriptor() ); - if ( descriptor.isEmpty() ) + if ( !descriptor.isValid() ) { cError() << "Module" << instanceKey.toString() << "not found in module search paths." << Logger::DebugList( m_paths ); @@ -258,7 +257,7 @@ ModuleManager::loadModules() = Calamares::moduleFromDescriptor( descriptor, instanceKey.id(), configFileName, - m_moduleDirectoriesByModuleName.value( instanceKey.module() ) ); + descriptor.directory() ); if ( !thisModule ) { cError() << "Module" << instanceKey.toString() << "cannot be created from descriptor" @@ -358,7 +357,7 @@ ModuleManager::checkRequirements() } static QStringList -missingRequiredModules( const QStringList& required, const QMap< QString, QVariantMap >& available ) +missingRequiredModules( const QStringList& required, const QMap< QString, ModuleSystem::Descriptor >& available ) { QStringList l; for ( const QString& depName : required ) @@ -386,12 +385,12 @@ ModuleManager::checkDependencies() for ( auto it = m_availableDescriptorsByModuleName.begin(); it != m_availableDescriptorsByModuleName.end(); ++it ) { - QStringList unmet = missingRequiredModules( it->value( "requiredModules" ).toStringList(), + QStringList unmet = missingRequiredModules( it->requiredModules(), m_availableDescriptorsByModuleName ); if ( unmet.count() > 0 ) { - QString moduleName = it->value( "name" ).toString(); + QString moduleName = it->name(); somethingWasRemovedBecauseOfUnmetDependencies = true; m_availableDescriptorsByModuleName.erase( it ); numberRemoved++; @@ -415,7 +414,7 @@ ModuleManager::checkModuleDependencies( const Module& m ) bool allRequirementsFound = true; QStringList requiredModules - = m_availableDescriptorsByModuleName[ m.name() ].value( "requiredModules" ).toStringList(); + = m_availableDescriptorsByModuleName[ m.name() ].requiredModules(); for ( const QString& required : requiredModules ) { diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index d079baee7..ed4cef8ba 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -162,7 +162,6 @@ private: bool checkModuleDependencies( const Module& ); QMap< QString, ModuleSystem::Descriptor > m_availableDescriptorsByModuleName; - QMap< QString, QString > m_moduleDirectoriesByModuleName; QMap< ModuleSystem::InstanceKey, Module* > m_loadedModulesByInstanceKey; const QStringList m_paths; RequirementsModel* m_requirementsModel; diff --git a/src/libcalamaresui/modulesystem/ProcessJobModule.cpp b/src/libcalamaresui/modulesystem/ProcessJobModule.cpp index 3fb0579d3..b5eed6e43 100644 --- a/src/libcalamaresui/modulesystem/ProcessJobModule.cpp +++ b/src/libcalamaresui/modulesystem/ProcessJobModule.cpp @@ -61,32 +61,14 @@ ProcessJobModule::jobs() const void -ProcessJobModule::initFrom( const QVariantMap& moduleDescriptor ) +ProcessJobModule::initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) { QDir directory( location() ); m_workingPath = directory.absolutePath(); - if ( !moduleDescriptor.value( "command" ).toString().isEmpty() ) - { - m_command = moduleDescriptor.value( "command" ).toString(); - } - - m_secondsTimeout = std::chrono::seconds( 30 ); - if ( moduleDescriptor.contains( "timeout" ) && !moduleDescriptor.value( "timeout" ).isNull() ) - { - int sec = moduleDescriptor.value( "timeout" ).toInt(); - if ( sec < 0 ) - { - sec = 0; - } - m_secondsTimeout = std::chrono::seconds( sec ); - } - - m_runInChroot = false; - if ( moduleDescriptor.contains( "chroot" ) && !moduleDescriptor.value( "chroot" ).isNull() ) - { - m_runInChroot = moduleDescriptor.value( "chroot" ).toBool(); - } + m_command = moduleDescriptor.command(); + m_secondsTimeout = std::chrono::seconds( moduleDescriptor.timeout() ); + m_runInChroot = moduleDescriptor.chroot(); } diff --git a/src/libcalamaresui/modulesystem/ProcessJobModule.h b/src/libcalamaresui/modulesystem/ProcessJobModule.h index 87c6e2da8..b59539a04 100644 --- a/src/libcalamaresui/modulesystem/ProcessJobModule.h +++ b/src/libcalamaresui/modulesystem/ProcessJobModule.h @@ -38,7 +38,7 @@ public: JobList jobs() const override; protected: - void initFrom( const QVariantMap& moduleDescriptor ) override; + void initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) override; private: explicit ProcessJobModule(); diff --git a/src/libcalamaresui/modulesystem/PythonJobModule.cpp b/src/libcalamaresui/modulesystem/PythonJobModule.cpp index d5d5563c7..72ca116fb 100644 --- a/src/libcalamaresui/modulesystem/PythonJobModule.cpp +++ b/src/libcalamaresui/modulesystem/PythonJobModule.cpp @@ -62,15 +62,11 @@ PythonJobModule::jobs() const void -PythonJobModule::initFrom( const QVariantMap& moduleDescriptor ) +PythonJobModule::initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) { QDir directory( location() ); m_workingPath = directory.absolutePath(); - - if ( !moduleDescriptor.value( "script" ).toString().isEmpty() ) - { - m_scriptFileName = moduleDescriptor.value( "script" ).toString(); - } + m_scriptFileName = moduleDescriptor.script(); } diff --git a/src/libcalamaresui/modulesystem/PythonJobModule.h b/src/libcalamaresui/modulesystem/PythonJobModule.h index 85f25ab74..db5554b9b 100644 --- a/src/libcalamaresui/modulesystem/PythonJobModule.h +++ b/src/libcalamaresui/modulesystem/PythonJobModule.h @@ -35,7 +35,7 @@ public: JobList jobs() const override; protected: - void initFrom( const QVariantMap& moduleDescriptor ) override; + void initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) override; private: explicit PythonJobModule(); diff --git a/src/libcalamaresui/modulesystem/ViewModule.cpp b/src/libcalamaresui/modulesystem/ViewModule.cpp index 54a79ab66..8f3b9a340 100644 --- a/src/libcalamaresui/modulesystem/ViewModule.cpp +++ b/src/libcalamaresui/modulesystem/ViewModule.cpp @@ -89,13 +89,12 @@ ViewModule::jobs() const void -ViewModule::initFrom( const QVariantMap& moduleDescriptor ) +ViewModule::initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) { QDir directory( location() ); - QString load; - if ( !moduleDescriptor.value( "load" ).toString().isEmpty() ) + QString load = moduleDescriptor.load(); + if ( !load.isEmpty() ) { - load = moduleDescriptor.value( "load" ).toString(); load = directory.absoluteFilePath( load ); } // If a load path is not specified, we look for a plugin to load in the directory. diff --git a/src/libcalamaresui/modulesystem/ViewModule.h b/src/libcalamaresui/modulesystem/ViewModule.h index 1d24ca811..ac56e7607 100644 --- a/src/libcalamaresui/modulesystem/ViewModule.h +++ b/src/libcalamaresui/modulesystem/ViewModule.h @@ -42,7 +42,7 @@ public: RequirementsList checkRequirements() override; protected: - void initFrom( const QVariantMap& moduleDescriptor ) override; + void initFrom( const ModuleSystem::Descriptor& moduleDescriptor ) override; private: explicit ViewModule(); From 60fbf04594393689a7305e0df82bbf6fd01a547d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 11 Aug 2020 22:13:04 +0200 Subject: [PATCH 28/34] [calamares] Adjust module test-loader for changed API - the test-loader needs to create modules (and does so hackishly, outside of the ModuleManager) so it needs to chase the API as well. --- src/calamares/testmain.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/calamares/testmain.cpp b/src/calamares/testmain.cpp index c9ac00f2b..d70ee70b5 100644 --- a/src/calamares/testmain.cpp +++ b/src/calamares/testmain.cpp @@ -184,13 +184,13 @@ public: void loadSelf() override; - virtual Type type() const override; - virtual Interface interface() const override; + virtual Calamares::ModuleSystem::Type type() const override; + virtual Calamares::ModuleSystem::Interface interface() const override; virtual Calamares::JobList jobs() const override; protected: - void initFrom( const QVariantMap& ) override; + void initFrom( const Calamares::ModuleSystem::Descriptor& ) override; }; ExecViewModule::ExecViewModule() @@ -201,13 +201,13 @@ ExecViewModule::ExecViewModule() // We don't have one, so build one -- this gives us "x@x". QVariantMap m; m.insert( "name", "x" ); - Calamares::Module::initFrom( m, "x" ); + Calamares::Module::initFrom( Calamares::ModuleSystem::Descriptor::fromDescriptorData(m), "x" ); } ExecViewModule::~ExecViewModule() {} void -ExecViewModule::initFrom( const QVariantMap& ) +ExecViewModule::initFrom( const Calamares::ModuleSystem::Descriptor& ) { } @@ -332,7 +332,7 @@ load_module( const ModuleConfig& moduleConfig ) cDebug() << "Module" << moduleName << "job-configuration:" << configFile; - Calamares::Module* module = Calamares::moduleFromDescriptor( descriptor, name, configFile, moduleDirectory ); + Calamares::Module* module = Calamares::moduleFromDescriptor( Calamares::ModuleSystem::Descriptor::fromDescriptorData( descriptor ), name, configFile, moduleDirectory ); return module; } From 65273a262b0d5de3a40f39fa3fe5975b8ddccff9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 12 Aug 2020 00:33:08 +0200 Subject: [PATCH 29/34] [libcalamares] Start putting data into the module Descriptor --- src/libcalamares/modulesystem/Descriptor.cpp | 17 +++++++++++++++++ src/libcalamares/modulesystem/Descriptor.h | 9 ++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/modulesystem/Descriptor.cpp b/src/libcalamares/modulesystem/Descriptor.cpp index 6927e74a6..b233686f0 100644 --- a/src/libcalamares/modulesystem/Descriptor.cpp +++ b/src/libcalamares/modulesystem/Descriptor.cpp @@ -55,6 +55,23 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) cDebug() << moduleDesc; + { + bool typeOk = false; + Type t = typeNames().find( moduleDesc.value( "type" ).toString(), typeOk ); + bool interfaceOk = false; + Interface i = interfaceNames().find( moduleDesc.value( "interface" ).toString(), interfaceOk ); + if ( typeOk && interfaceOk ) + { + d.m_type = t; + d.m_interface = i; + d.m_isValid = true; + } + } + if ( !d.m_isValid ) + { + return d; + } + return d; } diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index a1a04472e..709f37ff3 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -72,11 +72,11 @@ public: */ static Descriptor fromDescriptorData( const QVariantMap& moduleDesc ); - bool isValid() const { return false; } + bool isValid() const { return m_isValid; } QString name() const { return QString(); } - Type type() const { return Type::Job; } - Interface interface() const { return Interface::QtPlugin; } + Type type() const { return m_type; } + Interface interface() const { return m_interface; } bool isEmergency() const { return false; } bool hasConfig() const { return true; } @@ -120,6 +120,9 @@ public: private: QString m_directory; + Type m_type; + Interface m_interface; + bool m_isValid = false; }; } // namespace ModuleSystem From e1e81bb133bdebe7af79a4246419b6c59e9e504b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 12 Aug 2020 00:39:01 +0200 Subject: [PATCH 30/34] [libcalamaresui] Warnings--, don't shadow a parameter --- src/libcalamaresui/modulesystem/ModuleManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 56888b1b4..283ea8b5f 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -166,9 +166,9 @@ ModuleManager::moduleInstance( const ModuleSystem::InstanceKey& instanceKey ) static QString getConfigFileName( const Settings::InstanceDescriptionList& descriptorList, const ModuleSystem::InstanceKey& instanceKey, - const ModuleSystem::Descriptor& descriptor ) + const ModuleSystem::Descriptor& thisModule ) { - if ( !descriptor.hasConfig() ) + if ( !thisModule.hasConfig() ) { // Explicitly set to no-configuration. This doesn't apply // to custom instances (above) since the only reason to From e406ae19670b72d8032efefb478d7e183abdcfd9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 12 Aug 2020 00:52:54 +0200 Subject: [PATCH 31/34] [libcalamares] Add name to module descriptor - introduce basic tests of the data structure - interpret name when passed in as descriptor data --- src/libcalamares/modulesystem/Descriptor.cpp | 3 +- src/libcalamares/modulesystem/Descriptor.h | 3 +- src/libcalamares/modulesystem/Tests.cpp | 44 ++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/modulesystem/Descriptor.cpp b/src/libcalamares/modulesystem/Descriptor.cpp index b233686f0..439c7f48c 100644 --- a/src/libcalamares/modulesystem/Descriptor.cpp +++ b/src/libcalamares/modulesystem/Descriptor.cpp @@ -60,7 +60,8 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) Type t = typeNames().find( moduleDesc.value( "type" ).toString(), typeOk ); bool interfaceOk = false; Interface i = interfaceNames().find( moduleDesc.value( "interface" ).toString(), interfaceOk ); - if ( typeOk && interfaceOk ) + d.m_name = moduleDesc.value( "name" ).toString(); + if ( typeOk && interfaceOk && !d.m_name.isEmpty() ) { d.m_type = t; d.m_interface = i; diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index 709f37ff3..c6fa5e351 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -74,7 +74,7 @@ public: bool isValid() const { return m_isValid; } - QString name() const { return QString(); } + QString name() const { return m_name; } Type type() const { return m_type; } Interface interface() const { return m_interface; } @@ -119,6 +119,7 @@ public: QString script() const { return QString(); } private: + QString m_name; QString m_directory; Type m_type; Interface m_interface; diff --git a/src/libcalamares/modulesystem/Tests.cpp b/src/libcalamares/modulesystem/Tests.cpp index 282300eec..2fcd5353d 100644 --- a/src/libcalamares/modulesystem/Tests.cpp +++ b/src/libcalamares/modulesystem/Tests.cpp @@ -18,6 +18,7 @@ * */ +#include "modulesystem/Descriptor.h" #include "modulesystem/InstanceKey.h" #include @@ -40,6 +41,8 @@ private Q_SLOTS: void testBadSimpleCases(); void testBadFromStringCases(); + + void testBasicDescriptor(); }; void @@ -136,6 +139,47 @@ ModuleSystemTests::testBadFromStringCases() assert_is_invalid( k0 ); } +void +ModuleSystemTests::testBasicDescriptor() +{ + { + QVariantMap m; + auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m ); + + QVERIFY( !d.isValid() ); + QVERIFY( d.name().isEmpty() ); + } + { + QVariantMap m; + m.insert( "name", QVariant() ); + auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m ); + + QVERIFY( !d.isValid() ); + QVERIFY( d.name().isEmpty() ); + } + { + QVariantMap m; + m.insert( "name", 17 ); + auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m ); + + QVERIFY( !d.isValid() ); + QVERIFY( !d.name().isEmpty() ); + QCOMPARE( d.name(), QStringLiteral( "17" ) ); // Strange but true + } + { + QVariantMap m; + m.insert( "name", "welcome" ); + m.insert( "type", "viewmodule" ); + m.insert( "interface", "qtplugin" ); + auto d = Calamares::ModuleSystem::Descriptor::fromDescriptorData( m ); + + // QVERIFY( !d.isValid() ); + QCOMPARE( d.name(), QStringLiteral( "welcome" ) ); + QCOMPARE( d.type(), Calamares::ModuleSystem::Type::View ); + QCOMPARE( d.interface(), Calamares::ModuleSystem::Interface::QtPlugin ); + } +} + QTEST_GUILESS_MAIN( ModuleSystemTests ) From c8b96c278be0010df1e3ae5722870806dd7ccdff Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 12 Aug 2020 08:42:52 +0200 Subject: [PATCH 32/34] [libcalamares] Complete the generic module descriptor - loads emergency, noconfig, requiredModules keys - warns (and marks descriptor invalid) if there are unused / unknown keys left over in the descriptor data. --- src/libcalamares/modulesystem/Descriptor.cpp | 37 ++++++++++++++++++-- src/libcalamares/modulesystem/Descriptor.h | 9 +++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/libcalamares/modulesystem/Descriptor.cpp b/src/libcalamares/modulesystem/Descriptor.cpp index 439c7f48c..ecfd6b898 100644 --- a/src/libcalamares/modulesystem/Descriptor.cpp +++ b/src/libcalamares/modulesystem/Descriptor.cpp @@ -8,6 +8,7 @@ #include "Descriptor.h" #include "utils/Logger.h" +#include "utils/Variant.h" namespace Calamares { @@ -57,9 +58,21 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) { bool typeOk = false; - Type t = typeNames().find( moduleDesc.value( "type" ).toString(), typeOk ); + QString typeValue = moduleDesc.value( "type" ).toString(); + Type t = typeNames().find( typeValue, typeOk ); + if ( !typeOk ) + { + cWarning() << "Module descriptor contains invalid *type*" << typeValue; + } + bool interfaceOk = false; - Interface i = interfaceNames().find( moduleDesc.value( "interface" ).toString(), interfaceOk ); + QString interfaceValue = moduleDesc.value( "interface" ).toString(); + Interface i = interfaceNames().find( interfaceValue, interfaceOk ); + if ( !interfaceOk ) + { + cWarning() << "Module descriptor contains invalid *interface*" << interfaceValue; + } + d.m_name = moduleDesc.value( "name" ).toString(); if ( typeOk && interfaceOk && !d.m_name.isEmpty() ) { @@ -73,6 +86,26 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) return d; } + d.m_isEmergeny = CalamaresUtils::getBool( moduleDesc, "emergency", false ); + d.m_hasConfig = !CalamaresUtils::getBool( moduleDesc, "noconfig", false ); // Inverted logic during load + d.m_requiredModules = CalamaresUtils::getStringList( moduleDesc, "requiredModules" ); + + QStringList consumedKeys { "type", "interface", "name", "emergency", "noconfig", "requiredModules" }; + + QStringList superfluousKeys; + for ( auto kv = moduleDesc.keyBegin(); kv != moduleDesc.keyEnd(); ++kv ) + { + if ( !consumedKeys.contains( *kv ) ) + { + superfluousKeys << *kv; + } + } + if ( !superfluousKeys.isEmpty() ) + { + cWarning() << "Module descriptor contains extra keys:" << Logger::DebugList( superfluousKeys ); + d.m_isValid = false; + } + return d; } diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index c6fa5e351..b44b53c48 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -78,14 +78,14 @@ public: Type type() const { return m_type; } Interface interface() const { return m_interface; } - bool isEmergency() const { return false; } - bool hasConfig() const { return true; } + bool isEmergency() const { return m_isEmergeny; } + bool hasConfig() const { return m_hasConfig; } /// @brief The directory where the module.desc lives QString directory() const { return m_directory; } void setDirectory( const QString& d ) { m_directory = d; } - QStringList requiredModules() const { return QStringList {}; } + const QStringList& requiredModules() const { return m_requiredModules; } /** @section C++ Modules * @@ -121,9 +121,12 @@ public: private: QString m_name; QString m_directory; + QStringList m_requiredModules; Type m_type; Interface m_interface; bool m_isValid = false; + bool m_isEmergeny = false; + bool m_hasConfig = true; }; } // namespace ModuleSystem From efd7145f765f2930156eec0caca42bd82ab7532b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 12 Aug 2020 09:06:47 +0200 Subject: [PATCH 33/34] [libcalamares] Implement the interface-specific fields for descriptor --- src/libcalamares/modulesystem/Descriptor.cpp | 28 ++++++++++++++++++-- src/libcalamares/modulesystem/Descriptor.h | 27 +++++++++++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/modulesystem/Descriptor.cpp b/src/libcalamares/modulesystem/Descriptor.cpp index ecfd6b898..1ac4dc6c0 100644 --- a/src/libcalamares/modulesystem/Descriptor.cpp +++ b/src/libcalamares/modulesystem/Descriptor.cpp @@ -54,8 +54,6 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) { Descriptor d; - cDebug() << moduleDesc; - { bool typeOk = false; QString typeValue = moduleDesc.value( "type" ).toString(); @@ -92,6 +90,32 @@ Descriptor::fromDescriptorData( const QVariantMap& moduleDesc ) QStringList consumedKeys { "type", "interface", "name", "emergency", "noconfig", "requiredModules" }; + switch ( d.interface() ) + { + case Interface::QtPlugin: + d.m_script = CalamaresUtils::getString( moduleDesc, "load" ); + consumedKeys << "load"; + break; + case Interface::Python: + case Interface::PythonQt: + d.m_script = CalamaresUtils::getString( moduleDesc, "script" ); + consumedKeys << "script"; + break; + case Interface::Process: + d.m_script = CalamaresUtils::getString( moduleDesc, "command" ); + d.m_processTimeout = CalamaresUtils::getInteger( moduleDesc, "timeout", 30 ); + d.m_processChroot = CalamaresUtils::getBool( moduleDesc, "chroot", false ); + consumedKeys << "command" + << "timeout" + << "chroot"; + + if ( d.m_processTimeout < 0 ) + { + d.m_processTimeout = 0; + } + break; + } + QStringList superfluousKeys; for ( auto kv = moduleDesc.keyBegin(); kv != moduleDesc.keyEnd(); ++kv ) { diff --git a/src/libcalamares/modulesystem/Descriptor.h b/src/libcalamares/modulesystem/Descriptor.h index b44b53c48..8166f9869 100644 --- a/src/libcalamares/modulesystem/Descriptor.h +++ b/src/libcalamares/modulesystem/Descriptor.h @@ -95,7 +95,7 @@ public: */ /// @brief Short path to the shared-library; no extension. - QString load() const { return QString(); } + QString load() const { return m_interface == Interface::QtPlugin ? m_script : QString(); } /** @section Process Job modules * @@ -106,17 +106,20 @@ public: * Process Jobs execute one command. */ /// @brief The command to execute; passed to the shell - QString command() const { return QString(); } + QString command() const { return m_interface == Interface::Process ? m_script : QString(); } /// @brief Timeout in seconds - int timeout() const { return 30; } + int timeout() const { return m_processTimeout; } /// @brief Run command in chroot? - bool chroot() const { return false; } + bool chroot() const { return m_processChroot; } /** @section Python Job modules * * Python job modules have one specific script to load and run. */ - QString script() const { return QString(); } + QString script() const + { + return ( m_interface == Interface::Python || m_interface == Interface::PythonQt ) ? m_script : QString(); + } private: QString m_name; @@ -127,6 +130,20 @@ private: bool m_isValid = false; bool m_isEmergeny = false; bool m_hasConfig = true; + + /** @brief The name of the thing to load + * + * - A C++ module loads a shared library (via key *load*), + * - A Python module loads a Python script (via key *script*), + * - A process runs a specific command (via key *command*) + * + * This name-of-the-thing is stored here, regardless of which + * interface is being used. + */ + QString m_script; + + int m_processTimeout = 30; + bool m_processChroot = false; }; } // namespace ModuleSystem From f73f94da27bfd77e30ef5698ecdc83d5039d652d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 12 Aug 2020 09:36:30 +0200 Subject: [PATCH 34/34] Repait module.desc - a handful of modules had an unused *requires* key in module.desc; this is probably from previous intentions around prerequisites-testing. Since the settings were empty anyway, they have been removed. - [unpackfs] Compacted the way *requiredModules* list is written --- src/modules/displaymanager/module.desc | 1 - src/modules/hwclock/module.desc | 1 - src/modules/networkcfg/module.desc | 1 - src/modules/services-systemd/module.desc | 1 - src/modules/unpackfs/module.desc | 3 +-- 5 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/modules/displaymanager/module.desc b/src/modules/displaymanager/module.desc index ad7ae4423..56d3fcb07 100644 --- a/src/modules/displaymanager/module.desc +++ b/src/modules/displaymanager/module.desc @@ -2,5 +2,4 @@ type: "job" name: "displaymanager" interface: "python" -requires: [] script: "main.py" diff --git a/src/modules/hwclock/module.desc b/src/modules/hwclock/module.desc index 47d1b6c14..ba3dbbaf8 100644 --- a/src/modules/hwclock/module.desc +++ b/src/modules/hwclock/module.desc @@ -2,6 +2,5 @@ type: "job" name: "hwclock" interface: "python" -requires: [] script: "main.py" noconfig: true diff --git a/src/modules/networkcfg/module.desc b/src/modules/networkcfg/module.desc index c02305db2..68024cb8d 100644 --- a/src/modules/networkcfg/module.desc +++ b/src/modules/networkcfg/module.desc @@ -2,6 +2,5 @@ type: "job" name: "networkcfg" interface: "python" -requires: [] script: "main.py" noconfig: true diff --git a/src/modules/services-systemd/module.desc b/src/modules/services-systemd/module.desc index 4a72b658b..4305b1141 100644 --- a/src/modules/services-systemd/module.desc +++ b/src/modules/services-systemd/module.desc @@ -2,5 +2,4 @@ type: "job" name: "services-systemd" interface: "python" -requires: [] script: "main.py" diff --git a/src/modules/unpackfs/module.desc b/src/modules/unpackfs/module.desc index 4b3086e44..54d95df1d 100644 --- a/src/modules/unpackfs/module.desc +++ b/src/modules/unpackfs/module.desc @@ -4,5 +4,4 @@ type: "job" name: "unpackfs" interface: "python" script: "main.py" -requiredModules: - - mount +requiredModules: [ mount ]