From bc2713ccbbb857ece8329b1b0eb0ef68e17e4d50 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 8 Dec 2021 14:08:37 +0100 Subject: [PATCH 01/22] [libcalamares] Add string functions for lstrip() and rstrip()-like --- src/libcalamares/utils/String.cpp | 21 ++++++++++ src/libcalamares/utils/String.h | 13 ++++++ src/libcalamares/utils/Tests.cpp | 68 +++++++++++++++++++++++++++++-- 3 files changed, 99 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/utils/String.cpp b/src/libcalamares/utils/String.cpp index 615d30309..c88c23693 100644 --- a/src/libcalamares/utils/String.cpp +++ b/src/libcalamares/utils/String.cpp @@ -224,5 +224,26 @@ truncateMultiLine( const QString& string, CalamaresUtils::LinesStartEnd lines, C return front + back.right( chars.total / 2 ); } +void +removeLeading( QString& string, QChar c ) +{ + int count = 0; + while ( string.length() > count && string[ count ] == c ) + { + count++; + } + string.remove( 0, count ); +} + +void +removeTrailing( QString& string, QChar c ) +{ + int lastIndex = string.length(); + while ( lastIndex > 0 && string[ lastIndex - 1 ] == c ) + { + lastIndex--; + } + string.remove( lastIndex, string.length() ); +} } // namespace CalamaresUtils diff --git a/src/libcalamares/utils/String.h b/src/libcalamares/utils/String.h index e08255f86..1adc2336a 100644 --- a/src/libcalamares/utils/String.h +++ b/src/libcalamares/utils/String.h @@ -100,6 +100,19 @@ DLLEXPORT QString truncateMultiLine( const QString& string, LinesStartEnd lines = LinesStartEnd { 3, 5 }, CharCount chars = CharCount { 812 } ); +/** @brief Remove all @p c at the beginning of @p string + * + * Modifies the @p string in-place. If @p c is not the first character + * of @p string, the string is left unchanged; otherwise the first character + * is removed and the process repeats. + */ +DLLEXPORT void removeLeading( QString& string, QChar c ); +/** @brief Remove all @p c at the end of @p string + * + * Like removeLeading(), but at the end of the string. + */ +DLLEXPORT void removeTrailing( QString& string, QChar c ); + } // namespace CalamaresUtils #endif diff --git a/src/libcalamares/utils/Tests.cpp b/src/libcalamares/utils/Tests.cpp index 8fd11cd87..3dde75338 100644 --- a/src/libcalamares/utils/Tests.cpp +++ b/src/libcalamares/utils/Tests.cpp @@ -70,6 +70,10 @@ private Q_SLOTS: void testStringTruncation(); void testStringTruncationShorter(); void testStringTruncationDegenerate(); + void testStringRemoveLeading_data(); + void testStringRemoveLeading(); + void testStringRemoveTrailing_data(); + void testStringRemoveTrailing(); /** @section Test Runner directory-manipulation. */ void testRunnerDirs(); @@ -754,6 +758,64 @@ LibCalamaresTests::testStringTruncationDegenerate() } } +void +LibCalamaresTests::testStringRemoveLeading_data() +{ + QTest::addColumn< QString >( "string" ); + QTest::addColumn< char >( "c" ); + QTest::addColumn< QString >( "result" ); + + QTest::newRow( "empty" ) << QString() << '/' << QString(); + QTest::newRow( "one-slash" ) << QStringLiteral( "/tmp" ) << '/' << QStringLiteral( "tmp" ); + QTest::newRow( "two-slash" ) << QStringLiteral( "//tmp" ) << '/' << QStringLiteral( "tmp" ); + QTest::newRow( "multi-slash" ) << QStringLiteral( "/tmp/p" ) << '/' << QStringLiteral( "tmp/p" ); + QTest::newRow( "later-slash" ) << QStringLiteral( "@/tmp" ) << '/' << QStringLiteral( "@/tmp" ); + QTest::newRow( "all-one-slash" ) << QStringLiteral( "/" ) << '/' << QString(); + QTest::newRow( "all-many-slash" ) << QStringLiteral( "////////////////////" ) << '/' << QString(); + QTest::newRow( "trailing" ) << QStringLiteral( "tmp/" ) << '/' << QStringLiteral( "tmp/" ); +} + +void +LibCalamaresTests::testStringRemoveLeading() +{ + QFETCH( QString, string ); + QFETCH( char, c ); + QFETCH( QString, result ); + + const QString initial = string; + CalamaresUtils::removeLeading( string, c ); + QCOMPARE( string, result ); +} + +void +LibCalamaresTests::testStringRemoveTrailing_data() +{ + QTest::addColumn< QString >( "string" ); + QTest::addColumn< char >( "c" ); + QTest::addColumn< QString >( "result" ); + + QTest::newRow( "empty" ) << QString() << '/' << QString(); + QTest::newRow( "one-slash" ) << QStringLiteral( "/tmp" ) << '/' << QStringLiteral( "/tmp" ); + QTest::newRow( "two-slash" ) << QStringLiteral( "//tmp" ) << '/' << QStringLiteral( "//tmp" ); + QTest::newRow( "multi-slash" ) << QStringLiteral( "/tmp//p/" ) << '/' << QStringLiteral( "/tmp//p" ); + QTest::newRow( "later-slash" ) << QStringLiteral( "@/tmp/@" ) << '/' << QStringLiteral( "@/tmp/@" ); + QTest::newRow( "later-slash2" ) << QStringLiteral( "@/tmp/@//" ) << '/' << QStringLiteral( "@/tmp/@" ); + QTest::newRow( "all-one-slash" ) << QStringLiteral( "/" ) << '/' << QString(); + QTest::newRow( "all-many-slash" ) << QStringLiteral( "////////////////////" ) << '/' << QString(); + QTest::newRow( "trailing" ) << QStringLiteral( "tmp/" ) << '/' << QStringLiteral( "tmp" ); +} + +void +LibCalamaresTests::testStringRemoveTrailing() +{ + QFETCH( QString, string ); + QFETCH( char, c ); + QFETCH( QString, result ); + + const QString initial = string; + CalamaresUtils::removeTrailing( string, c ); + QCOMPARE( string, result ); +} static QString dirname( const QTemporaryDir& d ) @@ -1001,10 +1063,10 @@ LibCalamaresTests::testReadWriteFile() { auto fullPath = ss->createTargetFile( "test0", otherContents ); QVERIFY( !fullPath ); // Failed, because it won't overwrite - QCOMPARE( fullPath.code(), decltype(fullPath)::Code::AlreadyExists ); - QVERIFY( fullPath.path().isEmpty() ); // Because it wasn't written + QCOMPARE( fullPath.code(), decltype( fullPath )::Code::AlreadyExists ); + QVERIFY( fullPath.path().isEmpty() ); // Because it wasn't written - QFileInfo fi( tempRoot.filePath( "test0" ) ); // Compute the name some other way + QFileInfo fi( tempRoot.filePath( "test0" ) ); // Compute the name some other way QVERIFY( fi.exists() ); QVERIFY( fi.isFile() ); QCOMPARE( fi.size(), 0 ); From 1d96c5af4615d17fdc744538bcf31b62d8f04ea2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 5 Dec 2021 01:32:51 +0100 Subject: [PATCH 02/22] [partition] Table type 'none' is a late addition. --- src/modules/partition/gui/DeviceInfoWidget.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/modules/partition/gui/DeviceInfoWidget.cpp b/src/modules/partition/gui/DeviceInfoWidget.cpp index 05a8c4b63..39d9413e1 100644 --- a/src/modules/partition/gui/DeviceInfoWidget.cpp +++ b/src/modules/partition/gui/DeviceInfoWidget.cpp @@ -100,7 +100,9 @@ DeviceInfoWidget::retranslateUi() "that makes a file accessible as a block device. " "This kind of setup usually only contains a single filesystem." ); break; +#if defined( WITH_KPMCORE42API ) case PartitionTable::none: +#endif case PartitionTable::unknownTableType: typeString = " ? "; toolTipString = tr( "This installer cannot detect a partition table on the " From c834a5066d079bd8f554bab3fc463c0f32cdfb73 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 5 Dec 2021 02:26:23 +0100 Subject: [PATCH 03/22] [umount] Make it much more clear that the logfiles-thing is going away. --- CHANGES-3.2 | 6 ++++++ src/modules/umount/main.py | 1 + src/modules/umount/umount.conf | 18 +++++++++++------- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index b658191fb..566a61ed9 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -13,6 +13,9 @@ This release contains contributions from (alphabetically by first name): - Artem Grinev - Evan James +Distributions are **specifically** reminded to update the *umount* module +configuration (and to use *preservefiles* if needed). + ## Core ## - Errors (e.g. when an installation fails for whatever reason) are displayed in a dialog with a scrollable details panel, rather than growing up @@ -21,6 +24,9 @@ This release contains contributions from (alphabetically by first name): ## Modules ## - *partition* now supports "deep" btrfs subvolume names, e.g. a separate subvolume for `/usr/local`. (Thanks Evan) + - The *umount* module now warns if the "preserve log file" feature is used. + This has been deprecated for a long time: use the *preservefiles* module + instead. A future release will turn this into an error. # 3.2.48 (2021-12-03) # diff --git a/src/modules/umount/main.py b/src/modules/umount/main.py index 77ea91e34..5fdb36014 100644 --- a/src/modules/umount/main.py +++ b/src/modules/umount/main.py @@ -80,6 +80,7 @@ def run(): if(libcalamares.job.configuration and "srcLog" in libcalamares.job.configuration and "destLog" in libcalamares.job.configuration): + libcalamares.utils.warning("Log-file preserving is **deprecated** in the *umount* module") log_source = libcalamares.job.configuration["srcLog"] log_destination = libcalamares.job.configuration["destLog"] # Relocate log_destination into target system diff --git a/src/modules/umount/umount.conf b/src/modules/umount/umount.conf index b6d86e353..062f7ac4b 100644 --- a/src/modules/umount/umount.conf +++ b/src/modules/umount/umount.conf @@ -10,16 +10,20 @@ # The "copy log files" functionality is deprecated; use the *preservefiles* # module instead, which is more flexible. # -# This module has two configuration keys: -# srcLog location in the live system where the log is -# destLog location in the target system to copy the log # --- -# example when using the normal Calamares log: -srcLog: "/root/.cache/calamares/session.log" -destLog: "/var/log/Calamares.log" - +# This is a **deprecated** example. Use the *preservefiles* module +# instead, where the equivalent configuration is this: +# +# files: +# - from: log +# dest: /var/log/installation.log +# +# Note that the "equivalent configuration" always finds the log, +# and is not dependent on specific user names or the vagaries of +# polkit configuration -- so it is a **better** "equivalent". +# # example when using a log created by `sudo calamares -d`: #srcLog: "/home/live/installation.log" #destLog: "/var/log/installation.log" From b4afedc79e49fc678ba23d5d93a18756a89c2593 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 10 Dec 2021 15:46:11 +0100 Subject: [PATCH 04/22] Changes: pre-release housekeeping --- CHANGES-3.2 | 4 +++- CMakeLists.txt | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index 566a61ed9..cb342ed53 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -7,7 +7,7 @@ contributors are listed. Note that Calamares does not have a historical changelog -- this log starts with version 3.2.0. The release notes on the website will have to do for older versions. -# 3.2.49 (unreleased) # +# 3.2.49 (2021-12-10) # This release contains contributions from (alphabetically by first name): - Artem Grinev @@ -22,6 +22,8 @@ configuration (and to use *preservefiles* if needed). to the size of the screen. (Thanks Artem) ## Modules ## + - *bootloader* better supports multiple installations of the same OS. + - *mount* supports btrfs subvolumes on subdirectories of / now. - *partition* now supports "deep" btrfs subvolume names, e.g. a separate subvolume for `/usr/local`. (Thanks Evan) - The *umount* module now warns if the "preserve log file" feature is used. diff --git a/CMakeLists.txt b/CMakeLists.txt index 1a55eae1c..03dfdad22 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,7 +45,7 @@ project( CALAMARES LANGUAGES C CXX ) -set( CALAMARES_VERSION_RC 1 ) # Set to 0 during release cycle, 1 during development +set( CALAMARES_VERSION_RC 0 ) # Set to 0 during release cycle, 1 during development if( CALAMARES_VERSION_RC EQUAL 1 AND CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR ) message( FATAL_ERROR "Do not build development versions in the source-directory." ) endif() From 3cdb019de7f2ad6c5cfa92a39a70212a3a3ec42a Mon Sep 17 00:00:00 2001 From: Calamares CI Date: Fri, 10 Dec 2021 15:55:58 +0100 Subject: [PATCH 05/22] i18n: [calamares] Automatic merge of Transifex translations --- lang/calamares_sv.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/calamares_sv.ts b/lang/calamares_sv.ts index 64946e710..bd6add603 100644 --- a/lang/calamares_sv.ts +++ b/lang/calamares_sv.ts @@ -703,7 +703,7 @@ Alla ändringar kommer att gå förlorade. Successfully closed mapper device %1. - + Framgångsrikt stängde krypterad enhet %1. From 7ac42b5f4060f97dc560891d4a3178feb7c41362 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 10 Dec 2021 16:44:01 +0100 Subject: [PATCH 06/22] [umount] Tests don't like an empty config - modules with no configuration should be marked 'noconfig', but umount is special: it has no **useful** configuration (maybe no **non-deprecated** configuration), but isn't marked 'noconfig' **yet**. --- src/modules/umount/umount.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/src/modules/umount/umount.conf b/src/modules/umount/umount.conf index 062f7ac4b..7c11f4db6 100644 --- a/src/modules/umount/umount.conf +++ b/src/modules/umount/umount.conf @@ -27,3 +27,4 @@ # example when using a log created by `sudo calamares -d`: #srcLog: "/home/live/installation.log" #destLog: "/var/log/installation.log" +srcLog: "/bogus/just/do/not/use/this/anymore.txt" From adaed528183fb4f245c374cd5b535b850ae10944 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 10 Dec 2021 17:01:42 +0100 Subject: [PATCH 07/22] Changes: post-release housekeeping --- CHANGES-3.2 | 12 ++++++++++++ CMakeLists.txt | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index cb342ed53..5f032ab58 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -7,6 +7,18 @@ contributors are listed. Note that Calamares does not have a historical changelog -- this log starts with version 3.2.0. The release notes on the website will have to do for older versions. +# 3.2.50 (unreleased) # + +This release contains contributions from (alphabetically by first name): + - No external contributors yet + +## Core ## + - No core changes yet + +## Modules ## + - No module changes yet + + # 3.2.49 (2021-12-10) # This release contains contributions from (alphabetically by first name): diff --git a/CMakeLists.txt b/CMakeLists.txt index 03dfdad22..15f40803e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,11 +41,11 @@ # TODO:3.3: Require CMake 3.12 cmake_minimum_required( VERSION 3.3 FATAL_ERROR ) project( CALAMARES - VERSION 3.2.49 + VERSION 3.2.50 LANGUAGES C CXX ) -set( CALAMARES_VERSION_RC 0 ) # Set to 0 during release cycle, 1 during development +set( CALAMARES_VERSION_RC 1 ) # Set to 0 during release cycle, 1 during development if( CALAMARES_VERSION_RC EQUAL 1 AND CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR ) message( FATAL_ERROR "Do not build development versions in the source-directory." ) endif() From 03da766b396d57d6dc050ee1c5c25e5d1a73f862 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 11 Dec 2021 13:19:08 +0100 Subject: [PATCH 08/22] [partition] Keep 64-bit integers for swap sizes FIXES #1849 --- src/modules/partition/core/PartitionActions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/core/PartitionActions.cpp b/src/modules/partition/core/PartitionActions.cpp index 8226499b4..a56446a39 100644 --- a/src/modules/partition/core/PartitionActions.cpp +++ b/src/modules/partition/core/PartitionActions.cpp @@ -71,7 +71,7 @@ swapSuggestion( const qint64 availableSpaceB, Config::SwapChoice swap ) // Allow for a fudge factor - suggestedSwapSizeB = qRound( suggestedSwapSizeB * overestimationFactor ); + suggestedSwapSizeB = qRound64( suggestedSwapSizeB * overestimationFactor ); // don't use more than 10% of available space if ( !ensureSuspendToDisk ) From db86c2463858efae726178860183c2d8dfb5ded0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 11 Dec 2021 13:23:23 +0100 Subject: [PATCH 09/22] Changes: pre-hotfix-release housekeeping --- CHANGES-3.2 | 12 +++--------- CMakeLists.txt | 4 ++-- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index 5f032ab58..bd04ee64c 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -7,16 +7,10 @@ contributors are listed. Note that Calamares does not have a historical changelog -- this log starts with version 3.2.0. The release notes on the website will have to do for older versions. -# 3.2.50 (unreleased) # +# 3.2.49.1 (2021-12-11) # -This release contains contributions from (alphabetically by first name): - - No external contributors yet - -## Core ## - - No core changes yet - -## Modules ## - - No module changes yet +This is a hot-fix release, to fix a regression in the calculation of +swap-size. Reported by EndeavourOS (Joe Kamprad) and Xero Linux. # 3.2.49 (2021-12-10) # diff --git a/CMakeLists.txt b/CMakeLists.txt index 15f40803e..5ad72ae47 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,11 +41,11 @@ # TODO:3.3: Require CMake 3.12 cmake_minimum_required( VERSION 3.3 FATAL_ERROR ) project( CALAMARES - VERSION 3.2.50 + VERSION 3.2.49.1 LANGUAGES C CXX ) -set( CALAMARES_VERSION_RC 1 ) # Set to 0 during release cycle, 1 during development +set( CALAMARES_VERSION_RC 0 ) # Set to 0 during release cycle, 1 during development if( CALAMARES_VERSION_RC EQUAL 1 AND CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR ) message( FATAL_ERROR "Do not build development versions in the source-directory." ) endif() From 132ebd2c2dfbab92c2b4b1d8e072f51f79ebc025 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 11 Dec 2021 15:12:51 +0100 Subject: [PATCH 10/22] [networkcfg] NetworkManager files are UTF-8 encoded The filenames don't matter, but the contents of the file are also UTF-8, and depending on the default encoding of the Python interpreter, this can fail on non-ASCII characters in the file. Set the encoding explicitly while reading and writing the NetworkManager configuration files. FIXES #1848 --- src/modules/networkcfg/main.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/networkcfg/main.py b/src/modules/networkcfg/main.py index 0ee47c1aa..807fdf613 100644 --- a/src/modules/networkcfg/main.py +++ b/src/modules/networkcfg/main.py @@ -73,12 +73,12 @@ def replace_username(nm_config_filename, live_user, target_user): if not os.path.exists(nm_config_filename): return - with open(nm_config_filename, "r") as network_conf: + with open(nm_config_filename, "r", encoding="UTF-8") as network_conf: text = network_conf.readlines() live_permissions = 'permissions=user:{}:;'.format(live_user) target_permissions = 'permissions=user:{}:;\n'.format(target_user) - with open(nm_config_filename, "w") as network_conf: + with open(nm_config_filename, "w", encoding="UTF-8") as network_conf: for line in text: if live_permissions in line: line = target_permissions From 6261f8a5cb420e38521f2cb4aeb78009e1f79de7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 11 Dec 2021 15:31:59 +0100 Subject: [PATCH 11/22] Changes: post-release housekeeping --- CHANGES-3.2 | 15 +++++++++++++++ CMakeLists.txt | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index bd04ee64c..cca35f9e7 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -7,6 +7,21 @@ contributors are listed. Note that Calamares does not have a historical changelog -- this log starts with version 3.2.0. The release notes on the website will have to do for older versions. +# 3.2.50 (unreleased) # + +This release contains contributions from (alphabetically by first name): + - No external contributors yet + +## Core ## + - No core changes yet + +## Modules ## + - *networkcfg* could fail to update the NetworkManager configuration + if the SSID or username contained non-ASCII characters **and** the + default Python text-file encoding was set to ASCII. The files are + now read and written in UTF-8, explicitly. #1848 + + # 3.2.49.1 (2021-12-11) # This is a hot-fix release, to fix a regression in the calculation of diff --git a/CMakeLists.txt b/CMakeLists.txt index 5ad72ae47..15f40803e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,11 +41,11 @@ # TODO:3.3: Require CMake 3.12 cmake_minimum_required( VERSION 3.3 FATAL_ERROR ) project( CALAMARES - VERSION 3.2.49.1 + VERSION 3.2.50 LANGUAGES C CXX ) -set( CALAMARES_VERSION_RC 0 ) # Set to 0 during release cycle, 1 during development +set( CALAMARES_VERSION_RC 1 ) # Set to 0 during release cycle, 1 during development if( CALAMARES_VERSION_RC EQUAL 1 AND CMAKE_SOURCE_DIR STREQUAL CMAKE_BINARY_DIR ) message( FATAL_ERROR "Do not build development versions in the source-directory." ) endif() From 5b225cf960e9c948f590f3c3f4d8775ad073cdf5 Mon Sep 17 00:00:00 2001 From: arcolinuxz Date: Sat, 11 Dec 2021 18:11:45 +0100 Subject: [PATCH 12/22] [preservefiles] Put the logs in /var/log --- CHANGES-3.2 | 2 +- src/modules/preservefiles/preservefiles.conf | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index cca35f9e7..93231b57a 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -10,7 +10,7 @@ website will have to do for older versions. # 3.2.50 (unreleased) # This release contains contributions from (alphabetically by first name): - - No external contributors yet + - Erik Dubois ## Core ## - No core changes yet diff --git a/src/modules/preservefiles/preservefiles.conf b/src/modules/preservefiles/preservefiles.conf index 6af0872d7..d862e35e9 100644 --- a/src/modules/preservefiles/preservefiles.conf +++ b/src/modules/preservefiles/preservefiles.conf @@ -34,15 +34,13 @@ # - *log*, for the complete log file (up to the moment the preservefiles # module is run), # - *config*, for a JSON dump of the contents of global storage ---- files: - - /etc/oem-information - from: log - dest: /root/install.log - perm: root:wheel:644 + dest: /var/log/Calamares.log + perm: root:wheel:600 - from: config - dest: /root/install.json - perm: root:wheel:400 + dest: /var/log/Camamares-install.json + perm: root:wheel:600 # The *perm* key contains a default value to apply to all files listed # above that do not have a *perm* key of their own. If not set, From becb1d57103eb0f423404f6e91ac6c14f119ba4d Mon Sep 17 00:00:00 2001 From: Johannes Kamprad Date: Sun, 12 Dec 2021 01:22:22 +0100 Subject: [PATCH 13/22] Update preservefiles.conf --- src/modules/preservefiles/preservefiles.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/preservefiles/preservefiles.conf b/src/modules/preservefiles/preservefiles.conf index d862e35e9..1c23a93de 100644 --- a/src/modules/preservefiles/preservefiles.conf +++ b/src/modules/preservefiles/preservefiles.conf @@ -39,7 +39,7 @@ files: dest: /var/log/Calamares.log perm: root:wheel:600 - from: config - dest: /var/log/Camamares-install.json + dest: /var/log/Calamares-install.json perm: root:wheel:600 # The *perm* key contains a default value to apply to all files listed From b1ecbb4151f22a74be4b4997dbbafbc77ef8b9ba Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 12 Dec 2021 00:33:55 +0100 Subject: [PATCH 14/22] [preservefiles] Start cleanup of structure, polymorphism --- src/modules/preservefiles/PreserveFiles.cpp | 51 +++++++++++++++------ src/modules/preservefiles/PreserveFiles.h | 25 ++-------- 2 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index b235aac93..d46563185 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -18,6 +18,30 @@ #include +enum class ItemType +{ + None, + Path, + Log, + Config +}; + +struct Item +{ + QString source; + QString dest; + CalamaresUtils::Permissions perm; + ItemType type; + + Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t ) + : source( src ) + , dest( d ) + , perm( std::move( p ) ) + , type( t ) + { + } +}; + using namespace CalamaresUtils::Units; QString @@ -113,7 +137,7 @@ copy_file( const QString& source, const QString& dest ) Calamares::JobResult PreserveFiles::exec() { - if ( m_items.isEmpty() ) + if ( m_items.empty() ) { return Calamares::JobResult::error( tr( "No files configured to save for later." ) ); } @@ -124,18 +148,18 @@ PreserveFiles::exec() prefix.append( '/' ); } - int count = 0; - for ( const auto& it : m_items ) + size_t count = 0; + for ( const auto& it : qAsConst( m_items ) ) { - QString source = it.source; - QString bare_dest = atReplacements( it.dest ); + QString source = it->source; + QString bare_dest = atReplacements( it->dest ); QString dest = prefix + bare_dest; - if ( it.type == ItemType::Log ) + if ( it->type == ItemType::Log ) { source = Logger::logFile(); } - if ( it.type == ItemType::Config ) + if ( it->type == ItemType::Config ) { if ( !Calamares::JobQueue::instance()->globalStorage()->saveJson( dest ) ) { @@ -154,9 +178,9 @@ PreserveFiles::exec() { if ( copy_file( source, dest ) ) { - if ( it.perm.isValid() ) + if ( it->perm.isValid() ) { - if ( !it.perm.apply( CalamaresUtils::System::instance()->targetPath( bare_dest ) ) ) + if ( !it->perm.apply( CalamaresUtils::System::instance()->targetPath( bare_dest ) ) ) { cWarning() << "Could not set attributes of" << bare_dest; } @@ -167,7 +191,7 @@ PreserveFiles::exec() } } - return count == m_items.count() + return count == m_items.size() ? Calamares::JobResult::ok() : Calamares::JobResult::error( tr( "Not all of the configured files could be preserved." ) ); } @@ -202,8 +226,8 @@ PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap ) { QString filename = li.toString(); if ( !filename.isEmpty() ) - m_items.append( - Item { filename, filename, CalamaresUtils::Permissions( defaultPermissions ), ItemType::Path } ); + m_items.push_back( std::make_unique< Item >( + filename, filename, CalamaresUtils::Permissions( defaultPermissions ), ItemType::Path ) ); else { cDebug() << "Empty filename for preservefiles, item" << c; @@ -231,7 +255,8 @@ PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap ) } else { - m_items.append( Item { QString(), dest, CalamaresUtils::Permissions( perm ), t } ); + m_items.push_back( + std::make_unique< Item >( QString(), dest, CalamaresUtils::Permissions( perm ), t ) ); } } else diff --git a/src/modules/preservefiles/PreserveFiles.h b/src/modules/preservefiles/PreserveFiles.h index 7a0aab34d..a54005254 100644 --- a/src/modules/preservefiles/PreserveFiles.h +++ b/src/modules/preservefiles/PreserveFiles.h @@ -13,31 +13,16 @@ #include "utils/Permissions.h" #include "utils/PluginFactory.h" -#include -#include -#include +#include +#include + +struct Item; class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob { Q_OBJECT - enum class ItemType - { - None, - Path, - Log, - Config - }; - - struct Item - { - QString source; - QString dest; - CalamaresUtils::Permissions perm; - ItemType type; - }; - - using ItemList = QList< Item >; + using ItemList = std::vector< std::unique_ptr< Item > >; public: explicit PreserveFiles( QObject* parent = nullptr ); From 238672ef78a43e4261bedf2ac419c3f0ac606952 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 13:00:28 +0100 Subject: [PATCH 15/22] [preservefiles] Split file-items into separate header Put the Item class in a separate header; give it functionality to create itself from Variants (e.g. from the configuration data) and to run itself (do whatever the item is supposed to do). This makes the polymorphic approach unnecessary: we just have items that are sufficiently smart. This moves do-a-thing to the Item, while the Job now has one job: be a loop around creating Items and running items. --- src/modules/preservefiles/CMakeLists.txt | 1 + src/modules/preservefiles/Item.cpp | 160 +++++++++++++++++ src/modules/preservefiles/Item.h | 72 ++++++++ src/modules/preservefiles/PreserveFiles.cpp | 184 ++------------------ src/modules/preservefiles/PreserveFiles.h | 8 +- 5 files changed, 249 insertions(+), 176 deletions(-) create mode 100644 src/modules/preservefiles/Item.cpp create mode 100644 src/modules/preservefiles/Item.h diff --git a/src/modules/preservefiles/CMakeLists.txt b/src/modules/preservefiles/CMakeLists.txt index 820c50a2b..e5cbe819c 100644 --- a/src/modules/preservefiles/CMakeLists.txt +++ b/src/modules/preservefiles/CMakeLists.txt @@ -9,6 +9,7 @@ calamares_add_plugin( preservefiles TYPE job EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES + Item.cpp PreserveFiles.cpp # REQUIRES mount # To set the rootMountPoint SHARED_LIB diff --git a/src/modules/preservefiles/Item.cpp b/src/modules/preservefiles/Item.cpp new file mode 100644 index 000000000..bc2357876 --- /dev/null +++ b/src/modules/preservefiles/Item.cpp @@ -0,0 +1,160 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2018, 2021 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + */ + +#include "Item.h" + +#include "GlobalStorage.h" +#include "JobQueue.h" +#include "utils/CalamaresUtilsSystem.h" +#include "utils/Logger.h" +#include "utils/Units.h" + +#include + +using namespace CalamaresUtils::Units; + +static bool +copy_file( const QString& source, const QString& dest ) +{ + QFile sourcef( source ); + if ( !sourcef.open( QFile::ReadOnly ) ) + { + cWarning() << "Could not read" << source; + return false; + } + + QFile destf( dest ); + if ( !destf.open( QFile::WriteOnly ) ) + { + sourcef.close(); + cWarning() << "Could not open" << destf.fileName() << "for writing; could not copy" << source; + return false; + } + + QByteArray b; + do + { + b = sourcef.read( 1_MiB ); + destf.write( b ); + } while ( b.count() > 0 ); + + sourcef.close(); + destf.close(); + + return true; +} + +Item +Item::fromVariant( const QVariant& v, const CalamaresUtils::Permissions& defaultPermissions ) +{ + if ( v.type() == QVariant::String ) + { + QString filename = v.toString(); + if ( !filename.isEmpty() ) + { + return { filename, filename, defaultPermissions, ItemType::Path }; + } + else + { + cWarning() << "Empty filename for preservefiles, item" << v; + return {}; + } + } + else if ( v.type() == QVariant::Map ) + { + const auto map = v.toMap(); + + CalamaresUtils::Permissions perm( defaultPermissions ); + ItemType t = ItemType::None; + + { + QString perm_string = map[ "perm" ].toString(); + if ( !perm_string.isEmpty() ) + { + perm = CalamaresUtils::Permissions( perm_string ); + } + } + + { + QString from = map[ "from" ].toString(); + t = ( from == "log" ) ? ItemType::Log : ( from == "config" ) ? ItemType::Config : ItemType::None; + + if ( t == ItemType::None && !map[ "src" ].toString().isEmpty() ) + { + t = ItemType::Path; + } + } + + QString dest = map[ "dest" ].toString(); + if ( dest.isEmpty() ) + { + cWarning() << "Empty dest for preservefiles, item" << v; + return {}; + } + + switch ( t ) + { + case ItemType::Config: + return { QString(), dest, perm, t }; + case ItemType::Log: + return { QString(), dest, perm, t }; + case ItemType::Path: + return { map[ "src" ].toString(), dest, perm, t }; + case ItemType::None: + cWarning() << "Invalid type for preservefiles, item" << v; + return {}; + } + } + else + { + cWarning() << "Invalid type for preservefiles, item" << v; + return {}; + } +} + + +bool +Item::exec( const std::function< QString( QString ) >& replacements ) const +{ + QString expanded_dest = replacements( dest ); + QString full_dest = CalamaresUtils::System::instance()->targetPath( expanded_dest ); + + bool success = false; + switch ( type ) + { + case ItemType::None: + cWarning() << "Invalid item for preservefiles skipped."; + return false; + case ItemType::Config: + if ( !( success = Calamares::JobQueue::instance()->globalStorage()->saveJson( full_dest ) ) ) + { + cWarning() << "Could not write a JSON dump of global storage to" << full_dest; + } + break; + case ItemType::Log: + if ( !( success = copy_file( Logger::logFile(), full_dest ) ) ) + { + cWarning() << "Could not preserve log file to" << full_dest; + } + break; + case ItemType::Path: + if ( !( success = copy_file( source, full_dest ) ) ) + { + cWarning() << "Could not preserve" << source << "to" << full_dest; + } + break; + } + if ( !success ) + { + CalamaresUtils::System::instance()->removeTargetFile( expanded_dest ); + return false; + } + else + { + return perm.apply( full_dest ); + } +} diff --git a/src/modules/preservefiles/Item.h b/src/modules/preservefiles/Item.h new file mode 100644 index 000000000..cd15e2042 --- /dev/null +++ b/src/modules/preservefiles/Item.h @@ -0,0 +1,72 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2018, 2021 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + */ +#ifndef PRESERVEFILES_ITEM_H +#define PRESERVEFILES_ITEM_H + +#include "utils/Permissions.h" + +#include +#include + +#include + +enum class ItemType +{ + None, + Path, + Log, + Config +}; + +/** @brief Represents one item to copy + * + * All item types need a destination (to place the data), this is + * intepreted within the target system. All items need a permission, + * which is applied to the data once written. + * + * The source may be a path, but not all types need a source. + */ +class Item +{ + QString source; + QString dest; + CalamaresUtils::Permissions perm; + ItemType type; + +public: + Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t ) + : source( src ) + , dest( d ) + , perm( std::move( p ) ) + , type( t ) + { + } + + Item() + : type( ItemType::None ) + { + } + + operator bool() const { return type != ItemType::None; } + + bool exec( const std::function< QString( QString ) >& replacements ) const; + + + /** @brief Create an Item -- or one of its subclasses -- from @p v + * + * Depending on the structure and contents of @p v, a pointer + * to an Item is returned. If @p v cannot be interpreted meaningfully, + * then a nullptr is returned. + * + * When the entry contains a *perm* key, use that permission, otherwise + * apply @p defaultPermissions to the item. + */ + static Item fromVariant( const QVariant& v, const CalamaresUtils::Permissions& defaultPermissions ); +}; + + +#endif diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index d46563185..4adde85bb 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -7,70 +7,20 @@ #include "PreserveFiles.h" +#include "Item.h" + #include "CalamaresVersion.h" #include "GlobalStorage.h" #include "JobQueue.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/CommandList.h" #include "utils/Logger.h" -#include "utils/Permissions.h" #include "utils/Units.h" #include -enum class ItemType -{ - None, - Path, - Log, - Config -}; - -struct Item -{ - QString source; - QString dest; - CalamaresUtils::Permissions perm; - ItemType type; - - Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t ) - : source( src ) - , dest( d ) - , perm( std::move( p ) ) - , type( t ) - { - } -}; - using namespace CalamaresUtils::Units; -QString -targetPrefix() -{ - if ( CalamaresUtils::System::instance()->doChroot() ) - { - Calamares::GlobalStorage* gs = Calamares::JobQueue::instance()->globalStorage(); - if ( gs && gs->contains( "rootMountPoint" ) ) - { - QString r = gs->value( "rootMountPoint" ).toString(); - if ( !r.isEmpty() ) - { - return r; - } - else - { - cDebug() << "RootMountPoint is empty"; - } - } - else - { - cDebug() << "No rootMountPoint defined, preserving files to '/'"; - } - } - - return QLatin1String( "/" ); -} - QString atReplacements( QString s ) { @@ -103,37 +53,6 @@ PreserveFiles::prettyName() const return tr( "Saving files for later ..." ); } -static bool -copy_file( const QString& source, const QString& dest ) -{ - QFile sourcef( source ); - if ( !sourcef.open( QFile::ReadOnly ) ) - { - cWarning() << "Could not read" << source; - return false; - } - - QFile destf( dest ); - if ( !destf.open( QFile::WriteOnly ) ) - { - sourcef.close(); - cWarning() << "Could not open" << destf.fileName() << "for writing; could not copy" << source; - return false; - } - - QByteArray b; - do - { - b = sourcef.read( 1_MiB ); - destf.write( b ); - } while ( b.count() > 0 ); - - sourcef.close(); - destf.close(); - - return true; -} - Calamares::JobResult PreserveFiles::exec() { @@ -142,52 +61,20 @@ PreserveFiles::exec() return Calamares::JobResult::error( tr( "No files configured to save for later." ) ); } - QString prefix = targetPrefix(); - if ( !prefix.endsWith( '/' ) ) - { - prefix.append( '/' ); - } - - size_t count = 0; + int count = 0; for ( const auto& it : qAsConst( m_items ) ) { - QString source = it->source; - QString bare_dest = atReplacements( it->dest ); - QString dest = prefix + bare_dest; - - if ( it->type == ItemType::Log ) + if ( !it ) { - source = Logger::logFile(); + // Invalid entries are nullptr, ignore them but count as a success + // because they shouldn't block the installation. There are + // warnings in the log showing what the configuration problem is. + ++count; + continue; } - if ( it->type == ItemType::Config ) + if ( it.exec( atReplacements ) ) { - if ( !Calamares::JobQueue::instance()->globalStorage()->saveJson( dest ) ) - { - cWarning() << "Could not write a JSON dump of global storage to" << dest; - } - else - { - ++count; - } - } - else if ( source.isEmpty() ) - { - cWarning() << "Skipping unnamed source file for" << dest; - } - else - { - if ( copy_file( source, dest ) ) - { - if ( it->perm.isValid() ) - { - if ( !it->perm.apply( CalamaresUtils::System::instance()->targetPath( bare_dest ) ) ) - { - cWarning() << "Could not set attributes of" << bare_dest; - } - } - - ++count; - } + ++count; } } @@ -217,54 +104,11 @@ PreserveFiles::setConfigurationMap( const QVariantMap& configurationMap ) { defaultPermissions = QStringLiteral( "root:root:0400" ); } + CalamaresUtils::Permissions perm( defaultPermissions ); - QVariantList l = files.toList(); - unsigned int c = 0; - for ( const auto& li : l ) + for ( const auto& li : files.toList() ) { - if ( li.type() == QVariant::String ) - { - QString filename = li.toString(); - if ( !filename.isEmpty() ) - m_items.push_back( std::make_unique< Item >( - filename, filename, CalamaresUtils::Permissions( defaultPermissions ), ItemType::Path ) ); - else - { - cDebug() << "Empty filename for preservefiles, item" << c; - } - } - else if ( li.type() == QVariant::Map ) - { - const auto map = li.toMap(); - QString dest = map[ "dest" ].toString(); - QString from = map[ "from" ].toString(); - ItemType t = ( from == "log" ) ? ItemType::Log : ( from == "config" ) ? ItemType::Config : ItemType::None; - QString perm = map[ "perm" ].toString(); - if ( perm.isEmpty() ) - { - perm = defaultPermissions; - } - - if ( dest.isEmpty() ) - { - cDebug() << "Empty dest for preservefiles, item" << c; - } - else if ( t == ItemType::None ) - { - cDebug() << "Invalid type for preservefiles, item" << c; - } - else - { - m_items.push_back( - std::make_unique< Item >( QString(), dest, CalamaresUtils::Permissions( perm ), t ) ); - } - } - else - { - cDebug() << "Invalid type for preservefiles, item" << c; - } - - ++c; + m_items.push_back( Item::fromVariant( li, perm ) ); } } diff --git a/src/modules/preservefiles/PreserveFiles.h b/src/modules/preservefiles/PreserveFiles.h index a54005254..dfd2804e3 100644 --- a/src/modules/preservefiles/PreserveFiles.h +++ b/src/modules/preservefiles/PreserveFiles.h @@ -10,19 +10,15 @@ #include "CppJob.h" #include "DllMacro.h" -#include "utils/Permissions.h" #include "utils/PluginFactory.h" -#include -#include - -struct Item; +class Item; class PLUGINDLLEXPORT PreserveFiles : public Calamares::CppJob { Q_OBJECT - using ItemList = std::vector< std::unique_ptr< Item > >; + using ItemList = QList< Item >; public: explicit PreserveFiles( QObject* parent = nullptr ); From 90f6ea1fc834eeb6d3ccde7539b1f9ce0830e273 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 14:51:42 +0100 Subject: [PATCH 16/22] [preservefiles] polish the documentation --- src/modules/preservefiles/preservefiles.conf | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/modules/preservefiles/preservefiles.conf b/src/modules/preservefiles/preservefiles.conf index 1c23a93de..2853ac037 100644 --- a/src/modules/preservefiles/preservefiles.conf +++ b/src/modules/preservefiles/preservefiles.conf @@ -11,8 +11,9 @@ # is true, then these items are ignored (since the destination is the same # as the source). # - a map with a *dest* key. The *dest* value is a path interpreted in the -# target system (if dontChroot is true, in the host system). Relative paths -# are not recommended. There are three possible other keys in the map: +# target system (if the global *dontChroot* is true, then the host is the +# target as well). Relative paths are not recommended. There are three +# possible other keys in the map: # - *from*, which must have one of the values, below; it is used to # preserve files whose pathname is known to Calamares internally. # - *src*, to refer to a path interpreted in the host system. Relative @@ -23,17 +24,23 @@ # by owner). If set, the file's ownership and permissions are set to # those values within the target system; if not set, no permissions # are changed. -# Only one of the two source keys (either *from* or *src*) may be set. +# Exactly one of the two source keys (either *from* or *src*) must be set. # -# The target filename is modified as follows: -# - `@@ROOT@@` is replaced by the path to the target root (may be /) +# The target path (*dest*) is modified as follows: +# - `@@ROOT@@` is replaced by the path to the target root (may be /). +# There is never any reason to use this, since the *dest* is already +# interpreted in the target system. # - `@@USER@@` is replaced by the username entered by on the user # page (may be empty, for instance if no user page is enabled) # # Special values for the key *from* are: # - *log*, for the complete log file (up to the moment the preservefiles # module is run), -# - *config*, for a JSON dump of the contents of global storage +# - *config*, for a JSON dump of the contents of global storage. +# Note that this may contain sensitive information, and should be +# given restrictive permissions. +# +# files: - from: log dest: /var/log/Calamares.log From 8b5e49d9806b2c9fbfc5149a7e0107bceabfa40b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 15:04:23 +0100 Subject: [PATCH 17/22] [preservefiles] Add (stub) tests --- src/modules/preservefiles/CMakeLists.txt | 9 ++- src/modules/preservefiles/Tests.cpp | 70 ++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 src/modules/preservefiles/Tests.cpp diff --git a/src/modules/preservefiles/CMakeLists.txt b/src/modules/preservefiles/CMakeLists.txt index e5cbe819c..5df637321 100644 --- a/src/modules/preservefiles/CMakeLists.txt +++ b/src/modules/preservefiles/CMakeLists.txt @@ -3,8 +3,6 @@ # SPDX-FileCopyrightText: 2020 Adriaan de Groot # SPDX-License-Identifier: BSD-2-Clause # -include_directories( ${PROJECT_BINARY_DIR}/src/libcalamaresui ) - calamares_add_plugin( preservefiles TYPE job EXPORT_MACRO PLUGINDLLEXPORT_PRO @@ -15,3 +13,10 @@ calamares_add_plugin( preservefiles SHARED_LIB EMERGENCY ) + +calamares_add_test( + preservefilestest + SOURCES + Item.cpp + Tests.cpp +) diff --git a/src/modules/preservefiles/Tests.cpp b/src/modules/preservefiles/Tests.cpp new file mode 100644 index 000000000..3bf4aa833 --- /dev/null +++ b/src/modules/preservefiles/Tests.cpp @@ -0,0 +1,70 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2021 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +#include "Item.h" + +#include "Settings.h" +#include "utils/CalamaresUtilsSystem.h" +#include "utils/Logger.h" +#include "utils/Yaml.h" + +#include + +class PreserveFilesTests : public QObject +{ + Q_OBJECT +public: + PreserveFilesTests(); + ~PreserveFilesTests() override {} + +private Q_SLOTS: + void initTestCase(); + + /* + void testOneUrl(); + void testUrls_data(); + void testUrls(); + + void testBadConfigDoesNotResetUrls(); + */ + void testTrue(); +}; + +PreserveFilesTests::PreserveFilesTests() {} + +void +PreserveFilesTests::initTestCase() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); + cDebug() << "PreserveFiles test started."; + + // Ensure we have a system object, expect it to be a "bogus" one + CalamaresUtils::System* system = CalamaresUtils::System::instance(); + QVERIFY( system ); + cDebug() << Logger::SubEntry << "System @" << Logger::Pointer( system ); + + const auto* settings = Calamares::Settings::instance(); + if ( !settings ) + { + (void)new Calamares::Settings( true ); + } +} + +void +PreserveFilesTests::testTrue() +{ + QVERIFY( true ); +} + + +QTEST_GUILESS_MAIN( PreserveFilesTests ) + +#include "utils/moc-warnings.h" + +#include "Tests.moc" From a1b7ba0dc59cd0d70146cc7872808f68209f7c1e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 15:44:07 +0100 Subject: [PATCH 18/22] [preservefiles] Accessor for item-type (needed for tests) --- src/modules/preservefiles/Item.cpp | 2 +- src/modules/preservefiles/Item.h | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/modules/preservefiles/Item.cpp b/src/modules/preservefiles/Item.cpp index bc2357876..335110b75 100644 --- a/src/modules/preservefiles/Item.cpp +++ b/src/modules/preservefiles/Item.cpp @@ -124,7 +124,7 @@ Item::exec( const std::function< QString( QString ) >& replacements ) const QString full_dest = CalamaresUtils::System::instance()->targetPath( expanded_dest ); bool success = false; - switch ( type ) + switch ( m_type ) { case ItemType::None: cWarning() << "Invalid item for preservefiles skipped."; diff --git a/src/modules/preservefiles/Item.h b/src/modules/preservefiles/Item.h index cd15e2042..9bd8f9613 100644 --- a/src/modules/preservefiles/Item.h +++ b/src/modules/preservefiles/Item.h @@ -35,23 +35,24 @@ class Item QString source; QString dest; CalamaresUtils::Permissions perm; - ItemType type; + ItemType m_type; public: Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t ) : source( src ) , dest( d ) , perm( std::move( p ) ) - , type( t ) + , m_type( t ) { } Item() - : type( ItemType::None ) + : m_type( ItemType::None ) { } - operator bool() const { return type != ItemType::None; } + operator bool() const { return m_type != ItemType::None; } + ItemType type() const { return m_type; } bool exec( const std::function< QString( QString ) >& replacements ) const; From 3be52f8b37b841bcf420626cfcfed931b9175747 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 15:44:32 +0100 Subject: [PATCH 19/22] [preservefiles] Expand tests with reading some existing config-items --- src/modules/preservefiles/Tests.cpp | 43 ++++++++++++++----- src/modules/preservefiles/tests/1a-log.conf | 7 +++ .../preservefiles/tests/1b-config.conf | 6 +++ src/modules/preservefiles/tests/1c-src.conf | 6 +++ .../preservefiles/tests/1d-filename.conf | 6 +++ src/modules/preservefiles/tests/1e-empty.conf | 3 ++ src/modules/preservefiles/tests/1f-bad.conf | 4 ++ 7 files changed, 65 insertions(+), 10 deletions(-) create mode 100644 src/modules/preservefiles/tests/1a-log.conf create mode 100644 src/modules/preservefiles/tests/1b-config.conf create mode 100644 src/modules/preservefiles/tests/1c-src.conf create mode 100644 src/modules/preservefiles/tests/1d-filename.conf create mode 100644 src/modules/preservefiles/tests/1e-empty.conf create mode 100644 src/modules/preservefiles/tests/1f-bad.conf diff --git a/src/modules/preservefiles/Tests.cpp b/src/modules/preservefiles/Tests.cpp index 3bf4aa833..57cefcf9d 100644 --- a/src/modules/preservefiles/Tests.cpp +++ b/src/modules/preservefiles/Tests.cpp @@ -12,6 +12,7 @@ #include "Settings.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" +#include "utils/NamedEnum.h" #include "utils/Yaml.h" #include @@ -26,14 +27,8 @@ public: private Q_SLOTS: void initTestCase(); - /* - void testOneUrl(); - void testUrls_data(); - void testUrls(); - - void testBadConfigDoesNotResetUrls(); - */ - void testTrue(); + void testItems_data(); + void testItems(); }; PreserveFilesTests::PreserveFilesTests() {} @@ -57,11 +52,39 @@ PreserveFilesTests::initTestCase() } void -PreserveFilesTests::testTrue() +PreserveFilesTests::testItems_data() { - QVERIFY( true ); + QTest::addColumn< QString >( "filename" ); + QTest::addColumn< bool >( "ok" ); + QTest::addColumn< int >( "type_i" ); + + QTest::newRow( "log " ) << QString( "1a-log.conf" ) << true << smash( ItemType::Log ); + QTest::newRow( "config " ) << QString( "1b-config.conf" ) << true << smash( ItemType::Config ); + QTest::newRow( "src " ) << QString( "1c-src.conf" ) << true << smash( ItemType::Path ); + QTest::newRow( "filename" ) << QString( "1d-filename.conf" ) << true << smash( ItemType::Path ); + QTest::newRow( "empty " ) << QString( "1e-empty.conf" ) << false << smash( ItemType::None ); + QTest::newRow( "bad " ) << QString( "1f-bad.conf" ) << false << smash( ItemType::None ); } +void +PreserveFilesTests::testItems() +{ + QFETCH( QString, filename ); + QFETCH( bool, ok ); + QFETCH( int, type_i ); + + QFile fi( QString( "%1/tests/%2" ).arg( BUILD_AS_TEST, filename ) ); + QVERIFY( fi.exists() ); + + bool config_file_ok = false; + const auto map = CalamaresUtils::loadYaml( fi, &config_file_ok ); + QVERIFY( config_file_ok ); + + CalamaresUtils::Permissions perm( QStringLiteral( "adridg:adridg:0750" ) ); + auto i = Item::fromVariant( map[ "item" ], perm ); + QCOMPARE( bool( i ), ok ); + QCOMPARE( smash( i.type() ), type_i ); +} QTEST_GUILESS_MAIN( PreserveFilesTests ) diff --git a/src/modules/preservefiles/tests/1a-log.conf b/src/modules/preservefiles/tests/1a-log.conf new file mode 100644 index 000000000..d589d4dfb --- /dev/null +++ b/src/modules/preservefiles/tests/1a-log.conf @@ -0,0 +1,7 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +# +item: + from: log + dest: /var/log/Calamares.log + perm: root:wheel:601 diff --git a/src/modules/preservefiles/tests/1b-config.conf b/src/modules/preservefiles/tests/1b-config.conf new file mode 100644 index 000000000..409dc89d9 --- /dev/null +++ b/src/modules/preservefiles/tests/1b-config.conf @@ -0,0 +1,6 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +item: + from: config + dest: /var/log/Calamares-install.json + perm: root:wheel:600 diff --git a/src/modules/preservefiles/tests/1c-src.conf b/src/modules/preservefiles/tests/1c-src.conf new file mode 100644 index 000000000..130ddd06f --- /dev/null +++ b/src/modules/preservefiles/tests/1c-src.conf @@ -0,0 +1,6 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +item: + src: /root/.cache/calamares/session.log + dest: /var/log/Calamares.log + perm: root:wheel:600 diff --git a/src/modules/preservefiles/tests/1d-filename.conf b/src/modules/preservefiles/tests/1d-filename.conf new file mode 100644 index 000000000..130ddd06f --- /dev/null +++ b/src/modules/preservefiles/tests/1d-filename.conf @@ -0,0 +1,6 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +item: + src: /root/.cache/calamares/session.log + dest: /var/log/Calamares.log + perm: root:wheel:600 diff --git a/src/modules/preservefiles/tests/1e-empty.conf b/src/modules/preservefiles/tests/1e-empty.conf new file mode 100644 index 000000000..183d4e456 --- /dev/null +++ b/src/modules/preservefiles/tests/1e-empty.conf @@ -0,0 +1,3 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +item: [] diff --git a/src/modules/preservefiles/tests/1f-bad.conf b/src/modules/preservefiles/tests/1f-bad.conf new file mode 100644 index 000000000..b2c008955 --- /dev/null +++ b/src/modules/preservefiles/tests/1f-bad.conf @@ -0,0 +1,4 @@ +# SPDX-FileCopyrightText: no +# SPDX-License-Identifier: CC0-1.0 +item: + bop: 1 From 445ed870ccf5be0237b29ea83da708487b702ea3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 15:53:15 +0100 Subject: [PATCH 20/22] [preservefiles] Simplify code to help gcc warnings --- src/modules/preservefiles/Item.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/modules/preservefiles/Item.cpp b/src/modules/preservefiles/Item.cpp index 335110b75..4354c2e08 100644 --- a/src/modules/preservefiles/Item.cpp +++ b/src/modules/preservefiles/Item.cpp @@ -109,11 +109,8 @@ Item::fromVariant( const QVariant& v, const CalamaresUtils::Permissions& default return {}; } } - else - { - cWarning() << "Invalid type for preservefiles, item" << v; - return {}; - } + cWarning() << "Invalid type for preservefiles, item" << v; + return {}; } From 778c2855f41d6d544d196c6ff60ed9ce40ff19f0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 16:34:38 +0100 Subject: [PATCH 21/22] [preservefiles] Introduce the notion of optionally-preserved files --- src/modules/preservefiles/Item.cpp | 10 +++--- src/modules/preservefiles/Item.h | 7 ++-- src/modules/preservefiles/PreserveFiles.cpp | 4 ++- src/modules/preservefiles/preservefiles.conf | 35 +++++++++++++------- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/modules/preservefiles/Item.cpp b/src/modules/preservefiles/Item.cpp index 4354c2e08..2ae929e67 100644 --- a/src/modules/preservefiles/Item.cpp +++ b/src/modules/preservefiles/Item.cpp @@ -12,6 +12,7 @@ #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" #include "utils/Units.h" +#include "utils/Variant.h" #include @@ -56,7 +57,7 @@ Item::fromVariant( const QVariant& v, const CalamaresUtils::Permissions& default QString filename = v.toString(); if ( !filename.isEmpty() ) { - return { filename, filename, defaultPermissions, ItemType::Path }; + return { filename, filename, defaultPermissions, ItemType::Path, false }; } else { @@ -70,6 +71,7 @@ Item::fromVariant( const QVariant& v, const CalamaresUtils::Permissions& default CalamaresUtils::Permissions perm( defaultPermissions ); ItemType t = ItemType::None; + bool optional = CalamaresUtils::getBool( map, "optional", false ); { QString perm_string = map[ "perm" ].toString(); @@ -99,11 +101,11 @@ Item::fromVariant( const QVariant& v, const CalamaresUtils::Permissions& default switch ( t ) { case ItemType::Config: - return { QString(), dest, perm, t }; + return { QString(), dest, perm, t, optional }; case ItemType::Log: - return { QString(), dest, perm, t }; + return { QString(), dest, perm, t, optional }; case ItemType::Path: - return { map[ "src" ].toString(), dest, perm, t }; + return { map[ "src" ].toString(), dest, perm, t, optional }; case ItemType::None: cWarning() << "Invalid type for preservefiles, item" << v; return {}; diff --git a/src/modules/preservefiles/Item.h b/src/modules/preservefiles/Item.h index 9bd8f9613..896b9471f 100644 --- a/src/modules/preservefiles/Item.h +++ b/src/modules/preservefiles/Item.h @@ -35,14 +35,16 @@ class Item QString source; QString dest; CalamaresUtils::Permissions perm; - ItemType m_type; + ItemType m_type = ItemType::None; + bool m_optional = false; public: - Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t ) + Item( const QString& src, const QString& d, CalamaresUtils::Permissions p, ItemType t, bool optional ) : source( src ) , dest( d ) , perm( std::move( p ) ) , m_type( t ) + , m_optional( optional ) { } @@ -53,6 +55,7 @@ public: operator bool() const { return m_type != ItemType::None; } ItemType type() const { return m_type; } + bool isOptional() const { return m_optional; } bool exec( const std::function< QString( QString ) >& replacements ) const; diff --git a/src/modules/preservefiles/PreserveFiles.cpp b/src/modules/preservefiles/PreserveFiles.cpp index 4adde85bb..f904ded8c 100644 --- a/src/modules/preservefiles/PreserveFiles.cpp +++ b/src/modules/preservefiles/PreserveFiles.cpp @@ -72,7 +72,9 @@ PreserveFiles::exec() ++count; continue; } - if ( it.exec( atReplacements ) ) + // Try to preserve the file. If it's marked as optional, count it + // as a success regardless. + if ( it.exec( atReplacements ) || it.isOptional() ) { ++count; } diff --git a/src/modules/preservefiles/preservefiles.conf b/src/modules/preservefiles/preservefiles.conf index 2853ac037..4fb393b2e 100644 --- a/src/modules/preservefiles/preservefiles.conf +++ b/src/modules/preservefiles/preservefiles.conf @@ -7,24 +7,38 @@ # the list should have one of these forms: # # - an absolute path (probably within the host system). This will be preserved -# as the same path within the target system (chroot). If, globally, dontChroot -# is true, then these items are ignored (since the destination is the same -# as the source). +# as the same path within the target system (chroot). If, globally, +# *dontChroot* is true, then these items will be ignored (since the +# destination is the same as the source). # - a map with a *dest* key. The *dest* value is a path interpreted in the # target system (if the global *dontChroot* is true, then the host is the -# target as well). Relative paths are not recommended. There are three -# possible other keys in the map: +# target as well). Relative paths are not recommended. There are two +# ways to select the source data for the file: # - *from*, which must have one of the values, below; it is used to # preserve files whose pathname is known to Calamares internally. # - *src*, to refer to a path interpreted in the host system. Relative # paths are not recommended, and are interpreted relative to where # Calamares is being run. +# Exactly one of the two source keys (either *from* or *src*) must be set. +# +# Special values for the key *from* are: +# - *log*, for the complete log file (up to the moment the preservefiles +# module is run), +# - *config*, for a JSON dump of the contents of global storage. +# Note that this may contain sensitive information, and should be +# given restrictive permissions. +# +# A map with a *dest* key can have these additional fields: # - *perm*, is a colon-separated tuple of :: # where is in octal (e.g. 4777 for wide-open, 0400 for read-only # by owner). If set, the file's ownership and permissions are set to # those values within the target system; if not set, no permissions # are changed. -# Exactly one of the two source keys (either *from* or *src*) must be set. +# - *optional*, is a boolean; if this is set to `true` then failure to +# preserve the file will **not** be counted as a failure of the +# module, and installation will proceed. Set this for files that might +# not exist in the host system (e.g. nvidia configuration files that +# are created in some boot scenarios and not in others). # # The target path (*dest*) is modified as follows: # - `@@ROOT@@` is replaced by the path to the target root (may be /). @@ -33,12 +47,6 @@ # - `@@USER@@` is replaced by the username entered by on the user # page (may be empty, for instance if no user page is enabled) # -# Special values for the key *from* are: -# - *log*, for the complete log file (up to the moment the preservefiles -# module is run), -# - *config*, for a JSON dump of the contents of global storage. -# Note that this may contain sensitive information, and should be -# given restrictive permissions. # # files: @@ -48,6 +56,9 @@ files: - from: config dest: /var/log/Calamares-install.json perm: root:wheel:600 +# - src: /var/log/nvidia.conf +# dest: /var/log/Calamares-nvidia.conf +# optional: true # The *perm* key contains a default value to apply to all files listed # above that do not have a *perm* key of their own. If not set, From d3ed5663d08a8d76054f67e9fa57a72a7eda24dd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 Dec 2021 16:56:07 +0100 Subject: [PATCH 22/22] [preservefiles] Add a schema-file --- .../preservefiles/preservefiles.schema.yaml | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 src/modules/preservefiles/preservefiles.schema.yaml diff --git a/src/modules/preservefiles/preservefiles.schema.yaml b/src/modules/preservefiles/preservefiles.schema.yaml new file mode 100644 index 000000000..65067ea97 --- /dev/null +++ b/src/modules/preservefiles/preservefiles.schema.yaml @@ -0,0 +1,37 @@ +# SPDX-FileCopyrightText: 2020 Adriaan de Groot +# SPDX-License-Identifier: GPL-3.0-or-later +--- +$schema: https://json-schema.org/schema# +$id: https://calamares.io/schemas/preservefiles +additionalProperties: false +type: object +properties: + # TODO: it's a particularly-formatted string + perm: { type: string } + files: + type: array + items: + # There are three entries here because: string, or an entry with + # a src (but no from) or an entry with from (but no src). + anyOf: + - type: string + - type: object + properties: + dest: { type: string } + src: { type: string } + # TODO: it's a particularly-formatted string + perm: { type: string } + optional: { type: boolean } + required: [ dest ] + additionalProperties: false + - type: object + properties: + dest: { type: string } + from: { type: string, enum: [config, log] } + # TODO: it's a particularly-formatted string + perm: { type: string } + optional: { type: boolean } + required: [ dest ] + additionalProperties: false + +required: [ files ]