From bdbeadcd65393c205f1ce50154cf5ba6edd5cc19 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 28 Apr 2024 16:59:25 +0200 Subject: [PATCH] [partition] Move fstab-handling This was declared in osprober, but implemented elsewhere. Move it to the "right" source file, add tests. While here, repair the listing of fstab entries (invalid entries were not stripped). --- src/modules/partition/CMakeLists.txt | 1 + src/modules/partition/core/OsproberEntry.cpp | 63 ++++++++++++++++++++ src/modules/partition/core/OsproberEntry.h | 15 ++++- src/modules/partition/core/PartUtils.cpp | 53 +++------------- src/modules/partition/tests/CMakeLists.txt | 12 ++-- src/modules/partition/tests/ConfigTests.cpp | 47 +++++++++++++++ 6 files changed, 140 insertions(+), 51 deletions(-) create mode 100644 src/modules/partition/core/OsproberEntry.cpp diff --git a/src/modules/partition/CMakeLists.txt b/src/modules/partition/CMakeLists.txt index 5a92e713e..3af8e4465 100644 --- a/src/modules/partition/CMakeLists.txt +++ b/src/modules/partition/CMakeLists.txt @@ -60,6 +60,7 @@ if(KPMcore_FOUND) core/DeviceList.cpp core/DeviceModel.cpp core/KPMHelpers.cpp + core/OsproberEntry.cpp core/PartitionActions.cpp core/PartitionCoreModule.cpp core/PartitionInfo.cpp diff --git a/src/modules/partition/core/OsproberEntry.cpp b/src/modules/partition/core/OsproberEntry.cpp new file mode 100644 index 000000000..4a59f7d07 --- /dev/null +++ b/src/modules/partition/core/OsproberEntry.cpp @@ -0,0 +1,63 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2015-2016 Teo Mrnjavac + * SPDX-FileCopyrightText: 2018-2019, 2024 Adriaan de Groot + * SPDX-FileCopyrightText: 2019 Collabora Ltd + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +#include "OsproberEntry.h" + + +bool +FstabEntry::isValid() const +{ + return !partitionNode.isEmpty() && !mountPoint.isEmpty() && !fsType.isEmpty(); +} + +FstabEntry +FstabEntry::fromEtcFstab( const QString& rawLine ) +{ + QString line = rawLine.simplified(); + if ( line.startsWith( '#' ) ) + { + return FstabEntry { QString(), QString(), QString(), QString(), 0, 0 }; + } + + QStringList splitLine = line.split( ' ' ); + if ( splitLine.length() != 6 ) + { + return FstabEntry { QString(), QString(), QString(), QString(), 0, 0 }; + } + + return FstabEntry { + splitLine.at( 0 ), // path, or UUID, or LABEL, etc. + splitLine.at( 1 ), // mount point + splitLine.at( 2 ), // fs type + splitLine.at( 3 ), // options + splitLine.at( 4 ).toInt(), //dump + splitLine.at( 5 ).toInt() //pass + }; +} + +namespace Calamares +{ +FstabEntryList +fromEtcFstabContents( const QStringList& fstabLines ) +{ + FstabEntryList fstabEntries; + + for ( const QString& rawLine : fstabLines ) + { + fstabEntries.append( FstabEntry::fromEtcFstab( rawLine ) ); + } + const auto invalidEntries = std::remove_if( + fstabEntries.begin(), fstabEntries.end(), []( const FstabEntry& x ) { return !x.isValid(); } ); + fstabEntries.erase( invalidEntries, fstabEntries.end() ); + return fstabEntries; +} + +} // namespace Calamares diff --git a/src/modules/partition/core/OsproberEntry.h b/src/modules/partition/core/OsproberEntry.h index 86b7691b8..0db7c9a51 100644 --- a/src/modules/partition/core/OsproberEntry.h +++ b/src/modules/partition/core/OsproberEntry.h @@ -32,11 +32,24 @@ struct FstabEntry * If the string isn't valid (e.g. comment-line, or broken * fstab entry) then the entry that is returned is invalid. */ - static FstabEntry fromEtcFstab( const QString& ); // implemented in Partutils.cpp + static FstabEntry fromEtcFstab( const QString& ); }; typedef QList< FstabEntry > FstabEntryList; +namespace Calamares +{ +/** @brief Returns valid entries from the lines of a fstab file */ +FstabEntryList fromEtcFstabContents( const QStringList& fstabLines ); + +/** @brief Returns valid entries from the byte-contents of a fstab file */ +inline FstabEntryList +fromEtcFstabContents( const QByteArray& contents ) +{ + return fromEtcFstabContents( QString::fromLocal8Bit( contents ).split( '\n' ) ); +} +} // namespace Calamares + struct OsproberEntry { QString prettyName; diff --git a/src/modules/partition/core/PartUtils.cpp b/src/modules/partition/core/PartUtils.cpp index 2b6fdbbea..9818ea71f 100644 --- a/src/modules/partition/core/PartUtils.cpp +++ b/src/modules/partition/core/PartUtils.cpp @@ -254,31 +254,25 @@ lookForFstabEntries( const QString& partitionPath ) if ( fstabFile.open( QIODevice::ReadOnly | QIODevice::Text ) ) { - const QStringList fstabLines = QString::fromLocal8Bit( fstabFile.readAll() ).split( '\n' ); - - for ( const QString& rawLine : fstabLines ) - { - fstabEntries.append( FstabEntry::fromEtcFstab( rawLine ) ); - } + const auto fstabLines = QString::fromLocal8Bit( fstabFile.readAll() ).split( '\n' ); fstabFile.close(); - const int lineCount = fstabEntries.count(); - const auto invalidEntries = std::remove_if( - fstabEntries.begin(), fstabEntries.end(), []( const FstabEntry& x ) { return !x.isValid(); } ); - fstabEntries.erase(invalidEntries); - cDebug() << Logger::SubEntry << "got" << fstabEntries.count() << "fstab entries from" << lineCount + + const auto fstabEntries = Calamares::fromEtcFstabContents( fstabLines ); + cDebug() << Logger::SubEntry << "got" << fstabEntries.count() << "fstab entries from" << fstabLines.count() << "lines in" << fstabFile.fileName(); + return fstabEntries; } else { cWarning() << "Could not read fstab from mounted fs"; + return {}; } } else { cWarning() << "Could not mount existing fs"; + return {}; } - - return fstabEntries; } static QString @@ -642,36 +636,3 @@ canonicalFilesystemName( const QString& fsName, FileSystem::Type* fsType ) } } // namespace PartUtils - -/* Implementation of methods for FstabEntry, from OsproberEntry.h */ - -bool -FstabEntry::isValid() const -{ - return !partitionNode.isEmpty() && !mountPoint.isEmpty() && !fsType.isEmpty(); -} - -FstabEntry -FstabEntry::fromEtcFstab( const QString& rawLine ) -{ - QString line = rawLine.simplified(); - if ( line.startsWith( '#' ) ) - { - return FstabEntry { QString(), QString(), QString(), QString(), 0, 0 }; - } - - QStringList splitLine = line.split( ' ' ); - if ( splitLine.length() != 6 ) - { - return FstabEntry { QString(), QString(), QString(), QString(), 0, 0 }; - } - - return FstabEntry { - splitLine.at( 0 ), // path, or UUID, or LABEL, etc. - splitLine.at( 1 ), // mount point - splitLine.at( 2 ), // fs type - splitLine.at( 3 ), // options - splitLine.at( 4 ).toInt(), //dump - splitLine.at( 5 ).toInt() //pass - }; -} diff --git a/src/modules/partition/tests/CMakeLists.txt b/src/modules/partition/tests/CMakeLists.txt index 1cb4ff7a1..3418859c4 100644 --- a/src/modules/partition/tests/CMakeLists.txt +++ b/src/modules/partition/tests/CMakeLists.txt @@ -14,6 +14,12 @@ include_directories( ${CMAKE_CURRENT_BINARY_DIR} ) +set(PartitionModule_basic_SRC + ${PartitionModule_SOURCE_DIR}/core/OsproberEntry.cpp + ${PartitionModule_SOURCE_DIR}/core/PartitionInfo.cpp + ${PartitionModule_SOURCE_DIR}/core/PartUtils.cpp + ) + calamares_add_test( partitionjobtest SOURCES @@ -40,10 +46,9 @@ calamares_add_test( partitioncreatelayoutstest SOURCES CreateLayoutsTests.cpp + ${PartitionModule_basic_SRC} ${PartitionModule_SOURCE_DIR}/core/KPMHelpers.cpp - ${PartitionModule_SOURCE_DIR}/core/PartitionInfo.cpp ${PartitionModule_SOURCE_DIR}/core/PartitionLayout.cpp - ${PartitionModule_SOURCE_DIR}/core/PartUtils.cpp ${PartitionModule_SOURCE_DIR}/core/DeviceModel.cpp LIBRARIES calamares::kpmcore Calamares::calamaresui DEFINITIONS ${_partition_defs} @@ -66,9 +71,8 @@ calamares_add_test( partitionconfigtest SOURCES ConfigTests.cpp + ${PartitionModule_basic_SRC} ${PartitionModule_SOURCE_DIR}/core/DeviceModel.cpp - ${PartitionModule_SOURCE_DIR}/core/PartitionInfo.cpp - ${PartitionModule_SOURCE_DIR}/core/PartUtils.cpp ${PartitionModule_SOURCE_DIR}/Config.cpp LIBRARIES calamares::kpmcore Calamares::calamaresui DEFINITIONS diff --git a/src/modules/partition/tests/ConfigTests.cpp b/src/modules/partition/tests/ConfigTests.cpp index eae912f71..0e030d57e 100644 --- a/src/modules/partition/tests/ConfigTests.cpp +++ b/src/modules/partition/tests/ConfigTests.cpp @@ -9,6 +9,7 @@ #include "Config.h" +#include "core/OsproberEntry.h" #include "core/PartUtils.h" #include "GlobalStorage.h" @@ -17,6 +18,7 @@ #include "utils/System.h" #include "utils/Yaml.h" +#include #include #include @@ -35,6 +37,9 @@ private Q_SLOTS: void testLegacySize(); void testAll(); void testWeirdConfig(); + + void testNormalFstab(); + void testWeirdFstab(); }; ConfigTests::ConfigTests() = default; @@ -222,6 +227,48 @@ ConfigTests::testWeirdConfig() } } +void +ConfigTests::testNormalFstab() +{ + const auto contents + = QByteArrayLiteral( "# A FreeBSD fstab\n" + "/dev/nvd0p3 none swap sw 0 0\n" ); + const auto entries = Calamares::fromEtcFstabContents( contents ); + for ( const auto& e : entries ) + { + QVERIFY( e.isValid() ); + } + QCOMPARE( entries.count(), 1 ); +} + +void +ConfigTests::testWeirdFstab() +{ + const auto contents + = QByteArrayLiteral( "# \n" + "UUID=dae80d0a-f6c7-46f4-a04a-6761f2cfd9b6 / ext4 defaults,noatime 0 1\n" + "UUID=423892d5-a929-41a9-a846-f410cf3fe25b swap swap defaults,noatime 0 2\n" + "# another comment\n" + "borked 2\n" + "ok /dev1 ext4 none 0 0\n" + "bogus /dev2 ext4 none no later\n" + "# comment\n" ); + const auto entries = Calamares::fromEtcFstabContents( contents ); + QCOMPARE( entries.count(), 4 ); + + QStringList mountPoints; + for ( const auto& e : entries ) + { + mountPoints.append( e.mountPoint ); + } + mountPoints.sort(); + QCOMPARE( mountPoints, + QStringList() << "/" + << "/dev1" + << "/dev2" + << "swap" ); +} + QTEST_GUILESS_MAIN( ConfigTests )