From 9ef04192dbc621dbc2864c856d12b94eb139d4ce Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 11:03:40 +0100 Subject: [PATCH 01/18] [libcalamares] Simplify returns in targetPath() --- src/libcalamares/utils/CalamaresUtilsSystem.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index ea8f507bd..5d4b61a2f 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -266,8 +266,6 @@ System::runCommand( System::RunLocation location, QString System::targetPath( const QString& path ) const { - QString completePath; - if ( doChroot() ) { Calamares::GlobalStorage* gs @@ -279,14 +277,12 @@ System::targetPath( const QString& path ) const return QString(); } - completePath = gs->value( "rootMountPoint" ).toString() + '/' + path; + return gs->value( "rootMountPoint" ).toString() + '/' + path; } else { - completePath = QStringLiteral( "/" ) + path; + return QStringLiteral( "/" ) + path; } - - return completePath; } QString From bf882cec1d8ce7669645268d2ef4a2e5c49d9ecd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 11:14:57 +0100 Subject: [PATCH 02/18] [machineid] Migrate removeFile() to libcalamares - Becomes removeTargetFile() --- .../utils/CalamaresUtilsSystem.cpp | 20 ++++++++++++++++++- src/libcalamares/utils/CalamaresUtilsSystem.h | 11 +++++++++- src/modules/machineid/MachineIdJob.cpp | 10 ++++++---- src/modules/machineid/Workers.cpp | 11 ---------- src/modules/machineid/Workers.h | 3 --- 5 files changed, 35 insertions(+), 20 deletions(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 5d4b61a2f..ede96305a 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014, Teo Mrnjavac - * Copyright 2017-2018, Adriaan de Groot + * Copyright 2017-2018, 2020, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -285,6 +285,13 @@ System::targetPath( const QString& path ) const } } +/// @brief Cheap check if a path is absolute. +static inline bool +isAbsolutePath( const QString& path ) +{ + return path.startsWith( '/' ); +} + QString System::createTargetFile( const QString& path, const QByteArray& contents ) const { @@ -323,6 +330,17 @@ 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; + } + QFile::remove( targetPath( path ) ); +} + QPair< quint64, float > System::getTotalMemoryB() const diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index 8265f0fdb..5c5e04eb2 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -1,7 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2014, Teo Mrnjavac - * Copyright 2017-2018, Adriaan de Groot + * Copyright 2017-2018, 2020, Adriaan de Groot * * 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,15 @@ 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 getTotalMemoryB returns the total main memory, in bytes. * diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index 393950ded..af3a68d90 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -3,7 +3,7 @@ * Copyright 2014, Kevin Kofler * Copyright 2016, Philip Müller * Copyright 2017, Alf Gaida - * Copyright 2019, Adriaan de Groot + * Copyright 2019-2020, Adriaan de Groot * * 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 auto* 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 diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index fefaf24b9..f1f7bb369 100644 --- a/src/modules/machineid/Workers.cpp +++ b/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 ) { diff --git a/src/modules/machineid/Workers.h b/src/modules/machineid/Workers.h index 5cf6270d9..31561af1b 100644 --- a/src/modules/machineid/Workers.h +++ b/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 ); From 4af68365c9bd955ee66001ad54b5d8bb71ef8cf5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 11:16:40 +0100 Subject: [PATCH 03/18] [machineid] Remove obsolete implementation --- src/modules/machineid/main.py | 73 ----------------------------------- 1 file changed, 73 deletions(-) delete mode 100644 src/modules/machineid/main.py diff --git a/src/modules/machineid/main.py b/src/modules/machineid/main.py deleted file mode 100644 index 384193ed6..000000000 --- a/src/modules/machineid/main.py +++ /dev/null @@ -1,73 +0,0 @@ -#!/usr/bin/env python3 -# -*- coding: utf-8 -*- -# -# === This file is part of Calamares - === -# -# Copyright 2014, Kevin Kofler -# Copyright 2016,2020 Philip Müller -# Copyright 2017, Alf Gaida -# Copyright 2019, Adriaan de Groot -# -# 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 . - -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 
{!s}
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" - - 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 From 95936549e2be8e8f8c1aa343bf4b794d9c14204a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 11:30:37 +0100 Subject: [PATCH 04/18] [libcalamares] Add a createTargetBasedirs() - Used to ensure that the directories leading up to a given path exist. Implementation is incomplete and broken for now. - While here, avoid removing an empty pathname in removeTargetFile() (the empty pathname indicates a broken configuration). --- .../utils/CalamaresUtilsSystem.cpp | 30 ++++++++++++++++++- src/libcalamares/utils/CalamaresUtilsSystem.h | 11 +++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index ede96305a..3b36a7ccf 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -338,7 +338,35 @@ System::removeTargetFile( const QString& path ) const cWarning() << "Will not remove non-absolute path" << path; return; } - QFile::remove( targetPath( path ) ); + QString target = targetPath( path ); + if ( !target.isEmpty() ) + { + QFile::remove( target ); + } + // If it was empty, a warning was already printed +} + +int +System::createTargetBasedirs(const QString& path) const +{ + if ( !isAbsolutePath( path ) ) + { + cWarning() << "Will not create basedirs for non-absolute path" << path; + return -1; + } + + QString target = targetPath( path ); + if ( target.isEmpty() ) + { + // If it was empty, a warning was already printed + return -1; + } + + QString base( "/" ); + QStringList parts = target.split( '/', QString::SplitBehavior::SkipEmptyParts ); + + cDebug() << parts; + return -1; } diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index 5c5e04eb2..09a606e2c 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -255,6 +255,17 @@ public: */ DLLEXPORT void removeTargetFile( const QString& path ) const; + /** @brief Ensure that the directories above @p path exist + * + * @param path a full pathname to a desired file. + * + * All the directory components before the last path component are + * created, as needed, with 0755 permissions. Returns the number + * of components that needed to be created; 0 if they all already + * existed, and < 0 on failure. + */ + DLLEXPORT int createTargetBasedirs( const QString& path ) const; + /** * @brief getTotalMemoryB returns the total main memory, in bytes. * From 15bca702c1adefa0ab71ea2ee2181601544858d0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 11:51:13 +0100 Subject: [PATCH 05/18] [libcalamares] Add tests for path functions (part 1) --- src/libcalamares/utils/Tests.cpp | 27 +++++++++++++++++++++++++++ src/libcalamares/utils/Tests.h | 3 +++ 2 files changed, 30 insertions(+) diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index 16faec9a1..7f9f7f2bf 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -24,6 +24,9 @@ #include "UMask.h" #include "Yaml.h" +#include "GlobalStorage.h" +#include "JobQueue.h" + #include #include @@ -221,3 +224,27 @@ LibCalamaresTests::testPrintableEntropy() QVERIFY( c.cell() < 127 ); } } + +void +LibCalamaresTests::testTargetPath() +{ + // Ensure we have a system object, expect it to be a "bogus" one + const 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() ) + { + (void)new Calamares::JobQueue(); + } + Calamares::GlobalStorage* gs + = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; + QVERIFY( gs ); + gs->insert( "rootMountPoint", "/tmp" ); + + // Paths mapped normally + QCOMPARE( system->targetPath( "/etc/calamares" ), QStringLiteral( "/tmp/etc/calamares" ) ); + QCOMPARE( system->targetPath( "//etc//calamares" ), QStringLiteral( "/tmp/etc/calamares" ) ); // extra / + QCOMPARE( system->targetPath( "etc/calamares" ), QString() ); // NOT ABSOLUTE +} diff --git a/src/libcalamares/utils/Tests.h b/src/libcalamares/utils/Tests.h index d369ed4cb..8ea9aa0ce 100644 --- a/src/libcalamares/utils/Tests.h +++ b/src/libcalamares/utils/Tests.h @@ -43,6 +43,9 @@ private Q_SLOTS: /** @brief Tests the entropy functions. */ void testEntropy(); void testPrintableEntropy(); + + /** @brief Tests file creation and removal. */ + void testTargetPath(); }; #endif From 31878dd43b0f6adb879585228d4ce61ed1e8a7b1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 13:29:35 +0100 Subject: [PATCH 06/18] [libcalamares] Avoid double / between root and path in targetPath() --- .../utils/CalamaresUtilsSystem.cpp | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 3b36a7ccf..b5293c252 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -263,6 +263,13 @@ 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 { @@ -277,21 +284,15 @@ System::targetPath( const QString& path ) const return QString(); } - return gs->value( "rootMountPoint" ).toString() + '/' + path; + QString root = gs->value( "rootMountPoint" ).toString(); + return isAbsolutePath( path ) ? ( root + path ) : ( root + '/' + path ); } else { - return QStringLiteral( "/" ) + path; + return isAbsolutePath( path ) ? path : ( QStringLiteral( "/" ) + path ); } } -/// @brief Cheap check if a path is absolute. -static inline bool -isAbsolutePath( const QString& path ) -{ - return path.startsWith( '/' ); -} - QString System::createTargetFile( const QString& path, const QByteArray& contents ) const { @@ -347,7 +348,7 @@ System::removeTargetFile( const QString& path ) const } int -System::createTargetBasedirs(const QString& path) const +System::createTargetBasedirs( const QString& path ) const { if ( !isAbsolutePath( path ) ) { From daa5b804b3e7b78a6f0431f43cf270a85d2e0951 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 13:49:06 +0100 Subject: [PATCH 07/18] [libcalamares] Split paths-tests into own test executable - Since these tests all want a system object, and a GS with a sensible setup, give them one with its own initTestCase(). This could have been done with one executable, running tests from multiple classes, but there's not much overall benefit there. --- src/libcalamares/CMakeLists.txt | 12 ++++ src/libcalamares/utils/TestPaths.cpp | 89 ++++++++++++++++++++++++++++ src/libcalamares/utils/Tests.cpp | 24 -------- src/libcalamares/utils/Tests.h | 3 - 4 files changed, 101 insertions(+), 27 deletions(-) create mode 100644 src/libcalamares/utils/TestPaths.cpp diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 0e7f7c6e7..cad7e7a6e 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/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} diff --git a/src/libcalamares/utils/TestPaths.cpp b/src/libcalamares/utils/TestPaths.cpp new file mode 100644 index 000000000..5071b1289 --- /dev/null +++ b/src/libcalamares/utils/TestPaths.cpp @@ -0,0 +1,89 @@ +/* === This file is part of Calamares - === + * + * Copyright 2018, Adriaan de Groot + * + * 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 . + */ + +#include "CalamaresUtilsSystem.h" +#include "Entropy.h" +#include "Logger.h" +#include "UMask.h" +#include "Yaml.h" + +#include "GlobalStorage.h" +#include "JobQueue.h" + +#include + +#include + +#include +#include +#include + +class TestPaths : public QObject +{ + Q_OBJECT +public: + TestPaths() {}; + virtual ~TestPaths() {}; + +private Q_SLOTS: + void initTestCase(); + + void testTargetPath(); + +private: + CalamaresUtils::System* m_system; // Points to singleton instance, not owned +}; + +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() ) + { + (void)new Calamares::JobQueue(); + } + Calamares::GlobalStorage* gs + = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; + QVERIFY( gs ); + gs->insert( "rootMountPoint", "/tmp" ); + + m_system = system; +} + + +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 / + QCOMPARE( m_system->targetPath( "etc/calamares" ), QString() ); // NOT ABSOLUTE +} + +QTEST_GUILESS_MAIN( TestPaths ) + +#include "utils/moc-warnings.h" + +#include "TestPaths.moc" diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index 7f9f7f2bf..e39d182ea 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -224,27 +224,3 @@ LibCalamaresTests::testPrintableEntropy() QVERIFY( c.cell() < 127 ); } } - -void -LibCalamaresTests::testTargetPath() -{ - // Ensure we have a system object, expect it to be a "bogus" one - const 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() ) - { - (void)new Calamares::JobQueue(); - } - Calamares::GlobalStorage* gs - = Calamares::JobQueue::instance() ? Calamares::JobQueue::instance()->globalStorage() : nullptr; - QVERIFY( gs ); - gs->insert( "rootMountPoint", "/tmp" ); - - // Paths mapped normally - QCOMPARE( system->targetPath( "/etc/calamares" ), QStringLiteral( "/tmp/etc/calamares" ) ); - QCOMPARE( system->targetPath( "//etc//calamares" ), QStringLiteral( "/tmp/etc/calamares" ) ); // extra / - QCOMPARE( system->targetPath( "etc/calamares" ), QString() ); // NOT ABSOLUTE -} diff --git a/src/libcalamares/utils/Tests.h b/src/libcalamares/utils/Tests.h index 8ea9aa0ce..d369ed4cb 100644 --- a/src/libcalamares/utils/Tests.h +++ b/src/libcalamares/utils/Tests.h @@ -43,9 +43,6 @@ private Q_SLOTS: /** @brief Tests the entropy functions. */ void testEntropy(); void testPrintableEntropy(); - - /** @brief Tests file creation and removal. */ - void testTargetPath(); }; #endif From 8d23e665ea82e4aa75bfab3060e2f8298bea7f82 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 13:53:49 +0100 Subject: [PATCH 08/18] [libcalamares] Fix targetPath() tests - there is less simplification done than you might think --- src/libcalamares/utils/TestPaths.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/utils/TestPaths.cpp b/src/libcalamares/utils/TestPaths.cpp index 5071b1289..2232ca084 100644 --- a/src/libcalamares/utils/TestPaths.cpp +++ b/src/libcalamares/utils/TestPaths.cpp @@ -78,8 +78,8 @@ 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 / - QCOMPARE( m_system->targetPath( "etc/calamares" ), QString() ); // NOT ABSOLUTE + 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 } QTEST_GUILESS_MAIN( TestPaths ) From 394eee395414b844ad27ac15b002d069883a3827 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 13:57:14 +0100 Subject: [PATCH 09/18] [libcalamares] Test more targetPath() scenario's --- src/libcalamares/utils/TestPaths.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libcalamares/utils/TestPaths.cpp b/src/libcalamares/utils/TestPaths.cpp index 2232ca084..7411dc5bb 100644 --- a/src/libcalamares/utils/TestPaths.cpp +++ b/src/libcalamares/utils/TestPaths.cpp @@ -80,6 +80,13 @@ TestPaths::testTargetPath() 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 + Calamares::JobQueue::instance()->globalStorage()->remove( "rootMountPoint" ); + QCOMPARE( m_system->targetPath( QString() ), QString() ); // Without root, no path } QTEST_GUILESS_MAIN( TestPaths ) From b502d78984e9b5406600f9eeea93ebc26171344f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 13:58:44 +0100 Subject: [PATCH 10/18] [libcalamares] Fix warning message - "create" was when this function was used elsewhere --- src/libcalamares/utils/CalamaresUtilsSystem.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index b5293c252..0c21034af 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -280,7 +280,7 @@ 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(); } From 8b8ecf7b7bcdf42adff3e8c5082a4d67c3dd827e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 15:23:02 +0100 Subject: [PATCH 11/18] [libcalamars] Improve test init and cleanup - Test createTargetFile and removeTargetFile - Clean up afterwards - Ensure /tmp is the RMP for each test --- src/libcalamares/utils/TestPaths.cpp | 45 +++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/src/libcalamares/utils/TestPaths.cpp b/src/libcalamares/utils/TestPaths.cpp index 7411dc5bb..5de8ae4fc 100644 --- a/src/libcalamares/utils/TestPaths.cpp +++ b/src/libcalamares/utils/TestPaths.cpp @@ -42,13 +42,20 @@ public: private Q_SLOTS: void initTestCase(); + void init(); + void cleanupTestCase(); void testTargetPath(); + void testCreateTarget(); private: - CalamaresUtils::System* m_system; // Points to singleton instance, not owned + 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() { @@ -62,14 +69,28 @@ TestPaths::initTestCase() // 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" ); m_system = system; + m_gs = gs; +} + +void +TestPaths::cleanupTestCase() +{ + QFile::remove( absFile ); +} + +void +TestPaths::init() +{ + cDebug() << "Setting rootMountPoint"; + m_gs->insert( "rootMountPoint", "/tmp" ); } @@ -78,17 +99,33 @@ 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" ) ); // 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 - Calamares::JobQueue::instance()->globalStorage()->remove( "rootMountPoint" ); + 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() ); +} + QTEST_GUILESS_MAIN( TestPaths ) #include "utils/moc-warnings.h" From e65969d5871e79b4a4fdaf46eab3af4b0baba7b8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 15:51:46 +0100 Subject: [PATCH 12/18] [libcalamares] Re-do createTargetDirs() - Drop the basedirs idea, replace return with just bool - Use QDir::mkpath, with some extra validation - Test it a bit --- .../utils/CalamaresUtilsSystem.cpp | 24 ++++++++---- src/libcalamares/utils/CalamaresUtilsSystem.h | 12 +++--- src/libcalamares/utils/TestPaths.cpp | 37 +++++++++++++++++-- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 0c21034af..7e6ef99d6 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -347,27 +347,35 @@ System::removeTargetFile( const QString& path ) const // If it was empty, a warning was already printed } -int -System::createTargetBasedirs( const QString& path ) const +bool +System::createTargetDirs( const QString& path ) const { if ( !isAbsolutePath( path ) ) { cWarning() << "Will not create basedirs for non-absolute path" << path; - return -1; + return false; } QString target = targetPath( path ); if ( target.isEmpty() ) { // If it was empty, a warning was already printed - return -1; + return false; } - QString base( "/" ); - QStringList parts = target.split( '/', QString::SplitBehavior::SkipEmptyParts ); + QString root = Calamares::JobQueue::instance()->globalStorage()->value( "rootMountPoint" ).toString(); + if ( root.isEmpty() ) + { + return false; + } - cDebug() << parts; - return -1; + 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** / } diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index 09a606e2c..e95d5731e 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -257,14 +257,14 @@ public: /** @brief Ensure that the directories above @p path exist * - * @param path a full pathname to a desired file. + * @param path a full pathname to a desired directory. * - * All the directory components before the last path component are - * created, as needed, with 0755 permissions. Returns the number - * of components that needed to be created; 0 if they all already - * existed, and < 0 on failure. + * All the directory components including the last path component are + * created, as needed, with 0755 permissions. Returns true on success. + * + * @see QDir::mkpath */ - DLLEXPORT int createTargetBasedirs( const QString& path ) const; + DLLEXPORT bool createTargetDirs( const QString& path ) const; /** * @brief getTotalMemoryB returns the total main memory, in bytes. diff --git a/src/libcalamares/utils/TestPaths.cpp b/src/libcalamares/utils/TestPaths.cpp index 5de8ae4fc..36e8afe5e 100644 --- a/src/libcalamares/utils/TestPaths.cpp +++ b/src/libcalamares/utils/TestPaths.cpp @@ -25,13 +25,14 @@ #include "GlobalStorage.h" #include "JobQueue.h" -#include +#include +// #include #include -#include -#include -#include +// #include +// #include +// #include class TestPaths : public QObject { @@ -47,6 +48,7 @@ private Q_SLOTS: void testTargetPath(); void testCreateTarget(); + void testCreateTargetBasedirs(); private: CalamaresUtils::System* m_system = nullptr; // Points to singleton instance, not owned @@ -126,6 +128,33 @@ TestPaths::testCreateTarget() 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() ); +} + QTEST_GUILESS_MAIN( TestPaths ) #include "utils/moc-warnings.h" From 6ede9f2c7ce40859275fead6882f49679932d7a8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 16:08:38 +0100 Subject: [PATCH 13/18] [libcalamares] Test QFileInfo::dir() for completeness --- src/libcalamares/utils/TestPaths.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libcalamares/utils/TestPaths.cpp b/src/libcalamares/utils/TestPaths.cpp index 36e8afe5e..2f9f4e657 100644 --- a/src/libcalamares/utils/TestPaths.cpp +++ b/src/libcalamares/utils/TestPaths.cpp @@ -153,6 +153,9 @@ TestPaths::testCreateTargetBasedirs() 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 ) From 240fe2a56481d59c2f4b92aa927f02900fee2f96 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 16:09:02 +0100 Subject: [PATCH 14/18] [libcalamares] Add convenience createTargetParentDirs() --- src/libcalamares/utils/CalamaresUtilsSystem.cpp | 6 ++++++ src/libcalamares/utils/CalamaresUtilsSystem.h | 11 +++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.cpp b/src/libcalamares/utils/CalamaresUtilsSystem.cpp index 7e6ef99d6..61e05976a 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.cpp +++ b/src/libcalamares/utils/CalamaresUtilsSystem.cpp @@ -378,6 +378,12 @@ System::createTargetDirs( const QString& path ) const 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 diff --git a/src/libcalamares/utils/CalamaresUtilsSystem.h b/src/libcalamares/utils/CalamaresUtilsSystem.h index e95d5731e..ca8e0d797 100644 --- a/src/libcalamares/utils/CalamaresUtilsSystem.h +++ b/src/libcalamares/utils/CalamaresUtilsSystem.h @@ -255,17 +255,24 @@ public: */ DLLEXPORT void removeTargetFile( const QString& path ) const; - /** @brief Ensure that the directories above @p path exist + /** @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, with 0755 permissions. Returns true on success. + * 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. * From b62004aae9039b44fd9d2c8e6eb9d74f5c88d863 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 16:10:13 +0100 Subject: [PATCH 15/18] [machineid] Create the DBus data directory - before running dbus-uuidgen or linking to systemd's UUID, create /var/lib/dbus; some distro's don't create that beforehand. FIXES #1314 --- src/modules/machineid/MachineIdJob.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/MachineIdJob.cpp b/src/modules/machineid/MachineIdJob.cpp index af3a68d90..fc535e356 100644 --- a/src/modules/machineid/MachineIdJob.cpp +++ b/src/modules/machineid/MachineIdJob.cpp @@ -68,7 +68,7 @@ MachineIdJob::exec() QString target_dbus_machineid_file = QStringLiteral( "/var/lib/dbus/machine-id" ); QString target_entropy_file = QStringLiteral( "/var/lib/urandom/random-seed" ); - const auto* system = CalamaresUtils::System::instance(); + const CalamaresUtils::System* system = CalamaresUtils::System::instance(); // Clear existing files if ( m_entropy ) @@ -106,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 ); From 4cdcb48de6f0dd79264df56dbaf6ad961098931a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 16:12:17 +0100 Subject: [PATCH 16/18] [machineid] Functionality moved to libcalamares --- src/modules/machineid/Tests.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/modules/machineid/Tests.cpp b/src/modules/machineid/Tests.cpp index 273645e22..87853af82 100644 --- a/src/modules/machineid/Tests.cpp +++ b/src/modules/machineid/Tests.cpp @@ -35,7 +35,6 @@ public: private Q_SLOTS: void initTestCase(); - void testRemoveFile(); void testCopyFile(); void testPoolSize(); @@ -84,11 +83,6 @@ MachineIdTests::testCopyFile() } } -void -MachineIdTests::testRemoveFile() -{ -} - void MachineIdTests::testPoolSize() { From 110a84344bd8e88a6da156c0323de26d057890cd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 20:29:42 +0100 Subject: [PATCH 17/18] [machineid] Test job function - Create a job and ask it to create dbus files -- either directly, or as a symlink. Since the target chroot isn't viable, this will fail but we can at least see that directories are created, etc. --- src/modules/machineid/CMakeLists.txt | 1 + src/modules/machineid/Tests.cpp | 63 +++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/src/modules/machineid/CMakeLists.txt b/src/modules/machineid/CMakeLists.txt index efb6454e8..a57d5163d 100644 --- a/src/modules/machineid/CMakeLists.txt +++ b/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 diff --git a/src/modules/machineid/Tests.cpp b/src/modules/machineid/Tests.cpp index 87853af82..53abd0482 100644 --- a/src/modules/machineid/Tests.cpp +++ b/src/modules/machineid/Tests.cpp @@ -16,11 +16,14 @@ * along with Calamares. If not, see . */ +#include "MachineIdJob.h" #include "Workers.h" +#include "GlobalStorage.h" +#include "JobQueue.h" +#include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" - #include #include #include @@ -38,6 +41,8 @@ private Q_SLOTS: void testCopyFile(); void testPoolSize(); + + void testJob(); }; void @@ -95,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 ) From 3e2908ea166e3726e1cdd5bd0e961413ab2f5f38 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 7 Feb 2020 20:31:15 +0100 Subject: [PATCH 18/18] [machineid] Follow Manjaro flags - add -f to ln(1) flags --- src/modules/machineid/Workers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/machineid/Workers.cpp b/src/modules/machineid/Workers.cpp index f1f7bb369..39acdfbf2 100644 --- a/src/modules/machineid/Workers.cpp +++ b/src/modules/machineid/Workers.cpp @@ -181,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