From e39782576d52da8abb577dd56910b8f2e9853b97 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 23 May 2019 12:17:53 +0200 Subject: [PATCH 01/12] Documentation: explain module-paths better - how "local" is interpreted - recommendations for other paths --- settings.conf | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/settings.conf b/settings.conf index 938817ef1..b12e379d5 100644 --- a/settings.conf +++ b/settings.conf @@ -7,8 +7,20 @@ # path to a directory or the keyword "local". # # "local" means: -# - modules in $LIBDIR/calamares/moduleswith +# - modules in $LIBDIR/calamares/modules, with # - settings in SHARE/calamares/modules or /etc/calamares/modules. +# In debug-mode (e.g. calamares -d) "local" also adds some paths +# that make sense from inside the build-directory, so that you +# can build-and-run with the latest modules immediately. +# +# Strings other than "local" are taken as paths and interpreted +# relative to wherever Calamares is started. It is therefore **strongly** +# recommended to use only absolute paths here. This is mostly useful +# if your distro has forks of standard Calamares modules, but also +# uses some form of upstream packaging which might overwrite those +# forked modules -- then you can keep modules somewhere outside of +# the "regular" module tree. +# # # YAML: list of strings. modules-search: [ local ] @@ -157,8 +169,8 @@ dont-chroot: false # If this is set to true, the "Cancel" button will be disabled entirely. # The button is also hidden from view. # -# This can be useful if when e.g. Calamares is used as a post-install -# configuration tool and you require the user to go through all the +# This can be useful if when e.g. Calamares is used as a post-install +# configuration tool and you require the user to go through all the # configuration steps. # # Default is false, but Calamares will complain if this is not explicitly set. From 9acf80db8b449b9b3e060a4b80c31b62899711bc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 23 May 2019 12:25:14 +0200 Subject: [PATCH 02/12] Documentation: go over the instances section. --- settings.conf | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/settings.conf b/settings.conf index b12e379d5..1c7b773ff 100644 --- a/settings.conf +++ b/settings.conf @@ -26,7 +26,7 @@ modules-search: [ local ] # Instances section. This section is optional, and it defines custom instances -# for modules of any kind. An instance entry has an instance name, a module +# for modules of any kind. An instance entry has an module name, an instance # name, and a configuration file name. The primary goal of this mechanism is # to allow loading multiple instances of the same module, with different # configuration. If you don't need this, the instances section can safely be @@ -34,24 +34,27 @@ modules-search: [ local ] # # Module name plus instance name makes an instance key, e.g. # "webview@owncloud", where "webview" is the module name (for the webview -# viewmodule) and "owncloud" is the instance name, which loads a configuration -# file named "owncloud.conf" from any of the configuration file paths, -# including the webview module directory. This instance key can then be -# referenced in the sequence section. +# viewmodule) and "owncloud" is the instance name. In the *sequence* +# section below, use instance-keys to name instances (instead of just +# a module name, for modules which have only a single instance). # -# For all modules without a custom instance specification, a default instance -# is generated automatically by Calamares. Therefore a statement such as -# "webview" in the sequence section automatically implies an instance key of -# "webview@webview" even without explicitly defining this instance, and the -# configuration file for this default instance "@" is -# always assumed to be ".conf". +# Every module implicitly has an instance with the instance name equal +# to its module name, e.g. "welcome@welcome". In the *sequence* section, +# mentioning a module without a full instance key (e.g. "welcome") +# means that implicit module. +# +# An instance must specify its configuration file (e.g. `webview-home.conf`). +# The implicit instances all have configuration files named `.conf`. +# This (implict) way matches the source examples, where the welcome +# module contains an example `welcome.conf`. # # For more information on running module instances, run Calamares in debug # mode and check the Modules page in the Debug information interface. # -# A module that is often used with instances is dummyprocess, which will -# run a single (shell) command. By configuring more than one instance of -# the module, multiple shell commands can be run during install. +# A module that is often used with instances is shellprocess, which will +# run shell commands specified in the configuration file. By configuring +# more than one instance of the module, multiple shell sessions can be run +# during install. # # YAML: list of maps of string:string key-value pairs. #instances: From c741470b6094d2d9cdf49e5a73345489821cf14e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 23 May 2019 13:30:37 +0200 Subject: [PATCH 03/12] [calamares] Name the debug button for styling purposes --- src/calamares/CalamaresWindow.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/calamares/CalamaresWindow.cpp b/src/calamares/CalamaresWindow.cpp index 25c8432e2..626b8588c 100644 --- a/src/calamares/CalamaresWindow.cpp +++ b/src/calamares/CalamaresWindow.cpp @@ -132,6 +132,7 @@ CalamaresWindow::CalamaresWindow( QWidget* parent ) if ( Calamares::Settings::instance()->debugMode() ) { QPushButton* debugWindowBtn = new QPushButton; + debugWindowBtn->setObjectName( "debugButton" ); CALAMARES_RETRANSLATE( debugWindowBtn->setText( tr( "Show debug information" ) ); ) From 36fe3ed188fbc4436eaa0719b1f292de340be870 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 14:06:05 +0200 Subject: [PATCH 04/12] [packages] Add -Su --noconfirm - When updating the system (-Su) it may want to install newer packages; it asks for confirmation before doing so. FIXES #1154 --- src/modules/packages/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/packages/main.py b/src/modules/packages/main.py index aac1aa6f9..aa4bb8b12 100644 --- a/src/modules/packages/main.py +++ b/src/modules/packages/main.py @@ -290,7 +290,7 @@ class PMPacman(PackageManager): check_target_env_call(["pacman", "-Sy"]) def update_system(self): - check_target_env_call(["pacman", "-Su"]) + check_target_env_call(["pacman", "-Su", "--noconfirm"]) class PMPortage(PackageManager): From 7f5e61480b0418fe57e98b17fb111950e899f382 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 14:36:47 +0200 Subject: [PATCH 05/12] [calamares] Once the steps are loaded, activate the first view step - Using next and back buttons calls onActivate() on the view step that you end up on. - The first view step to be shown, though, doesn't get an onActivate() (unless you go, say, next and then back). - Explicitly call onActivate() on the first view step once they're all loaded. FIXES #1156 --- src/calamares/CalamaresApplication.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/calamares/CalamaresApplication.cpp b/src/calamares/CalamaresApplication.cpp index 2d5ea41f0..704cde71c 100644 --- a/src/calamares/CalamaresApplication.cpp +++ b/src/calamares/CalamaresApplication.cpp @@ -363,6 +363,12 @@ CalamaresApplication::initViewSteps() ProgressTreeModel* m = new ProgressTreeModel( nullptr ); ProgressTreeView::instance()->setModel( m ); cDebug() << "STARTUP: Window now visible and ProgressTreeView populated"; + + const auto steps = Calamares::ViewManager::instance()->viewSteps(); + cDebug() << Logger::SubEntry << steps.count() << "view steps loaded."; + // Tell the first view that it's been shown. + if ( steps.count() > 0 ) + steps[0]->onActivate(); } void From b41cac6556178159510562a404b77ddaea110fb9 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 14:54:30 +0200 Subject: [PATCH 06/12] [interactiveterminal] Warn when called in exec: context FIXES #1157 --- .../interactiveterminal/InteractiveTerminalViewStep.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/modules/interactiveterminal/InteractiveTerminalViewStep.cpp b/src/modules/interactiveterminal/InteractiveTerminalViewStep.cpp index d57eb7d05..874c8a9cc 100644 --- a/src/modules/interactiveterminal/InteractiveTerminalViewStep.cpp +++ b/src/modules/interactiveterminal/InteractiveTerminalViewStep.cpp @@ -20,6 +20,8 @@ #include "InteractiveTerminalPage.h" +#include "utils/Logger.h" + #include CALAMARES_PLUGIN_FACTORY_DEFINITION( InteractiveTerminalViewStepFactory, registerPlugin(); ) @@ -84,6 +86,7 @@ InteractiveTerminalViewStep::isAtEnd() const QList< Calamares::job_ptr > InteractiveTerminalViewStep::jobs() const { + cDebug() << "InteractiveTerminal" << prettyName() << "asked for jobs(), this is probably wrong."; return QList< Calamares::job_ptr >(); } @@ -91,6 +94,7 @@ InteractiveTerminalViewStep::jobs() const void InteractiveTerminalViewStep::onActivate() { + cDebug() << "InteractiveTerminal" << prettyName() << "activated."; m_widget->onActivate(); } From 822bbaad9c3021a922aaf7b85c6663b0375a72e6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 15:26:57 +0200 Subject: [PATCH 07/12] [libcalamaresui] Allow icon names in branding images - It's ok to use path / filenames in images, but you can also use icon names according to the FDO icon spec. This makes sense for at least *productLogo*, possibly *productIcon*, but not really for *productWelcome*. --- src/libcalamaresui/Branding.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/libcalamaresui/Branding.cpp b/src/libcalamaresui/Branding.cpp index a27b39ae6..680e9b926 100644 --- a/src/libcalamaresui/Branding.cpp +++ b/src/libcalamaresui/Branding.cpp @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014-2015, Teo Mrnjavac - * Copyright 2017-2018, Adriaan de Groot + * Copyright 2017-2019, Adriaan de Groot * Copyright 2018, Raul Rodrigo Segura (raurodse) * * Calamares is free software: you can redistribute it and/or modify @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -185,9 +186,16 @@ Branding::Branding( const QString& brandingFilePath, loadStrings( m_images, doc, "images", [&]( const QString& s ) -> QString { - QFileInfo imageFi( componentDir.absoluteFilePath( expand( s ) ) ); + const QString imageName( expand( s ) ); + QFileInfo imageFi( componentDir.absoluteFilePath( imageName ) ); if ( !imageFi.exists() ) - bail( QString( "Image file %1 does not exist." ).arg( imageFi.absoluteFilePath() ) ); + { + const auto icon = QIcon::fromTheme( imageName ); + // Not found, bail out with the filename used + if ( icon.isNull() ) + bail( QString( "Image file %1 does not exist." ).arg( imageFi.absoluteFilePath() ) ); + return imageName; // Not turned into a path + } return imageFi.absoluteFilePath(); } ); From 976ad7e3e72025bf81add89b75a0968dd5d587fe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 15:35:56 +0200 Subject: [PATCH 08/12] [libcalamaresui] Look up icons via theme - Don't cache icons, because they could be changed via the active desktop theme. --- src/libcalamaresui/Branding.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/libcalamaresui/Branding.cpp b/src/libcalamaresui/Branding.cpp index 680e9b926..4dc07ff19 100644 --- a/src/libcalamaresui/Branding.cpp +++ b/src/libcalamaresui/Branding.cpp @@ -297,15 +297,21 @@ Branding::imagePath( Branding::ImageEntry imageEntry ) const QPixmap Branding::image( Branding::ImageEntry imageEntry, const QSize& size ) const { - QPixmap pixmap = - ImageRegistry::instance()->pixmap( imagePath( imageEntry ), size ); - - if ( pixmap.isNull() ) + const auto path = imagePath( imageEntry ); + if ( path.contains( '/' ) ) { - Q_ASSERT( false ); - return QPixmap(); + QPixmap pixmap = ImageRegistry::instance()->pixmap( path, size ); + + Q_ASSERT( !pixmap.isNull() ); + return pixmap; + } + else + { + auto icon = QIcon::fromTheme(path); + + Q_ASSERT( !icon.isNull() ); + return icon.pixmap( size ); } - return pixmap; } QString From f64e55f0dc19a18475770c4fe9ca278aff346620 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 15:38:37 +0200 Subject: [PATCH 09/12] [libcalamaresui] Use meaningful asserts - In debug mode, hitting assert(false) is meaningless, - In release mode, the assert is optimized out. - So assert the condition we're actually testing, for better messages. --- src/libcalamaresui/utils/ImageRegistry.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/libcalamaresui/utils/ImageRegistry.cpp b/src/libcalamaresui/utils/ImageRegistry.cpp index 442017048..96ea92bbb 100644 --- a/src/libcalamaresui/utils/ImageRegistry.cpp +++ b/src/libcalamaresui/utils/ImageRegistry.cpp @@ -61,9 +61,9 @@ ImageRegistry::cacheKey( const QSize& size, float opacity, QColor tint ) QPixmap ImageRegistry::pixmap( const QString& image, const QSize& size, CalamaresUtils::ImageMode mode, float opacity, QColor tint ) { + Q_ASSERT( !(size.width() < 0 || size.height() < 0) ); if ( size.width() < 0 || size.height() < 0 ) { - Q_ASSERT( false ); return QPixmap(); } @@ -159,15 +159,9 @@ ImageRegistry::putInCache( const QString& image, const QSize& size, CalamaresUti if ( s_cache.contains( image ) ) { subcache = s_cache.value( image ); - if ( subcache.contains( mode ) ) { subsubcache = subcache.value( mode ); - -/* if ( subsubcache.contains( size.width() * size.height() ) ) - { - Q_ASSERT( false ); - }*/ } } From 966604892b25f3df776d83d080336afbb202a5b4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 16:38:06 +0200 Subject: [PATCH 10/12] CHANGES: mention icon use in branding --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 731465158..6de55bff7 100644 --- a/CHANGES +++ b/CHANGES @@ -15,6 +15,10 @@ This release contains contributions from (alphabetically by first name): - *branding* now supports os-release variables in the *strings* section, which allows re-using (at runtime) information set in /etc/os-release . This requires KDE Frameworks 5.58. #1150 + - *branding* allows the use of FreeDesktop.org icon names for the + *productLogo* and *productIcon* keys. If a file is named there, then + the file is used, and otherwise the icon is looked up in the current + theme. # 3.2.8 (2019-05-10) # From 2b91608b8287be79c9c1bbfa91e7388e5925638b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 17:10:21 +0200 Subject: [PATCH 11/12] [libcalamaresui] Avoid requirements-results UI duplication If the requirements checking is **really fast**, e.g. you don't have a check for internet connectivity, then the checks might be done as fast as the 0-timeout single-shot timer, which means that finished() is called once by the QFutureWatcher, and then after that by the QTimer .. leading to two messages "All requirements have been checked", but also twice requirementsComplete being emitted, so you end up with two results lists being added by the CheckerContainer. Stop that by using the results-progress timer as an additional flag: the first time everything is complete, delete that timer and set the pointer back to nullptr. --- src/libcalamaresui/modulesystem/RequirementsChecker.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libcalamaresui/modulesystem/RequirementsChecker.cpp b/src/libcalamaresui/modulesystem/RequirementsChecker.cpp index 7eccdbbc0..562204a2c 100644 --- a/src/libcalamaresui/modulesystem/RequirementsChecker.cpp +++ b/src/libcalamaresui/modulesystem/RequirementsChecker.cpp @@ -99,12 +99,15 @@ RequirementsChecker::run() void RequirementsChecker::finished() { - if ( std::all_of( m_watchers.cbegin(), m_watchers.cend(), []( const Watcher *w ) { return w && w->isFinished(); } ) ) + if ( m_progressTimer && std::all_of( m_watchers.cbegin(), m_watchers.cend(), []( const Watcher *w ) { return w && w->isFinished(); } ) ) { cDebug() << "All requirements have been checked."; - if ( m_progressTimer ) + { m_progressTimer->stop(); + delete m_progressTimer; + m_progressTimer = nullptr; + } bool acceptable = true; int count = 0; From 2e39f24bb0c72ab0921742316a6540f150e1e4a8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 27 May 2019 17:15:49 +0200 Subject: [PATCH 12/12] [libcalamaresui] Make sure finished() is processed once - Avoid races which might double-delete the timer, or enter the if twice (which would lead to duplicate emissions of requirementsComplete and the associated UI glitches). --- src/libcalamaresui/modulesystem/RequirementsChecker.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcalamaresui/modulesystem/RequirementsChecker.cpp b/src/libcalamaresui/modulesystem/RequirementsChecker.cpp index 562204a2c..f71c24b8b 100644 --- a/src/libcalamaresui/modulesystem/RequirementsChecker.cpp +++ b/src/libcalamaresui/modulesystem/RequirementsChecker.cpp @@ -99,6 +99,9 @@ RequirementsChecker::run() void RequirementsChecker::finished() { + static QMutex finishedMutex; + QMutexLocker lock( &finishedMutex ); + if ( m_progressTimer && std::all_of( m_watchers.cbegin(), m_watchers.cend(), []( const Watcher *w ) { return w && w->isFinished(); } ) ) { cDebug() << "All requirements have been checked.";