From 73d09977fc9e333fc693eae4ca77e89de9b95fdd Mon Sep 17 00:00:00 2001 From: abalfoort Date: Thu, 21 Apr 2022 16:39:07 +0200 Subject: [PATCH 01/17] Support unencrypted boot partition --- src/modules/fstab/main.py | 28 ++++++++++++++++--- .../luksbootkeyfile/LuksBootKeyFileJob.cpp | 27 ++++++++++++++++-- 2 files changed, 48 insertions(+), 7 deletions(-) mode change 100644 => 100755 src/modules/fstab/main.py mode change 100644 => 100755 src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp diff --git a/src/modules/fstab/main.py b/src/modules/fstab/main.py old mode 100644 new mode 100755 index 9bc427b13..e65e36445 --- a/src/modules/fstab/main.py +++ b/src/modules/fstab/main.py @@ -142,13 +142,21 @@ class FstabGenerator(object): with open(crypttab_path, "w") as crypttab_file: print(CRYPTTAB_HEADER, file=crypttab_file) + # Check if /boot is unencrypted + unencrypted_separate_boot = False for partition in self.partitions: - dct = self.generate_crypttab_line_info(partition) + if (partition["mountPoint"] == "/boot" + and "luksMapperName" not in partition): + unencrypted_separate_boot = True + break + + for partition in self.partitions: + dct = self.generate_crypttab_line_info(partition, unencrypted_separate_boot) if dct: self.print_crypttab_line(dct, file=crypttab_file) - def generate_crypttab_line_info(self, partition): + def generate_crypttab_line_info(self, partition, unencrypted_separate_boot): """ Generates information for each crypttab entry. """ if "luksMapperName" not in partition or "luksUuid" not in partition: return None @@ -158,11 +166,19 @@ class FstabGenerator(object): if not mapper_name or not luks_uuid: return None + # Set crypttab password for partition to none and remove crypttab options + # on root partition when /boot is unencrypted + password = "/crypto_keyfile.bin" + crypttab_options = self.crypttab_options + if partition["mountPoint"] == "/" and unencrypted_separate_boot: + password = 'none' + crypttab_options = '' + return dict( name=mapper_name, device="UUID=" + luks_uuid, - password="/crypto_keyfile.bin", - options=self.crypttab_options, + password=password, + options=crypttab_options, ) def print_crypttab_line(self, dct, file=None): @@ -264,6 +280,10 @@ class FstabGenerator(object): options = "subvol={},".format(partition["subvol"]) + options if has_luks: + # Check if user mounted a previously encrypted partition + if not partition["luksMapperName"]: + return None + device = "/dev/mapper/" + partition["luksMapperName"] elif partition["uuid"]: device = "UUID=" + partition["uuid"] diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp old mode 100644 new mode 100755 index 137cb750d..38658a7a9 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -172,6 +172,22 @@ hasUnencryptedSeparateBoot() return false; } +static bool +hasEncryptedRoot() +{ + const QVariantList partitions = ::partitions(); + for ( const QVariant& partition : partitions ) + { + QVariantMap partitionMap = partition.toMap(); + QString mountPoint = partitionMap.value( QStringLiteral( "mountPoint" ) ).toString(); + if ( QDir::cleanPath( mountPoint ) == QStringLiteral( "/" ) ) + { + return partitionMap.contains( QStringLiteral( "luksMapperName" ) ); + } + } + return false; +} + Calamares::JobResult LuksBootKeyFileJob::exec() { @@ -218,9 +234,10 @@ LuksBootKeyFileJob::exec() } // /boot partition is not encrypted, keyfile must not be used - if ( hasUnencryptedSeparateBoot() ) + // But only if root partition is not encrypted + if ( hasUnencryptedSeparateBoot() && !hasEncryptedRoot() ) { - cDebug() << Logger::SubEntry << "/boot partition is not encrypted, skipping keyfile creation."; + cDebug() << Logger::SubEntry << "/boot partition is not encrypted, skipping keyfile creation."; return Calamares::JobResult::ok(); } @@ -241,7 +258,11 @@ LuksBootKeyFileJob::exec() for ( const auto& d : s.devices ) { - if ( !setupLuks( d ) ) + // Skip setupLuks for root partition if system has an unencrypted /boot + if ( d.isRoot && hasUnencryptedSeparateBoot() ) + continue; + + if ( !setupLuks( d ) ) return Calamares::JobResult::error( tr( "Encrypted rootfs setup error" ), tr( "Could not configure LUKS key file on partition %1." ).arg( d.device ) ); From dcbb83ebe5a971f42c074dbba1775f06158278c0 Mon Sep 17 00:00:00 2001 From: abalfoort Date: Thu, 21 Apr 2022 17:13:50 +0200 Subject: [PATCH 02/17] Replace tabs with 4 spaces --- src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp index 38658a7a9..d4a718acf 100755 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -237,7 +237,7 @@ LuksBootKeyFileJob::exec() // But only if root partition is not encrypted if ( hasUnencryptedSeparateBoot() && !hasEncryptedRoot() ) { - cDebug() << Logger::SubEntry << "/boot partition is not encrypted, skipping keyfile creation."; + cDebug() << Logger::SubEntry << "/boot partition is not encrypted, skipping keyfile creation."; return Calamares::JobResult::ok(); } @@ -258,11 +258,11 @@ LuksBootKeyFileJob::exec() for ( const auto& d : s.devices ) { - // Skip setupLuks for root partition if system has an unencrypted /boot + // Skip setupLuks for root partition if system has an unencrypted /boot if ( d.isRoot && hasUnencryptedSeparateBoot() ) - continue; + continue; - if ( !setupLuks( d ) ) + if ( !setupLuks( d ) ) return Calamares::JobResult::error( tr( "Encrypted rootfs setup error" ), tr( "Could not configure LUKS key file on partition %1." ).arg( d.device ) ); From 5d1b02423784776b48164a0c9cc8f5fc66213fd5 Mon Sep 17 00:00:00 2001 From: abalfoort Date: Sat, 23 Apr 2022 12:44:39 +0200 Subject: [PATCH 03/17] Implement comments - Unencrypted /boot check moved to generate_crypttab_line_info. - has_luks in class FstabGenerator changed to luks_mapper_name. --- src/modules/fstab/main.py | 42 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/src/modules/fstab/main.py b/src/modules/fstab/main.py index e65e36445..539457174 100755 --- a/src/modules/fstab/main.py +++ b/src/modules/fstab/main.py @@ -142,21 +142,13 @@ class FstabGenerator(object): with open(crypttab_path, "w") as crypttab_file: print(CRYPTTAB_HEADER, file=crypttab_file) - # Check if /boot is unencrypted - unencrypted_separate_boot = False for partition in self.partitions: - if (partition["mountPoint"] == "/boot" - and "luksMapperName" not in partition): - unencrypted_separate_boot = True - break - - for partition in self.partitions: - dct = self.generate_crypttab_line_info(partition, unencrypted_separate_boot) + dct = self.generate_crypttab_line_info(partition) if dct: self.print_crypttab_line(dct, file=crypttab_file) - def generate_crypttab_line_info(self, partition, unencrypted_separate_boot): + def generate_crypttab_line_info(self, partition): """ Generates information for each crypttab entry. """ if "luksMapperName" not in partition or "luksUuid" not in partition: return None @@ -166,13 +158,17 @@ class FstabGenerator(object): if not mapper_name or not luks_uuid: return None - # Set crypttab password for partition to none and remove crypttab options - # on root partition when /boot is unencrypted password = "/crypto_keyfile.bin" crypttab_options = self.crypttab_options - if partition["mountPoint"] == "/" and unencrypted_separate_boot: - password = 'none' - crypttab_options = '' + + # Set crypttab password for partition to none and remove crypttab options + # on root partition when /boot is unencrypted + if partition["mountPoint"] == "/": + if any([p["mountPoint"] == "/boot" + and "luksMapperName" not in p + for p in self.partitions]): + password = "none" + crypttab_options = "" return dict( name=mapper_name, @@ -236,7 +232,7 @@ class FstabGenerator(object): # Some "fs" names need special handling in /etc/fstab, so remap them. filesystem = partition["fs"].lower() filesystem = FS_MAP.get(filesystem, filesystem) - has_luks = "luksMapperName" in partition + luks_mapper_name = partition.get("luksMapperName", None) mount_point = partition["mountPoint"] disk_name = disk_name_for_partition(partition) is_ssd = disk_name in self.ssd_disks @@ -279,17 +275,19 @@ class FstabGenerator(object): if filesystem == "btrfs" and partition.get("subvol",None): options = "subvol={},".format(partition["subvol"]) + options - if has_luks: - # Check if user mounted a previously encrypted partition - if not partition["luksMapperName"]: - return None - - device = "/dev/mapper/" + partition["luksMapperName"] + device = None + if luks_mapper_name: + device = "/dev/mapper/" + luks_mapper_name elif partition["uuid"]: device = "UUID=" + partition["uuid"] else: device = partition["device"] + if not device: + # TODO: we get here when the user mounted a previously mounted partition + # This should be catched early in the process + return None + return dict(device=device, mount_point=mount_point, fs=filesystem, From 29733819f5ccd29ca62fefdced17062377ad8c4d Mon Sep 17 00:00:00 2001 From: abalfoort Date: Sat, 23 Apr 2022 13:06:44 +0200 Subject: [PATCH 04/17] Implement comments - Unencrypted /boot check moved to generate_crypttab_line_info. - has_luks in class FstabGenerator changed to luks_mapper_name. --- src/modules/fstab/main.py | 42 +++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/src/modules/fstab/main.py b/src/modules/fstab/main.py index e65e36445..539457174 100755 --- a/src/modules/fstab/main.py +++ b/src/modules/fstab/main.py @@ -142,21 +142,13 @@ class FstabGenerator(object): with open(crypttab_path, "w") as crypttab_file: print(CRYPTTAB_HEADER, file=crypttab_file) - # Check if /boot is unencrypted - unencrypted_separate_boot = False for partition in self.partitions: - if (partition["mountPoint"] == "/boot" - and "luksMapperName" not in partition): - unencrypted_separate_boot = True - break - - for partition in self.partitions: - dct = self.generate_crypttab_line_info(partition, unencrypted_separate_boot) + dct = self.generate_crypttab_line_info(partition) if dct: self.print_crypttab_line(dct, file=crypttab_file) - def generate_crypttab_line_info(self, partition, unencrypted_separate_boot): + def generate_crypttab_line_info(self, partition): """ Generates information for each crypttab entry. """ if "luksMapperName" not in partition or "luksUuid" not in partition: return None @@ -166,13 +158,17 @@ class FstabGenerator(object): if not mapper_name or not luks_uuid: return None - # Set crypttab password for partition to none and remove crypttab options - # on root partition when /boot is unencrypted password = "/crypto_keyfile.bin" crypttab_options = self.crypttab_options - if partition["mountPoint"] == "/" and unencrypted_separate_boot: - password = 'none' - crypttab_options = '' + + # Set crypttab password for partition to none and remove crypttab options + # on root partition when /boot is unencrypted + if partition["mountPoint"] == "/": + if any([p["mountPoint"] == "/boot" + and "luksMapperName" not in p + for p in self.partitions]): + password = "none" + crypttab_options = "" return dict( name=mapper_name, @@ -236,7 +232,7 @@ class FstabGenerator(object): # Some "fs" names need special handling in /etc/fstab, so remap them. filesystem = partition["fs"].lower() filesystem = FS_MAP.get(filesystem, filesystem) - has_luks = "luksMapperName" in partition + luks_mapper_name = partition.get("luksMapperName", None) mount_point = partition["mountPoint"] disk_name = disk_name_for_partition(partition) is_ssd = disk_name in self.ssd_disks @@ -279,17 +275,19 @@ class FstabGenerator(object): if filesystem == "btrfs" and partition.get("subvol",None): options = "subvol={},".format(partition["subvol"]) + options - if has_luks: - # Check if user mounted a previously encrypted partition - if not partition["luksMapperName"]: - return None - - device = "/dev/mapper/" + partition["luksMapperName"] + device = None + if luks_mapper_name: + device = "/dev/mapper/" + luks_mapper_name elif partition["uuid"]: device = "UUID=" + partition["uuid"] else: device = partition["device"] + if not device: + # TODO: we get here when the user mounted a previously mounted partition + # This should be catched early in the process + return None + return dict(device=device, mount_point=mount_point, fs=filesystem, From fae515c3a25168dd35ebfc983badded458cb9a34 Mon Sep 17 00:00:00 2001 From: abalfoort Date: Sat, 23 Apr 2022 13:21:27 +0200 Subject: [PATCH 05/17] Improve todo comment --- src/modules/fstab/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/fstab/main.py b/src/modules/fstab/main.py index 539457174..1cfb5e660 100755 --- a/src/modules/fstab/main.py +++ b/src/modules/fstab/main.py @@ -284,7 +284,7 @@ class FstabGenerator(object): device = partition["device"] if not device: - # TODO: we get here when the user mounted a previously mounted partition + # TODO: we get here when the user mounted a previously encrypted partition # This should be catched early in the process return None From 466c01b524371827137bcd77069a8750957d5892 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 13:33:49 +0200 Subject: [PATCH 06/17] Changes: post-release housekeeping & credits for new features --- CHANGES-3.2 | 13 +++++++++++++ CMakeLists.txt | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index 3171a939a..5efd645bb 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -8,6 +8,19 @@ 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.57 (unreleased) # + +This release contains contributions from (alphabetically by first name): + - Arjen Balfoort (new contributor! Welcome!) + +## Core ## + - No core changes yet + +## Modules ## + - *fstab* and *luksbootkeyfile* have better support for an **un**encrypted + `/boot` partition. #1931 (thanks Arjen) + + # 3.2.56 (2022-04-22) # As of this release, Calamares 3.2 development is winding down. The diff --git a/CMakeLists.txt b/CMakeLists.txt index 87fec664f..a11f683d0 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.56 + VERSION 3.2.57 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 19af46faeaf3b51d99fec81a733a87a132710182 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 13:35:06 +0200 Subject: [PATCH 07/17] [luksbootkeyfile] Apply coding style --- src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp | 2 ++ 1 file changed, 2 insertions(+) mode change 100755 => 100644 src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp old mode 100755 new mode 100644 index d4a718acf..41fde97b8 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -260,7 +260,9 @@ LuksBootKeyFileJob::exec() { // Skip setupLuks for root partition if system has an unencrypted /boot if ( d.isRoot && hasUnencryptedSeparateBoot() ) + { continue; + } if ( !setupLuks( d ) ) return Calamares::JobResult::error( From 1752dd573b36d3cc27c37d05c0124c1d779ffc49 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 14:07:00 +0200 Subject: [PATCH 08/17] [luksbootkeyfile] Modernize and refactor C++ finding-a-partition --- .../luksbootkeyfile/LuksBootKeyFileJob.cpp | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp index 41fde97b8..ba772aee1 100644 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -150,42 +150,50 @@ setupLuks( const LuksDevice& d ) } static QVariantList -partitions() +partitionsFromGlobalStorage() { Calamares::GlobalStorage* globalStorage = Calamares::JobQueue::instance()->globalStorage(); return globalStorage->value( QStringLiteral( "partitions" ) ).toList(); } +/// Checks if the partition (represented by @p map) mounts to the given @p path +static bool +hasMountPoint( const QVariantMap& map, const QString& path ) +{ + const QString mountPoint = map.value( QStringLiteral( "mountPoint" ) ).toString(); + return QDir::cleanPath( mountPoint ) == path; +} + +static bool +isEncrypted( const QVariantMap& map ) +{ + return map.contains( QStringLiteral( "luksMapperName" ) ); +} + +/// Checks for any partition satisfying @p pred +static bool +anyPartition( bool ( *pred )( const QVariantMap& ) ) +{ + const auto partitions = partitionsFromGlobalStorage(); + return std::find_if( partitions.cbegin(), + partitions.cend(), + [ &pred ]( const QVariant& partitionVariant ) { return pred( partitionVariant.toMap() ); } ) + != partitions.cend(); +} + static bool hasUnencryptedSeparateBoot() { - const QVariantList partitions = ::partitions(); - for ( const QVariant& partition : partitions ) - { - QVariantMap partitionMap = partition.toMap(); - QString mountPoint = partitionMap.value( QStringLiteral( "mountPoint" ) ).toString(); - if ( QDir::cleanPath( mountPoint ) == QStringLiteral( "/boot" ) ) - { - return !partitionMap.contains( QStringLiteral( "luksMapperName" ) ); - } - } - return false; + return anyPartition( + []( const QVariantMap& partition ) + { return hasMountPoint( partition, QStringLiteral( "/boot" ) ) && !isEncrypted( partition ); } ); } static bool hasEncryptedRoot() { - const QVariantList partitions = ::partitions(); - for ( const QVariant& partition : partitions ) - { - QVariantMap partitionMap = partition.toMap(); - QString mountPoint = partitionMap.value( QStringLiteral( "mountPoint" ) ).toString(); - if ( QDir::cleanPath( mountPoint ) == QStringLiteral( "/" ) ) - { - return partitionMap.contains( QStringLiteral( "luksMapperName" ) ); - } - } - return false; + return anyPartition( []( const QVariantMap& partition ) + { return hasMountPoint( partition, QStringLiteral( "/" ) ) && isEncrypted( partition ); } ); } Calamares::JobResult From 4466e360e16770b7bc7a9617b4c943509ada2180 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 14:29:10 +0200 Subject: [PATCH 09/17] [luksbootkeyfile] Start adding tests --- src/modules/luksbootkeyfile/CMakeLists.txt | 9 +- .../luksbootkeyfile/LuksBootKeyFileJob.cpp | 10 +-- src/modules/luksbootkeyfile/Tests.cpp | 83 +++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 src/modules/luksbootkeyfile/Tests.cpp diff --git a/src/modules/luksbootkeyfile/CMakeLists.txt b/src/modules/luksbootkeyfile/CMakeLists.txt index 6e98af468..dff682521 100644 --- a/src/modules/luksbootkeyfile/CMakeLists.txt +++ b/src/modules/luksbootkeyfile/CMakeLists.txt @@ -3,7 +3,7 @@ # SPDX-FileCopyrightText: 2020 Adriaan de Groot # SPDX-License-Identifier: BSD-2-Clause # -calamares_add_plugin( luksbootkeyfile +calamares_add_plugin(luksbootkeyfile TYPE job EXPORT_MACRO PLUGINDLLEXPORT_PRO SOURCES @@ -11,3 +11,10 @@ calamares_add_plugin( luksbootkeyfile SHARED_LIB NO_CONFIG ) + +calamares_add_test( + luksbootkeyfiletest + SOURCES + Tests.cpp + LuksBootKeyFileJob.cpp +) diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp index ba772aee1..ecee92467 100644 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -157,21 +157,21 @@ partitionsFromGlobalStorage() } /// Checks if the partition (represented by @p map) mounts to the given @p path -static bool +STATICTEST bool hasMountPoint( const QVariantMap& map, const QString& path ) { const QString mountPoint = map.value( QStringLiteral( "mountPoint" ) ).toString(); return QDir::cleanPath( mountPoint ) == path; } -static bool +STATICTEST bool isEncrypted( const QVariantMap& map ) { return map.contains( QStringLiteral( "luksMapperName" ) ); } /// Checks for any partition satisfying @p pred -static bool +STATICTEST bool anyPartition( bool ( *pred )( const QVariantMap& ) ) { const auto partitions = partitionsFromGlobalStorage(); @@ -181,7 +181,7 @@ anyPartition( bool ( *pred )( const QVariantMap& ) ) != partitions.cend(); } -static bool +STATICTEST bool hasUnencryptedSeparateBoot() { return anyPartition( @@ -189,7 +189,7 @@ hasUnencryptedSeparateBoot() { return hasMountPoint( partition, QStringLiteral( "/boot" ) ) && !isEncrypted( partition ); } ); } -static bool +STATICTEST bool hasEncryptedRoot() { return anyPartition( []( const QVariantMap& partition ) diff --git a/src/modules/luksbootkeyfile/Tests.cpp b/src/modules/luksbootkeyfile/Tests.cpp new file mode 100644 index 000000000..bb295919b --- /dev/null +++ b/src/modules/luksbootkeyfile/Tests.cpp @@ -0,0 +1,83 @@ +/* === This file is part of Calamares - === + * + * SPDX-FileCopyrightText: 2022 Adriaan de Groot + * SPDX-License-Identifier: GPL-3.0-or-later + * + * Calamares is Free Software: see the License-Identifier above. + * + */ + +#include "JobQueue.h" +#include "utils/Logger.h" + +#include + +#undef STATICTEST +#define STATICTEST extern + +// Implementation details +STATICTEST bool hasMountPoint( const QVariantMap& map, const QString& path ); + +STATICTEST bool isEncrypted( const QVariantMap& map ); + +STATICTEST bool anyPartition( bool ( *pred )( const QVariantMap& ) ); + +STATICTEST bool hasUnencryptedSeparateBoot(); + +STATICTEST bool hasEncryptedRoot(); + +class LuksBootKeyFileTests : public QObject +{ + Q_OBJECT +public: + LuksBootKeyFileTests() {} + ~LuksBootKeyFileTests() override {} + +private Q_SLOTS: + void initTestCase(); + + void testMountPoint(); +}; + +void +LuksBootKeyFileTests::initTestCase() +{ + Logger::setupLogLevel( Logger::LOGDEBUG ); + cDebug() << "LuksBootKeyFile test started."; + + if ( !Calamares::JobQueue::instance() ) + { + (void)new Calamares::JobQueue(); + } +} + +void +LuksBootKeyFileTests::testMountPoint() +{ + QVariantMap m; // As if this is a partition data + const QString key = QStringLiteral( "mountPoint" ); + const QString boot = QStringLiteral( "/boot" ); + const QString root = QStringLiteral( "/" ); + + QVERIFY( !hasMountPoint( m, QString() ) ); + QVERIFY( !hasMountPoint( m, boot ) ); + + m.insert( key, boot ); + QVERIFY( hasMountPoint( m, boot ) ); + QVERIFY( !hasMountPoint( m, QString() ) ); + QVERIFY( !hasMountPoint( m, root ) ); + + m.insert( key, root ); + QVERIFY( !hasMountPoint( m, boot ) ); + QVERIFY( !hasMountPoint( m, QString() ) ); + QVERIFY( hasMountPoint( m, root ) ); + + m.remove( key ); + QVERIFY( !hasMountPoint( m, root ) ); +} + +QTEST_GUILESS_MAIN( LuksBootKeyFileTests ) + +#include "utils/moc-warnings.h" + +#include "Tests.moc" From 4613386863159421f0de58d62a364e292e873c09 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 14:33:25 +0200 Subject: [PATCH 10/17] [luksbootkeyfile] fix test for empty mount point --- src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp index ecee92467..e8dfd4724 100644 --- a/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp +++ b/src/modules/luksbootkeyfile/LuksBootKeyFileJob.cpp @@ -160,8 +160,8 @@ partitionsFromGlobalStorage() STATICTEST bool hasMountPoint( const QVariantMap& map, const QString& path ) { - const QString mountPoint = map.value( QStringLiteral( "mountPoint" ) ).toString(); - return QDir::cleanPath( mountPoint ) == path; + const auto v = map.value( QStringLiteral( "mountPoint" ) ); + return v.isValid() && QDir::cleanPath( v.toString() ) == path; } STATICTEST bool From 70911b92758ab8d07bea020c8955f093451b211a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 14:51:15 +0200 Subject: [PATCH 11/17] [luksbootkeyfile] Flesh out the tests some more --- src/modules/luksbootkeyfile/Tests.cpp | 86 +++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/modules/luksbootkeyfile/Tests.cpp b/src/modules/luksbootkeyfile/Tests.cpp index bb295919b..6b6c7be81 100644 --- a/src/modules/luksbootkeyfile/Tests.cpp +++ b/src/modules/luksbootkeyfile/Tests.cpp @@ -7,6 +7,7 @@ * */ +#include "GlobalStorage.h" #include "JobQueue.h" #include "utils/Logger.h" @@ -37,6 +38,8 @@ private Q_SLOTS: void initTestCase(); void testMountPoint(); + void testIsEncrypted(); + void testAnyPartition(); }; void @@ -76,6 +79,89 @@ LuksBootKeyFileTests::testMountPoint() QVERIFY( !hasMountPoint( m, root ) ); } +void +LuksBootKeyFileTests::testIsEncrypted() +{ + QVariantMap m; // As if this is a partition data + const QString key = QStringLiteral( "luksMapperName" ); + const QString name = QStringLiteral( "any-name" ); + + QVERIFY( !isEncrypted( m ) ); + + // Even an empty string is considered encrypted + m.insert( key, QString() ); + QVERIFY( isEncrypted( m ) ); + + m.insert( key, name ); + QVERIFY( isEncrypted( m ) ); + + m.insert( key, QString() ); + QVERIFY( isEncrypted( m ) ); + + m.remove( key ); + QVERIFY( !isEncrypted( m ) ); +} + + +void +LuksBootKeyFileTests::testAnyPartition() +{ + // This is kind of annoying: we need to build up + // partition data in GS because the functions we're testing + // go straight to GS. + auto* gs = Calamares::JobQueue::instanceGlobalStorage(); + QVERIFY( gs ); + + const QString partitionsKey = QStringLiteral( "partitions" ); + const QString mountPointKey = QStringLiteral( "mountPoint" ); + const QString boot = QStringLiteral( "/boot" ); + const QString root = QStringLiteral( "/" ); + + QVariantList partitions; + QVariantMap p; + QVERIFY( !gs->contains( partitionsKey ) ); + + // Empty list! + QVERIFY( !anyPartition( []( const QVariantMap& ) { return true; } ) ); + + gs->insert( partitionsKey, partitions ); + QVERIFY( !anyPartition( []( const QVariantMap& ) { return true; } ) ); // Still an empty list + + partitions.append( p ); + QCOMPARE( partitions.count(), 1 ); + gs->insert( partitionsKey, partitions ); + QVERIFY( anyPartition( []( const QVariantMap& ) { return true; } ) ); // Now a one-element list + QVERIFY( !anyPartition( []( const QVariantMap& ) { return false; } ) ); // Now a one-element list + + p.insert( mountPointKey, boot ); + QVERIFY( hasMountPoint( p, boot ) ); + partitions.append( p ); + QCOMPARE( partitions.count(), 2 ); + + // Note that GS is not updated yet, so we expect this to fail + QEXPECT_FAIL( "", "GS not updated", Continue ); + QVERIFY( anyPartition( + []( const QVariantMap& partdata ) + { + cDebug() << partdata; + return hasMountPoint( partdata, QStringLiteral( "/boot" ) ); + } ) ); + + gs->insert( partitionsKey, partitions ); // Update GS + QVERIFY( anyPartition( + []( const QVariantMap& partdata ) + { + cDebug() << partdata; + return hasMountPoint( partdata, QStringLiteral( "/boot" ) ); + } ) ); + QVERIFY( !anyPartition( []( const QVariantMap& partdata ) + { return hasMountPoint( partdata, QStringLiteral( "/" ) ); } ) ); + QVERIFY( !anyPartition( []( const QVariantMap& partdata ) { return hasMountPoint( partdata, QString() ); } ) ); + + QVERIFY( !hasEncryptedRoot() ); + QVERIFY( hasUnencryptedSeparateBoot() ); +} + QTEST_GUILESS_MAIN( LuksBootKeyFileTests ) #include "utils/moc-warnings.h" From 1aee8401db89306d64749a20303d2dc2b10c5228 Mon Sep 17 00:00:00 2001 From: Victor Fuentes Date: Sat, 23 Apr 2022 13:51:02 -0400 Subject: [PATCH 12/17] [packagechooserq]: allow changing step name --- src/modules/packagechooser/Config.cpp | 16 ++++++++++++++++ src/modules/packagechooser/Config.h | 2 ++ .../packagechooser/PackageChooserViewStep.cpp | 14 +------------- .../packagechooser/PackageChooserViewStep.h | 1 - .../PackageChooserQmlViewStep.cpp | 2 +- src/modules/packagechooserq/packagechooserq.conf | 13 +++++++++++++ 6 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/modules/packagechooser/Config.cpp b/src/modules/packagechooser/Config.cpp index 491fe5c25..667621597 100644 --- a/src/modules/packagechooser/Config.cpp +++ b/src/modules/packagechooser/Config.cpp @@ -237,6 +237,12 @@ Config::setPackageChoice( const QString& packageChoice ) emit packageChoiceChanged( m_packageChoice.value_or( QString() ) ); } +QString +Config::prettyName() const +{ + return m_stepName ? m_stepName->get() : tr( "Packages" ); +} + QString Config::prettyStatus() const { @@ -343,4 +349,14 @@ Config::setConfigurationMap( const QVariantMap& configurationMap ) cWarning() << "Single-selection QML module must use 'Legacy' method."; } } + + bool labels_ok = false; + auto labels = CalamaresUtils::getSubMap( configurationMap, "labels", labels_ok ); + if ( labels_ok ) + { + if ( labels.contains( "step" ) ) + { + m_stepName = new CalamaresUtils::Locale::TranslatedString( labels, "step" ); + } + } } diff --git a/src/modules/packagechooser/Config.h b/src/modules/packagechooser/Config.h index b04b1c30b..d1b783a8d 100644 --- a/src/modules/packagechooser/Config.h +++ b/src/modules/packagechooser/Config.h @@ -98,6 +98,7 @@ public: QString packageChoice() const { return m_packageChoice.value_or( QString() ); } void setPackageChoice( const QString& packageChoice ); + QString prettyName() const; QString prettyStatus() const; signals: @@ -120,6 +121,7 @@ private: * Reading the property will return an empty QString. */ std::optional< QString > m_packageChoice; + CalamaresUtils::Locale::TranslatedString* m_stepName; // As it appears in the sidebar }; diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index 9057004de..c3f2ce6e2 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -29,7 +29,6 @@ PackageChooserViewStep::PackageChooserViewStep( QObject* parent ) : Calamares::ViewStep( parent ) , m_config( new Config( this ) ) , m_widget( nullptr ) - , m_stepName( nullptr ) { emit nextStatusChanged( false ); } @@ -41,14 +40,13 @@ PackageChooserViewStep::~PackageChooserViewStep() { m_widget->deleteLater(); } - delete m_stepName; } QString PackageChooserViewStep::prettyName() const { - return m_stepName ? m_stepName->get() : tr( "Packages" ); + return m_config->prettyName(); } @@ -139,16 +137,6 @@ PackageChooserViewStep::setConfigurationMap( const QVariantMap& configurationMap m_config->setDefaultId( moduleInstanceKey() ); m_config->setConfigurationMap( configurationMap ); - bool labels_ok = false; - auto labels = CalamaresUtils::getSubMap( configurationMap, "labels", labels_ok ); - if ( labels_ok ) - { - if ( labels.contains( "step" ) ) - { - m_stepName = new CalamaresUtils::Locale::TranslatedString( labels, "step" ); - } - } - if ( m_widget ) { hookupModel(); diff --git a/src/modules/packagechooser/PackageChooserViewStep.h b/src/modules/packagechooser/PackageChooserViewStep.h index 7561f2bd7..76b35aed8 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.h +++ b/src/modules/packagechooser/PackageChooserViewStep.h @@ -50,7 +50,6 @@ private: Config* m_config; PackageChooserPage* m_widget; - CalamaresUtils::Locale::TranslatedString* m_stepName; // As it appears in the sidebar }; CALAMARES_PLUGIN_FACTORY_DECLARATION( PackageChooserViewStepFactory ) diff --git a/src/modules/packagechooserq/PackageChooserQmlViewStep.cpp b/src/modules/packagechooserq/PackageChooserQmlViewStep.cpp index 543c9771d..ae4aa3c48 100644 --- a/src/modules/packagechooserq/PackageChooserQmlViewStep.cpp +++ b/src/modules/packagechooserq/PackageChooserQmlViewStep.cpp @@ -29,7 +29,7 @@ PackageChooserQmlViewStep::PackageChooserQmlViewStep( QObject* parent ) QString PackageChooserQmlViewStep::prettyName() const { - return tr( "Packages" ); + return m_config->prettyName(); } QString diff --git a/src/modules/packagechooserq/packagechooserq.conf b/src/modules/packagechooserq/packagechooserq.conf index 803c6f670..9c1878f34 100644 --- a/src/modules/packagechooserq/packagechooserq.conf +++ b/src/modules/packagechooserq/packagechooserq.conf @@ -42,6 +42,19 @@ # method: legacy +# Human-visible strings in this module. These are all optional. +# The following translated keys are used: +# - *step*, used in the overall progress view (left-hand pane) +# +# Each key can have a [locale] added to it, which is used as +# the translated string for that locale. For the strings +# associated with the "no-selection" item, see *items*, below +# with the explicit item-*id* "". +# +labels: + step: "Packages" + step[nl]: "Pakketten" + # The *packageChoice* value is used for setting the default selection # in the QML view; this should match one of the keys used in the QML # module for package names. From 78067110c909281c0dcc3a05f436257cb05c0852 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 16 Apr 2022 12:42:29 +0200 Subject: [PATCH 13/17] Git: add a blame-ignore file for large-scale reformatting --- .git-blame-ignore-revs | 1 + 1 file changed, 1 insertion(+) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 000000000..cbf236645 --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1 @@ +18fef8dfe5d926ec0bc979562553adf4db8db2e9 From cab4be1bd0d934e6f8a3086c76b3343abb787d18 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 23 Apr 2022 21:29:00 +0200 Subject: [PATCH 14/17] Changes: credits for packagechooser-naming --- CHANGES-3.2 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES-3.2 b/CHANGES-3.2 index 5efd645bb..c590fe13f 100644 --- a/CHANGES-3.2 +++ b/CHANGES-3.2 @@ -12,6 +12,7 @@ website will have to do for older versions. This release contains contributions from (alphabetically by first name): - Arjen Balfoort (new contributor! Welcome!) + - Victor Fuentes ## Core ## - No core changes yet @@ -19,6 +20,8 @@ This release contains contributions from (alphabetically by first name): ## Modules ## - *fstab* and *luksbootkeyfile* have better support for an **un**encrypted `/boot` partition. #1931 (thanks Arjen) + - *packagechooser* and *packagechooserq* can now be given a custom name + in the side-panel. #1932 (thanks Victor) # 3.2.56 (2022-04-22) # From eb5be9cd34dd9605404cb6cb0cff990356af3cbe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 25 Apr 2022 11:11:40 +0200 Subject: [PATCH 15/17] [bootloader] Allow skipping an EFI bootloader, too - while here, remove C-style if() --- src/modules/bootloader/main.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/modules/bootloader/main.py b/src/modules/bootloader/main.py index fb8e19ac2..814da37f3 100644 --- a/src/modules/bootloader/main.py +++ b/src/modules/bootloader/main.py @@ -761,8 +761,12 @@ def run(): fw_type = libcalamares.globalstorage.value("firmwareType") - if (libcalamares.globalstorage.value("bootLoader") is None and fw_type != "efi"): - libcalamares.utils.warning( "Non-EFI system, and no bootloader is set." ) + if libcalamares.globalstorage.value("bootLoader") is None: + # Don't want a bootloader, but do log that this has an effect: + if fw_type != "efi": + libcalamares.utils.warning( "Non-EFI system, and no bootloader is set." ) + else: + libcalamares.utils.warning( "EFI system, and no bootloader is set." ) return None partitions = libcalamares.globalstorage.value("partitions") From d70d1ebfcd5f46c48a6f41c938755d2453a3927a Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 25 Apr 2022 11:23:04 +0200 Subject: [PATCH 16/17] [partition] Always offer a 'no bootloader' item - this branch is importing parts of a PR from Anubhav, so add relevant SPDX tag as well. --- src/modules/partition/core/BootLoaderModel.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/modules/partition/core/BootLoaderModel.cpp b/src/modules/partition/core/BootLoaderModel.cpp index fd66c8514..f659fc593 100644 --- a/src/modules/partition/core/BootLoaderModel.cpp +++ b/src/modules/partition/core/BootLoaderModel.cpp @@ -3,6 +3,7 @@ * SPDX-FileCopyrightText: 2014 Aurélien Gâteau * SPDX-FileCopyrightText: 2015 Teo Mrnjavac * SPDX-FileCopyrightText: 2019 Adriaan de Groot + * SPDX-FileCopyrightText: 2021 Anubhav Choudhary * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is Free Software: see the License-Identifier above. @@ -79,7 +80,7 @@ BootLoaderModel::updateInternal() clear(); createMbrItems(); - // An empty model is possible if you don't havee permissions: don't crash though. + // An empty model is possible if you don't have permissions: don't crash though. if ( rowCount() < 1 ) { return; @@ -124,10 +125,10 @@ BootLoaderModel::updateInternal() { appendRow( createBootLoaderItem( partitionText, PartitionInfo::mountPoint( partition ), true ) ); } - - // Create "don't install bootloader" item - appendRow( createBootLoaderItem( tr( "Do not install a boot loader" ), QString(), false ) ); } + // Create "don't install bootloader" item. This is always available, + // also if there was no /boot or / partition found. + appendRow( createBootLoaderItem( tr( "Do not install a boot loader" ), QString(), false ) ); } From d02afa20b8a66f5879818974e2d29792f149a68c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 25 Apr 2022 12:14:00 +0200 Subject: [PATCH 17/17] [partition] Factor out creating-bootloader-panel - this makes createBootloaderComboBox obsolete, since that was an implementation detail for creating the panel. - add the panel also when doing an alongside install. - credits to Anubhav, whose PR started this code. --- src/modules/partition/gui/ChoicePage.cpp | 151 ++++++++++++----------- src/modules/partition/gui/ChoicePage.h | 7 +- 2 files changed, 83 insertions(+), 75 deletions(-) diff --git a/src/modules/partition/gui/ChoicePage.cpp b/src/modules/partition/gui/ChoicePage.cpp index c0845da7f..a7299e715 100644 --- a/src/modules/partition/gui/ChoicePage.cpp +++ b/src/modules/partition/gui/ChoicePage.cpp @@ -3,6 +3,7 @@ * SPDX-FileCopyrightText: 2014-2017 Teo Mrnjavac * SPDX-FileCopyrightText: 2017-2019 Adriaan de Groot * SPDX-FileCopyrightText: 2019 Collabora Ltd + * SPDX-FileCopyrightText: 2021 Anubhav Choudhary * SPDX-License-Identifier: GPL-3.0-or-later * * Calamares is Free Software: see the License-Identifier above. @@ -1023,6 +1024,12 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) QLabel* sizeLabel = new QLabel( m_previewAfterFrame ); layout->addWidget( sizeLabel ); sizeLabel->setWordWrap( true ); + + if ( !m_isEfi ) + { + layout->addWidget( createBootloaderPanel() ); + } + connect( m_afterPartitionSplitterWidget, &PartitionSplitterWidget::partitionResized, this, @@ -1081,51 +1088,7 @@ ChoicePage::updateActionChoicePreview( InstallChoice choice ) if ( !m_isEfi ) { - QWidget* eraseWidget = new QWidget; - - QHBoxLayout* eraseLayout = new QHBoxLayout; - eraseWidget->setLayout( eraseLayout ); - eraseLayout->setContentsMargins( 0, 0, 0, 0 ); - QLabel* eraseBootloaderLabel = new QLabel( eraseWidget ); - eraseLayout->addWidget( eraseBootloaderLabel ); - eraseBootloaderLabel->setText( tr( "Boot loader location:" ) ); - - m_bootloaderComboBox = createBootloaderComboBox( eraseWidget ); - connect( m_core->bootLoaderModel(), - &QAbstractItemModel::modelReset, - [ this ]() - { - if ( !m_bootloaderComboBox.isNull() ) - { - Calamares::restoreSelectedBootLoader( *m_bootloaderComboBox, - m_core->bootLoaderInstallPath() ); - } - } ); - connect( - m_core, - &PartitionCoreModule::deviceReverted, - this, - [ this ]( Device* dev ) - { - Q_UNUSED( dev ) - if ( !m_bootloaderComboBox.isNull() ) - { - if ( m_bootloaderComboBox->model() != m_core->bootLoaderModel() ) - { - m_bootloaderComboBox->setModel( m_core->bootLoaderModel() ); - } - - m_bootloaderComboBox->setCurrentIndex( m_lastSelectedDeviceIndex ); - } - }, - Qt::QueuedConnection ); - // ^ Must be Queued so it's sure to run when the widget is already visible. - - eraseLayout->addWidget( m_bootloaderComboBox ); - eraseBootloaderLabel->setBuddy( m_bootloaderComboBox ); - eraseLayout->addStretch(); - - layout->addWidget( eraseWidget ); + layout->addWidget( createBootloaderPanel() ); } m_previewAfterFrame->show(); @@ -1237,35 +1200,6 @@ ChoicePage::setupEfiSystemPartitionSelector() } } - -QComboBox* -ChoicePage::createBootloaderComboBox( QWidget* parent ) -{ - QComboBox* comboForBootloader = new QComboBox( parent ); - comboForBootloader->setModel( m_core->bootLoaderModel() ); - - // When the chosen bootloader device changes, we update the choice in the PCM - connect( comboForBootloader, - QOverload< int >::of( &QComboBox::currentIndexChanged ), - this, - [ this ]( int newIndex ) - { - QComboBox* bootloaderCombo = qobject_cast< QComboBox* >( sender() ); - if ( bootloaderCombo ) - { - QVariant var = bootloaderCombo->itemData( newIndex, BootLoaderModel::BootLoaderPathRole ); - if ( !var.isValid() ) - { - return; - } - m_core->setBootLoaderInstallPath( var.toString() ); - } - } ); - - return comboForBootloader; -} - - static inline void force_uncheck( QButtonGroup* grp, PrettyRadioButton* button ) { @@ -1737,3 +1671,72 @@ ChoicePage::setLastSelectedDeviceIndex( int index ) m_lastSelectedDeviceIndex = index; m_drivesCombo->setCurrentIndex( m_lastSelectedDeviceIndex ); } + +QWidget* +ChoicePage::createBootloaderPanel() +{ + QWidget* panelWidget = new QWidget; + + QHBoxLayout* mainLayout = new QHBoxLayout; + panelWidget->setLayout( mainLayout ); + mainLayout->setContentsMargins( 0, 0, 0, 0 ); + QLabel* widgetLabel = new QLabel( panelWidget ); + mainLayout->addWidget( widgetLabel ); + widgetLabel->setText( tr( "Boot loader location:" ) ); + + QComboBox* comboForBootloader = new QComboBox( panelWidget ); + comboForBootloader->setModel( m_core->bootLoaderModel() ); + + // When the chosen bootloader device changes, we update the choice in the PCM + connect( comboForBootloader, + QOverload< int >::of( &QComboBox::currentIndexChanged ), + this, + [ this ]( int newIndex ) + { + QComboBox* bootloaderCombo = qobject_cast< QComboBox* >( sender() ); + if ( bootloaderCombo ) + { + QVariant var = bootloaderCombo->itemData( newIndex, BootLoaderModel::BootLoaderPathRole ); + if ( !var.isValid() ) + { + return; + } + m_core->setBootLoaderInstallPath( var.toString() ); + } + } ); + m_bootloaderComboBox = comboForBootloader; + + connect( m_core->bootLoaderModel(), + &QAbstractItemModel::modelReset, + [ this ]() + { + if ( !m_bootloaderComboBox.isNull() ) + { + Calamares::restoreSelectedBootLoader( *m_bootloaderComboBox, m_core->bootLoaderInstallPath() ); + } + } ); + connect( + m_core, + &PartitionCoreModule::deviceReverted, + this, + [ this ]( Device* ) + { + if ( !m_bootloaderComboBox.isNull() ) + { + if ( m_bootloaderComboBox->model() != m_core->bootLoaderModel() ) + { + m_bootloaderComboBox->setModel( m_core->bootLoaderModel() ); + } + + m_bootloaderComboBox->setCurrentIndex( m_lastSelectedDeviceIndex ); + } + }, + Qt::QueuedConnection ); + // ^ Must be Queued so it's sure to run when the widget is already visible. + + mainLayout->addWidget( m_bootloaderComboBox ); + widgetLabel->setBuddy( m_bootloaderComboBox ); + mainLayout->addStretch(); + + return panelWidget; +} diff --git a/src/modules/partition/gui/ChoicePage.h b/src/modules/partition/gui/ChoicePage.h index 12222ac63..d1699200a 100644 --- a/src/modules/partition/gui/ChoicePage.h +++ b/src/modules/partition/gui/ChoicePage.h @@ -108,7 +108,12 @@ private: void updateNextEnabled(); void setupChoices(); void checkInstallChoiceRadioButton( Config::InstallChoice choice ); ///< Sets the chosen button to "on" - QComboBox* createBootloaderComboBox( QWidget* parentButton ); + /** @brief Create a panel with "boot loader location:" + * + * Panel + dropdown and handling for model updates. Returns a pointer + * to the panel's widget. + */ + QWidget* createBootloaderPanel(); Device* selectedDevice(); /* Change the UI depending on the device selected. */