Merge branch 'issue-1740' into calamares

FIXES #1740

src/libcalamares/CMakeLists.txt

@@ -77,6 +77,7 @@ set( libSources
     utils/Permissions.cpp
     utils/PluginFactory.cpp
     utils/Retranslator.cpp
+    utils/Runner.cpp
     utils/String.cpp
     utils/UMask.cpp
     utils/Variant.cpp
@@ -303,6 +304,7 @@ calamares_add_test(
     libcalamaresutilstest
     SOURCES
         utils/Tests.cpp
+        utils/Runner.cpp
 )
 
 calamares_add_test(

src/libcalamares/ProcessJob.cpp

@@ -14,7 +14,6 @@
 #include "utils/Logger.h"
 
 #include <QDir>
-#include <QProcess>
 
 namespace Calamares
 {

src/libcalamares/PythonJob.cpp

@@ -36,6 +36,12 @@ BOOST_PYTHON_FUNCTION_OVERLOADS( check_target_env_output_list_overloads,
                                  CalamaresPython::check_target_env_output,
                                  1,
                                  3 );
+BOOST_PYTHON_FUNCTION_OVERLOADS( target_env_process_output_overloads,
+                                 CalamaresPython::target_env_process_output,
+                                 1,
+                                 4 );
+BOOST_PYTHON_FUNCTION_OVERLOADS( host_env_process_output_overloads, CalamaresPython::host_env_process_output, 1, 4 );
+
 BOOST_PYTHON_MODULE( libcalamares )
 {
     bp::object package = bp::scope();
@@ -137,6 +143,15 @@ BOOST_PYTHON_MODULE( libcalamares )
                                                      "Runs the specified command in the chroot of the target system.\n"
                                                      "Returns the program's standard output, and raises a "
                                                      "subprocess.CalledProcessError if something went wrong." ) );
+    bp::def( "target_env_process_output",
+             &CalamaresPython::target_env_process_output,
+             target_env_process_output_overloads( bp::args( "command", "callback", "stdin", "timeout" ),
+                                                  "Runs the specified @p command in the target system." ) );
+    bp::def( "host_env_process_output",
+             &CalamaresPython::host_env_process_output,
+             host_env_process_output_overloads( bp::args( "command", "callback", "stdin", "timeout" ),
+                                                "Runs the specified command in the host system." ) );
+
     bp::def( "obscure",
              &CalamaresPython::obscure,
              bp::args( "s" ),

src/libcalamares/PythonJobApi.cpp

@@ -17,6 +17,7 @@
 #include "utils/CalamaresUtilsSystem.h"
 #include "utils/Logger.h"
 #include "utils/RAII.h"
+#include "utils/Runner.h"
 #include "utils/String.h"
 
 #include <QCoreApplication>
@@ -171,6 +172,68 @@ PythonJobInterface::setprogress( qreal progress )
     }
 }
 
+static inline int
+_process_output( Calamares::Utils::RunLocation location,
+                 const boost::python::list& args,
+                 const boost::python::object& callback,
+                 const std::string& stdin,
+                 int timeout )
+{
+    Calamares::Utils::Runner r( _bp_list_to_qstringlist( args ) );
+    r.setLocation( location );
+    if ( !callback.is_none() )
+    {
+        bp::extract< bp::list > x( callback );
+        if ( x.check() )
+        {
+            QObject::connect( &r, &decltype( r )::output, [cb = callback.attr( "append" )]( const QString& s ) {
+                cb( s.toStdString() );
+            } );
+        }
+        else
+        {
+            QObject::connect(
+                &r, &decltype( r )::output, [&callback]( const QString& s ) { callback( s.toStdString() ); } );
+        }
+        r.enableOutputProcessing();
+    }
+    if ( !stdin.empty() )
+    {
+        r.setInput( QString::fromStdString( stdin ) );
+    }
+    if ( timeout > 0 )
+    {
+        r.setTimeout( std::chrono::seconds( timeout ) );
+    }
+
+    auto result = r.run();
+
+    if ( result.getExitCode() )
+    {
+        return _handle_check_target_env_call_error( result, r.executable() );
+    }
+    return 0;
+}
+
+int
+target_env_process_output( const boost::python::list& args,
+                           const boost::python::object& callback,
+                           const std::string& stdin,
+                           int timeout )
+{
+    return _process_output( Calamares::Utils::RunLocation::RunInTarget, args, callback, stdin, timeout );
+}
+
+int
+host_env_process_output( const boost::python::list& args,
+                         const boost::python::object& callback,
+                         const std::string& stdin,
+                         int timeout )
+{
+    return _process_output( Calamares::Utils::RunLocation::RunInHost, args, callback, stdin, timeout );
+}
+
+
 std::string
 obscure( const std::string& string )
 {

src/libcalamares/PythonJobApi.h

@@ -42,6 +42,16 @@ check_target_env_output( const std::string& command, const std::string& stdin =
 std::string
 check_target_env_output( const boost::python::list& args, const std::string& stdin = std::string(), int timeout = 0 );
 
+int target_env_process_output( const boost::python::list& args,
+                               const boost::python::object& callback = boost::python::object(),
+                               const std::string& stdin = std::string(),
+                               int timeout = 0 );
+
+int host_env_process_output( const boost::python::list& args,
+                             const boost::python::object& callback = boost::python::object(),
+                             const std::string& stdin = std::string(),
+                             int timeout = 0 );
+
 std::string obscure( const std::string& string );
 
 boost::python::object gettext_path();

src/libcalamares/utils/CalamaresUtilsSystem.cpp

@@ -13,12 +13,11 @@
 
 #include "GlobalStorage.h"
 #include "JobQueue.h"
-#include "Settings.h"
+#include "Runner.h"
 #include "utils/Logger.h"
 
 #include <QCoreApplication>
 #include <QDir>
-#include <QProcess>
 #include <QRegularExpression>
 
 #ifdef Q_OS_LINUX
@@ -33,47 +32,6 @@
 // clang-format on
 #endif
 
-/** @brief When logging commands, don't log everything.
- *
- * The command-line arguments to some commands may contain the
- * encrypted password set by the user. Don't log that password,
- * since the log may get posted to bug reports, or stored in
- * the target system.
- */
-struct RedactedList
-{
-    RedactedList( const QStringList& l )
-        : list( l )
-    {
-    }
-
-    const QStringList& list;
-};
-
-QDebug&
-operator<<( QDebug& s, const RedactedList& l )
-{
-    // Special case logging: don't log the (encrypted) password.
-    if ( l.list.contains( "usermod" ) )
-    {
-        for ( const auto& item : l.list )
-            if ( item.startsWith( "$6$" ) )
-            {
-                s << "<password>";
-            }
-            else
-            {
-                s << item;
-            }
-    }
-    else
-    {
-        s << l.list;
-    }
-
-    return s;
-}
-
 namespace CalamaresUtils
 {
 
@@ -116,112 +74,9 @@ System::runCommand( System::RunLocation location,
                     const QString& stdInput,
                     std::chrono::seconds timeoutSec )
 {
-    if ( args.isEmpty() )
+    Calamares::Utils::Runner r( args );
-    {
+    r.setLocation( location ).setInput( stdInput ).setTimeout( timeoutSec ).setWorkingDirectory( workingPath );
-        cWarning() << "Cannot run an empty program list";
+    return r.run();
-        return ProcessResult::Code::FailedToStart;
-    }
-
-    Calamares::GlobalStorage* gs
-        = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
-
-    if ( ( location == System::RunLocation::RunInTarget ) && ( !gs || !gs->contains( "rootMountPoint" ) ) )
-    {
-        cWarning() << "No rootMountPoint in global storage, while RunInTarget is specified";
-        return ProcessResult::Code::NoWorkingDirectory;
-    }
-
-    QString program;
-    QStringList arguments( args );
-
-    if ( location == System::RunLocation::RunInTarget )
-    {
-        QString destDir = gs->value( "rootMountPoint" ).toString();
-        if ( !QDir( destDir ).exists() )
-        {
-            cWarning() << "rootMountPoint points to a dir which does not exist";
-            return ProcessResult::Code::NoWorkingDirectory;
-        }
-
-        program = "chroot";
-        arguments.prepend( destDir );
-    }
-    else
-    {
-        program = "env";
-    }
-
-    QProcess process;
-    process.setProgram( program );
-    process.setArguments( arguments );
-    process.setProcessChannelMode( QProcess::MergedChannels );
-
-    if ( !workingPath.isEmpty() )
-    {
-        if ( QDir( workingPath ).exists() )
-        {
-            process.setWorkingDirectory( QDir( workingPath ).absolutePath() );
-        }
-        else
-        {
-            cWarning() << "Invalid working directory:" << workingPath;
-            return ProcessResult::Code::NoWorkingDirectory;
-        }
-    }
-
-    cDebug() << Logger::SubEntry << "Running" << program << RedactedList( arguments );
-    process.start();
-    if ( !process.waitForStarted() )
-    {
-        cWarning() << "Process" << args.first() << "failed to start" << process.error();
-        return ProcessResult::Code::FailedToStart;
-    }
-
-    if ( !stdInput.isEmpty() )
-    {
-        process.write( stdInput.toLocal8Bit() );
-    }
-    process.closeWriteChannel();
-
-    if ( !process.waitForFinished( timeoutSec > std::chrono::seconds::zero()
-                                       ? ( static_cast< int >( std::chrono::milliseconds( timeoutSec ).count() ) )
-                                       : -1 ) )
-    {
-        cWarning() << "Process" << args.first() << "timed out after" << timeoutSec.count() << "s. Output so far:\n"
-                   << Logger::NoQuote << process.readAllStandardOutput();
-        return ProcessResult::Code::TimedOut;
-    }
-
-    QString output = QString::fromLocal8Bit( process.readAllStandardOutput() ).trimmed();
-
-    if ( process.exitStatus() == QProcess::CrashExit )
-    {
-        cWarning() << "Process" << args.first() << "crashed. Output so far:\n" << Logger::NoQuote << output;
-        return ProcessResult::Code::Crashed;
-    }
-
-    auto r = process.exitCode();
-    bool showDebug = ( !Calamares::Settings::instance() ) || ( Calamares::Settings::instance()->debugMode() );
-    if ( r == 0 )
-    {
-        if ( showDebug && !output.isEmpty() )
-        {
-            cDebug() << Logger::SubEntry << "Finished. Exit code:" << r << "output:\n" << Logger::NoQuote << output;
-        }
-    }
-    else  // if ( r != 0 )
-    {
-        if ( !output.isEmpty() )
-        {
-            cDebug() << Logger::SubEntry << "Target cmd:" << RedactedList( args ) << "Exit code:" << r << "output:\n"
-                     << Logger::NoQuote << output;
-        }
-        else
-        {
-            cDebug() << Logger::SubEntry << "Target cmd:" << RedactedList( args ) << "Exit code:" << r << "(no output)";
-        }
-    }
-    return ProcessResult( r, output );
 }
 
 /// @brief Cheap check if a path is absolute.

src/libcalamares/utils/Logger.cpp

@@ -228,4 +228,28 @@ toString( const QVariant& v )
     }
 }
 
+QDebug&
+operator<<( QDebug& s, const Redacted& l )
+{
+    // Special case logging: don't log the (encrypted) password.
+    if ( l.list.contains( "usermod" ) )
+    {
+        for ( const auto& item : l.list )
+            if ( item.startsWith( "$6$" ) )
+            {
+                s << "<password>";
+            }
+            else
+            {
+                s << item;
+            }
+    }
+    else
+    {
+        s << l.list;
+    }
+
+    return s;
+}
+
 }  // namespace Logger

src/libcalamares/utils/Logger.h

@@ -207,6 +207,25 @@ public:
     const QVariantMap& map;
 };
 
+/** @brief When logging commands, don't log everything.
+ *
+ * The command-line arguments to some commands may contain the
+ * encrypted password set by the user. Don't log that password,
+ * since the log may get posted to bug reports, or stored in
+ * the target system.
+ */
+struct Redacted
+{
+    Redacted( const QStringList& l )
+        : list( l )
+    {
+    }
+
+    const QStringList& list;
+};
+
+QDebug& operator<<( QDebug& s, const Redacted& l );
+
 /**
  * @brief Formatted logging of a pointer
  *

src/libcalamares/utils/Permissions.cpp

@@ -7,9 +7,9 @@
 
 #include "Permissions.h"
 
+#include "CalamaresUtilsSystem.h"
 #include "Logger.h"
 
-#include <QProcess>
 #include <QString>
 #include <QStringList>
 
@@ -105,7 +105,9 @@ Permissions::apply( const QString& path, const CalamaresUtils::Permissions& p )
         // uid_t and gid_t values to pass to that system call.
         //
         // Do a lame cop-out and let the chown(8) utility do the heavy lifting.
-        if ( QProcess::execute( "chown", { p.username() + ':' + p.group(), path } ) )
+        if ( CalamaresUtils::System::runCommand( { "chown", p.username() + ':' + p.group(), path },
+                                                 std::chrono::seconds( 3 ) )
+                 .getExitCode() )
         {
             r = false;
             cDebug() << Logger::SubEntry << "Could not set owner of" << path << "to"

src/libcalamares/utils/Runner.cpp

@@ -0,0 +1,221 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2021 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ *
+ */
+
+#include "Runner.h"
+
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+#include "Settings.h"
+#include "utils/Logger.h"
+
+#include <QProcess>
+
+/** @brief Descend from directory, always relative
+ *
+ * If @p subdir begins with a "/" or "../" or "./" those are stripped
+ * until none are left, then changes @p directory into that
+ * subdirectory.
+ *
+ * Returns @c false if the @p subdir doesn't make sense.
+ */
+STATICTEST bool
+relativeChangeDirectory( QDir& directory, const QString& subdir )
+{
+    const QString rootPath = directory.absolutePath();
+    const QString concatenatedPath = rootPath + '/' + subdir;
+    const QString relPath = QDir::cleanPath( concatenatedPath );
+
+    if ( !relPath.startsWith( rootPath ) )
+    {
+        cWarning() << "Relative path" << subdir << "escapes from" << rootPath;
+        return false;
+    }
+
+    return directory.cd( relPath );
+}
+
+
+STATICTEST std::pair< bool, QDir >
+calculateWorkingDirectory( Calamares::Utils::RunLocation location, const QString& directory )
+{
+    Calamares::GlobalStorage* gs
+        = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
+
+    if ( location == Calamares::Utils::RunLocation::RunInTarget )
+    {
+        if ( !gs || !gs->contains( "rootMountPoint" ) )
+        {
+            cWarning() << "No rootMountPoint in global storage, while RunInTarget is specified";
+            return std::make_pair( false, QDir() );
+        }
+
+        QDir rootMountPoint( gs->value( "rootMountPoint" ).toString() );
+        if ( !rootMountPoint.exists() )
+        {
+            cWarning() << "rootMountPoint points to a dir which does not exist";
+            return std::make_pair( false, QDir() );
+        }
+
+        if ( !directory.isEmpty() )
+        {
+
+            if ( !relativeChangeDirectory( rootMountPoint, directory ) || !rootMountPoint.exists() )
+            {
+                cWarning() << "Working directory" << directory << "does not exist in target";
+                return std::make_pair( false, QDir() );
+            }
+        }
+        return std::make_pair( true, rootMountPoint );  // Now changed to subdir
+    }
+    else
+    {
+        QDir root;
+        if ( !directory.isEmpty() )
+        {
+            root = QDir::root();
+
+            if ( !relativeChangeDirectory( root, directory ) || !root.exists() )
+            {
+                cWarning() << "Working directory" << directory << "does not exist in host";
+                return std::make_pair( false, QDir() );
+            }
+        }
+        return std::make_pair( true, root );  // Now changed to subdir
+    }
+}
+
+namespace Calamares
+{
+namespace Utils
+{
+
+Runner::Runner() {}
+
+
+}  // namespace Utils
+}  // namespace Calamares
+
+
+Calamares::Utils::Runner::Runner( const QStringList& command )
+{
+    setCommand( command );
+}
+
+Calamares::Utils::Runner::~Runner() {}
+
+Calamares::Utils::ProcessResult
+Calamares::Utils::Runner::run()
+{
+    if ( m_command.isEmpty() )
+    {
+        cWarning() << "Cannot run an empty program list";
+        return ProcessResult::Code::FailedToStart;
+    }
+
+    auto [ ok, workingDirectory ] = calculateWorkingDirectory( m_location, m_directory );
+    if ( !ok || !workingDirectory.exists() )
+    {
+        // Warnings have already been printed
+        return ProcessResult::Code::NoWorkingDirectory;
+    }
+
+    QProcess process;
+    process.setProcessChannelMode( QProcess::MergedChannels );
+    if ( !m_directory.isEmpty() )
+    {
+        process.setWorkingDirectory( workingDirectory.absolutePath() );
+    }
+    if ( m_location == RunLocation::RunInTarget )
+    {
+        process.setProgram( "chroot" );
+        process.setArguments( QStringList { workingDirectory.absolutePath() } << m_command );
+    }
+    else
+    {
+        process.setProgram( "env" );
+        process.setArguments( m_command );
+    }
+
+    if ( m_output )
+    {
+        connect( &process, &QProcess::readyReadStandardOutput, [this, &process]() {
+            while ( process.canReadLine() )
+            {
+                QString s = process.readLine();
+                if ( !s.isEmpty() )
+                {
+                    Q_EMIT this->output( s );
+                }
+            }
+        } );
+    }
+
+    cDebug() << Logger::SubEntry << "Running" << Logger::Redacted( m_command );
+    process.start();
+    if ( !process.waitForStarted() )
+    {
+        cWarning() << "Process" << m_command.first() << "failed to start" << process.error();
+        return ProcessResult::Code::FailedToStart;
+    }
+
+    if ( !m_input.isEmpty() )
+    {
+        process.write( m_input.toLocal8Bit() );
+    }
+    process.closeWriteChannel();
+
+    if ( !process.waitForFinished( m_timeout > std::chrono::seconds::zero()
+                                       ? ( 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();
+        return ProcessResult::Code::TimedOut;
+    }
+
+    QString output = m_output ? process.readAllStandardOutput()
+                              : QString::fromLocal8Bit( process.readAllStandardOutput() ).trimmed();
+    if ( m_output && !output.isEmpty() )
+    {
+        Q_EMIT this->output( output );
+        output = QString();
+    }
+
+    if ( process.exitStatus() == QProcess::CrashExit )
+    {
+        cWarning() << "Process" << m_command.first() << "crashed. Output so far:\n" << Logger::NoQuote << output;
+        return ProcessResult::Code::Crashed;
+    }
+
+    auto r = process.exitCode();
+    const bool showDebug = ( !Calamares::Settings::instance() ) || ( Calamares::Settings::instance()->debugMode() );
+    if ( r == 0 )
+    {
+        if ( showDebug && !output.isEmpty() )
+        {
+            cDebug() << Logger::SubEntry << "Finished. Exit code:" << r << "output:\n" << Logger::NoQuote << output;
+        }
+    }
+    else  // if ( r != 0 )
+    {
+        if ( !output.isEmpty() )
+        {
+            cDebug() << Logger::SubEntry << "Target cmd:" << Logger::Redacted( m_command ) << "Exit code:" << r
+                     << "output:\n"
+                     << Logger::NoQuote << output;
+        }
+        else
+        {
+            cDebug() << Logger::SubEntry << "Target cmd:" << Logger::Redacted( m_command ) << "Exit code:" << r
+                     << "(no output)";
+        }
+    }
+    return ProcessResult( r, output );
+}

src/libcalamares/utils/Runner.h

@@ -0,0 +1,133 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2021 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ *
+ */
+
+#ifndef UTILS_RUNNER_H
+#define UTILS_RUNNER_H
+
+#include "CalamaresUtilsSystem.h"
+
+#include <QDir>
+#include <QObject>
+#include <QStringList>
+
+#include <chrono>
+#include <memory>
+#include <optional>
+
+namespace Calamares
+{
+namespace Utils
+{
+
+using RunLocation = CalamaresUtils::System::RunLocation;
+using ProcessResult = CalamaresUtils::ProcessResult;
+
+/** @brief A Runner wraps a process and handles running it and processing output
+ *
+ * This is basically a QProcess, but handles both running in the
+ * host system (through env(1)) or in the target (by calling chroot(8)).
+ * It has an output signal that handles output one line at a time
+ * (unlike QProcess that lets you do the buffering yourself).
+ * This output processing is only enabled if you do so explicitly.
+ *
+ * Use the set*() methods to configure the runner.
+ *
+ * If you call enableOutputProcessing(), then you can connect to
+ * the output() signal to receive each line (including trailing newline!).
+ */
+class Runner : public QObject
+{
+    Q_OBJECT
+
+public:
+    /** @brief Create an empty runner
+     *
+     * This is a runner with no commands, nothing; call set*() methods
+     * to configure it.
+     */
+    Runner();
+    /** @brief Create a runner with a specified command
+     *
+     * Equivalent to Calamares::Utils::Runner::Runner() followed by
+     * calling setCommand().
+     */
+    Runner( const QStringList& command );
+    virtual ~Runner() override;
+
+    Runner& setCommand( const QStringList& command )
+    {
+        m_command = command;
+        return *this;
+    }
+    Runner& setLocation( RunLocation r )
+    {
+        m_location = r;
+        return *this;
+    }
+    Runner& setWorkingDirectory( const QDir& directory )
+    {
+        m_directory = directory.absolutePath();
+        return *this;
+    }
+    Runner& setWorkingDirectory( const QString& directory )
+    {
+        m_directory = directory;
+        return *this;
+    }
+    Runner& setTimeout( std::chrono::seconds timeout )
+    {
+        m_timeout = timeout;
+        return *this;
+    }
+    Runner& setInput( const QString& stdin )
+    {
+        m_input = stdin;
+        return *this;
+    }
+    Runner& setOutputProcessing( bool enable )
+    {
+        m_output = enable;
+        return *this;
+    }
+
+    Runner& enableOutputProcessing()
+    {
+        m_output = true;
+        return *this;
+    }
+
+    ProcessResult run();
+    /** @brief The executable (argv[0]) that this runner will run
+     *
+     * This is the first element of the command; it does not include
+     * env(1) or chroot(8) which are injected when actually running
+     * the command.
+     */
+    QString executable() const { return m_command.isEmpty() ? QString() : m_command.first(); }
+
+signals:
+    void output( QString line );
+
+private:
+    // What to run, and where.
+    QStringList m_command;
+    QString m_directory;
+    RunLocation m_location { RunLocation::RunInHost };
+
+    // Settings for when it actually runs
+    QString m_input;
+    std::chrono::milliseconds m_timeout { 0 };
+    bool m_output = false;
+};
+
+}  // namespace Utils
+}  // namespace Calamares
+
+#endif

src/libcalamares/utils/Tests.cpp

@@ -13,6 +13,7 @@
 #include "Entropy.h"
 #include "Logger.h"
 #include "RAII.h"
+#include "Runner.h"
 #include "String.h"
 #include "Traits.h"
 #include "UMask.h"
@@ -70,6 +71,11 @@ private Q_SLOTS:
     void testStringTruncationShorter();
     void testStringTruncationDegenerate();
 
+    /** @section Test Runner directory-manipulation. */
+    void testRunnerDirs();
+    void testCalculateWorkingDirectory();
+    void testRunnerOutput();
+
 private:
     void recursiveCompareMap( const QVariantMap& a, const QVariantMap& b, int depth );
 };
@@ -746,6 +752,204 @@ LibCalamaresTests::testStringTruncationDegenerate()
 }
 
 
+static QString
+dirname( const QTemporaryDir& d )
+{
+    return d.path().split( '/' ).last();
+}
+static QString
+dirname( const QDir& d )
+{
+    return d.absolutePath().split( '/' ).last();
+}
+
+// Method under test
+extern bool relativeChangeDirectory( QDir& directory, const QString& subdir );
+
+void
+LibCalamaresTests::testRunnerDirs()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+
+    QDir startDir( QDir::current() );
+    QTemporaryDir tempDir( "./utilstest" );
+    QVERIFY( tempDir.isValid() );
+    QVERIFY( startDir.isReadable() );
+
+    // Test changing "downward"
+    {
+        QDir testDir( QDir::current() );
+        QCOMPARE( startDir, testDir );
+    }
+
+    {
+        QDir testDir( QDir::current() );
+        const bool could_change_to_dot = relativeChangeDirectory( testDir, QStringLiteral( "." ) );
+        QVERIFY( could_change_to_dot );
+        QCOMPARE( startDir, testDir );
+    }
+
+    {
+        // The tempDir was created inside the current directory, we want only the subdir-name
+        QDir testDir( QDir::current() );
+        const bool could_change_to_temp = relativeChangeDirectory( testDir, dirname( tempDir ) );
+        QVERIFY( could_change_to_temp );
+        QVERIFY( startDir != testDir );
+        QVERIFY( testDir.absolutePath().startsWith( startDir.absolutePath() ) );
+    }
+
+    // Test changing to something that doesn't exist
+    {
+        QDir testDir( QDir::current() );
+        const bool could_change_to_bogus = relativeChangeDirectory( testDir, QStringLiteral( "bogus" ) );
+        QVERIFY( !could_change_to_bogus );
+        QCOMPARE( startDir, testDir );  // Must be unchanged
+    }
+
+    // Testing escape-from-start
+    {
+        // Escape briefly from the start
+        QDir testDir( QDir::current() );
+        const bool could_change_to_current
+            = relativeChangeDirectory( testDir, QStringLiteral( "../" ) + dirname( startDir ) );
+        QVERIFY( could_change_to_current );
+        QCOMPARE( startDir, testDir );  // The change succeeded, but net effect is zero
+
+        const bool could_change_to_temp = relativeChangeDirectory(
+            testDir, QStringLiteral( "../" ) + dirname( startDir ) + QStringLiteral( "/./" ) + dirname( tempDir ) );
+        QVERIFY( could_change_to_temp );
+        QVERIFY( startDir != testDir );
+        QVERIFY( testDir.absolutePath().startsWith( startDir.absolutePath() ) );
+    }
+
+    {
+        // Escape?
+        QDir testDir( QDir::current() );
+        const bool could_change_to_parent = relativeChangeDirectory( testDir, QStringLiteral( "../" ) );
+        QVERIFY( !could_change_to_parent );
+        QCOMPARE( startDir, testDir );  // Change failed
+
+        const bool could_change_to_tmp = relativeChangeDirectory( testDir, QStringLiteral( "/tmp" ) );
+        QVERIFY( !could_change_to_tmp );
+        QCOMPARE( startDir, testDir );
+
+        const bool could_change_to_elsewhere = relativeChangeDirectory( testDir, QStringLiteral( "../src" ) );
+        QVERIFY( !could_change_to_elsewhere );
+        QCOMPARE( startDir, testDir );
+    }
+}
+
+// Method under test
+extern std::pair< bool, QDir > calculateWorkingDirectory( Calamares::Utils::RunLocation location,
+                                                          const QString& directory );
+
+void
+LibCalamaresTests::testCalculateWorkingDirectory()
+{
+    Calamares::GlobalStorage* gs
+        = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
+
+    if ( !gs )
+    {
+        cDebug() << "Creating new JobQueue";
+        (void)new Calamares::JobQueue();
+        gs = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
+    }
+    QVERIFY( gs );
+
+    // Working with a rootMountPoint set
+    QTemporaryDir tempRoot( QDir::tempPath() + QStringLiteral( "/test-job-XXXXXX" ) );
+    gs->insert( "rootMountPoint", tempRoot.path() );
+
+    {
+        auto [ ok, d ] = calculateWorkingDirectory( CalamaresUtils::System::RunLocation::RunInHost, QString() );
+        QVERIFY( ok );
+        QCOMPARE( d, QDir::current() );
+    }
+    {
+        auto [ ok, d ] = calculateWorkingDirectory( CalamaresUtils::System::RunLocation::RunInTarget, QString() );
+        QVERIFY( ok );
+        QCOMPARE( d.absolutePath(), tempRoot.path() );
+    }
+
+    gs->remove( "rootMountPoint" );
+    {
+        auto [ ok, d ] = calculateWorkingDirectory( CalamaresUtils::System::RunLocation::RunInHost, QString() );
+        QVERIFY( ok );
+        QCOMPARE( d, QDir::current() );
+    }
+    {
+        auto [ ok, d ] = calculateWorkingDirectory( CalamaresUtils::System::RunLocation::RunInTarget, QString() );
+        QVERIFY( !ok );
+        QCOMPARE( d, QDir::current() );
+    }
+}
+
+void
+LibCalamaresTests::testRunnerOutput()
+{
+    cDebug() << "Testing ls";
+    {
+        Calamares::Utils::Runner r( { "ls", "-d", "." } );
+        QSignalSpy spy( &r, &decltype( r )::output );
+        r.enableOutputProcessing();
+
+        auto result = r.run();
+        QCOMPARE( result.getExitCode(), 0 );
+        QCOMPARE( result.getOutput(), QString() );
+        QCOMPARE( spy.count(), 1 );
+    }
+
+    cDebug() << "Testing cat";
+    {
+        Calamares::Utils::Runner r( { "cat" } );
+        QSignalSpy spy( &r, &decltype( r )::output );
+        r.enableOutputProcessing().setInput( QStringLiteral( "hello\nworld\n\n!\n" ) );
+
+        {
+            auto result = r.run();
+            QCOMPARE( result.getExitCode(), 0 );
+            QCOMPARE( result.getOutput(), QString() );
+            QCOMPARE( spy.count(), 4 );
+        }
+
+        r.setInput( QStringLiteral( "yo\ndogg" ) );
+        {
+            auto result = r.run();
+            QCOMPARE( result.getExitCode(), 0 );
+            QCOMPARE( result.getOutput(), QString() );
+            QCOMPARE( spy.count(), 6 );  // 4 from before, +2 here
+        }
+    }
+
+    cDebug() << "Testing cat (again)";
+    {
+        QStringList collectedOutput;
+
+        Calamares::Utils::Runner r( { "cat" } );
+        r.enableOutputProcessing().setInput( QStringLiteral( "hello\nworld\n\n!\n" ) );
+        QObject::connect( &r, &decltype( r )::output, [&collectedOutput]( QString s ) { collectedOutput << s; } );
+
+        {
+            auto result = r.run();
+            QCOMPARE( result.getExitCode(), 0 );
+            QCOMPARE( result.getOutput(), QString() );
+            QCOMPARE( collectedOutput.count(), 4 );
+            QVERIFY( collectedOutput.contains( QStringLiteral( "world\n" ) ) );
+        }
+
+        r.setInput( QStringLiteral( "yo\ndogg" ) );
+        {
+            auto result = r.run();
+            QCOMPARE( result.getExitCode(), 0 );
+            QCOMPARE( result.getOutput(), QString() );
+            QCOMPARE( collectedOutput.count(), 6 );
+            QVERIFY( collectedOutput.contains( QStringLiteral( "dogg" ) ) );  // no newline
+        }
+    }
+}
+
+
 QTEST_GUILESS_MAIN( LibCalamaresTests )
 
 #include "utils/moc-warnings.h"

src/modules/README.md

@@ -14,9 +14,9 @@ Each Calamares module lives in its own directory.
 All modules are installed in `$DESTDIR/lib/calamares/modules`.
 
 There are two **types** of Calamares module:
-* viewmodule, for user-visible modules. These use C++ and QWidgets or QML
+* viewmodule, for user-visible modules. These use C++ and either Widgets or QML
 * jobmodule, for not-user-visible modules. These may be done in C++,
-  Python, or as external processes.
+  Python, or as external processes (external processes not recommended).
 
 A viewmodule exposes a UI to the user.
 
@@ -39,7 +39,7 @@ recommended way to create such modules -- the module descriptor
 file is optional, since it can be generated by the build system.
 For other module interfaces, the module descriptor file is required.
 
-The module descriptor file must be placed in the module's directory.
+The module descriptor file, if required, is placed in the module's directory.
 The module descriptor file is a YAML 1.2 document which defines the
 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
@@ -63,6 +63,7 @@ Module descriptors for process modules **must** have the following key:
 Module descriptors for process modules **may** have the following keys:
 - *timeout* (how long, in seconds, to wait for the command to run)
 - *chroot* (if true, run the command in the target system rather than the host)
+Note that process modules are not recommended.
 
 Module descriptors **may** have the following keys:
 - *emergency* (a boolean value, set to true to mark the module
@@ -134,7 +135,7 @@ in `/etc/calamares/modules`.
 During the *exec* phase of an installation, where jobs are run and
 things happen to the target system, there is a running progress bar.
 It goes from 0% to 100% while all of the jobs for that exec phase
-are run. Generally, one module creates on job, but this varies a little
+are run. Generally, one module creates one job, but this varies a little
 (e.g. the partition module can spawn a whole bunch of jobs to
 deal with each disk, and the users module has separate jobs for
 the regular user and the root user).
@@ -162,7 +163,10 @@ visible.
 
 It is also possible to set a weight on a specific module **instance**,
 which can be done in `settings.conf`. This overrides any weight
-set in the module descriptor.
+set in the module descriptor. Doing so is the recommended approach,
+since that is where the specific installation-process is configured;
+it is possible to take the whole installation-process into account
+for determining the relative weights there.
 
 
 ## C++ modules
@@ -252,7 +256,7 @@ it has a `module.desc`. It will be picked up automatically by our CMake magic.
 For all kinds of Python jobs, the key *script* must be set to the name of
 the main python file for the job. This is almost universally `main.py`.
 
-`CMakeLists.txt` is *not* used for Python and process jobmodules.
+`CMakeLists.txt` is *not* used for Python jobmodules.
 
 Calamares offers a Python API for module developers, the core Calamares
 functionality is exposed as `libcalamares.job` for job data,
@@ -280,8 +284,163 @@ description if something went wrong.
 
 ### Python API
 
-**TODO:** this needs documentation
+The interface from a Python module to Calamares internals is
+found in the *libcalamares* module. This is not a standard Python
+module, and is only available inside the Calamares "runtime" for
+Python modules (it is implemented through Boost::Python in C++).
 
+A module should start by importing the Calamares internals:
+
+```
+import libcalamares
+```
+
+There are three important (sub)modules in *libcalamares*:
+- *globalstorage* behaves like a dictionary, and interfaces
+  with the global storage in Calamares; use it to transfer
+  information between modules (e.g. the *partition* module
+  shares the partition layout it creates). Note that some information
+  in global storage is expected to be structured, and it may be
+  dicts-within-dicts.
+
+  An example of using globalstorage:
+  ```
+  if not libcalamares.globalstorage.contains("lala"):
+      libcalamares.globalstorage.insert("lala", 72)
+  ```
+- *job* is the interface to the job's behavior, with one important
+  data member: *configuration* which is a dictionary derived from the
+  configuration file for the module (if there is one, empty otherwise).
+  Less important data is *pretty_name* (a string) and *working_path*
+  which are normally not needed. The *pretty_name* value is
+  obtained by the Calamares internals by calling the `pretty_name()`
+  function inside the Python module.
+
+  There is one function: `setprogress(p)` which can be passed a float
+  *p* between 0 and 1 to indicate 0% to 100% completion of the module's
+  work.
+- *utils* is where non-job-specific functions are placed:
+  - `debug(s)` and `warning(s)` are logger functions, which send output
+    to the usual Calamares logging functions. Use these over `print()`
+    which may not be visible at all.
+  - `mount(device, path, type, options)` mounts a filesystem from
+    *device* onto *path*, as if running the mount command from the shell.
+    Use this in preference to running mount by hand. In Calamares 3.3
+    this function also handles privilege escalation.
+  - `gettext_path()` and `gettext_languages()` are support functions
+    for translations, which would normally be called only once when
+    setting up gettext (see below).
+  - `obscure(s)` is a lousy string obfuscation mechanism. Do not use it.
+  - A half-dozen functions for running a command and dealing with its
+    output. These are recommended over using `os.system()` or the *subprocess*
+    module because they handle the chroot behavior for running in the
+    target system transparently. In Calamares 3.3 these functions also
+    handle privilege escalation. See below, *Running Commands in Python* for details.
+
+A module **must** contain a `run()` function to do the actual work
+of the module. The module **may** define the following functions
+to provide information to Calamares:
+- `pretty_name()` returns a string that is a human-readable name or
+  short description of the module. Since it is human-readable,
+  return a translated string.
+- `pretty_status_message()` returns a (longer) string that is a human-readable
+  description of the state of the module, or what it is doing. This is
+  primarily of importance for long-running modules. The function is called
+  by the Calamares framework when the module reports progress through the
+  `job.setprogress()` function. Since the status is human-readable,
+  return a translated string.
+
+### Python Translations
+
+Translations in Python modules -- at least the ones in the Calamares core
+repository -- are handled through gettext. You should import the standard
+Python *gettext* module. Conventionally, `_` is used to mark translations.
+That function needs to be configured specifically for use in Calamares
+so that it can find the translations. A boilerplate solution is this:
+
+```
+import gettext
+_ = gettext.translation("calamares-python",
+                        localedir=libcalamares.utils.gettext_path(),
+                        languages=libcalamares.utils.gettext_languages(),
+                        fallback=True).gettext
+```
+
+Error messages should be logged in English, and given to the user
+in translated form. In particular, when returning an error message
+and description from the `run()` function, return translated forms,
+like the following:
+
+```
+return (
+    _("No configuration found"),
+    _("<a longer description of the problem>"))
+```
+
+### Running Commands in Python
+
+The use of the `os.system()` function and *subprocess* modules is
+discouraged. Using these makes the caller responsible for handling
+any chroot or other target-versus-host-system manipulation, and in
+Calamares 3.3 may require additional privilege escalation handling.
+
+The primary functions for running a command from Python are:
+- `target_env_process_output(command, callback, stdin, timeout)`
+- `host_env_process_output(command, callback, stdin, timeout)`
+They run the given *command* (which must be a list of strings, like
+`sys.argv` or what would be passed to a *subprocess* module call)
+either in the target system (within the chroot) or in the host system.
+Except for *command*, the arguments are optional.
+
+A very simple example is running `ls` from a Python module (with `libcalamares.utils.` qualification omitted):
+```
+target_env_process_output(["ls"])
+```
+
+The functions return 0. If the exit code of *command* is not 0, an exception
+is raised instead of returning 0.
+
+Parameter *stdin* may be a string which is fed to the command as standard input.
+The *timeout* is in seconds, with 0 (or a negative number) treated as no-timeout.
+
+Parameter *callback* is special:
+- If it is `None`, no special handling of the command's output is done.
+  The output will be logged, though (if there is any).
+- If it is a list, then the output of the command will be appended to the list,
+  one line at a time. Lines will still contain the trailing newline character
+  (if there is one; output may end without a newline).
+  Use this approach to process the command output after it has completed.
+- Anything else is assumed to be a callable function that takes one parameter.
+  The function is called once for each line of output produced by the command.
+  The line of output still contains the trailing newline character (if there is one).
+  Use this approach to process the command output while it is running.
+
+Here are three examples of running `ls` with different callbacks:
+```
+# No processing at all, output is logged
+target_env_process_output(["ls"])
+target_env_process_output(["ls"], None)
+
+# Appends to the list
+ls_output = []
+target_env_process_output(["ls"], ls_output)
+
+# Calls the function for each line, which then calls debug()
+def handle_output(s):
+    debug(f"ls said {s}")
+target_env_process_output(["ls"], handle_output)
+```
+
+
+There are additional functions for running commands in the target,
+which can select what they return and whether exceptions are raised
+or only an exit code is returned. These functions have an overload
+that takes a single string (the name of an executable) as well. They should
+all be considered deprecated by the callback-enabled functions, above.
+
+- `target_env_call(command, stdin, timeout)` returns the exit code, does not raise.
+- `check_target_env_call(command, stdin, timeout)` raises on a non-zero exit code.
+- `check_target_env_output(command, stdin, timeout)` returns a single string with the output of *command*, raises on a non-zero exit code.
 
 
 ## PythonQt modules (deprecated)
@@ -309,6 +468,8 @@ passed to the shell -- remember to quote it properly. It is generally
 recommended to use a *shellprocess* job module instead (less configuration,
 easier to have multiple instances).
 
+`CMakeLists.txt` is *not* used for process jobmodules.
+
 
 ## Testing Modules
 

src/modules/dummycpp/DummyCppJob.cpp

@@ -12,7 +12,6 @@
 #include "DummyCppJob.h"
 
 #include <QDateTime>
-#include <QProcess>
 #include <QThread>
 
 #include "CalamaresVersion.h"

src/modules/fsresizer/ResizeFSJob.cpp

@@ -18,7 +18,6 @@
 #include "utils/Variant.h"
 
 #include <QDateTime>
-#include <QProcess>
 #include <QThread>
 
 #include <kpmcore/backend/corebackend.h>

src/modules/keyboard/KeyboardPage.cpp

@@ -28,7 +28,6 @@
 #include "utils/String.h"
 
 #include <QComboBox>
-#include <QProcess>
 #include <QPushButton>
 
 class LayoutItem : public QListWidgetItem

src/modules/keyboard/keyboardwidget/keyboardpreview.cpp

@@ -18,6 +18,8 @@
 #include "utils/Logger.h"
 #include "utils/String.h"
 
+#include <QProcess>
+
 KeyBoardPreview::KeyBoardPreview( QWidget* parent )
     : QWidget( parent )
     , layout( "us" )

src/modules/keyboard/keyboardwidget/keyboardpreview.h

@@ -21,7 +21,6 @@
 #include <QPainterPath>
 #include <QPen>
 #include <QPixmap>
-#include <QProcess>
 #include <QRectF>
 #include <QString>
 #include <QStringList>

src/modules/partition/core/PartitionCoreModule.cpp

@@ -60,7 +60,6 @@
 // Qt
 #include <QDir>
 #include <QFutureWatcher>
-#include <QProcess>
 #include <QStandardItemModel>
 #include <QtConcurrent/QtConcurrent>
 

src/modules/shellprocess/ShellProcessJob.cpp

@@ -18,7 +18,6 @@
 #include "utils/Variant.h"
 
 #include <QDateTime>
-#include <QProcess>
 #include <QThread>
 
 ShellProcessJob::ShellProcessJob( QObject* parent )

src/modules/users/CreateUserJob.cpp

@@ -19,7 +19,6 @@
 #include <QDir>
 #include <QFile>
 #include <QFileInfo>
-#include <QProcess>
 #include <QTextStream>