From f9f888fade7472c0d09ed8f9c1141cfcfc767e33 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 25 May 2024 22:27:47 +0200 Subject: [PATCH 01/10] [shellprocess] Introduce configuration for verbosity --- src/modules/shellprocess/shellprocess.conf | 18 ++++++++++++++++-- .../shellprocess/shellprocess.schema.yaml | 10 +++++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/modules/shellprocess/shellprocess.conf b/src/modules/shellprocess/shellprocess.conf index 41a7d2733..7c1cb8f53 100644 --- a/src/modules/shellprocess/shellprocess.conf +++ b/src/modules/shellprocess/shellprocess.conf @@ -84,8 +84,20 @@ --- # Set to true to run in host, rather than target system dontChroot: false -# Tune this for the commands you're actually running -# timeout: 10 + +# Tune this for the commands you're actually running, or +# use the list-of-items form of commands to tune the timeout +# for each command individually. +timeout: 10 + +# This will copy the output from the command into the Calamares +# log file. No processing is done beyond log-each-line-separately, +# so this can introduce weirdness in the log if the script +# outputs e.g. escape codes. +# +# The default is `false`. This can also be set for each +# command individually. +verbose: false # Script may be a single string (because false returns an error exit # code, this will trigger a failure in the installation): @@ -109,6 +121,8 @@ script: - "/usr/bin/true" - command: "/usr/local/bin/slowloris" timeout: 3600 + - command: "echo -e '\e[33;2mred\e[33;0m'" + verbose: true # You can change the description of the job (as it is displayed in the # progress bar during installation) by defining an *i18n* key, which diff --git a/src/modules/shellprocess/shellprocess.schema.yaml b/src/modules/shellprocess/shellprocess.schema.yaml index af56707b7..c9f6c3410 100644 --- a/src/modules/shellprocess/shellprocess.schema.yaml +++ b/src/modules/shellprocess/shellprocess.schema.yaml @@ -19,7 +19,10 @@ definitions: timeout: type: number description: the (optional) timeout for this specific command (differently - from the global setting) + from the global setting). + verbose: + type: boolean + description: when true, log output from the command to the Calamares log. required: - command type: object @@ -34,6 +37,9 @@ properties: type: number description: The (global) timeout for the command list in seconds. If unset, defaults to 30 seconds. + verbose: + type: boolean + description: when true, log output from the command to the Calamares log. script: anyOf: - $ref: '#definitions/command' @@ -55,3 +61,5 @@ properties: type: string required: - name +required: + - script From b0614bb79c3604ab402b07aa994fe05f24e01c35 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 25 May 2024 22:44:02 +0200 Subject: [PATCH 02/10] [libcalamares] Add verbosity to CommandLine This is the data structure that stores a single command for execution by the shell. There is no back-end implementation for verbosity yet. --- src/libcalamares/utils/CommandList.cpp | 11 +++++++++++ src/libcalamares/utils/CommandList.h | 24 ++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index 0f5111eac..120cf5aa5 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -140,6 +140,11 @@ CommandLine::CommandLine( const QVariantMap& m ) m_command = command; m_timeout = timeout >= 0 ? std::chrono::seconds( timeout ) : CommandLine::TimeoutNotSet(); m_environment = Calamares::getStringList( m, "environment" ); + + if ( m.contains( "verbose" ) ) + { + m_verbose = Calamares::getBool( m, "verbose", false ); + } } else { @@ -289,4 +294,10 @@ CommandList::expand() const return expand( expander ); } +void +CommandList::updateVerbose( bool verbose ) +{ + std::for_each( begin(), end(), [ verbose ]( CommandLine& command ) { command.updateVerbose( verbose ); } ); +} + } // namespace Calamares diff --git a/src/libcalamares/utils/CommandList.h b/src/libcalamares/utils/CommandList.h index 331127c1e..b5b4fe7ad 100644 --- a/src/libcalamares/utils/CommandList.h +++ b/src/libcalamares/utils/CommandList.h @@ -18,6 +18,7 @@ #include #include +#include #include class KMacroExpanderBase; @@ -63,6 +64,7 @@ public: QString command() const { return m_command; } [[nodiscard]] QStringList environment() const { return m_environment; } std::chrono::seconds timeout() const { return m_timeout; } + bool isVerbose() const { return m_verbose.value_or( false ); } bool isValid() const { return !m_command.isEmpty(); } @@ -81,10 +83,20 @@ public: */ DLLEXPORT CommandLine expand() const; + /** @brief If nothing has set verbosity yet, update to @p verbose */ + void updateVerbose( bool verbose ) + { + if ( !m_verbose.has_value() ) + { + m_verbose = verbose; + } + } + private: QString m_command; QStringList m_environment; std::chrono::seconds m_timeout = TimeoutNotSet(); + std::optional< bool > m_verbose; }; /** @brief Abbreviation, used internally. */ @@ -103,6 +115,11 @@ class DLLEXPORT CommandList : protected CommandList_t public: /** @brief empty command-list with timeout to apply to entries. */ CommandList( bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); + /** @brief command-list constructed from script-entries in @p v + * + * The global settings @p doChroot and @p timeout can be overridden by + * the individual script-entries. + */ CommandList( const QVariant& v, bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); CommandList( int ) = delete; CommandList( const QVariant&, int ) = delete; @@ -126,14 +143,17 @@ public: * Each command-line in the list is expanded with the given @p expander. * @see CommandLine::expand() for details. */ - CommandList expand( KMacroExpanderBase& expander ) const; + DLLEXPORT CommandList expand( KMacroExpanderBase& expander ) const; /** @brief As above, with a default macro-expander. * * Each command-line in the list is expanded with that default macro-expander. * @see CommandLine::expand() for details. */ - CommandList expand() const; + DLLEXPORT CommandList expand() const; + + /** @brief Applies default-value @p verbose to each entry without an explicit setting. */ + DLLEXPORT void updateVerbose( bool verbose ); private: bool m_doChroot; From 0d63ebf14f857777ced01aa03ebadb7cffe6819f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 25 May 2024 22:54:46 +0200 Subject: [PATCH 03/10] [shellprocess] Apply verbosity from configuration file --- src/modules/shellprocess/ShellProcessJob.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/shellprocess/ShellProcessJob.cpp b/src/modules/shellprocess/ShellProcessJob.cpp index 3fe8cc613..d6fa9acfa 100644 --- a/src/modules/shellprocess/ShellProcessJob.cpp +++ b/src/modules/shellprocess/ShellProcessJob.cpp @@ -60,6 +60,7 @@ ShellProcessJob::setConfigurationMap( const QVariantMap& configurationMap ) { timeout = 30; } + bool verbose = Calamares::getBool( configurationMap, "verbose", false ); if ( configurationMap.contains( "script" ) ) { @@ -69,6 +70,7 @@ ShellProcessJob::setConfigurationMap( const QVariantMap& configurationMap ) { cDebug() << "ShellProcessJob: \"script\" contains no commands for" << moduleInstanceKey(); } + m_commands->updateVerbose( verbose ); } else { From 4815bf6963ea0fa1435c853f7d879e4c7d86a49b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 15:07:03 +0200 Subject: [PATCH 04/10] [libcalamares] Expand runCommand() inline for CommandLine Since we want to hook up more to the runner, the simplified API in System::runCommand() is not sufficient. --- src/libcalamares/utils/CommandList.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index 120cf5aa5..39254ec05 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -16,6 +16,7 @@ #include "compat/Variant.h" #include "locale/Global.h" #include "utils/Logger.h" +#include "utils/Runner.h" #include "utils/StringExpander.h" #include "utils/System.h" #include "utils/Variant.h" @@ -257,7 +258,10 @@ CommandList::run() shell_cmd << ( environmentSetting + processed_cmd ); std::chrono::seconds timeout = i->timeout() >= std::chrono::seconds::zero() ? i->timeout() : m_timeout; - ProcessResult r = System::runCommand( location, shell_cmd, QString(), QString(), timeout ); + + Calamares::Utils::Runner runner( shell_cmd ); + runner.setLocation( location ).setTimeout( timeout ).setWorkingDirectory( QString() ); + ProcessResult r = runner.run(); if ( r.getExitCode() != 0 ) { From 6969a5e01dd4b15476401218a7070ccda5993d3c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 15:15:52 +0200 Subject: [PATCH 05/10] [libcalamares] log-as-you-go for verbose commands --- src/libcalamares/utils/CommandList.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index 39254ec05..b976c7928 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -261,6 +261,12 @@ CommandList::run() Calamares::Utils::Runner runner( shell_cmd ); runner.setLocation( location ).setTimeout( timeout ).setWorkingDirectory( QString() ); + if ( i->isVerbose() ) + { + runner.enableOutputProcessing(); + QObject::connect( + &runner, &Calamares::Utils::Runner::output, []( QString output ) { cDebug() << output; } ); + } ProcessResult r = runner.run(); if ( r.getExitCode() != 0 ) From 091c00e4efaab9f22af6d99c7326801defad68ea Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 15:27:02 +0200 Subject: [PATCH 06/10] [libcalamares] use test to demonstrate verbose CommandLine --- src/libcalamares/utils/Tests.cpp | 49 ++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index fd1411d38..e38aed0f2 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -56,6 +56,7 @@ private Q_SLOTS: void testCommandConstructors(); void testCommandConstructorsYAML(); void testCommandRunning(); + void testCommandVerbose(); /** @section Test that all the UMask objects work correctly. */ void testUmask(); @@ -454,6 +455,54 @@ LibCalamaresTests::testCommandRunning() tempRoot.setAutoRemove( true ); } +void +LibCalamaresTests::testCommandVerbose() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); + + QTemporaryDir tempRoot( QDir::tempPath() + QStringLiteral( "/test-job-XXXXXX" ) ); + tempRoot.setAutoRemove( false ); + + const QString testExecutable = tempRoot.filePath( "example.sh" ); + + cDebug() << "Creating example executable" << testExecutable; + + { + QFile f( testExecutable ); + QVERIFY( f.open( QIODevice::WriteOnly ) ); + f.write( "#! /bin/sh\necho one\necho two\necho error 1>&2\nsleep 1; echo three\n" ); + f.close(); + Calamares::Permissions::apply( testExecutable, 0755 ); + } + + cDebug() << "Running command non-verbose"; + { + Calamares::CommandList l( false ); // no chroot + Calamares::CommandLine c( testExecutable, {}, std::chrono::seconds( 2 ) ); + c.updateVerbose( false ); + QVERIFY( !c.isVerbose() ); + + l.push_back( c ); + + const auto r = l.run(); + QVERIFY( bool( r ) ); + } + + cDebug() << "Running command verbosely"; + + { + Calamares::CommandList l( false ); // no chroot + Calamares::CommandLine c( testExecutable, {}, std::chrono::seconds( 2 ) ); + c.updateVerbose( true ); + QVERIFY( c.isVerbose() ); + + l.push_back( c ); + + const auto r = l.run(); + QVERIFY( bool( r ) ); + } +} + void LibCalamaresTests::testUmask() { From 8bb6c6393101d8955ebe82e388f135f2e84f31a0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 15:33:16 +0200 Subject: [PATCH 07/10] [libcalamares] Avoid stray space when logging command-output --- src/libcalamares/utils/Runner.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/utils/Runner.cpp b/src/libcalamares/utils/Runner.cpp index 6a1eb85f8..f7872a7d0 100644 --- a/src/libcalamares/utils/Runner.cpp +++ b/src/libcalamares/utils/Runner.cpp @@ -189,8 +189,9 @@ Calamares::Utils::Runner::run() ? ( static_cast< int >( std::chrono::milliseconds( m_timeout ).count() ) ) : -1 ) ) { - cWarning() << "Process" << m_command.first() << "timed out after" << m_timeout.count() << "ms. Output so far:\n" - << Logger::NoQuote << process.readAllStandardOutput(); + cWarning() << "Process" << m_command.first() << "timed out after" << m_timeout.count() << "ms." + << Logger::NoQuote << "Output so far:\n" + << process.readAllStandardOutput(); return ProcessResult::Code::TimedOut; } @@ -216,7 +217,7 @@ Calamares::Utils::Runner::run() if ( process.exitStatus() == QProcess::CrashExit ) { - cWarning() << "Process" << m_command.first() << "crashed. Output so far:\n" << Logger::NoQuote << output; + cWarning() << "Process" << m_command.first() << "crashed." << Logger::NoQuote << "Output so far:\n" << output; return ProcessResult::Code::Crashed; } @@ -226,7 +227,7 @@ Calamares::Utils::Runner::run() { if ( showDebug && !output.isEmpty() ) { - cDebug() << Logger::SubEntry << "Finished. Exit code:" << r << "output:\n" << Logger::NoQuote << output; + cDebug() << Logger::SubEntry << "Finished. Exit code:" << r << Logger::NoQuote << "output:\n" << output; } } else // if ( r != 0 ) @@ -234,8 +235,8 @@ Calamares::Utils::Runner::run() if ( !output.isEmpty() ) { cDebug() << Logger::SubEntry << "Target cmd:" << Logger::RedactedCommand( m_command ) << "Exit code:" << r - << "output:\n" - << Logger::NoQuote << output; + << Logger::NoQuote << "output:\n" + << output; } else { From e6de798228e72cf9151127e61c8b13eacce116ce Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 15:49:27 +0200 Subject: [PATCH 08/10] [libcalamares] Add a test for command-timeout --- src/libcalamares/utils/Tests.cpp | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index e38aed0f2..acd6ee2c0 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -31,6 +31,8 @@ #include +#include + #include #include #include @@ -56,6 +58,7 @@ private Q_SLOTS: void testCommandConstructors(); void testCommandConstructorsYAML(); void testCommandRunning(); + void testCommandTimeout(); void testCommandVerbose(); /** @section Test that all the UMask objects work correctly. */ @@ -455,6 +458,38 @@ LibCalamaresTests::testCommandRunning() tempRoot.setAutoRemove( true ); } +void +LibCalamaresTests::testCommandTimeout() +{ + + QTemporaryDir tempRoot( QDir::tempPath() + QStringLiteral( "/test-job-XXXXXX" ) ); + tempRoot.setAutoRemove( false ); + + const QString testExecutable = tempRoot.filePath( "example.sh" ); + + cDebug() << "Creating example executable" << testExecutable; + + { + QFile f( testExecutable ); + QVERIFY( f.open( QIODevice::WriteOnly ) ); + f.write( "#! /bin/sh\necho early\nsleep 3\necho late" ); + f.close(); + Calamares::Permissions::apply( testExecutable, 0755 ); + } + + { + Calamares::CommandList l( false ); // no chroot + Calamares::CommandLine c( testExecutable, {}, std::chrono::seconds( 2 ) ); + l.push_back( c ); + + const auto r = l.run(); + QVERIFY( !bool( r ) ); // Because it times out after 2 seconds + // The **command** timed out, but the job result is a generic "error" + // QCOMPARE( r.errorCode(), static_cast>(Calamares::ProcessResult::Code::TimedOut)); + QCOMPARE( r.errorCode(), -1 ); + } +} + void LibCalamaresTests::testCommandVerbose() { From 265469c9cd2ce91b07782d0a758e4532d9fb5594 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 17:34:44 +0200 Subject: [PATCH 09/10] explain-verbose-test --- src/libcalamares/utils/Tests.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index acd6ee2c0..81877029b 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -501,7 +501,6 @@ LibCalamaresTests::testCommandVerbose() const QString testExecutable = tempRoot.filePath( "example.sh" ); cDebug() << "Creating example executable" << testExecutable; - { QFile f( testExecutable ); QVERIFY( f.open( QIODevice::WriteOnly ) ); @@ -510,6 +509,11 @@ LibCalamaresTests::testCommandVerbose() Calamares::Permissions::apply( testExecutable, 0755 ); } + // Note that, because of the blocking way run() works, + // in this single-threaded test with no event loop, + // there's nothing for the verbose version to connect + // to for sending output. + cDebug() << "Running command non-verbose"; { Calamares::CommandList l( false ); // no chroot @@ -524,7 +528,6 @@ LibCalamaresTests::testCommandVerbose() } cDebug() << "Running command verbosely"; - { Calamares::CommandList l( false ); // no chroot Calamares::CommandLine c( testExecutable, {}, std::chrono::seconds( 2 ) ); From bbe015e732b7cc7bf41c60fd53eac7de687fc278 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 30 Jun 2024 18:05:54 +0200 Subject: [PATCH 10/10] [libcalamares] Command verbosity was lost during expansion --- src/libcalamares/utils/CommandList.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index b976c7928..6e3434aa3 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -165,7 +165,12 @@ CommandLine::expand( KMacroExpanderBase& expander ) const QStringList e = m_environment; std::for_each( e.begin(), e.end(), [ &expander ]( QString& s ) { expander.expandMacrosShellQuote( s ); } ); - return { c, m_environment, m_timeout }; + CommandLine l { c, m_environment, m_timeout }; + if ( m_verbose.has_value() ) + { + l.updateVerbose( m_verbose.value() ); + } + return l; } Calamares::CommandLine