[libcalamares] Add an expand() to command lines and lists

- While this is primarily convenient for testing (e.g. checking
  that a command is expanded the way we expect), it simplifies
  some of the code because it's now clear that run() uses an
  expanded copy of the command-list to do the actual work.

src/libcalamares/utils/CommandList.cpp

@@ -68,6 +68,48 @@ get_variant_stringlist( const QVariantList& l )
     return retl;
 }
 
+static Calamares::String::DictionaryExpander
+get_gs_expander( System::RunLocation location )
+{
+    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
+
+    Calamares::String::DictionaryExpander expander;
+
+    // Figure out the replacement for ${ROOT}
+    if ( location == System::RunLocation::RunInTarget )
+    {
+        expander.insert( QStringLiteral( "ROOT" ), QStringLiteral( "/" ) );
+    }
+    else if ( gs && gs->contains( "rootMountPoint" ) )
+    {
+        expander.insert( QStringLiteral( "ROOT" ), gs->value( "rootMountPoint" ).toString() );
+    }
+
+    // Replacement for ${USER}
+    if ( gs && gs->contains( "username" ) )
+    {
+        expander.insert( QStringLiteral( "USER" ), gs->value( "username" ).toString() );
+    }
+
+    return expander;
+}
+
+CommandLine
+CommandLine::expand( KMacroExpanderBase& expander ) const
+{
+    QString c = first;
+    expander.expandMacrosShellQuote( c );
+    return { c, second };
+}
+
+CalamaresUtils::CommandLine
+CommandLine::expand() const
+{
+    auto expander = get_gs_expander( System::RunLocation::RunInHost );
+    return expand( expander );
+}
+
+
 CommandList::CommandList( bool doChroot, std::chrono::seconds timeout )
     : m_doChroot( doChroot )
     , m_timeout( timeout )
@@ -91,7 +133,7 @@ CommandList::CommandList::CommandList( const QVariant& v, bool doChroot, std::ch
     }
     else if ( v.type() == QVariant::String )
     {
-        append( v.toString() );
+        append( { v.toString(), m_timeout } );
     }
     else if ( v.type() == QVariant::Map )
     {
@@ -114,37 +156,9 @@ Calamares::JobResult
 CommandList::run()
 {
     System::RunLocation location = m_doChroot ? System::RunLocation::RunInTarget : System::RunLocation::RunInHost;
-    Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage();
-
-    Calamares::String::DictionaryExpander expander;
-
-    // Figure out the replacement for ${ROOT}
-    if ( location == System::RunLocation::RunInTarget )
-    {
-        expander.insert( QStringLiteral( "ROOT" ), QStringLiteral( "/" ) );
-    }
-    else if ( gs && gs->contains( "rootMountPoint" ) )
-    {
-        expander.insert( QStringLiteral( "ROOT" ), gs->value( "rootMountPoint" ).toString() );
-    }
-
-    // Replacement for ${USER}
-    if ( gs && gs->contains( "username" ) )
-    {
-        expander.insert( QStringLiteral( "USER" ), gs->value( "username" ).toString() );
-    }
-
-    // Copy and expand the list, collecting missing variables (so don't call expand())
-    CommandList_t expandedList;
-    std::transform( cbegin(),
-                    cend(),
-                    std::back_inserter( expandedList ),
-                    [ &expander ]( CommandLine c )
-                    {
-                        expander.expandMacros( c.first );
-                        return c;
-                    } );
 
+    auto expander = get_gs_expander( location );
+    auto expandedList = expand( expander );
     if ( expander.hasErrors() )
     {
         const auto missing = expander.errorNames();
@@ -157,7 +171,6 @@ CommandList::run()
                 .arg( missing.join( ',' ) ) );
     }
 
-
     for ( CommandList::const_iterator i = expandedList.cbegin(); i != expandedList.cend(); ++i )
     {
         QString processed_cmd = i->command();
@@ -190,10 +203,24 @@ CommandList::run()
     return Calamares::JobResult::ok();
 }
 
-void
+CommandList
-CommandList::append( const QString& s )
+CommandList::expand( KMacroExpanderBase& expander ) const
 {
-    append( CommandLine( s, m_timeout ) );
+    // Copy and expand the list, collecting missing variables (so don't call expand())
+    CommandList expandedList( m_doChroot, m_timeout );
+    std::transform( cbegin(),
+                    cend(),
+                    std::back_inserter( expandedList ),
+                    [ &expander ]( const CommandLine& c ) { return c.expand( expander ); } );
+    return expandedList;
 }
 
+CommandList
+CommandList::expand() const
+{
+    auto expander = get_gs_expander( System::RunLocation::RunInHost );
+    return expand( expander );
+}
+
+
 }  // namespace CalamaresUtils

src/libcalamares/utils/CommandList.h

@@ -18,6 +18,8 @@
 
 #include <chrono>
 
+class KMacroExpanderBase;
+
 namespace CalamaresUtils
 {
 
@@ -50,6 +52,20 @@ struct CommandLine : public QPair< QString, std::chrono::seconds >
     std::chrono::seconds timeout() const { return second; }
 
     bool isValid() const { return !first.isEmpty(); }
+
+    /** @brief Returns a copy of this one command, with variables expanded
+     *
+     * The given macro-expander is used to expand the command-line.
+     * This will normally be a Calamares::String::DictionaryExpander
+     * instance, which handles the ROOT and USER variables.
+     */
+    CommandLine expand( KMacroExpanderBase& expander ) const;
+    /** @brief As above, with a default macro-expander.
+     *
+     * The default macro-expander assumes RunInHost (e.g. ROOT will
+     * expand to the RootMountPoint set in Global Storage).
+     */
+    CommandLine expand() const;
 };
 
 /** @brief Abbreviation, used internally. */
@@ -81,10 +97,21 @@ public:
     using CommandList_t::const_iterator;
     using CommandList_t::count;
     using CommandList_t::isEmpty;
+    using CommandList_t::push_back;
+    using CommandList_t::value_type;
 
-protected:
+    /** @brief Return a copy of this command-list, with variables expanded
-    using CommandList_t::append;
+     *
-    void append( const QString& );
+     * Each command-line in the list is expanded with the given @p expander.
+     * @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.
+     * @see CommandLine::expand() for details.
+     */
+    CommandList expand() const;
 
 private:
     bool m_doChroot;

src/libcalamares/utils/StringExpander.cpp

@@ -28,6 +28,13 @@ DictionaryExpander::DictionaryExpander()
 {
 }
 
+DictionaryExpander::DictionaryExpander( Calamares::String::DictionaryExpander&& other )
+    : KWordMacroExpander( other.escapeChar() )
+    , d( std::move( other.d ) )
+{
+}
+
+
 DictionaryExpander::~DictionaryExpander() {}
 
 

src/libcalamares/utils/StringExpander.h

@@ -42,6 +42,7 @@ class DictionaryExpander : public KWordMacroExpander
 {
 public:
     DictionaryExpander();
+    DictionaryExpander( DictionaryExpander&& );
     virtual ~DictionaryExpander() override;
 
     void insert( const QString& key, const QString& value );

src/libcalamares/utils/Tests.cpp

@@ -10,6 +10,7 @@
  */
 
 #include "CalamaresUtilsSystem.h"
+#include "CommandList.h"
 #include "Entropy.h"
 #include "Logger.h"
 #include "RAII.h"
@@ -46,7 +47,10 @@ private Q_SLOTS:
     void testLoadSaveYaml();  // Just settings.conf
     void testLoadSaveYamlExtended();  // Do a find() in the src dir
 
+    /** @section Test running commands and command-expansion. */
     void testCommands();
+    void testCommandExpansion_data();
+    void testCommandExpansion();  // See also shellprocess tests
 
     /** @section Test that all the UMask objects work correctly. */
     void testUmask();
@@ -99,6 +103,16 @@ LibCalamaresTests::~LibCalamaresTests() {}
 void
 LibCalamaresTests::initTestCase()
 {
+    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 );
 }
 
 void
@@ -256,6 +270,34 @@ LibCalamaresTests::testCommands()
     QVERIFY( r.getOutput().contains( tfn.fileName() ) );
 }
 
+void
+LibCalamaresTests::testCommandExpansion_data()
+{
+    QTest::addColumn< QString >( "command" );
+    QTest::addColumn< QString >( "expected" );
+
+    QTest::newRow( "empty" ) << QString() << QString();
+    QTest::newRow( "ls   " ) << QStringLiteral( "ls" ) << QStringLiteral( "ls" );
+    QTest::newRow( "user " ) << QStringLiteral( "chmod $USER" ) << QStringLiteral( "chmod alice" );
+}
+
+void
+LibCalamaresTests::testCommandExpansion()
+{
+    Calamares::GlobalStorage* gs
+        = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
+    QVERIFY( gs );
+    gs->insert( QStringLiteral( "username" ), QStringLiteral( "alice" ) );
+
+    QFETCH( QString, command );
+    QFETCH( QString, expected );
+    CalamaresUtils::CommandLine c( command, std::chrono::seconds( 0 ) );
+    CalamaresUtils::CommandLine e = c.expand();
+
+    QCOMPARE( c.command(), command );
+    QCOMPARE( e.command(), expected );
+}
+
 void
 LibCalamaresTests::testUmask()
 {