Merge branch 'issue-2212' into calamares

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)
 
 

src/libcalamares/ProcessJob.cpp

@@ -10,8 +10,8 @@
 
 #include "ProcessJob.h"
 
+#include "utils/CommandList.h"
 #include "utils/Logger.h"
-#include "utils/System.h"
 
 #include <QDir>
 
@@ -57,23 +57,9 @@ ProcessJob::prettyStatusMessage() const
 JobResult
 ProcessJob::exec()
 {
-    using Calamares::System;
+    Calamares::CommandList l( m_runInChroot, m_timeoutSec );
-
+    l.push_back( Calamares::CommandLine { m_command } );
-    if ( m_runInChroot )
+    return l.run();
-    {
-        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 );
-    }
 }
 
 }  // namespace Calamares

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 );

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 )
+        : m_command( s )
-        , second( TimeoutNotSet() )
     {
     }
 
     CommandLine( const QString& s, std::chrono::seconds t )
-        : first( s )
+        : m_command( s )
-        , second( t )
+        , 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;
+    QString m_command;
-    std::chrono::seconds second = std::chrono::seconds( -1 );
+    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.

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()
 {

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.
 

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.

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
+# Using a single object is not generally useful because the same effect
-# be obtained with a single string and a global timeout, but when
+# can be obtained with a single string and a global timeout, except
-# there are multiple commands to execute, one of them might have
+# 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