From a6688504e801e1eecc2a3e68bcb51e3f8ff7ad1e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 04:17:03 -0400 Subject: [PATCH 02/13] CMake: switch INSTALL_CONFIG to OFF by default - The examples files are not harmless, so distro's should take a explicit decision to install the config examples (instead of putting files in /etc/calamares). --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5eed360f8..d72463bbd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,7 +36,7 @@ cmake_minimum_required( VERSION 3.2 ) ### OPTIONS # -option( INSTALL_CONFIG "Install configuration files" ON ) +option( INSTALL_CONFIG "Install configuration files" OFF ) option( INSTALL_POLKIT "Install Polkit configuration" ON ) option( BUILD_TESTING "Build the testing tree." ON ) option( WITH_PYTHON "Enable Python modules API (requires Boost.Python)." ON ) From 29830bc1e12c0a4481fd7b7e58701a73d03768ec Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 04:35:00 -0400 Subject: [PATCH 03/13] [services] Document the configuration file. - Change the example to be harmless (empty) - Document the structure of the entries --- src/modules/services/services.conf | 57 +++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 16 deletions(-) diff --git a/src/modules/services/services.conf b/src/modules/services/services.conf index d9c8895ea..eb971b222 100644 --- a/src/modules/services/services.conf +++ b/src/modules/services/services.conf @@ -1,20 +1,45 @@ +# Systemd services manipulation. +# +# This module can enable services and targets for systemd +# (if packaging doesn't already do that). It can calso +# disable services (but not targets). +# +# First, services are enabled; then targets; then services +# are disabled -- this order of operations is fixed. --- -#systemd services and targets are enabled in this precise order -services: - - name: "NetworkManager" #name of the service file - mandatory: false #true=> if enabling fails the installer errors out and quits - #false=>if enabling fails print warning to console and continue - - name: "cups" - mandatory: false +# There are three configuration keys for this module: +# *services*, *targets* and *disable*. The value of each +# key is a list of entries. Each entry has two keys: +# - *name* is the (string) name of the service or target that is being +# changed. Use quotes. +# - *mandatory* is a boolean option, which states whether the change +# must be done successfully. If systemd reports an error while changing +# a mandatory entry, the installation will fail. When mandatory is false, +# errors for that entry (service or target) are ignored. +# +# Use [] to express an empty list. -targets: - - name: "graphical" - mandatory: true +# # This example enables NetworkManager (and fails if it can't), +# # disables cups (and ignores failure). Then it enables the +# # graphical target (e.g. so that SDDM runs for login), and +# # finally disables pacman-init (an ArchLinux-only service). +# # +# services: +# - name: "NetworkManager" +# mandatory: true +# - name: "cups" +# mandatory: false +# +# targets: +# - name: "graphical" +# mandatory: true +# +# disable: +# - name: "pacman-init" +# mandatory: false -disable: - - name: "pacman-init" - mandatory: false - -# Example to express an empty list: -# disable: [] +# By default, no changes are made. +services: [] +targets: [] +disable: [] From b2c2b916456972b5ce5e0ac140c3a568b7c48a8a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 05:18:10 -0400 Subject: [PATCH 04/13] CMake: introduce USE_ When there are multiple modules doing a thing and it really only makes sense to have one of them in a given Calamares compilation, the USE_ variables allow you to select one, while ignoring all the other implementations. If USE_ is not set, all implementations are included (as usual). --- CMakeLists.txt | 16 ++++++++++++++++ src/modules/CMakeLists.txt | 20 +++++++++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d72463bbd..1fa59421c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,6 +43,22 @@ option( WITH_PYTHON "Enable Python modules API (requires Boost.Python)." ON ) option( WITH_PYTHONQT "Enable next generation Python modules API (experimental, requires PythonQt)." ON ) option( WITH_KF5Crash "Enable crash reporting with KCrash." ON ) +### USE_* +# +# By convention, when there are multiple modules that implement similar +# functionality, and it only makes sense to have **at most one** of them +# enabled at any time, those modules are named -. +# For example, services-systemd and services-openrc. +# +# Setting up SKIP_MODULES to ignore "the ones you don't want" can be +# annoying and error-prone (e.g. if a new module shows up). The USE_* +# modules provide a way to do automatic selection. To pick exactly +# one of the implementations from group , set USE_ to the +# name of the implementation. If USE_ is unset, or empty, then +# all the implementations are enabled (this just means they are +# **available** to `settings.conf`, not that they are used). +# +# Currently, no USE_ variables exist. ### Calamares application info # diff --git a/src/modules/CMakeLists.txt b/src/modules/CMakeLists.txt index 514d6b4f6..bf1817b52 100644 --- a/src/modules/CMakeLists.txt +++ b/src/modules/CMakeLists.txt @@ -15,13 +15,31 @@ string( REPLACE " " ";" SKIP_LIST "${SKIP_MODULES}" ) file( GLOB SUBDIRECTORIES RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "*" ) list( SORT SUBDIRECTORIES ) +# Handle the USE_ variables by looking for subdirectories +# with a - kind of name. +foreach( SUBDIRECTORY ${SUBDIRECTORIES} ) +endforeach() + foreach( SUBDIRECTORY ${SUBDIRECTORIES} ) list( FIND SKIP_LIST ${SUBDIRECTORY} DO_SKIP ) + set( _skip_reason "user request" ) + if( SUBDIRECTORY MATCHES "^[a-zA-Z0-9_]+-" ) + string( REGEX REPLACE "^[^-]+-" "" _implementation ${SUBDIRECTORY} ) + string( REGEX REPLACE "-.*" "" _category ${SUBDIRECTORY} ) + if( USE_${_category} ) + if( NOT "${_implementation}" STREQUAL "${USE_${_category}}" ) + list( APPEND SKIP_LIST ${SUBDIRECTORY} ) + set( _skip_reason "USE_${_category}=${USE_${_category}}" ) + set( DO_SKIP 1 ) + endif() + endif() + endif() + if( NOT DO_SKIP EQUAL -1 ) message( "${ColorReset}-- Skipping module ${BoldRed}${SUBDIRECTORY}${ColorReset}." ) message( "" ) - list( APPEND LIST_SKIPPED_MODULES "${SUBDIRECTORY} (user request)" ) + list( APPEND LIST_SKIPPED_MODULES "${SUBDIRECTORY} (${_skip_reason})" ) elseif( ( IS_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/${SUBDIRECTORY}" ) AND ( DO_SKIP EQUAL -1 ) ) set( SKIPPED_MODULES ) From c086d18a26a5f2d62f9dc1fb2ef646b47585b622 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 05:33:01 -0400 Subject: [PATCH 05/13] CMake: improve error-handling for USE_* If USE_ is given a value that doesn't match **anything**, then bail out. Since USE_* is an explicit distro choice for a specific implementation, it's an error if that implementation is not there. --- src/modules/CMakeLists.txt | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/modules/CMakeLists.txt b/src/modules/CMakeLists.txt index bf1817b52..0a8d1db70 100644 --- a/src/modules/CMakeLists.txt +++ b/src/modules/CMakeLists.txt @@ -15,20 +15,23 @@ string( REPLACE " " ";" SKIP_LIST "${SKIP_MODULES}" ) file( GLOB SUBDIRECTORIES RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} "*" ) list( SORT SUBDIRECTORIES ) -# Handle the USE_ variables by looking for subdirectories -# with a - kind of name. -foreach( SUBDIRECTORY ${SUBDIRECTORIES} ) -endforeach() +set( _use_categories "" ) +set( _found_categories "" ) foreach( SUBDIRECTORY ${SUBDIRECTORIES} ) list( FIND SKIP_LIST ${SUBDIRECTORY} DO_SKIP ) set( _skip_reason "user request" ) + # Handle the USE_ variables by looking for subdirectories + # with a - kind of name. if( SUBDIRECTORY MATCHES "^[a-zA-Z0-9_]+-" ) string( REGEX REPLACE "^[^-]+-" "" _implementation ${SUBDIRECTORY} ) string( REGEX REPLACE "-.*" "" _category ${SUBDIRECTORY} ) if( USE_${_category} ) - if( NOT "${_implementation}" STREQUAL "${USE_${_category}}" ) + list( APPEND _use_categories ${_category} ) + if( "${_implementation}" STREQUAL "${USE_${_category}}" ) + list( APPEND _found_categories ${_category} ) + else() list( APPEND SKIP_LIST ${SUBDIRECTORY} ) set( _skip_reason "USE_${_category}=${USE_${_category}}" ) set( DO_SKIP 1 ) @@ -54,5 +57,12 @@ endforeach() # both before and after the feature summary. calamares_explain_skipped_modules( ${LIST_SKIPPED_MODULES} ) +foreach( _category ${_use_categories} ) + list( FIND _found_categories ${_category} _found ) + if ( _found EQUAL -1 ) + message( FATAL_ERROR "USE_${_category} is set to ${USE_${_category}} and no module matches." ) + endif() +endforeach() + include( CalamaresAddTranslations ) add_calamares_python_translations( ${CALAMARES_TRANSLATION_LANGUAGES} ) From 52f09f7f462c36d559b1bd536bc1f140f0620365 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 05:47:23 -0400 Subject: [PATCH 06/13] [modules] Minor documentation work on modules a-g --- src/modules/bootloader/bootloader.conf | 2 ++ src/modules/displaymanager/displaymanager.conf | 2 ++ src/modules/dummycpp/dummycpp.conf | 5 ++++- src/modules/dummypython/dummypython.conf | 5 ++++- src/modules/dummypythonqt/dummypythonqt.conf | 3 +++ src/modules/grubcfg/grubcfg.conf | 1 + 6 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/modules/bootloader/bootloader.conf b/src/modules/bootloader/bootloader.conf index ee8680f72..808a5e6d0 100644 --- a/src/modules/bootloader/bootloader.conf +++ b/src/modules/bootloader/bootloader.conf @@ -12,8 +12,10 @@ kernel: "/vmlinuz-linux" img: "/initramfs-linux.img" fallback: "/initramfs-linux-fallback.img" timeout: "10" + # Optionally set the menu entry name and kernel name to use in systemd-boot. # If not specified here, these settings will be taken from branding.desc. +# # bootloaderEntryName: "Generic GNU/Linux" # kernelLine: ", with Stable-Kernel" # fallbackKernelLine: ", with Stable-Kernel (fallback initramfs)" diff --git a/src/modules/displaymanager/displaymanager.conf b/src/modules/displaymanager/displaymanager.conf index 1c30ed637..8f8e9c704 100644 --- a/src/modules/displaymanager/displaymanager.conf +++ b/src/modules/displaymanager/displaymanager.conf @@ -1,3 +1,5 @@ +# Configure one or more display managers (e.g. SDDM) +# with a "best effort" approach. --- #The DM module attempts to set up all the DMs found in this list, in that precise order. #It also sets up autologin, if the feature is enabled in globalstorage. diff --git a/src/modules/dummycpp/dummycpp.conf b/src/modules/dummycpp/dummycpp.conf index c90b6f3b9..1f2e1daee 100644 --- a/src/modules/dummycpp/dummycpp.conf +++ b/src/modules/dummycpp/dummycpp.conf @@ -1,3 +1,6 @@ +# This is a dummy (example) module for C++ Jobs. +# +# The code is the documentation for the configuration file. --- syntax: "YAML map of anything" example: @@ -15,4 +18,4 @@ a_list_of_maps: - "another element" - name: "another item" contents: - - "not much" \ No newline at end of file + - "not much" diff --git a/src/modules/dummypython/dummypython.conf b/src/modules/dummypython/dummypython.conf index fc985089a..c700120e7 100644 --- a/src/modules/dummypython/dummypython.conf +++ b/src/modules/dummypython/dummypython.conf @@ -1,3 +1,6 @@ +# This is a dummy (example) module for a Python Job Module. +# +# The code is the documentation for the configuration file. --- syntax: "YAML map of anything" example: @@ -15,4 +18,4 @@ a_list_of_maps: - "another element" - name: "another item" contents: - - "not much" \ No newline at end of file + - "not much" diff --git a/src/modules/dummypythonqt/dummypythonqt.conf b/src/modules/dummypythonqt/dummypythonqt.conf index f60e778e1..5bc64abfa 100644 --- a/src/modules/dummypythonqt/dummypythonqt.conf +++ b/src/modules/dummypythonqt/dummypythonqt.conf @@ -1,3 +1,6 @@ +# This is a dummy (example) module for PythonQt. +# +# The code is the documentation for the configuration file. --- syntax: "YAML map of anything" example: diff --git a/src/modules/grubcfg/grubcfg.conf b/src/modules/grubcfg/grubcfg.conf index 608c9b2b4..18951a8a6 100644 --- a/src/modules/grubcfg/grubcfg.conf +++ b/src/modules/grubcfg/grubcfg.conf @@ -2,6 +2,7 @@ # If set to true, always creates /etc/default/grub from scratch even if the file # already existed. If set to false, edits the existing file instead. overwrite: false + # Default entries to write to /etc/default/grub if it does not exist yet or if # we are overwriting it. Note that in addition, GRUB_CMDLINE_LINUX_DEFAULT and # GRUB_DISTRIBUTOR will always be written, with automatically detected values. From 63c03068c0cda4f71b8bd0bf2a8c1b870eaa3008 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 06:39:30 -0400 Subject: [PATCH 07/13] [modules] Documentation for fstab, grubcfg, mkinitcpio - These modules were entirely documented as "use the source", - The sources aren't terribly clear either. --- src/modules/fstab/fstab.conf | 15 +++++++++++++++ src/modules/grubcfg/grubcfg.conf | 15 +++++++++++++-- src/modules/initcpio/initcpio.conf | 1 + 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/modules/fstab/fstab.conf b/src/modules/fstab/fstab.conf index c3dbfc309..11adff2ed 100644 --- a/src/modules/fstab/fstab.conf +++ b/src/modules/fstab/fstab.conf @@ -1,13 +1,28 @@ +# Creates /etc/fstab and /etc/crypttab in the target system. +# Also creates mount points for all the filesystems. +# +# When creating fstab entries for a filesystem, this module +# uses the options for the filesystem type to write to the +# options field of the file. --- +# Mount options to use for all filesystems. If a specific filesystem +# is listed here, use those options, otherwise use the *default* +# options from this mapping. mountOptions: default: defaults,noatime btrfs: defaults,noatime,space_cache,autodefrag + +# If a filesystem is on an SSD, add the following options. If a specific +# filesystem is listed here, use those options, otherwise no additional +# options are set (i.e. there is no *default* like in *mountOptions*). ssdExtraMountOptions: ext4: discard jfs: discard xfs: discard swap: discard btrfs: discard,compress=lzo + +# Additional options added to each line in /etc/crypttab crypttabOptions: luks # For Debian and Debian-based distributions, change the above line to: # crypttabOptions: luks,keyscript=/bin/cat diff --git a/src/modules/grubcfg/grubcfg.conf b/src/modules/grubcfg/grubcfg.conf index 18951a8a6..b354ec35a 100644 --- a/src/modules/grubcfg/grubcfg.conf +++ b/src/modules/grubcfg/grubcfg.conf @@ -1,11 +1,22 @@ +# Write lines to /etc/default/grub (in the target system) based +# on calculated values and the values set in the *defaults* key +# in this configuration file. +# +# Calculated values are: +# - GRUB_DISTRIBUTOR, branding module, *bootloaderEntryName* +# - GRUB_ENABLE_CRYPTODISK, based on the presence of filesystems +# that use LUKS +# - GRUB_CMDLINE_LINUX_DEFAULT, adding LUKS setup and plymouth +# support to the kernel. + --- # If set to true, always creates /etc/default/grub from scratch even if the file # already existed. If set to false, edits the existing file instead. overwrite: false # Default entries to write to /etc/default/grub if it does not exist yet or if -# we are overwriting it. Note that in addition, GRUB_CMDLINE_LINUX_DEFAULT and -# GRUB_DISTRIBUTOR will always be written, with automatically detected values. +# we are overwriting it. +# defaults: GRUB_TIMEOUT: 5 GRUB_DEFAULT: "saved" diff --git a/src/modules/initcpio/initcpio.conf b/src/modules/initcpio/initcpio.conf index 21f5704cc..466a8785d 100644 --- a/src/modules/initcpio/initcpio.conf +++ b/src/modules/initcpio/initcpio.conf @@ -1,2 +1,3 @@ +# Run mkinitcpio(8) with the given preset value --- kernel: linux312 From 1eede6f79719377bac7004bdebed5ef54abc5232 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 06:50:16 -0400 Subject: [PATCH 08/13] [modules] Configuration documentation for mount and luksopenswaphookcfg --- .../luksopenswaphookcfg/luksopenswaphookcfg.conf | 2 ++ src/modules/mount/mount.conf | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/src/modules/luksopenswaphookcfg/luksopenswaphookcfg.conf b/src/modules/luksopenswaphookcfg/luksopenswaphookcfg.conf index 886867f8d..f5610cd7c 100644 --- a/src/modules/luksopenswaphookcfg/luksopenswaphookcfg.conf +++ b/src/modules/luksopenswaphookcfg/luksopenswaphookcfg.conf @@ -1,2 +1,4 @@ +# Writes an openswap configuration with LUKS settings to the given path --- +# Path of the configuration file to write (in the target system) configFilePath: /etc/openswap.conf diff --git a/src/modules/mount/mount.conf b/src/modules/mount/mount.conf index d8f8fb8cc..bb28eed66 100644 --- a/src/modules/mount/mount.conf +++ b/src/modules/mount/mount.conf @@ -1,4 +1,18 @@ +# Mount filesystems in the target (generally, before treating the +# target as a usable chroot / "live" system). Filesystems are +# automatically mounted from the partitioning module. Filesystems +# listed here are **extra**. The filesystems listed in *extraMounts* +# are mounted in all target systems. The filesystems listed in +# *extraMountsEfi* are mounted in the target system **only** if +# the host machine uses UEFI. --- +# Extra filesystems to mount. The key's value is a list of entries; each +# entry has four keys: +# - device The device node to mount +# - fs The filesystem type to use +# - mountPoint Where to mount the filesystem +# - options (optional) Extra options to pass to mount(8) +# extraMounts: - device: proc fs: proc From 40252f1000eb2453dc6661e4bc300bf0e462170f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 06:52:37 -0400 Subject: [PATCH 09/13] [removeuser] Minor documentation --- src/modules/removeuser/removeuser.conf | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/modules/removeuser/removeuser.conf b/src/modules/removeuser/removeuser.conf index a59961ec5..dab4b2526 100644 --- a/src/modules/removeuser/removeuser.conf +++ b/src/modules/removeuser/removeuser.conf @@ -1,2 +1,6 @@ +# Removes a single user (with userdel) from the system. +# This is typically used in OEM setups or if the live user +# spills into the target system. --- +# Username in the target system to be removed. username: live From 731594fb40456b1676725659e13213760bc180a2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 08:07:36 -0400 Subject: [PATCH 10/13] [libcalamaresui] Remove the requiredModules setting - The value set in module.desc was never stored for use, but isn't an attribute of the instance, either. It belongs with the descriptor, in ModuleManager. --- src/libcalamaresui/modulesystem/Module.cpp | 8 -------- src/libcalamaresui/modulesystem/Module.h | 8 -------- 2 files changed, 16 deletions(-) diff --git a/src/libcalamaresui/modulesystem/Module.cpp b/src/libcalamaresui/modulesystem/Module.cpp index c820b98b3..ed1cb33ea 100644 --- a/src/libcalamaresui/modulesystem/Module.cpp +++ b/src/libcalamaresui/modulesystem/Module.cpp @@ -224,13 +224,6 @@ Module::instanceKey() const } -QStringList -Module::requiredModules() const -{ - return m_requiredModules; -} - - QString Module::location() const { @@ -286,7 +279,6 @@ void Module::initFrom( const QVariantMap& moduleDescriptor ) { m_name = moduleDescriptor.value( "name" ).toString(); - if ( moduleDescriptor.contains( EMERGENCY ) ) m_maybe_emergency = moduleDescriptor[ EMERGENCY ].toBool(); } diff --git a/src/libcalamaresui/modulesystem/Module.h b/src/libcalamaresui/modulesystem/Module.h index 18c2e4cbe..f89c9eedb 100644 --- a/src/libcalamaresui/modulesystem/Module.h +++ b/src/libcalamaresui/modulesystem/Module.h @@ -106,13 +106,6 @@ public: */ virtual QString instanceKey() const final; - /** - * @brief requiredModules a list of names of modules required by this one. - * @return the list of names. - * The module dependencies system is currently incomplete and unused. - */ - virtual QStringList requiredModules() const; - /** * @brief location returns the full path of this module's directory. * @return the path. @@ -198,7 +191,6 @@ private: void loadConfigurationFile( const QString& configFileName ); //throws YAML::Exception QString m_name; - QStringList m_requiredModules; QString m_directory; QString m_instanceId; From 08966ff9334cc28ee01524bce5a113aff5466cf2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 08:29:33 -0400 Subject: [PATCH 11/13] [libcalamaresui] Check module dependencies - Module dependency-checking is done in two phases: first, catch any unknown modules that are listed in *requiredModules* and bail out before loading anything. Second, check that the modules required by X occur before X in the sequence. --- .../modulesystem/ModuleManager.cpp | 54 +++++++++++++++++-- .../modulesystem/ModuleManager.h | 20 ++++++- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index ed1e52f9f..24bc6bc53 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -126,7 +126,6 @@ ModuleManager::doInit() } // At this point m_availableModules is filled with whatever was found in the // search paths. - checkDependencies(); emit initDone(); } @@ -176,7 +175,13 @@ ModuleManager::loadModules() { QTimer::singleShot( 0, this, [ this ]() { - QStringList failedModules; + QStringList failedModules = checkDependencies(); + if ( !failedModules.isEmpty() ) + { + emit modulesFailed( failedModules ); + return; + } + Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); @@ -262,6 +267,14 @@ ModuleManager::loadModules() failedModules.append( instanceKey ); continue; } + + if ( !checkDependencies( *thisModule ) ) + { + // Error message is already printed + failedModules.append( instanceKey ); + continue; + } + // If it's a ViewModule, it also appends the ViewStep to the ViewManager. thisModule->loadSelf(); m_loadedModulesByInstanceKey.insert( instanceKey, thisModule ); @@ -301,24 +314,29 @@ ModuleManager::loadModules() } -void +QStringList ModuleManager::checkDependencies() { + QStringList failed; + // This goes through the map of available modules, and deletes those whose // dependencies are not met, if any. - bool somethingWasRemovedBecauseOfUnmetDependencies = false; forever { + bool somethingWasRemovedBecauseOfUnmetDependencies = false; for ( auto it = m_availableDescriptorsByModuleName.begin(); it != m_availableDescriptorsByModuleName.end(); ++it ) { foreach ( const QString& depName, - ( *it ).value( "requiredModules" ).toStringList() ) + it->value( "requiredModules" ).toStringList() ) { if ( !m_availableDescriptorsByModuleName.contains( depName ) ) { + QString moduleName = it->value( "name" ).toString(); somethingWasRemovedBecauseOfUnmetDependencies = true; m_availableDescriptorsByModuleName.erase( it ); + failed << moduleName; + cWarning() << "Module" << moduleName << "has unmet requirement" << depName; break; } } @@ -328,7 +346,33 @@ ModuleManager::checkDependencies() if ( !somethingWasRemovedBecauseOfUnmetDependencies ) break; } + + return failed; } +bool +ModuleManager::checkDependencies( const Module& m ) +{ + bool allRequirementsFound = true; + QStringList requiredModules = m_availableDescriptorsByModuleName[ m.name() ].value( "requiredModules" ).toStringList(); + + for ( const QString& required : requiredModules ) + { + bool requirementFound = false; + for( const Module* v : m_loadedModulesByInstanceKey ) + if ( required == v->name() ) + { + requirementFound = true; + break; + } + if ( !requirementFound ) + { + cError() << "Module" << m.name() << "requires" << required << "before it in sequence."; + allRequirementsFound = false; + } + } + + return allRequirementsFound; +} } diff --git a/src/libcalamaresui/modulesystem/ModuleManager.h b/src/libcalamaresui/modulesystem/ModuleManager.h index eff09b321..a0edc2528 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.h +++ b/src/libcalamaresui/modulesystem/ModuleManager.h @@ -90,7 +90,25 @@ private slots: void doInit(); private: - void checkDependencies(); + /** + * Check in a general sense whether the dependencies between + * modules are valid. Returns a list of module names that + * do **not** have their requirements met. + * + * Returns an empty list on success. + * + * Also modifies m_availableDescriptorsByModuleName to remove + * all the entries that fail. + */ + QStringList checkDependencies(); + + /** + * Check for this specific module if its required modules have + * already been loaded (i.e. are in sequence before it). + * + * Returns true if the requirements are met. + */ + bool checkDependencies( const Module& ); QMap< QString, QVariantMap > m_availableDescriptorsByModuleName; QMap< QString, QString > m_moduleDirectoriesByModuleName; From 0db8082ae1972f0d882c071427a3bc7ab8d5600a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 08:41:16 -0400 Subject: [PATCH 12/13] [libcalamares] Convenience type --- src/libcalamares/Settings.cpp | 2 +- src/libcalamares/Settings.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 1fdfc6daa..8fd4eeac3 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -212,7 +212,7 @@ Settings::customModuleInstances() const } -QList< QPair< ModuleAction, QStringList > > +Settings::ModuleSequence Settings::modulesSequence() const { return m_modulesSequence; diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index 2330f5747..4da65f710 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -47,7 +47,8 @@ public: using InstanceDescriptionList = QList< InstanceDescription >; InstanceDescriptionList customModuleInstances() const; - QList< QPair< ModuleAction, QStringList > > modulesSequence() const; + using ModuleSequence = QList< QPair< ModuleAction, QStringList > >; + ModuleSequence modulesSequence() const; QString brandingComponentName() const; @@ -63,7 +64,7 @@ private: QStringList m_modulesSearchPaths; InstanceDescriptionList m_customModuleInstances; - QList< QPair< ModuleAction, QStringList > > m_modulesSequence; + ModuleSequence m_modulesSequence; QString m_brandingComponentName; From d66393f1aeac1efbfa8b990bca6ff8f69df48429 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 26 Jun 2018 08:43:23 -0400 Subject: [PATCH 13/13] [libcalamares] Fix early failure mode - There is more to failing out of loadModules() than just emitting modulesFailed, so instead share the failure code with the code after loading modules -- but don't load any. --- src/libcalamaresui/modulesystem/ModuleManager.cpp | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 24bc6bc53..86d97d2db 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -176,16 +176,10 @@ ModuleManager::loadModules() QTimer::singleShot( 0, this, [ this ]() { QStringList failedModules = checkDependencies(); - if ( !failedModules.isEmpty() ) - { - emit modulesFailed( failedModules ); - return; - } - Settings::InstanceDescriptionList customInstances = Settings::instance()->customModuleInstances(); - const auto modulesSequence = Settings::instance()->modulesSequence(); + const auto modulesSequence = failedModules.isEmpty() ? Settings::instance()->modulesSequence() : Settings::ModuleSequence(); for ( const auto& modulePhase : modulesSequence ) { ModuleAction currentAction = modulePhase.first; @@ -336,7 +330,7 @@ ModuleManager::checkDependencies() somethingWasRemovedBecauseOfUnmetDependencies = true; m_availableDescriptorsByModuleName.erase( it ); failed << moduleName; - cWarning() << "Module" << moduleName << "has unmet requirement" << depName; + cWarning() << "Module" << moduleName << "has unknown requirement" << depName; break; } }