From a04915e6fac6b93452cba1f6013ea6466068769f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 29 May 2018 03:37:37 -0400 Subject: [PATCH 1/9] [libcalamaresui] Add 'emergency' concept to modules. --- src/libcalamaresui/modulesystem/Module.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/libcalamaresui/modulesystem/Module.h b/src/libcalamaresui/modulesystem/Module.h index 896d3b597..ae9379859 100644 --- a/src/libcalamaresui/modulesystem/Module.h +++ b/src/libcalamaresui/modulesystem/Module.h @@ -155,6 +155,17 @@ public: */ virtual void loadSelf() = 0; + /** + * @brief Is this an emergency module? + * + * An emergency module is run even if an error occurs + * which would terminate Calamares earlier in the same + * *exec* block. Emergency modules in later exec blocks + * are not run (in the common case where there is only + * one exec block, this doesn't really matter). + */ + bool isEmergency() const { return m_emergency; } + /** * @brief jobs returns any jobs exposed by this module. * @return a list of jobs (can be empty). @@ -176,11 +187,14 @@ protected: private: void loadConfigurationFile( const QString& configFileName ); //throws YAML::Exception + QString m_name; QStringList m_requiredModules; QString m_directory; QString m_instanceId; + bool m_emergency; + friend void ::operator>>( const QVariantMap& moduleDescriptor, Calamares::Module* m ); }; From eddee7d76acfaf7eae69eb1fc42420719363ed46 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 29 May 2018 03:38:17 -0400 Subject: [PATCH 2/9] [libcalamaresui] No point in isLoaded() being virtual. --- src/libcalamaresui/modulesystem/Module.cpp | 7 ------- src/libcalamaresui/modulesystem/Module.h | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/libcalamaresui/modulesystem/Module.cpp b/src/libcalamaresui/modulesystem/Module.cpp index 05394e69f..54b6639ff 100644 --- a/src/libcalamaresui/modulesystem/Module.cpp +++ b/src/libcalamaresui/modulesystem/Module.cpp @@ -276,13 +276,6 @@ Module::interfaceString() const } -bool -Module::isLoaded() const -{ - return m_loaded; -} - - QVariantMap Module::configurationMap() { diff --git a/src/libcalamaresui/modulesystem/Module.h b/src/libcalamaresui/modulesystem/Module.h index ae9379859..9b481b157 100644 --- a/src/libcalamaresui/modulesystem/Module.h +++ b/src/libcalamaresui/modulesystem/Module.h @@ -147,7 +147,7 @@ public: * @brief isLoaded reports on the loaded status of a module. * @return true if the module's loading phase has finished, otherwise false. */ - virtual bool isLoaded() const; + bool isLoaded() const { return m_loaded; } /** * @brief loadSelf initialized the module. From b66d4856e753241fc7de014cd884adc99f65a71d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 30 May 2018 07:51:23 -0400 Subject: [PATCH 3/9] [libcalamaresui] Use modern C++ for (auto)deleting failed modules --- src/libcalamaresui/modulesystem/Module.cpp | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/libcalamaresui/modulesystem/Module.cpp b/src/libcalamaresui/modulesystem/Module.cpp index 54b6639ff..2d0de8e55 100644 --- a/src/libcalamaresui/modulesystem/Module.cpp +++ b/src/libcalamaresui/modulesystem/Module.cpp @@ -64,7 +64,7 @@ Module::fromDescriptor( const QVariantMap& moduleDescriptor, const QString& configFileName, const QString& moduleDirectory ) { - Module* m = nullptr; + std::unique_ptr m; QString typeString = moduleDescriptor.value( "type" ).toString(); QString intfString = moduleDescriptor.value( "interface" ).toString(); @@ -79,12 +79,12 @@ Module::fromDescriptor( const QVariantMap& moduleDescriptor, { if ( intfString == "qtplugin" ) { - m = new ViewModule(); + m.reset( new ViewModule() ); } else if ( intfString == "pythonqt" ) { #ifdef WITH_PYTHONQT - m = new PythonQtViewModule(); + m.reset( new PythonQtViewModule() ); #else cError() << "PythonQt view modules are not supported in this version of Calamares."; #endif @@ -96,16 +96,16 @@ Module::fromDescriptor( const QVariantMap& moduleDescriptor, { if ( intfString == "qtplugin" ) { - m = new CppJobModule(); + m.reset( new CppJobModule() ); } else if ( intfString == "process" ) { - m = new ProcessJobModule(); + m.reset( new ProcessJobModule() ); } else if ( intfString == "python" ) { #ifdef WITH_PYTHON - m = new PythonJobModule(); + m.reset( new PythonJobModule() ); #else cError() << "Python modules are not supported in this version of Calamares."; #endif @@ -130,7 +130,6 @@ Module::fromDescriptor( const QVariantMap& moduleDescriptor, else { cError() << "Bad module directory" << moduleDirectory << "for" << instanceId; - delete m; return nullptr; } @@ -144,10 +143,9 @@ Module::fromDescriptor( const QVariantMap& moduleDescriptor, catch ( YAML::Exception& e ) { cError() << "YAML parser error " << e.what(); - delete m; return nullptr; } - return m; + return m.release(); } From def459a29d5d447a03ff228e3a4ccfa19725f6db Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 15 Jun 2018 07:11:17 -0400 Subject: [PATCH 4/9] [libcalamaresui] Read emergency setting from module.desc - Read setting from the module descriptor - Document optional settings - Add EMERGENCY keyword to the CMake helper functions --- CMakeModules/CalamaresAddPlugin.cmake | 6 +++++- src/libcalamaresui/modulesystem/Module.cpp | 6 ++++++ src/libcalamaresui/modulesystem/Module.h | 6 +++--- src/modules/README.md | 24 ++++++++++++++++++++-- 4 files changed, 36 insertions(+), 6 deletions(-) diff --git a/CMakeModules/CalamaresAddPlugin.cmake b/CMakeModules/CalamaresAddPlugin.cmake index e02102829..ade323689 100644 --- a/CMakeModules/CalamaresAddPlugin.cmake +++ b/CMakeModules/CalamaresAddPlugin.cmake @@ -38,6 +38,7 @@ # RESOURCES resource-file # [NO_INSTALL] # [SHARED_LIB] +# [EMERGENCY] # ) include( CMakeParseArguments ) @@ -47,7 +48,7 @@ include( CMakeColors ) function( calamares_add_plugin ) # parse arguments ( name needs to be saved before passing ARGN into the macro ) set( NAME ${ARGV0} ) - set( options NO_INSTALL SHARED_LIB ) + set( options NO_INSTALL SHARED_LIB EMERGENCY ) set( oneValueArgs NAME TYPE EXPORT_MACRO RESOURCES ) set( multiValueArgs SOURCES UI LINK_LIBRARIES LINK_PRIVATE_LIBRARIES COMPILE_DEFINITIONS ) cmake_parse_arguments( PLUGIN "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN} ) @@ -132,6 +133,9 @@ function( calamares_add_plugin ) set( _type ${PLUGIN_TYPE} ) file( WRITE ${_file} "# AUTO-GENERATED metadata file\n# Syntax is YAML 1.2\n---\n" ) file( APPEND ${_file} "type: \"${_type}\"\nname: \"${PLUGIN_NAME}\"\ninterface: \"qtplugin\"\nload: \"lib${target}.so\"\n" ) + if ( PLUGIN_EMERGENCY ) + file( APPEND ${_file} "emergency: true\n" ) + endif() endif() install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_DESC_FILE} diff --git a/src/libcalamaresui/modulesystem/Module.cpp b/src/libcalamaresui/modulesystem/Module.cpp index 2d0de8e55..d334fda17 100644 --- a/src/libcalamaresui/modulesystem/Module.cpp +++ b/src/libcalamaresui/modulesystem/Module.cpp @@ -290,6 +290,12 @@ void Module::initFrom( const QVariantMap& moduleDescriptor ) { m_name = moduleDescriptor.value( "name" ).toString(); + + auto em = QStringLiteral( "emergency" ); + if ( moduleDescriptor.contains( em ) ) + { + m_emergency = moduleDescriptor[ em ].toBool(); + } } } //ns diff --git a/src/libcalamaresui/modulesystem/Module.h b/src/libcalamaresui/modulesystem/Module.h index 9b481b157..219902b17 100644 --- a/src/libcalamaresui/modulesystem/Module.h +++ b/src/libcalamaresui/modulesystem/Module.h @@ -182,9 +182,11 @@ public: protected: explicit Module(); virtual void initFrom( const QVariantMap& moduleDescriptor ); - bool m_loaded; QVariantMap m_configurationMap; + bool m_loaded = false; + bool m_emergency = false; + private: void loadConfigurationFile( const QString& configFileName ); //throws YAML::Exception @@ -193,8 +195,6 @@ private: QString m_directory; QString m_instanceId; - bool m_emergency; - friend void ::operator>>( const QVariantMap& moduleDescriptor, Calamares::Module* m ); }; diff --git a/src/modules/README.md b/src/modules/README.md index a2ec06144..4b1d8ef2f 100644 --- a/src/modules/README.md +++ b/src/modules/README.md @@ -43,15 +43,21 @@ module's name, type, interface and possibly other properties. The name of the module as defined in `module.desc` must be the same as the name of the module's directory. -Module descriptors must have the following keys: +Module descriptors **must** have the following keys: - *name* (an identifier; must be the same as the directory name) - *type* ("job" or "view") - *interface* (see below for the different interfaces; generally we refer to the kinds of modules by their interface) +Module descriptors **may** have the following keys: +- *required* **unimplemented** (a list of modules which are required for this module + to operate properly) +- *emergency* (a boolean value, set to true to mark the module + as an emergency module) + ## Module-specific configuration -A Calamares module *may* read a module configuration file, +A Calamares module **may** read a module configuration file, named `.conf`. If such a file is present in the module's directory, it is shipped as a *default* configuration file. The module configuration file, if it exists, is a YAML 1.2 document @@ -125,3 +131,17 @@ while the module type must be "job" or "jobmodule". The key *command* should have a string as value, which is passed to the shell -- remember to quote it properly. +## Emergency Modules + +Only C++ modules and job modules may be emergency modules. If, during an +*exec* step in the sequence, a module fails, installation as a whole fails +and the install is aborted. If there are emergency modules in the **same** +exec block, those will be executed before the installation is aborted. +Non-emergency modules are not executed. + +If an emergency-module fails while processing emergency-modules for +another failed module, that failure is ignored and emergency-module +processing continues. + +Use the EMERGENCY keyword in the CMake description of a C++ module +to generate a suitable `module.desc`. From d325366e92284cae8c5fd9aef5e5bae6b574dac1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 15 Jun 2018 07:19:02 -0400 Subject: [PATCH 5/9] CMake: fix plugin NO_INSTALL keyword Although the NO_INSTALL keyword could be specified for Calamares plugins, it didn't actually do anything. Now it does. A NO_INSTALL module does not install configs or libraries. --- CMakeModules/CalamaresAddPlugin.cmake | 31 ++++++++++++++++----------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/CMakeModules/CalamaresAddPlugin.cmake b/CMakeModules/CalamaresAddPlugin.cmake index ade323689..d2f878381 100644 --- a/CMakeModules/CalamaresAddPlugin.cmake +++ b/CMakeModules/CalamaresAddPlugin.cmake @@ -72,7 +72,7 @@ function( calamares_add_plugin ) # message( " ${Green}NO_INSTALL:${ColorReset} ${PLUGIN_NO_INSTALL}" ) message( " ${Green}PLUGIN_DESTINATION:${ColorReset} ${PLUGIN_DESTINATION}" ) if( PLUGIN_CONFIG_FILES ) - if ( INSTALL_CONFIG ) + if ( INSTALL_CONFIG AND NOT PLUGIN_NO_INSTALL ) message( " ${Green}CONFIGURATION_FILES:${ColorReset} ${PLUGIN_CONFIG_FILES} => ${PLUGIN_DATA_DESTINATION}" ) else() message( " ${Green}CONFIGURATION_FILES:${ColorReset} ${PLUGIN_CONFIG_FILES} => [Skipping installation]" ) @@ -93,7 +93,7 @@ function( calamares_add_plugin ) set( target_type "SHARED" ) endif() - list( APPEND calamares_add_library_args + set( calamares_add_library_args "${target}" "EXPORT_MACRO" "${PLUGIN_EXPORT_MACRO}" "TARGET_TYPE" "${target_type}" @@ -116,9 +116,14 @@ function( calamares_add_plugin ) list( APPEND calamares_add_library_args "COMPILE_DEFINITIONS" ${PLUGIN_COMPILE_DEFINITIONS} ) endif() - list( APPEND calamares_add_library_args "NO_VERSION" ) + if ( PLUGIN_NO_INSTALL ) + list( APPEND calamares_add_library_args "NO_INSTALL" ) + endif() - list( APPEND calamares_add_library_args "INSTALL_BINDIR" "${PLUGIN_DESTINATION}" ) + list( APPEND calamares_add_library_args + "NO_VERSION" + "INSTALL_BINDIR" "${PLUGIN_DESTINATION}" + ) if( PLUGIN_RESOURCES ) list( APPEND calamares_add_library_args "RESOURCES" "${PLUGIN_RESOURCES}" ) @@ -138,14 +143,16 @@ function( calamares_add_plugin ) endif() endif() - install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_DESC_FILE} - DESTINATION ${PLUGIN_DESTINATION} ) + if ( NOT PLUGIN_NO_INSTALL ) + install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_DESC_FILE} + DESTINATION ${PLUGIN_DESTINATION} ) - if ( INSTALL_CONFIG ) - foreach( PLUGIN_CONFIG_FILE ${PLUGIN_CONFIG_FILES} ) - configure_file( ${PLUGIN_CONFIG_FILE} ${PLUGIN_CONFIG_FILE} COPYONLY ) - install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_CONFIG_FILE} - DESTINATION ${PLUGIN_DATA_DESTINATION} ) - endforeach() + if ( INSTALL_CONFIG ) + foreach( PLUGIN_CONFIG_FILE ${PLUGIN_CONFIG_FILES} ) + configure_file( ${PLUGIN_CONFIG_FILE} ${PLUGIN_CONFIG_FILE} COPYONLY ) + install( FILES ${CMAKE_CURRENT_BINARY_DIR}/${PLUGIN_CONFIG_FILE} + DESTINATION ${PLUGIN_DATA_DESTINATION} ) + endforeach() + endif() endif() endfunction() From 53161f6e36c46fbef1e8a61e081af157db32c3eb Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 15 Jun 2018 07:22:09 -0400 Subject: [PATCH 6/9] [preservefiles] Mark this as an emergency module - For C++ modules, don't need the module.desc file in the source repo, since the CMake macros can create it. --- src/modules/preservefiles/CMakeLists.txt | 1 + src/modules/preservefiles/module.desc | 5 ----- 2 files changed, 1 insertion(+), 5 deletions(-) delete mode 100644 src/modules/preservefiles/module.desc diff --git a/src/modules/preservefiles/CMakeLists.txt b/src/modules/preservefiles/CMakeLists.txt index 43602024c..1ac979d1b 100644 --- a/src/modules/preservefiles/CMakeLists.txt +++ b/src/modules/preservefiles/CMakeLists.txt @@ -8,4 +8,5 @@ calamares_add_plugin( preservefiles LINK_PRIVATE_LIBRARIES calamares SHARED_LIB + EMERGENCY ) diff --git a/src/modules/preservefiles/module.desc b/src/modules/preservefiles/module.desc deleted file mode 100644 index 953d8c81b..000000000 --- a/src/modules/preservefiles/module.desc +++ /dev/null @@ -1,5 +0,0 @@ ---- -type: "job" -name: "preservefiles" -interface: "qtplugin" -load: "libcalamares_job_preservefiles.so" From 3ed6f13fa8d293201dc616357230e12bcc67ce47 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 15 Jun 2018 09:32:19 -0400 Subject: [PATCH 7/9] [libcalamaresui] Adjust the emergency-ness of modules A potentially emergency module is one that has EMERGENCY (in CMake) or emergency: true (in module.desc) set. Any such module must also set emergency: true in the configuration of the module. This is to allow for instances of a module that **don't** run as emergency modules, alongside actual emergency ones. --- src/libcalamaresui/modulesystem/Module.cpp | 10 +++++++--- src/libcalamaresui/modulesystem/Module.h | 3 ++- src/modules/README.md | 6 ++++++ 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/libcalamaresui/modulesystem/Module.cpp b/src/libcalamaresui/modulesystem/Module.cpp index d334fda17..d131c9f13 100644 --- a/src/libcalamaresui/modulesystem/Module.cpp +++ b/src/libcalamaresui/modulesystem/Module.cpp @@ -52,6 +52,8 @@ name: "foo" #the module name. must be unique and same as the parent di interface: "qtplugin" #can be: qtplugin, python, process, ... */ +static const char EMERGENCY[] = "emergency"; + namespace Calamares { @@ -198,6 +200,9 @@ Module::loadConfigurationFile( const QString& configFileName ) //throws YAML::Ex } m_configurationMap = CalamaresUtils::yamlMapToVariant( doc ).toMap(); + m_emergency = m_maybe_emergency + && m_configurationMap.contains( EMERGENCY ) + && m_configurationMap[ EMERGENCY ].toBool(); return; } else @@ -291,10 +296,9 @@ Module::initFrom( const QVariantMap& moduleDescriptor ) { m_name = moduleDescriptor.value( "name" ).toString(); - auto em = QStringLiteral( "emergency" ); - if ( moduleDescriptor.contains( em ) ) + if ( moduleDescriptor.contains( EMERGENCY ) ) { - m_emergency = moduleDescriptor[ em ].toBool(); + m_maybe_emergency = moduleDescriptor[ EMERGENCY ].toBool(); } } diff --git a/src/libcalamaresui/modulesystem/Module.h b/src/libcalamaresui/modulesystem/Module.h index 219902b17..4fd0020f8 100644 --- a/src/libcalamaresui/modulesystem/Module.h +++ b/src/libcalamaresui/modulesystem/Module.h @@ -185,7 +185,8 @@ protected: QVariantMap m_configurationMap; bool m_loaded = false; - bool m_emergency = false; + bool m_emergency = false; // Based on module and local config + bool m_maybe_emergency = false; // Based on the module.desc private: void loadConfigurationFile( const QString& configFileName ); //throws YAML::Exception diff --git a/src/modules/README.md b/src/modules/README.md index 4b1d8ef2f..bd6cd4e37 100644 --- a/src/modules/README.md +++ b/src/modules/README.md @@ -145,3 +145,9 @@ processing continues. Use the EMERGENCY keyword in the CMake description of a C++ module to generate a suitable `module.desc`. + +A module that is marked as an emergency module in its module.desc +must **also** set the *emergency* key to *true* in its configuration file. +If it does not, the module is not considered to be an emergency module +after all (this is so that you can have modules that have several +instances, only some of which are actually needed for emergencies. From 8387d5d81f38e046bb58a79fadaa138854af3d20 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 15 Jun 2018 10:20:07 -0400 Subject: [PATCH 8/9] [libcalamares] Allow emergency jobs Any job can be an emergency job; emergency modules spawn emergency jobs (but conversely, a non-emergency module can spawn an emergency job explicitly). --- src/libcalamares/Job.h | 7 +++++++ src/libcalamaresui/ExecutionViewStep.cpp | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/Job.h b/src/libcalamares/Job.h index 6063633b8..040ead6ad 100644 --- a/src/libcalamares/Job.h +++ b/src/libcalamares/Job.h @@ -66,8 +66,15 @@ public: virtual QString prettyDescription() const; virtual QString prettyStatusMessage() const; virtual JobResult exec() = 0; + + bool isEmergency() const { return m_emergency; } + void setEmergency( bool e ) { m_emergency = e; } + signals: void progress( qreal percent ); + +private: + bool m_emergency = false; }; } // namespace Calamares diff --git a/src/libcalamaresui/ExecutionViewStep.cpp b/src/libcalamaresui/ExecutionViewStep.cpp index 109bd1384..b505102a4 100644 --- a/src/libcalamaresui/ExecutionViewStep.cpp +++ b/src/libcalamaresui/ExecutionViewStep.cpp @@ -21,6 +21,7 @@ #include #include "Branding.h" +#include "Job.h" #include "JobQueue.h" #include "modulesystem/Module.h" #include "modulesystem/ModuleManager.h" @@ -142,7 +143,15 @@ ExecutionViewStep::onActivate() Calamares::Module* module = Calamares::ModuleManager::instance() ->moduleInstance( instanceKey ); if ( module ) - queue->enqueue( module->jobs() ); + { + auto jl = module->jobs(); + if ( module->isEmergency() ) + { + for( auto& j : jl ) + j->setEmergency( true ); + } + queue->enqueue( jl ); + } } queue->start(); From 264d135776c327de05e74a0b3134c495c82e29e0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 15 Jun 2018 11:45:02 -0400 Subject: [PATCH 9/9] [libcalamares] Run emergency jobs - After a failure, skip non-emergency jobs. - After running all emergency jobs, then emit failure message. - In log, distinguish emergency and non-emergency jobs. --- src/libcalamares/JobQueue.cpp | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/libcalamares/JobQueue.cpp b/src/libcalamares/JobQueue.cpp index 339fd8457..b3d42d8f9 100644 --- a/src/libcalamares/JobQueue.cpp +++ b/src/libcalamares/JobQueue.cpp @@ -50,22 +50,36 @@ public: void run() override { + bool anyFailed = false; + QString message; + QString details; + m_jobIndex = 0; for( auto job : m_jobs ) { + if ( anyFailed && !job->isEmergency() ) + { + cDebug() << "Skipping non-emergency job" << job->prettyName(); + continue; + } + emitProgress(); - cDebug() << "Starting job" << job->prettyName(); + cDebug() << "Starting" << ( anyFailed ? "EMERGENCY JOB" : "job" ) << job->prettyName(); connect( job.data(), &Job::progress, this, &JobThread::emitProgress ); JobResult result = job->exec(); - if ( !result ) + if ( !anyFailed && !result ) { - emitFailed( result.message(), result.details() ); - emitFinished(); - return; + anyFailed = true; + message = result.message(); + details = result.details(); } - ++m_jobIndex; + if ( !anyFailed ) + ++m_jobIndex; } - emitProgress(); + if ( anyFailed ) + emitFailed( message, details ); + else + emitProgress(); emitFinished(); }