Merge branch 'issue-1314'

FIXES #1314

src/libcalamares/CMakeLists.txt

@@ -171,6 +171,18 @@ if ( ECM_FOUND AND BUILD_TESTING )
     )
     calamares_automoc( libcalamarestest )
 
+    ecm_add_test(
+            utils/TestPaths.cpp
+        TEST_NAME
+            libcalamarestestpaths
+        LINK_LIBRARIES
+            calamares
+            Qt5::Core
+            Qt5::Test
+    )
+    calamares_automoc( libcalamarestestpaths )
+
+
     ecm_add_test(
             geoip/GeoIPTests.cpp
             ${geoip_src}

src/libcalamares/utils/CalamaresUtilsSystem.cpp

@@ -1,7 +1,7 @@
 /* === This file is part of Calamares - <https://github.com/calamares> ===
  *
  *   Copyright 2014, Teo Mrnjavac <teo@kde.org>
- *   Copyright 2017-2018, Adriaan de Groot <groot@kde.org>
+ *   Copyright 2017-2018, 2020, 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
@@ -263,11 +263,16 @@ System::runCommand( System::RunLocation location,
     return ProcessResult( r, output );
 }
 
+/// @brief Cheap check if a path is absolute.
+static inline bool
+isAbsolutePath( const QString& path )
+{
+    return path.startsWith( '/' );
+}
+
 QString
 System::targetPath( const QString& path ) const
 {
-    QString completePath;
-
     if ( doChroot() )
     {
         Calamares::GlobalStorage* gs
@@ -275,18 +280,17 @@ System::targetPath( const QString& path ) const
 
         if ( !gs || !gs->contains( "rootMountPoint" ) )
         {
-            cWarning() << "No rootMountPoint in global storage, cannot create target file" << path;
+            cWarning() << "No rootMountPoint in global storage, cannot name target file" << path;
             return QString();
         }
 
-        completePath = gs->value( "rootMountPoint" ).toString() + '/' + path;
+        QString root = gs->value( "rootMountPoint" ).toString();
+        return isAbsolutePath( path ) ? ( root + path ) : ( root + '/' + path );
     }
     else
     {
-        completePath = QStringLiteral( "/" ) + path;
+        return isAbsolutePath( path ) ? path : ( QStringLiteral( "/" ) + path );
     }
-
-    return completePath;
 }
 
 QString
@@ -327,6 +331,59 @@ System::createTargetFile( const QString& path, const QByteArray& contents ) cons
     return QFileInfo( f ).canonicalFilePath();
 }
 
+void
+System::removeTargetFile( const QString& path ) const
+{
+    if ( !isAbsolutePath( path ) )
+    {
+        cWarning() << "Will not remove non-absolute path" << path;
+        return;
+    }
+    QString target = targetPath( path );
+    if ( !target.isEmpty() )
+    {
+        QFile::remove( target );
+    }
+    // If it was empty, a warning was already printed
+}
+
+bool
+System::createTargetDirs( const QString& path ) const
+{
+    if ( !isAbsolutePath( path ) )
+    {
+        cWarning() << "Will not create basedirs for non-absolute path" << path;
+        return false;
+    }
+
+    QString target = targetPath( path );
+    if ( target.isEmpty() )
+    {
+        // If it was empty, a warning was already printed
+        return false;
+    }
+
+    QString root = Calamares::JobQueue::instance()->globalStorage()->value( "rootMountPoint" ).toString();
+    if ( root.isEmpty() )
+    {
+        return false;
+    }
+
+    QDir d( root );
+    if ( !d.exists() )
+    {
+        cWarning() << "Root mountpoint" << root << "does not exist.";
+        return false;
+    }
+    return d.mkpath( target );  // This re-does everything starting from the **host** /
+}
+
+bool
+System::createTargetParentDirs( const QString& filePath ) const
+{
+    return createTargetDirs( QFileInfo( filePath ).dir().path() );
+}
+
 
 QPair< quint64, float >
 System::getTotalMemoryB() const

src/libcalamares/utils/CalamaresUtilsSystem.h

@@ -1,7 +1,7 @@
 /* === This file is part of Calamares - <https://github.com/calamares> ===
  *
  *   Copyright 2014, Teo Mrnjavac <teo@kde.org>
- *   Copyright 2017-2018, Adriaan de Groot <groot@kde.org>
+ *   Copyright 2017-2018, 2020, 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
@@ -246,6 +246,33 @@ public:
      */
     DLLEXPORT QString createTargetFile( const QString& path, const QByteArray& contents ) const;
 
+    /** @brief Remove a file from the target system.
+     *
+     * @param path Path to the file; this is interpreted from the root
+     *      of the target system (@see targetPath()).
+     *
+     * Does no error checking to see if the target file was really removed.
+     */
+    DLLEXPORT void removeTargetFile( const QString& path ) const;
+
+    /** @brief Ensure that the directory @p path exists
+     *
+     * @param path a full pathname to a desired directory.
+     *
+     * All the directory components including the last path component are
+     * created, as needed. Returns true on success.
+     *
+     * @see QDir::mkpath
+     */
+    DLLEXPORT bool createTargetDirs( const QString& path ) const;
+
+    /** @brief Convenience to create parent directories of a file path.
+     *
+     * Creates all the parent directories until the last
+     * component of @p filePath . @see createTargetDirs()
+     */
+    DLLEXPORT bool createTargetParentDirs( const QString& filePath ) const;
+
     /**
      * @brief getTotalMemoryB returns the total main memory, in bytes.
      *

src/libcalamares/utils/TestPaths.cpp

@@ -0,0 +1,165 @@
+/* === 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 "CalamaresUtilsSystem.h"
+#include "Entropy.h"
+#include "Logger.h"
+#include "UMask.h"
+#include "Yaml.h"
+
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+
+#include <QDir>
+// #include <QTemporaryFile>
+
+#include <QtTest/QtTest>
+
+// #include <fcntl.h>
+// #include <sys/stat.h>
+// #include <unistd.h>
+
+class TestPaths : public QObject
+{
+    Q_OBJECT
+public:
+    TestPaths() {};
+    virtual ~TestPaths() {};
+
+private Q_SLOTS:
+    void initTestCase();
+    void init();
+    void cleanupTestCase();
+
+    void testTargetPath();
+    void testCreateTarget();
+    void testCreateTargetBasedirs();
+
+private:
+    CalamaresUtils::System* m_system = nullptr;  // Points to singleton instance, not owned
+    Calamares::GlobalStorage* m_gs = nullptr;
+};
+
+static const char testFile[] = "/calamares-testcreate";
+static const char absFile[] = "/tmp/calamares-testcreate";  // With rootMountPoint prepended
+
+void
+TestPaths::initTestCase()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+
+    // Ensure we have a system object, expect it to be a "bogus" one
+    CalamaresUtils::System* system = CalamaresUtils::System::instance();
+    QVERIFY( system );
+    QVERIFY( system->doChroot() );
+
+    // Ensure we have a system-wide GlobalStorage with /tmp as root
+    if ( !Calamares::JobQueue::instance() )
+    {
+        cDebug() << "Creating new JobQueue";
+        (void)new Calamares::JobQueue();
+    }
+    Calamares::GlobalStorage* gs
+        = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
+    QVERIFY( gs );
+
+    m_system = system;
+    m_gs = gs;
+}
+
+void
+TestPaths::cleanupTestCase()
+{
+    QFile::remove( absFile );
+}
+
+void
+TestPaths::init()
+{
+    cDebug() << "Setting rootMountPoint";
+    m_gs->insert( "rootMountPoint", "/tmp" );
+}
+
+
+void
+TestPaths::testTargetPath()
+{
+    // Paths mapped normally
+    QCOMPARE( m_system->targetPath( "/etc/calamares" ), QStringLiteral( "/tmp/etc/calamares" ) );
+    QCOMPARE( m_system->targetPath( "//etc//calamares" ),
+              QStringLiteral( "/tmp//etc//calamares" ) );  // extra / are not cleaned up
+    QCOMPARE( m_system->targetPath( "etc/calamares" ), QStringLiteral( "/tmp/etc/calamares" ) );  // relative to root
+
+    // Weird Paths
+    QCOMPARE( m_system->targetPath( QString() ), QStringLiteral( "/tmp/" ) );
+
+    // Now break GS
+    m_gs->remove( "rootMountPoint" );
+    QCOMPARE( m_system->targetPath( QString() ), QString() );  // Without root, no path
+}
+
+
+void
+TestPaths::testCreateTarget()
+{
+    QCOMPARE( m_system->createTargetFile( testFile, "Hello" ), QString( absFile ) );  // Success
+
+    QFileInfo fi( absFile );
+    QVERIFY( fi.exists() );
+    QCOMPARE( fi.size(), 5 );
+
+    m_system->removeTargetFile( testFile );
+    QFileInfo fi2( absFile );  // fi caches information
+    QVERIFY( !fi2.exists() );
+}
+
+struct DirRemover
+{
+    DirRemover( const QString& base, const QString& dir )
+        : m_base( base )
+        , m_dir( dir )
+    {
+    }
+    ~DirRemover() { QDir( m_base ).rmpath( m_dir ); }
+
+    bool exists() const { return QDir( m_base ).exists( m_dir ); }
+
+    QString m_base, m_dir;
+};
+
+void
+TestPaths::testCreateTargetBasedirs()
+{
+    {
+        DirRemover dirrm( "/tmp", "var/lib/dbus" );
+        QVERIFY( m_system->createTargetDirs( "/" ) );
+        QVERIFY( m_system->createTargetDirs( "/var/lib/dbus" ) );
+        QVERIFY( QFile( "/tmp/var/lib/dbus" ).exists() );
+        QVERIFY( dirrm.exists() );
+    }
+    QVERIFY( !QFile( "/tmp/var/lib/dbus" ).exists() );
+
+    // QFileInfo.dir() behaves even when things don't exist
+    QCOMPARE( QFileInfo( "/tmp/var/lib/dbus/bogus" ).dir().path(), QStringLiteral( "/tmp/var/lib/dbus" ) );
+}
+
+QTEST_GUILESS_MAIN( TestPaths )
+
+#include "utils/moc-warnings.h"
+
+#include "TestPaths.moc"

src/libcalamares/utils/Tests.cpp

@@ -24,6 +24,9 @@
 #include "UMask.h"
 #include "Yaml.h"
 
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+
 #include <QTemporaryFile>
 
 #include <QtTest/QtTest>

src/modules/machineid/CMakeLists.txt

@@ -12,6 +12,7 @@ calamares_add_plugin( machineid
 if ( ECM_FOUND AND BUILD_TESTING )
     ecm_add_test(
             Tests.cpp
+            MachineIdJob.cpp
             Workers.cpp
         TEST_NAME
             machineidtest

src/modules/machineid/MachineIdJob.cpp

@@ -3,7 +3,7 @@
  *   Copyright 2014, Kevin Kofler <kevin.kofler@chello.at>
  *   Copyright 2016, Philip Müller <philm@manjaro.org>
  *   Copyright 2017, Alf Gaida <agaida@siduction.org>
- *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *   Copyright 2019-2020, 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
@@ -68,18 +68,20 @@ MachineIdJob::exec()
     QString target_dbus_machineid_file = QStringLiteral( "/var/lib/dbus/machine-id" );
     QString target_entropy_file = QStringLiteral( "/var/lib/urandom/random-seed" );
 
+    const CalamaresUtils::System* system = CalamaresUtils::System::instance();
+
     // Clear existing files
     if ( m_entropy )
     {
-        MachineId::removeFile( root, target_entropy_file );
+        system->removeTargetFile( target_entropy_file );
     }
     if ( m_dbus )
     {
-        MachineId::removeFile( root, target_dbus_machineid_file );
+        system->removeTargetFile( target_dbus_machineid_file );
     }
     if ( m_systemd )
     {
-        MachineId::removeFile( root, target_systemd_machineid_file );
+        system->removeTargetFile( target_systemd_machineid_file );
     }
 
     //Create new files
@@ -104,6 +106,10 @@ MachineIdJob::exec()
     }
     if ( m_dbus )
     {
+        if ( !system->createTargetParentDirs( target_dbus_machineid_file ) )
+        {
+            cWarning() << "Could not create DBus data-directory.";
+        }
         if ( m_dbus_symlink && QFile::exists( root + target_systemd_machineid_file ) )
         {
             auto r = MachineId::createDBusLink( root, target_dbus_machineid_file, target_systemd_machineid_file );

src/modules/machineid/Tests.cpp

@@ -16,11 +16,14 @@
  *   along with Calamares. If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "MachineIdJob.h"
 #include "Workers.h"
 
+#include "GlobalStorage.h"
+#include "JobQueue.h"
+#include "utils/CalamaresUtilsSystem.h"
 #include "utils/Logger.h"
 
-
 #include <QDir>
 #include <QFile>
 #include <QtTest/QtTest>
@@ -35,10 +38,11 @@ public:
 private Q_SLOTS:
     void initTestCase();
 
-    void testRemoveFile();
     void testCopyFile();
 
     void testPoolSize();
+
+    void testJob();
 };
 
 void
@@ -84,11 +88,6 @@ MachineIdTests::testCopyFile()
     }
 }
 
-void
-MachineIdTests::testRemoveFile()
-{
-}
-
 void
 MachineIdTests::testPoolSize()
 {
@@ -101,6 +100,62 @@ MachineIdTests::testPoolSize()
 #endif
 }
 
+void
+MachineIdTests::testJob()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+
+    // Ensure we have a system object, expect it to be a "bogus" one
+    CalamaresUtils::System* system = CalamaresUtils::System::instance();
+    QVERIFY( system );
+    QVERIFY( system->doChroot() );
+
+    // Ensure we have a system-wide GlobalStorage with /tmp as root
+    if ( !Calamares::JobQueue::instance() )
+    {
+        cDebug() << "Creating new JobQueue";
+        (void)new Calamares::JobQueue();
+    }
+    Calamares::GlobalStorage* gs
+        = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr;
+    QVERIFY( gs );
+    gs->insert( "rootMountPoint", "/tmp" );
+
+    // Prepare part of the target filesystem
+    QVERIFY( system->createTargetDirs("/etc") );
+    QVERIFY( !(system->createTargetFile( "/etc/machine-id", "Hello" ).isEmpty() ) );
+
+    MachineIdJob job( nullptr );
+    QVERIFY( !job.prettyName().isEmpty() );
+
+    QVariantMap config;
+    config.insert( "dbus", true );
+    job.setConfigurationMap( config );
+
+    {
+        auto r = job.exec();
+        QVERIFY( !r );  // It's supposed to fail, because no dbus-uuidgen executable exists
+        QVERIFY( QFile::exists( "/tmp/var/lib/dbus" ) );  // but the target dir exists
+    }
+
+    config.insert( "dbus-symlink", true );
+    job.setConfigurationMap( config );
+    {
+        auto r = job.exec();
+        QVERIFY( !r );  // It's supposed to fail, because no dbus-uuidgen executable exists
+        QVERIFY( QFile::exists( "/tmp/var/lib/dbus" ) );  // but the target dir exists
+
+        // These all (would) fail, because the chroot isn't viable
+#if 0
+        QVERIFY( QFile::exists( "/tmp/var/lib/dbus/machine-id" ) );
+
+        QFileInfo fi( "/tmp/var/lib/dbus/machine-id" );
+        QVERIFY( fi.exists() );
+        QVERIFY( fi.isSymLink() );
+        QCOMPARE( fi.size(), 5);
+#endif
+    }
+}
 
 QTEST_GUILESS_MAIN( MachineIdTests )
 

src/modules/machineid/Workers.cpp

@@ -36,17 +36,6 @@ isAbsolutePath( const QString& fileName )
     return fileName.startsWith( '/' );
 }
 
-// might need to use a helper to remove the file
-void
-removeFile( const QString& rootMountPoint, const QString& fileName )
-{
-    if ( isAbsolutePath( fileName ) )
-    {
-        QFile::remove( rootMountPoint + fileName );
-    }
-    // Otherwise, do nothing
-}
-
 Calamares::JobResult
 copyFile( const QString& rootMountPoint, const QString& fileName )
 {
@@ -192,7 +181,7 @@ Calamares::JobResult
 createDBusLink( const QString& rootMountPoint, const QString& fileName, const QString& systemdFileName )
 {
     Q_UNUSED( rootMountPoint )
-    return runCmd( QStringList { QStringLiteral( "ln" ), QStringLiteral( "-s" ), systemdFileName, fileName } );
+    return runCmd( QStringList { QStringLiteral( "ln" ), QStringLiteral( "-sf" ), systemdFileName, fileName } );
 }
 
 }  // namespace MachineId

src/modules/machineid/Workers.h

@@ -30,9 +30,6 @@ namespace MachineId
  * for moving files around in the target system.
  */
 
-/// @brief Remove @p fileName from the target system at @p rootMountPoint
-void removeFile( const QString& rootMountPoint, const QString& fileName );
-
 /// @brief Copy @p fileName from host into target system at @p rootMountPoint
 Calamares::JobResult copyFile( const QString& rootMountPoint, const QString& fileName );
 

src/modules/machineid/main.py

@@ -1,77 +0,0 @@
-#!/usr/bin/env python3
-# -*- coding: utf-8 -*-
-#
-# === This file is part of Calamares - <https://github.com/calamares> ===
-#
-#   Copyright 2014, Kevin Kofler <kevin.kofler@chello.at>
-#   Copyright 2016,2020 Philip Müller <philm@manjaro.org>
-#   Copyright 2017, Alf Gaida <agaida@siduction.org>
-#   Copyright 2019, 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/>.
-
-import libcalamares
-import os
-from libcalamares.utils import check_target_env_call, debug
-
-import gettext
-_ = gettext.translation("calamares-python",
-                        localedir=libcalamares.utils.gettext_path(),
-                        languages=libcalamares.utils.gettext_languages(),
-                        fallback=True).gettext
-
-
-def pretty_name():
-    return _("Generate machine-id.")
-
-
-def run():
-    """
-    Generate machine-id using dbus and systemd.
-
-    :return:
-    """
-    root_mount_point = libcalamares.globalstorage.value("rootMountPoint")
-
-    if root_mount_point is None:
-        libcalamares.utils.warning("rootMountPoint is empty, {!s}".format(root_mount_point))
-        return (_("Configuration Error"),
-                _("No root mount point is given for <pre>{!s}</pre> to use." ).format("machineid"))
-
-    enable_systemd = libcalamares.job.configuration["systemd"]
-    enable_dbus = libcalamares.job.configuration["dbus"]
-    enable_symlink = libcalamares.job.configuration["symlink"]
-    target_systemd_machineid_file = root_mount_point + "/etc/machine-id"
-    target_dbus_machineid_file = root_mount_point + "/var/lib/dbus/machine-id"
-    target_dbus_folder = root_mount_point + "/var/lib/dbus"
-    
-    if not os.path.exists(target_dbus_folder):
-        os.mkdir(target_dbus_folder, 0644)
-
-    if os.path.exists(target_dbus_machineid_file):
-        os.remove(target_dbus_machineid_file)
-
-    if enable_systemd:
-        if os.path.exists(target_systemd_machineid_file):
-            os.remove(target_systemd_machineid_file)
-        check_target_env_call("systemd-machine-id-setup")
-
-    if enable_dbus:
-        if enable_symlink and os.path.exists(target_systemd_machineid_file):
-            check_target_env_call(["ln", "-sf", "/etc/machine-id",
-                                   "/var/lib/dbus/machine-id"])
-        else:
-            check_target_env_call(["dbus-uuidgen", "--ensure"])
-
-    return None