diff --git a/CHANGES-3.3 b/CHANGES-3.3 index c40feefbd..60ee3e0c0 100644 --- a/CHANGES-3.3 +++ b/CHANGES-3.3 @@ -9,13 +9,29 @@ the history of the 3.2 series (2018-05 - 2022-08). # 3.3.4 (unreleased) +In this release, process jobmodules -- a particular kind of module +recognizable by `type: job` and `interface: process` in the descriptor +file -- undergo a large change to resemble *shellprocess* more. + +Users of process jobmodules are encouraged to double-check the Functionality +of those modules in this release. + This release contains contributions from (alphabetically by first name): - Adriaan de Groot - Victor Fuentes ## Core ## + - Process jobs (a job type provided by Calamares core) now share more + code with *contextualprocess* and *shellprocess* jobs. The execution + mechanism is the same, and always invokes the shell, whether the command + runs in the host or in the target system. It is no longer necessary to + add `/bin/sh` in the *command* key -- this is always present. ## Modules ## + - *contextualprocess* and *shellprocess* can now set environment variables + as part of the configuration. See *shellprocess* documentation for details. + This is optional, and does not do anything that could not already be done + by putting `export VAR=value ;` in front of the command before. - *partition* fixed a bug with an uninitialized variable. (thanks Victor) diff --git a/src/libcalamares/ProcessJob.cpp b/src/libcalamares/ProcessJob.cpp index 70506f5ba..d3d76afb8 100644 --- a/src/libcalamares/ProcessJob.cpp +++ b/src/libcalamares/ProcessJob.cpp @@ -10,8 +10,8 @@ #include "ProcessJob.h" +#include "utils/CommandList.h" #include "utils/Logger.h" -#include "utils/System.h" #include @@ -57,23 +57,9 @@ ProcessJob::prettyStatusMessage() const JobResult ProcessJob::exec() { - using Calamares::System; - - if ( m_runInChroot ) - { - return Calamares::System::instance() - ->targetEnvCommand( { m_command }, m_workingPath, QString(), m_timeoutSec ) - .explainProcess( m_command, m_timeoutSec ); - } - else - { - return System::runCommand( System::RunLocation::RunInHost, - { "/bin/sh", "-c", m_command }, - m_workingPath, - QString(), - m_timeoutSec ) - .explainProcess( m_command, m_timeoutSec ); - } + Calamares::CommandList l( m_runInChroot, m_timeoutSec ); + l.push_back( Calamares::CommandLine { m_command } ); + return l.run(); } } // namespace Calamares diff --git a/src/libcalamares/utils/CommandList.cpp b/src/libcalamares/utils/CommandList.cpp index 96d5ea5d1..86efe1d38 100644 --- a/src/libcalamares/utils/CommandList.cpp +++ b/src/libcalamares/utils/CommandList.cpp @@ -14,6 +14,7 @@ #include "JobQueue.h" #include "compat/Variant.h" +#include "locale/Global.h" #include "utils/Logger.h" #include "utils/StringExpander.h" #include "utils/System.h" @@ -25,20 +26,6 @@ namespace Calamares { -static CommandLine -get_variant_object( const QVariantMap& m ) -{ - QString command = Calamares::getString( m, "command" ); - qint64 timeout = Calamares::getInteger( m, "timeout", -1 ); - - if ( !command.isEmpty() ) - { - return CommandLine( command, timeout >= 0 ? std::chrono::seconds( timeout ) : CommandLine::TimeoutNotSet() ); - } - cWarning() << "Bad CommandLine element" << m; - return CommandLine(); -} - static CommandList_t get_variant_stringlist( const QVariantList& l ) { @@ -52,7 +39,7 @@ get_variant_stringlist( const QVariantList& l ) } else if ( Calamares::typeOf( v ) == Calamares::MapVariantType ) { - auto command( get_variant_object( v.toMap() ) ); + CommandLine command( v.toMap() ); if ( command.isValid() ) { retl.append( command ); @@ -91,15 +78,48 @@ get_gs_expander( System::RunLocation location ) expander.insert( QStringLiteral( "USER" ), gs->value( "username" ).toString() ); } + if ( gs ) + { + const auto key = QStringLiteral( "LANG" ); + const QString lang = Calamares::Locale::readGS( *gs, key ); + if ( !lang.isEmpty() ) + { + expander.insert( key, lang ); + } + } + return expander; } +CommandLine::CommandLine( const QVariantMap& m ) +{ + const QString command = Calamares::getString( m, "command" ); + const qint64 timeout = Calamares::getInteger( m, "timeout", -1 ); + if ( !command.isEmpty() ) + { + m_command = command; + m_timeout = timeout >= 0 ? std::chrono::seconds( timeout ) : CommandLine::TimeoutNotSet(); + m_environment = Calamares::getStringList( m, "environment" ); + } + else + { + cWarning() << "Bad CommandLine element" << m; + // this CommandLine is invalid + } +} + CommandLine CommandLine::expand( KMacroExpanderBase& expander ) const { - QString c = first; + // Calamares variable expansion in the command + QString c = m_command; expander.expandMacrosShellQuote( c ); - return { c, second }; + + // .. and expand in each environment key=value string. + QStringList e = m_environment; + std::for_each( e.begin(), e.end(), [ &expander ]( QString& s ) { expander.expandMacrosShellQuote( s ); } ); + + return { c, m_environment, m_timeout }; } Calamares::CommandLine @@ -136,7 +156,7 @@ CommandList::CommandList::CommandList( const QVariant& v, bool doChroot, std::ch } else if ( Calamares::typeOf( v ) == Calamares::MapVariantType ) { - auto c( get_variant_object( v.toMap() ) ); + CommandLine c( v.toMap() ); if ( c.isValid() ) { append( c ); @@ -178,8 +198,18 @@ CommandList::run() processed_cmd.remove( 0, 1 ); // Drop the - } + const QString environmentSetting = []( const QStringList& l ) -> QString + { + if ( l.isEmpty() ) + { + return {}; + } + + return QStringLiteral( "export " ) + l.join( " " ) + QStringLiteral( " ; " ); + }( i->environment() ); + QStringList shell_cmd { "/bin/sh", "-c" }; - shell_cmd << processed_cmd; + 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 ); diff --git a/src/libcalamares/utils/CommandList.h b/src/libcalamares/utils/CommandList.h index 9969dc165..17a251a7c 100644 --- a/src/libcalamares/utils/CommandList.h +++ b/src/libcalamares/utils/CommandList.h @@ -28,30 +28,43 @@ namespace Calamares * Each command can have an associated timeout in seconds. The timeout * defaults to 10 seconds. Provide some convenience naming and construction. */ -struct CommandLine +class CommandLine { +public: static inline constexpr std::chrono::seconds TimeoutNotSet() { return std::chrono::seconds( -1 ); } /// An invalid command line CommandLine() = default; CommandLine( const QString& s ) - : first( s ) - , second( TimeoutNotSet() ) + : m_command( s ) { } CommandLine( const QString& s, std::chrono::seconds t ) - : first( s ) - , second( t ) + : m_command( s ) + , m_timeout( t ) { } - QString command() const { return first; } + CommandLine( const QString& s, const QStringList& env, std::chrono::seconds t ) + : m_command( s ) + , m_environment( env ) + , m_timeout( t ) + { + } - std::chrono::seconds timeout() const { return second; } + /** @brief Constructs a CommandLine from a map with keys + * + * Relevant keys are *command*, *environment* and *timeout*. + */ + CommandLine( const QVariantMap& m ); - bool isValid() const { return !first.isEmpty(); } + QString command() const { return m_command; } + [[nodiscard]] QStringList environment() const { return m_environment; } + std::chrono::seconds timeout() const { return m_timeout; } + + bool isValid() const { return !m_command.isEmpty(); } /** @brief Returns a copy of this one command, with variables expanded * @@ -60,6 +73,7 @@ struct CommandLine * instance, which handles the ROOT and USER variables. */ DLLEXPORT CommandLine expand( KMacroExpanderBase& expander ) const; + /** @brief As above, with a default macro-expander. * * The default macro-expander assumes RunInHost (e.g. ROOT will @@ -68,8 +82,9 @@ struct CommandLine DLLEXPORT CommandLine expand() const; private: - QString first; - std::chrono::seconds second = std::chrono::seconds( -1 ); + QString m_command; + QStringList m_environment; + std::chrono::seconds m_timeout = TimeoutNotSet(); }; /** @brief Abbreviation, used internally. */ @@ -91,6 +106,7 @@ public: CommandList( const QVariant& v, bool doChroot = true, std::chrono::seconds timeout = std::chrono::seconds( 10 ) ); bool doChroot() const { return m_doChroot; } + std::chrono::seconds defaultTimeout() const { return m_timeout; } Calamares::JobResult run(); @@ -109,6 +125,7 @@ public: * @see CommandLine::expand() for details. */ 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. diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index ae0be4615..2e1b5655c 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -12,6 +12,7 @@ #include "CommandList.h" #include "Entropy.h" #include "Logger.h" +#include "Permissions.h" #include "RAII.h" #include "Runner.h" #include "String.h" @@ -52,6 +53,9 @@ private Q_SLOTS: void testCommands(); void testCommandExpansion_data(); void testCommandExpansion(); // See also shellprocess tests + void testCommandConstructors(); + void testCommandConstructorsYAML(); + void testCommandRunning(); /** @section Test that all the UMask objects work correctly. */ void testUmask(); @@ -300,6 +304,136 @@ LibCalamaresTests::testCommandExpansion() QCOMPARE( e.command(), expected ); } +void +LibCalamaresTests::testCommandConstructors() +{ + const QString command( "do this" ); + Calamares::CommandLine c0( command ); + + QCOMPARE( c0.command(), command ); + QCOMPARE( c0.timeout(), Calamares::CommandLine::TimeoutNotSet() ); + QVERIFY( c0.environment().isEmpty() ); + + const QStringList env { "-la", "/tmp" }; + Calamares::CommandLine c1( command, env, Calamares::CommandLine::TimeoutNotSet() ); + + QCOMPARE( c1.command(), command ); + QCOMPARE( c1.timeout(), Calamares::CommandLine::TimeoutNotSet() ); + QVERIFY( !c1.environment().isEmpty() ); + QCOMPARE( c1.environment().count(), 2 ); + QCOMPARE( c1.environment(), env ); +} + +void +LibCalamaresTests::testCommandConstructorsYAML() +{ + QTemporaryFile f; + QVERIFY( f.open() ); + f.write( R"(--- +commands: + - one-string-command + - command: only-command + - command: with-timeout + timeout: 12 + - command: all-three + timeout: 20 + environment: + - PATH=/USER + - DISPLAY=:0 + )" ); + f.close(); + bool ok = false; + QVariantMap m = Calamares::YAML::load( f.fileName(), &ok ); + + QVERIFY( ok ); + QCOMPARE( m.count(), 1 ); + QCOMPARE( m[ "commands" ].toList().count(), 4 ); + + { + // Take care! The second parameter is a bool, so "3" here means "true" + Calamares::CommandList cmds( m[ "commands" ], 3 ); + QCOMPARE( cmds.defaultTimeout(), std::chrono::seconds( 10 ) ); + // But the 4 commands are there anyway + QCOMPARE( cmds.count(), 4 ); + QCOMPARE( cmds.at( 0 ).command(), QString( "one-string-command" ) ); + QCOMPARE( cmds.at( 0 ).environment(), QStringList() ); + QCOMPARE( cmds.at( 0 ).timeout(), Calamares::CommandLine::TimeoutNotSet() ); + QCOMPARE( cmds.at( 1 ).command(), QString( "only-command" ) ); + QCOMPARE( cmds.at( 2 ).command(), QString( "with-timeout" ) ); + QCOMPARE( cmds.at( 2 ).environment(), QStringList() ); + QCOMPARE( cmds.at( 2 ).timeout(), std::chrono::seconds( 12 ) ); + + QStringList expectedEnvironment = { "PATH=/USER", "DISPLAY=:0" }; + QCOMPARE( cmds.at( 3 ).command(), QString( "all-three" ) ); + QCOMPARE( cmds.at( 3 ).environment(), expectedEnvironment ); + QCOMPARE( cmds.at( 3 ).timeout(), std::chrono::seconds( 20 ) ); + } + + { + Calamares::CommandList cmds( m[ "commands" ], true, std::chrono::seconds( 3 ) ); + QCOMPARE( cmds.defaultTimeout(), std::chrono::seconds( 3 ) ); + QCOMPARE( cmds.at( 0 ).timeout(), Calamares::CommandLine::TimeoutNotSet() ); + QCOMPARE( cmds.at( 2 ).timeout(), std::chrono::seconds( 12 ) ); + } +} + +void +LibCalamaresTests::testCommandRunning() +{ + + QTemporaryDir tempRoot( QDir::tempPath() + QStringLiteral( "/test-job-XXXXXX" ) ); + tempRoot.setAutoRemove( false ); + + const QString testExecutable = tempRoot.filePath( "example.sh" ); + const QString testFile = tempRoot.filePath( "example.txt" ); + + { + QFile f( testExecutable ); + QVERIFY( f.open( QIODevice::WriteOnly ) ); + f.write( "#! /bin/sh\necho \"$calamares_test_variable\"\n" ); + f.close(); + Calamares::Permissions::apply( testExecutable, 0755 ); + } + + const QString echoCommand = testExecutable + QStringLiteral( " > " ) + testFile; + + // Without an environment, the variable echoed in the example + // executable is empty, and we write a single newline to stdout, + // which is redirected to testFile. + { + Calamares::CommandList l( false ); // no chroot + Calamares::CommandLine c( echoCommand, {}, std::chrono::seconds( 2 ) ); + l.push_back( c ); + + const auto r = l.run(); + QVERIFY( bool( r ) ); + + QCOMPARE( QFileInfo( testFile ).size(), 1 ); // single newline + } + + // With an environment, echoes the value of the variable and a newline + { + const QString world = QStringLiteral( "Hello world" ); + Calamares::CommandList l( false ); // no chroot + Calamares::CommandLine c( + echoCommand, + { QStringLiteral( "calamares_test_variable=" ) + QChar( '"' ) + world + QChar( '"' ) }, + std::chrono::seconds( 2 ) ); + l.push_back( c ); + + const auto r = l.run(); + QVERIFY( bool( r ) ); + + QCOMPARE( QFileInfo( testFile ).size(), world.length() + 1 ); // plus newline + QFile f( testFile ); + QVERIFY( f.open( QIODevice::ReadOnly ) ); + QCOMPARE( f.readAll(), world + QChar( '\n' ) ); + } + + + tempRoot.setAutoRemove( true ); +} + void LibCalamaresTests::testUmask() { diff --git a/src/modules/README.md b/src/modules/README.md index 108b3200d..86aa4d726 100644 --- a/src/modules/README.md +++ b/src/modules/README.md @@ -497,7 +497,8 @@ LC_ALL and LANG to "C" for the called command. ## Process modules -Use of this kind of module is **not** recommended. +Use of this kind of module is **not** recommended. Use *shellprocess* +instead, which is more configurable. > Type: jobmodule > Interface: process @@ -506,9 +507,14 @@ A process jobmodule runs a (single) command. The interface is *process*, while the module type must be *job* or *jobmodule*. The module-descriptor key *command* should have a string as value, which is -passed to the shell -- remember to quote it properly. It is generally +passed to the shell -- remember to quote it properly in YAML. It is generally recommended to use a *shellprocess* job module instead (less configuration, -easier to have multiple instances). +easier to have multiple instances). There is no configuration outside +of the module-descriptor. The *command* undergoes Calamares variable- +expansion (e.g. replacing `${ROOT}` by the target of the installation). +See *shellprocess* documentation for details. + +Optional keys are *timeout* and *chroot*. `CMakeLists.txt` is *not* used for process jobmodules. diff --git a/src/modules/contextualprocess/contextualprocess.conf b/src/modules/contextualprocess/contextualprocess.conf index b86fd922c..ba8a8bf1d 100644 --- a/src/modules/contextualprocess/contextualprocess.conf +++ b/src/modules/contextualprocess/contextualprocess.conf @@ -7,14 +7,13 @@ # When a given global value (string) equals a given value, then # the associated command is executed. # -# The special top-level keys *dontChroot* and *timeout* have -# meaning just like in shellprocess.conf. They are excluded from -# the comparison with global variables. -# # Configuration consists of keys for global variable names (except # *dontChroot* and *timeout*), and the sub-keys are strings to compare # to the variable's value. If the variable has that particular value, the -# corresponding value (script) is executed. +# corresponding value (script) is executed. The top-level keys *dontChroot* +# and *timeout* are not global variable names. They have +# meaning just like in shellprocess.conf, that is they +# determine **where** the command runs and how long it has. # # The variable **may** contain dots, in which case the dot is used # to select into maps inside global storage, e.g. diff --git a/src/modules/shellprocess/shellprocess.conf b/src/modules/shellprocess/shellprocess.conf index 9ff83221e..87e31c58c 100644 --- a/src/modules/shellprocess/shellprocess.conf +++ b/src/modules/shellprocess/shellprocess.conf @@ -12,8 +12,15 @@ # system from the point of view of the command (when run in the target # system, e.g. when *dontChroot* is false, that will be `/`). # - `USER` is replaced by the username, set on the user page. +# - `LANG` is replaced by the language chosen for the user-interface +# of Calamares, set on the welcome page. This may not reflect the +# chosen system language from the locale page. # # Variables are written as `${var}`, e.g. `${ROOT}`. +# Write `$$` to get a shell-escaped `\$` in the shell command. +# It is not possible to get an un-escaped `$` in the shell command +# (either the command will fail because of undefined variables, or +# you get a shell-escaped `\$`). # # The (global) timeout for the command list can be set with # the *timeout* key. The value is a time in seconds, default @@ -35,20 +42,33 @@ # # The value of *script* may be: # - a single string; this is one command that is executed. -# - a single object (this is not useful). +# - a single object (see below). # - a list of items; these are executed one at a time, by # separate shells (/bin/sh -c is invoked for each command). # Each list item may be: # - a single string; this is one command that is executed. # - a single object, specifying a key *command* and (optionally) # a key *timeout* to set the timeout for this specific -# command differently from the global setting. +# command differently from the global setting. An optional +# key *environment* is a list of strings to put into the +# environment of the command. # -# Using a single object is not useful because the same effect can -# be obtained with a single string and a global timeout, but when -# there are multiple commands to execute, one of them might have +# Using a single object is not generally useful because the same effect +# can be obtained with a single string and a global timeout, except +# when the command needs environment-settings. When there are +# multiple commands to execute, one of them might have # a different timeout than the others. # +# The environment strings should all be "KEY='some value'" strings, +# as if they can be typed into the shell. Quoting the environment +# strings with "" in YAML is recommended. Adding the '' quotes ensures +# that the value will not be interpreted by the shell. Writing +# environment strings is the same as placing `export KEY='some value' ;` +# in front of the *command*. +# +# Calamares variable expansion is **also** done on the environment strings. +# Write `$$` to get a literal `$` in the shell command. +# # To change the description of the job, set the *name* entries in *i18n*. --- # Set to true to run in host, rather than target system