[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

@@ -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

src/modules/partition/core/OsproberEntry.cpp

@@ -0,0 +1,63 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2015-2016 Teo Mrnjavac <teo@kde.org>
+ *   SPDX-FileCopyrightText: 2018-2019, 2024 Adriaan de Groot <groot@kde.org>
+ *   SPDX-FileCopyrightText: 2019 Collabora Ltd <arnaud.ferraris@collabora.com>
+ *   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

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;

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' );
+            const auto fstabLines = QString::fromLocal8Bit( fstabFile.readAll() ).split( '\n' );
-
-            for ( const QString& rawLine : fstabLines )
-            {
-                fstabEntries.append( FstabEntry::fromEtcFstab( rawLine ) );
-            }
             fstabFile.close();
-            const int lineCount = fstabEntries.count();
+
-            const auto invalidEntries = std::remove_if(
+            const auto fstabEntries = Calamares::fromEtcFstabContents( fstabLines );
-                fstabEntries.begin(), fstabEntries.end(), []( const FstabEntry& x ) { return !x.isValid(); } );
+            cDebug() << Logger::SubEntry << "got" << fstabEntries.count() << "fstab entries from" << fstabLines.count()
-            fstabEntries.erase(invalidEntries);
-            cDebug() << Logger::SubEntry << "got" << fstabEntries.count() << "fstab entries from" << lineCount
                      << "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
-    };
-}

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

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 <QByteArray>
 #include <QObject>
 #include <QtTest/QtTest>
 
@@ -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( "# <file system>             <mount point>  <type>  <options>  <dump>  <pass>\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 )