[libcalamares] Move CommandList into libcalamares

- Move CommandList so it can be used from more modules than
   just ShellProcess
 - Allow a CommandList to run itself. This centralizes
   code for executing one or more commands and simplifies
   the ShellProcess module.

Various small cleanups:
 - mention instance id in log message
 - code formatting / style

src/libcalamares/CMakeLists.txt

@@ -22,6 +22,7 @@ set( libSources
 set( utilsSources
     utils/CalamaresUtils.cpp
     utils/CalamaresUtilsSystem.cpp
+    utils/CommandList.cpp
     utils/Logger.cpp
     utils/PluginFactory.cpp
     utils/Retranslator.cpp

src/libcalamares/utils/CommandList.cpp

@@ -0,0 +1,120 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2018, Adriaan de Groot <groot@kde.org>
+ *
+ *   Calamares is free software: you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, either version 3 of the License, or
+ *   (at your option) any later version.
+ *
+ *   Calamares is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *   GNU General Public License for more details.
+ *
+ *   You should have received a copy of the GNU General Public License
+ *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "CommandList.h"
+
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+
+#include "utils/CalamaresUtilsSystem.h"
+#include "utils/Logger.h"
+
+#include <QVariantList>
+
+static QStringList get_variant_stringlist( const QVariantList& l )
+{
+    QStringList retl;
+    unsigned int c = 0;
+    for ( const auto& v : l )
+    {
+        if ( v.type() == QVariant::String )
+            retl.append( v.toString() );
+        else
+            cDebug() << "WARNING Bad CommandList element" << c << v.type() << v;
+        ++c;
+    }
+    return retl;
+}
+
+namespace CalamaresUtils
+{
+
+CommandList::CommandList( bool doChroot )
+    : m_doChroot( doChroot )
+{
+}
+
+CommandList::CommandList::CommandList( const QVariant& v, bool doChroot )
+    : CommandList( doChroot )
+{
+    if ( v.type() == QVariant::List )
+    {
+        const auto v_list = v.toList();
+        if ( v_list.count() )
+            append( get_variant_stringlist( v_list ) );
+        else
+            cDebug() << "WARNING: Empty CommandList";
+    }
+    else if ( v.type() == QVariant::String )
+        append( v.toString() );
+    else
+        cDebug() << "WARNING: CommandList does not understand variant" << v.type();
+}
+
+CommandList::~CommandList()
+{
+}
+
+Calamares::JobResult CommandList::run( const QObject* parent )
+{
+    System::RunLocation location = m_doChroot ? System::RunLocation::RunInTarget : System::RunLocation::RunInHost;
+
+    /* Figure out the replacement for @@ROOT@@ */
+    QString root = QStringLiteral( "/" );
+    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
+    if ( location == System::RunLocation::RunInTarget )
+    {
+        if ( !gs || !gs->contains( "rootMountPoint" ) )
+        {
+            cDebug() << "ERROR: No rootMountPoint defined.";
+            return Calamares::JobResult::error( parent->tr( "Could not run command." ),
+                                                parent->tr( "No rootMountPoint is defined, so command cannot be run in the target environment." ) );
+        }
+        root = gs->value( "rootMountPoint" ).toString();
+    }
+
+    for ( CommandList::const_iterator i = cbegin(); i != cend(); ++i )
+    {
+        QString processed_cmd = *i;
+        processed_cmd.replace( "@@ROOT@@", root );  // FIXME?
+        bool suppress_result = false;
+        if ( processed_cmd.startsWith( '-' ) )
+        {
+            suppress_result = true;
+            processed_cmd.remove( 0, 1 );  // Drop the -  // FIXME?
+        }
+
+        QStringList shell_cmd { "/bin/sh", "-c" };
+        shell_cmd << processed_cmd;
+
+        ProcessResult r = System::runCommand(
+                              location, shell_cmd, QString(), QString(), 10 );
+
+        if ( r.getExitCode() != 0 )
+        {
+            if ( suppress_result )
+                cDebug() << "Error code" << r.getExitCode() << "ignored by ShellProcess configuration.";
+            else
+                return r.explainProcess( parent, processed_cmd, 10 );
+        }
+    }
+
+    return Calamares::JobResult::ok();
+}
+
+}  // namespace

src/libcalamares/utils/CommandList.h

@@ -19,9 +19,14 @@
 #ifndef COMMANDLIST_H
 #define COMMANDLIST_H
 
+#include "Job.h"
+
 #include <QStringList>
 #include <QVariant>
 
+namespace CalamaresUtils
+{
+
 class CommandList : protected QStringList
 {
 public:
@@ -29,7 +34,12 @@ public:
     CommandList( const QVariant& v, bool doChroot = true );
     ~CommandList();
 
-    bool doChroot() const { return m_doChroot; }
+    bool doChroot() const
+    {
+        return m_doChroot;
+    }
+
+    Calamares::JobResult run( const QObject* parent );
 
     using QStringList::isEmpty;
     using QStringList::count;
@@ -41,4 +51,5 @@ private:
     bool m_doChroot;
 } ;
 
+}  // namespace
 #endif // COMMANDLIST_H

src/modules/shellprocess/CMakeLists.txt

@@ -2,7 +2,6 @@ calamares_add_plugin( shellprocess
     TYPE job
     EXPORT_MACRO PLUGINDLLEXPORT_PRO
     SOURCES
-        CommandList.cpp
         ShellProcessJob.cpp
     LINK_PRIVATE_LIBRARIES
         calamares
@@ -16,7 +15,6 @@ if( ECM_FOUND )
 
     ecm_add_test(
             Tests.cpp
-            CommandList.cpp
         TEST_NAME
             shellprocesstest
         LINK_LIBRARIES

src/modules/shellprocess/CommandList.cpp

@@ -1,68 +0,0 @@
-/* === This file is part of Calamares - <https://github.com/calamares> ===
- *
- *   Copyright 2018, Adriaan de Groot <groot@kde.org>
- *
- *   Calamares is free software: you can redistribute it and/or modify
- *   it under the terms of the GNU General Public License as published by
- *   the Free Software Foundation, either version 3 of the License, or
- *   (at your option) any later version.
- *
- *   Calamares is distributed in the hope that it will be useful,
- *   but WITHOUT ANY WARRANTY; without even the implied warranty of
- *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- *   GNU General Public License for more details.
- *
- *   You should have received a copy of the GNU General Public License
- *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "CommandList.h"
-
-#include "utils/Logger.h"
-
-#include <QVariantList>
-
-static QStringList get_variant_stringlist(const QVariantList& l)
-{
-    QStringList retl;
-    unsigned int c = 0;
-    for ( const auto& v : l )
-    {
-        if ( v.type() == QVariant::String )
-            retl.append( v.toString() );
-        else
-            cDebug() << "WARNING Bad CommandList element" << c << v.type() << v;
-        ++c;
-    }
-    return retl;
-}
-
-CommandList::CommandList( bool doChroot )
-    : m_doChroot( doChroot )
-{
-}
-
-CommandList::CommandList::CommandList( const QVariant& v, bool doChroot )
-    : CommandList( doChroot )
-{
-    if ( v.type() == QVariant::List )
-    {
-        const auto v_list = v.toList();
-        if ( v_list.count() )
-        {
-            append( get_variant_stringlist( v_list ) );
-        }
-        else
-            cDebug() << "WARNING: Empty CommandList";
-    }
-    else if ( v.type() == QVariant::String )
-    {
-        append( v.toString() );
-    }
-    else
-        cDebug() << "WARNING: CommandList does not understand variant" << v.type();
-}
-
-CommandList::~CommandList()
-{
-}

src/modules/shellprocess/ShellProcessJob.cpp

@@ -18,8 +18,6 @@
 
 #include "ShellProcessJob.h"
 
-#include "CommandList.h"
-
 #include <QProcess>
 #include <QDateTime>
 #include <QThread>
@@ -30,10 +28,9 @@
 
 #include "utils/CalamaresUtils.h"
 #include "utils/CalamaresUtilsSystem.h"
+#include "utils/CommandList.h"
 #include "utils/Logger.h"
 
-class CommandList;
-
 ShellProcessJob::ShellProcessJob( QObject* parent )
     : Calamares::CppJob( parent )
     , m_commands( nullptr )
@@ -59,56 +56,14 @@ ShellProcessJob::prettyName() const
 Calamares::JobResult
 ShellProcessJob::exec()
 {
-    using CalamaresUtils::System;
-    System::RunLocation location = m_dontChroot ? System::RunLocation::RunInHost : System::RunLocation::RunInTarget;
 
     if ( ! m_commands || m_commands->isEmpty() )
     {
-        cDebug() << "WARNING: No commands to execute";
+        cDebug() << "WARNING: No commands to execute" << moduleInstanceKey();
         return Calamares::JobResult::ok();
     }
 
-    /* Figure out the replacement for @@ROOT@@ */
+    return m_commands->run( this );
-    QString root = QStringLiteral( "/" );
-    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
-    if ( location == System::RunLocation::RunInTarget )
-    {
-         if ( !gs || !gs->contains( "rootMountPoint" ) )
-         {
-             cDebug() << "ERROR: No rootMountPoint defined.";
-             return Calamares::JobResult::error( tr( "Could not run command." ),
-                                                 tr( "No rootMountPoint is defined, so command cannot be run in the target environment." ) );
-         }
-         root = gs->value( "rootMountPoint" ).toString();
-    }
-
-    for ( CommandList::const_iterator i = m_commands->cbegin(); i != m_commands->cend(); ++i )
-    {
-        QString processed_cmd = *i;
-        processed_cmd.replace( "@@ROOT@@", root );  // FIXME?
-        bool suppress_result = false;
-        if ( processed_cmd.startsWith( '-' ) )
-        {
-            suppress_result = true;
-            processed_cmd.remove( 0, 1 );  // Drop the -  // FIXME?
-        }
-
-        QStringList shell_cmd { "/bin/sh", "-c" };
-        shell_cmd << processed_cmd;
-
-        CalamaresUtils::ProcessResult r = System::runCommand(
-            location, shell_cmd, QString(), QString(), 10 );
-
-        if ( r.getExitCode() != 0 )
-        {
-            if ( suppress_result )
-                cDebug() << "Error code" << r.getExitCode() << "ignored by ShellProcess configuration.";
-            else
-                return r.explainProcess( this, processed_cmd, 10 );
-        }
-    }
-
-    return Calamares::JobResult::ok();
 }
 
 
@@ -119,7 +74,7 @@ ShellProcessJob::setConfigurationMap( const QVariantMap& configurationMap )
 
     if ( configurationMap.contains( "script" ) )
     {
-        m_commands = new CommandList( configurationMap.value( "script" ) );
+        m_commands = new CalamaresUtils::CommandList( configurationMap.value( "script" ), !m_dontChroot );
         if ( m_commands->isEmpty() )
             cDebug() << "ShellProcessJob: \"script\" contains no commands for" << moduleInstanceKey();
     }

src/modules/shellprocess/ShellProcessJob.h

@@ -24,11 +24,11 @@
 
 #include <CppJob.h>
 
+#include <utils/CommandList.h>
 #include <utils/PluginFactory.h>
 
 #include <PluginDllMacro.h>
 
-class CommandList;
 
 class PLUGINDLLEXPORT ShellProcessJob : public Calamares::CppJob
 {
@@ -45,7 +45,7 @@ public:
     void setConfigurationMap( const QVariantMap& configurationMap ) override;
 
 private:
-    CommandList *m_commands;
+    CalamaresUtils::CommandList* m_commands;
     bool m_dontChroot;
 };
 

src/modules/shellprocess/Tests.cpp

@@ -17,8 +17,8 @@
  */
 
 #include "Tests.h"
-#include "CommandList.h"
 
+#include "utils/CommandList.h"
 #include "utils/YamlUtils.h"
 
 #include <yaml-cpp/yaml.h>
@@ -30,6 +30,8 @@
 
 QTEST_GUILESS_MAIN( ShellProcessTests )
 
+using CommandList = CalamaresUtils::CommandList;
+
 ShellProcessTests::ShellProcessTests()
 {
 }