From ec7c5a4611182368e9c4844724d31ac2c8edc382 Mon Sep 17 00:00:00 2001 From: Kevin Kofler Date: Sun, 12 May 2019 16:11:35 +0200 Subject: [PATCH 01/36] [bootloader] Fix sb-shim mode to write grub.cfg into the ESP src/modules/bootloader/main.py (install_secureboot): Run the configured grubMkconfig command (should be `grub-mkconfig` or `grub2-mkconfig`) to create `/boot/efi/EFI/$efi_bootloader_id/grub.cfg`. The sb-shim is just a chainloader to GRUB 2, which expects a grub.cfg in that location, so something has to create it or the installed system will not boot beyond the GRUB rescue shell. (install_grub): Fix misleading comment above the grubMkconfig call: it is not the file specified in grubCfg that should be already filled out by the grubcfg job module, that file is written by `grub*-mkconfig` using `/etc/default/grub` as the input file. It is that input file `/etc/default/grub` that should already be filled out by the grubcfg job module. (The same input file is used in install_secureboot.) --- src/modules/bootloader/main.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/modules/bootloader/main.py b/src/modules/bootloader/main.py index 40e9f1613..9d705adfc 100644 --- a/src/modules/bootloader/main.py +++ b/src/modules/bootloader/main.py @@ -7,7 +7,7 @@ # Copyright 2014, Anke Boersma # Copyright 2014, Daniel Hillenbrand # Copyright 2014, Benjamin Vaudour -# Copyright 2014, Kevin Kofler +# Copyright 2014-2019, Kevin Kofler # Copyright 2015-2018, Philip Mueller # Copyright 2016-2017, Teo Mrnjavac # Copyright 2017, Alf Gaida @@ -339,8 +339,8 @@ def install_grub(efi_directory, fw_type): "--force", boot_loader["installPath"]]) - # The file specified in grubCfg should already be filled out - # by the grubcfg job module. + # The input file /etc/default/grub should already be filled out by the + # grubcfg job module. check_target_env_call([libcalamares.job.configuration["grubMkconfig"], "-o", libcalamares.job.configuration["grubCfg"]]) @@ -395,6 +395,12 @@ def install_secureboot(efi_directory): "-p", efi_partition_number, "-l", install_efi_directory + "/" + install_efi_bin]) + # The input file /etc/default/grub should already be filled out by the + # grubcfg job module. + check_target_env_call([libcalamares.job.configuration["grubMkconfig"], + "-o", os.path.join(efi_directory, "EFI", + efi_bootloader_id, "grub.cfg")]) + def vfat_correct_case(parent, name): for candidate in os.listdir(parent): From 8fcdbd5bd5288f205543a89d823fb62242f6e66d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 12 May 2019 16:29:50 +0200 Subject: [PATCH 02/36] [libcalamaresui] Improve warning message - Tell the packager / deployer that certain modules are missing --- src/libcalamaresui/modulesystem/ModuleManager.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libcalamaresui/modulesystem/ModuleManager.cpp b/src/libcalamaresui/modulesystem/ModuleManager.cpp index 852f41445..185ec37e9 100644 --- a/src/libcalamaresui/modulesystem/ModuleManager.cpp +++ b/src/libcalamaresui/modulesystem/ModuleManager.cpp @@ -358,7 +358,8 @@ ModuleManager::checkDependencies() somethingWasRemovedBecauseOfUnmetDependencies = true; m_availableDescriptorsByModuleName.erase( it ); failed << moduleName; - cWarning() << "Module" << moduleName << "has unknown requirements" << Logger::DebugList( unmet ); + cWarning() << "Module" << moduleName << "requires modules" << Logger::DebugList( unmet ); + cWarning() << Logger::SubEntry << "but these are not available (listed in settings, or installed)."; break; } } From 2ea69d08cc28ef0d7ec5dff339ba35f42254bc71 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 12 May 2019 16:34:25 +0200 Subject: [PATCH 03/36] Changes: Credit for SB bugfixing --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index 3a847b498..17a14f317 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,7 @@ website will have to do for older versions. # 3.2.9 (unreleased) # This release contains contributions from (alphabetically by first name): + - Kevin Kofler ## Core ## From 3b0d778d1eaf7650dae5e776b6633e17fefb3b10 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sun, 12 May 2019 16:34:57 +0200 Subject: [PATCH 04/36] [partition] Mention that reuse-swap isn't supported now --- src/modules/partition/partition.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/partition.conf b/src/modules/partition/partition.conf index b9262e86f..42d1f2ce2 100644 --- a/src/modules/partition/partition.conf +++ b/src/modules/partition/partition.conf @@ -30,7 +30,7 @@ efiSystemPartition: "/boot/efi" # 8.8GiB on disk in the end). userSwapChoices: - none # Create no swap, use no swap - - reuse # Re-use existing swap, but don't create any + - reuse # Re-use existing swap, but don't create any (unsupported right now) - small # Up to 4GB - suspend # At least main memory size - file # To swap file instead of partition (unsupported right now) From 859e95432e142a3d039635bcfe9f7a7d8e2f9619 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:23:19 +0200 Subject: [PATCH 05/36] [partition] Handle all enum values in the switch --- src/libcalamares/partition/PartitionSize.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index edff0fe1e..5c4c0e040 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -153,6 +153,9 @@ PartitionSize::toBytes() const switch ( m_unit ) { + case unit_t::None: + case unit_t::Percent: + return -1; case unit_t::Byte: return value(); case unit_t::KiB: @@ -161,12 +164,7 @@ PartitionSize::toBytes() const return CalamaresUtils::MiBtoBytes( static_cast( value() ) ); case unit_t::GiB: return CalamaresUtils::GiBtoBytes( static_cast( value() ) ); - default: - break; } - - // Reached only when unit is Percent or None - return -1; } bool From 7302b9c8510295128b2f291c64d3709eabdf3424 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:23:41 +0200 Subject: [PATCH 06/36] [libcalamares] Fix nested namespaces - Declaring namespace A::B is a C++17 extension, and Calamares is C++14. Split the namespace declarations. - While here, fix extra const warning as well. --- src/libcalamares/locale/Label.cpp | 5 ++++- src/libcalamares/locale/Label.h | 7 ++++--- src/libcalamares/locale/LabelModel.cpp | 7 +++++-- src/libcalamares/locale/LabelModel.h | 9 +++++---- src/libcalamares/locale/Lookup.cpp | 5 ++++- src/libcalamares/locale/Lookup.h | 6 ++++-- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/libcalamares/locale/Label.cpp b/src/libcalamares/locale/Label.cpp index ca528dc75..8d3cd443f 100644 --- a/src/libcalamares/locale/Label.cpp +++ b/src/libcalamares/locale/Label.cpp @@ -19,7 +19,9 @@ #include "Label.h" -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { Label::Label() @@ -70,4 +72,5 @@ QLocale Label::getLocale( const QString& localeName ) return QLocale( localeName ); } +} } // namespace diff --git a/src/libcalamares/locale/Label.h b/src/libcalamares/locale/Label.h index 65befc6b4..7935b0880 100644 --- a/src/libcalamares/locale/Label.h +++ b/src/libcalamares/locale/Label.h @@ -23,8 +23,9 @@ #include #include -namespace CalamaresUtils {} -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { /** @@ -120,7 +121,7 @@ protected: QString m_englishLabel; } ; - +} } // namespace #endif diff --git a/src/libcalamares/locale/LabelModel.cpp b/src/libcalamares/locale/LabelModel.cpp index 543417212..ef8ff8ea1 100644 --- a/src/libcalamares/locale/LabelModel.cpp +++ b/src/libcalamares/locale/LabelModel.cpp @@ -22,7 +22,9 @@ #include "CalamaresVersion.h" // For the list of translations -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { LabelModel::LabelModel( const QStringList& locales, QObject* parent ) @@ -121,10 +123,11 @@ LabelModel::find( const QString& countryCode ) const return find( [&]( const Label& l ){ return l.language() == c_l.second; } ); } -LabelModel* const availableTranslations() +LabelModel* availableTranslations() { static LabelModel model( QString( CALAMARES_TRANSLATION_LANGUAGES ).split( ';') ); return &model; } +} } // namespace diff --git a/src/libcalamares/locale/LabelModel.h b/src/libcalamares/locale/LabelModel.h index 178f76343..2f3bee629 100644 --- a/src/libcalamares/locale/LabelModel.h +++ b/src/libcalamares/locale/LabelModel.h @@ -26,8 +26,9 @@ #include -namespace CalamaresUtils {} -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { class DLLEXPORT LabelModel : public QAbstractListModel @@ -78,7 +79,7 @@ private: * * NOTE: While the model is not typed const, it should be. Do not modify. */ -DLLEXPORT LabelModel* const availableTranslations(); - +DLLEXPORT LabelModel* availableTranslations(); +} } // namespace #endif diff --git a/src/libcalamares/locale/Lookup.cpp b/src/libcalamares/locale/Lookup.cpp index a096bc679..73d706de6 100644 --- a/src/libcalamares/locale/Lookup.cpp +++ b/src/libcalamares/locale/Lookup.cpp @@ -20,7 +20,9 @@ #include "CountryData_p.cpp" -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { struct TwoChar @@ -87,4 +89,5 @@ QLocale::Language languageForCountry(QLocale::Country country) return p->l; } +} } // namespace diff --git a/src/libcalamares/locale/Lookup.h b/src/libcalamares/locale/Lookup.h index 5712a1120..9d1c23cd8 100644 --- a/src/libcalamares/locale/Lookup.h +++ b/src/libcalamares/locale/Lookup.h @@ -24,8 +24,9 @@ #include #include -namespace CalamaresUtils {} -namespace CalamaresUtils::Locale +namespace CalamaresUtils +{ +namespace Locale { /* All the functions in this file do lookups of locale data * based on CLDR tables; these are lookups that you can't (easily) @@ -48,6 +49,7 @@ namespace CalamaresUtils::Locale DLLEXPORT QPair< QLocale::Country, QLocale::Language > countryData( const QString& code ); /// @brief Get a likely locale for a 2-letter country code DLLEXPORT QLocale countryLocale( const QString& code ); +} } // namespace #endif From 2c94cbdb14cf63e4d7ab593dce34de12d6dba608 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:25:14 +0200 Subject: [PATCH 07/36] [libcalamares] namespace A::B is a C++17 extension --- src/libcalamares/geoip/GeoIPJSON.cpp | 7 ++++--- src/libcalamares/geoip/GeoIPJSON.h | 5 ++++- src/libcalamares/geoip/GeoIPXML.h | 5 ++++- src/libcalamares/geoip/Handler.cpp | 6 ++++-- src/libcalamares/geoip/Handler.h | 6 ++++-- src/libcalamares/geoip/Interface.cpp | 5 ++++- src/libcalamares/geoip/Interface.h | 6 ++++-- 7 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/libcalamares/geoip/GeoIPJSON.cpp b/src/libcalamares/geoip/GeoIPJSON.cpp index 61b9fd8d6..4b7562d7e 100644 --- a/src/libcalamares/geoip/GeoIPJSON.cpp +++ b/src/libcalamares/geoip/GeoIPJSON.cpp @@ -25,7 +25,9 @@ #include -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { GeoIPJSON::GeoIPJSON(const QString& attribute) @@ -88,6 +90,5 @@ GeoIPJSON::processReply( const QByteArray& data ) return splitTZString( rawReply( data ) ); } - - +} } // namespace diff --git a/src/libcalamares/geoip/GeoIPJSON.h b/src/libcalamares/geoip/GeoIPJSON.h index 584825d70..4d7ded631 100644 --- a/src/libcalamares/geoip/GeoIPJSON.h +++ b/src/libcalamares/geoip/GeoIPJSON.h @@ -21,7 +21,9 @@ #include "Interface.h" -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief GeoIP lookup for services that return JSON. * @@ -46,5 +48,6 @@ public: virtual QString rawReply(const QByteArray & ) override; } ; +} } // namespace #endif diff --git a/src/libcalamares/geoip/GeoIPXML.h b/src/libcalamares/geoip/GeoIPXML.h index 7dee2ecbe..356e88b12 100644 --- a/src/libcalamares/geoip/GeoIPXML.h +++ b/src/libcalamares/geoip/GeoIPXML.h @@ -21,7 +21,9 @@ #include "Interface.h" -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief GeoIP lookup with XML data * @@ -46,5 +48,6 @@ public: virtual QString rawReply(const QByteArray & ) override; } ; +} } // namespace #endif diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 1e8b03b26..4add3eb13 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -47,7 +47,9 @@ handlerTypes() return names; } -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { Handler::Handler() @@ -179,5 +181,5 @@ Handler::queryRaw() const } ); } - +} } // namespace diff --git a/src/libcalamares/geoip/Handler.h b/src/libcalamares/geoip/Handler.h index 92e5f326e..5c3207b60 100644 --- a/src/libcalamares/geoip/Handler.h +++ b/src/libcalamares/geoip/Handler.h @@ -25,8 +25,9 @@ #include #include -namespace CalamaresUtils {} -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief Handle one complete GeoIP lookup. @@ -86,6 +87,7 @@ private: const QString m_selector; }; +} } // namespace #endif diff --git a/src/libcalamares/geoip/Interface.cpp b/src/libcalamares/geoip/Interface.cpp index 50aa04683..36e680aab 100644 --- a/src/libcalamares/geoip/Interface.cpp +++ b/src/libcalamares/geoip/Interface.cpp @@ -20,7 +20,9 @@ #include "utils/Logger.h" -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { Interface::Interface(const QString& e) @@ -51,4 +53,5 @@ splitTZString( const QString& tz ) return RegionZonePair( QString(), QString() ); } +} } // namespace diff --git a/src/libcalamares/geoip/Interface.h b/src/libcalamares/geoip/Interface.h index 4b2ff3a5a..7db8c4c91 100644 --- a/src/libcalamares/geoip/Interface.h +++ b/src/libcalamares/geoip/Interface.h @@ -27,8 +27,9 @@ class QByteArray; -namespace CalamaresUtils {} -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { /** @brief A Region, Zone pair of strings * @@ -94,5 +95,6 @@ protected: QString m_element; // string for selecting from data } ; +} } // namespace #endif From 0b0fb93e75244d1e41a8555c45488596e6fc11a1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:33:03 +0200 Subject: [PATCH 08/36] [libcalamares] Remove redundant default: in case - the switch handles all values of the enum and the compiler should be smart enough to know that (therefore default isn't needed, nor the return afterwards). --- src/libcalamares/geoip/Handler.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 4add3eb13..60404c2d6 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -112,8 +112,6 @@ create_interface( Handler::Type t, const QString& selector ) #else return nullptr; #endif - default: // there are no others - return nullptr; } } From d048975f15ff89d09e8c8b732e92213ce728a4fe Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:06:28 +0200 Subject: [PATCH 09/36] [libcalamares] One more nested namespace --- src/libcalamares/geoip/GeoIPXML.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/geoip/GeoIPXML.cpp b/src/libcalamares/geoip/GeoIPXML.cpp index a4b9bb146..b658042bf 100644 --- a/src/libcalamares/geoip/GeoIPXML.cpp +++ b/src/libcalamares/geoip/GeoIPXML.cpp @@ -23,7 +23,9 @@ #include #include -namespace CalamaresUtils::GeoIP +namespace CalamaresUtils +{ +namespace GeoIP { GeoIPXML::GeoIPXML( const QString& element ) @@ -87,4 +89,5 @@ GeoIPXML::processReply( const QByteArray& data ) return RegionZonePair(); } +} } // namespace From 93a68c3d5f140805d993e06c019b46da260b4588 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:32:14 +0200 Subject: [PATCH 10/36] [libcalamares] Add convenience method to check for unit-comparability - Not all kinds of units are comparable. Introduce a method in PartitionSize to check for comparability (this could also be a free method, but seems more tidy here because it is specifically about comparing in the context of partition sizes). --- src/libcalamares/partition/PartitionSize.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index 13ffa5c70..975ebd887 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2019, Collabora Ltd + * Copyright 2019, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -96,6 +97,20 @@ public: * @return the size in bytes, or -1 if it cannot be calculated. */ qint64 toBytes() const; + + /** @brief Are the units comparable? + * + * None units cannot be compared with anything. Percentages can + * be compared with each other, and all the explicit sizes (KiB, ...) + * can be compared with each other. + */ + static constexpr bool unitsComparable( const SizeUnit u1, const SizeUnit u2 ) + { + return !( ( u1 == SizeUnit::None || u2 == SizeUnit::None ) || + ( u1 == SizeUnit::Percent && u2 != SizeUnit::Percent ) || + ( u1 != SizeUnit::Percent && u2 == SizeUnit::Percent ) ); + } + }; } // namespace Calamares From 7a368dc1d748fbb77926a3f25ab47c51e4580015 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:33:38 +0200 Subject: [PATCH 11/36] [libcalamares] Add tests for the partitioning service --- src/libcalamares/CMakeLists.txt | 10 ++ src/libcalamares/partition/Tests.cpp | 145 +++++++++++++++++++++++++++ src/libcalamares/partition/Tests.h | 41 ++++++++ 3 files changed, 196 insertions(+) create mode 100644 src/libcalamares/partition/Tests.cpp create mode 100644 src/libcalamares/partition/Tests.h diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index 959b0a9db..b414887c4 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -167,6 +167,16 @@ if ( ECM_FOUND AND BUILD_TESTING ) ${YAMLCPP_LIBRARY} ) calamares_automoc( geoiptest ) + + ecm_add_test( + partition/Tests.cpp + TEST_NAME + libcalamarespartitiontest + LINK_LIBRARIES + calamares + Qt5::Test + ) + calamares_automoc( libcalamarespartitiontest ) endif() if( BUILD_TESTING ) diff --git a/src/libcalamares/partition/Tests.cpp b/src/libcalamares/partition/Tests.cpp new file mode 100644 index 000000000..ac2d4f431 --- /dev/null +++ b/src/libcalamares/partition/Tests.cpp @@ -0,0 +1,145 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#include "Tests.h" + +#include "PartitionSize.h" + +Q_DECLARE_METATYPE( Calamares::SizeUnit ) + +#include "utils/Logger.h" + +#include + +QTEST_GUILESS_MAIN( PartitionSizeTests ) + +PartitionSizeTests::PartitionSizeTests() +{ +} + +PartitionSizeTests::~PartitionSizeTests() +{ +} + +void +PartitionSizeTests::initTestCase() +{ +} + +void +PartitionSizeTests::testUnitComparison_data() +{ + QTest::addColumn("u1"); + QTest::addColumn("u2"); + QTest::addColumn("comparable"); + + using Calamares::SizeUnit; + + QTest::newRow("nones") << SizeUnit::None << SizeUnit::None << false; + QTest::newRow("none+%") << SizeUnit::None << SizeUnit::Percent<< false; + QTest::newRow("%+none") << SizeUnit::Percent << SizeUnit::None << false; + QTest::newRow("KiB+none") << SizeUnit::KiB << SizeUnit::None << false; + QTest::newRow("none+MiB") << SizeUnit::None << SizeUnit::MiB << false; + + QTest::newRow("KiB+KiB") << SizeUnit::KiB << SizeUnit::KiB << true; + QTest::newRow("KiB+MiB") << SizeUnit::KiB << SizeUnit::MiB << true; + QTest::newRow("KiB+GiB") << SizeUnit::KiB << SizeUnit::GiB << true; + QTest::newRow("MiB+MiB") << SizeUnit::MiB << SizeUnit::MiB << true; + QTest::newRow("MiB+GiB") << SizeUnit::MiB << SizeUnit::GiB << true; + QTest::newRow("GiB+GiB") << SizeUnit::GiB << SizeUnit::GiB << true; + + QTest::newRow("%+None") << SizeUnit::Percent << SizeUnit::None << false; + QTest::newRow("%+%") << SizeUnit::Percent << SizeUnit::Percent << true; + QTest::newRow("%+KiB") << SizeUnit::Percent << SizeUnit::KiB << false; +} + + +static bool +original_compare( Calamares::SizeUnit m_unit, Calamares::SizeUnit other_m_unit ) +{ + if ( ( m_unit == Calamares::SizeUnit::None || other_m_unit == Calamares::SizeUnit::None ) || + ( m_unit == Calamares::SizeUnit::Percent && other_m_unit != Calamares::SizeUnit::Percent ) || + ( m_unit != Calamares::SizeUnit::Percent && other_m_unit == Calamares::SizeUnit::Percent ) ) + return false; + return true; +} + +void +PartitionSizeTests::testUnitComparison() +{ + QFETCH( Calamares::SizeUnit, u1 ); + QFETCH( Calamares::SizeUnit, u2 ); + QFETCH( bool, comparable ); + + if ( comparable ) + { + QVERIFY( Calamares::PartitionSize::unitsComparable( u1, u2 ) ); + QVERIFY( Calamares::PartitionSize::unitsComparable( u2, u1 ) ); + } + else + { + QVERIFY( !Calamares::PartitionSize::unitsComparable( u1, u2 ) ); + QVERIFY( !Calamares::PartitionSize::unitsComparable( u2, u1 ) ); + } + + QCOMPARE( original_compare( u1, u2 ), Calamares::PartitionSize::unitsComparable( u1, u2 ) ); +} + +void +PartitionSizeTests::testUnitNormalisation_data() +{ + QTest::addColumn("u1"); + QTest::addColumn("v"); + QTest::addColumn("bytes"); + + using Calamares::SizeUnit; + + QTest::newRow("none") << SizeUnit::None << 16 << -1; + QTest::newRow("none") << SizeUnit::None << 0 << -1; + QTest::newRow("none") << SizeUnit::None << -2 << -1; + + QTest::newRow("percent") << SizeUnit::Percent << 0 << -1; + QTest::newRow("percent") << SizeUnit::Percent << 16 << -1; + QTest::newRow("percent") << SizeUnit::Percent << -2 << -1; + + QTest::newRow("KiB") << SizeUnit::KiB << 0 << 0; + QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024; + QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000; + QTest::newRow("KiB") << SizeUnit::KiB << 1024 << 1024 * 1024; + QTest::newRow("KiB") << SizeUnit::KiB << -2 << -1; + + QTest::newRow("MiB") << SizeUnit::MiB << 0 << 0; + QTest::newRow("MiB") << SizeUnit::MiB << 1 << 1024 * 1024; + QTest::newRow("MiB") << SizeUnit::MiB << 1000 << 1024 * 1024000; + QTest::newRow("MiB") << SizeUnit::MiB << 1024 << 1024 * 1024 * 1024; + QTest::newRow("MiB") << SizeUnit::MiB << -2 << -1; + + QTest::newRow("GiB") << SizeUnit::GiB << 0 << 0; + QTest::newRow("GiB") << SizeUnit::GiB << 1 << 1024 * 1024 * 1024; + QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2 * 1024 * 1024 * 1024; +} + +void +PartitionSizeTests::testUnitNormalisation() +{ + QFETCH( Calamares::SizeUnit, u1 ); + QFETCH( int, v ); + QFETCH( int, bytes ); + + QCOMPARE( Calamares::PartitionSize( v, u1 ).toBytes(), static_cast( bytes ) ); +} diff --git a/src/libcalamares/partition/Tests.h b/src/libcalamares/partition/Tests.h new file mode 100644 index 000000000..24398233d --- /dev/null +++ b/src/libcalamares/partition/Tests.h @@ -0,0 +1,41 @@ +/* === This file is part of Calamares - === + * + * Copyright 2019, Adriaan de Groot + * + * Calamares is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Calamares is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Calamares. If not, see . + */ + +#ifndef LIBCALAMARES_PARTITION_TESTS_H +#define LIBCALAMARES_PARTITION_TESTS_H + +#include + +class PartitionSizeTests : public QObject +{ + Q_OBJECT +public: + PartitionSizeTests(); + ~PartitionSizeTests() override; + +private Q_SLOTS: + void initTestCase(); + + void testUnitComparison_data(); + void testUnitComparison(); + + void testUnitNormalisation_data(); + void testUnitNormalisation(); +}; + +#endif From 72e1a36752605df35acecde6a01e437a167e8391 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:38:30 +0200 Subject: [PATCH 12/36] [libcalamares] Update partition service tests - Use long so that 2GiB fits in the values - Document special case of 0[KMG]iB --- src/libcalamares/partition/PartitionSize.h | 3 +- src/libcalamares/partition/Tests.cpp | 42 +++++++++++----------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index 975ebd887..da15a5e80 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -92,7 +92,8 @@ public: /** @brief Convert the size to bytes. * * This method is only valid for sizes in Bytes, KiB, MiB or GiB. - * It will return -1 in any other case. + * It will return -1 in any other case. Note that 0KiB and 0MiB and + * 0GiB are considered **invalid** sizes and return -1. * * @return the size in bytes, or -1 if it cannot be calculated. */ diff --git a/src/libcalamares/partition/Tests.cpp b/src/libcalamares/partition/Tests.cpp index ac2d4f431..78d8a42a0 100644 --- a/src/libcalamares/partition/Tests.cpp +++ b/src/libcalamares/partition/Tests.cpp @@ -105,33 +105,33 @@ PartitionSizeTests::testUnitNormalisation_data() { QTest::addColumn("u1"); QTest::addColumn("v"); - QTest::addColumn("bytes"); + QTest::addColumn("bytes"); using Calamares::SizeUnit; - QTest::newRow("none") << SizeUnit::None << 16 << -1; - QTest::newRow("none") << SizeUnit::None << 0 << -1; - QTest::newRow("none") << SizeUnit::None << -2 << -1; + QTest::newRow("none") << SizeUnit::None << 16 << -1L; + QTest::newRow("none") << SizeUnit::None << 0 << -1L; + QTest::newRow("none") << SizeUnit::None << -2 << -1L; - QTest::newRow("percent") << SizeUnit::Percent << 0 << -1; - QTest::newRow("percent") << SizeUnit::Percent << 16 << -1; - QTest::newRow("percent") << SizeUnit::Percent << -2 << -1; + QTest::newRow("percent") << SizeUnit::Percent << 0 << -1L; + QTest::newRow("percent") << SizeUnit::Percent << 16 << -1L; + QTest::newRow("percent") << SizeUnit::Percent << -2 << -1L; - QTest::newRow("KiB") << SizeUnit::KiB << 0 << 0; - QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024; - QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000; - QTest::newRow("KiB") << SizeUnit::KiB << 1024 << 1024 * 1024; - QTest::newRow("KiB") << SizeUnit::KiB << -2 << -1; + QTest::newRow("KiB") << SizeUnit::KiB << 0 << -1L; + QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024L; + QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000L; + QTest::newRow("KiB") << SizeUnit::KiB << 1024 << 1024 * 1024L; + QTest::newRow("KiB") << SizeUnit::KiB << -2 << -1L; - QTest::newRow("MiB") << SizeUnit::MiB << 0 << 0; - QTest::newRow("MiB") << SizeUnit::MiB << 1 << 1024 * 1024; - QTest::newRow("MiB") << SizeUnit::MiB << 1000 << 1024 * 1024000; - QTest::newRow("MiB") << SizeUnit::MiB << 1024 << 1024 * 1024 * 1024; - QTest::newRow("MiB") << SizeUnit::MiB << -2 << -1; + QTest::newRow("MiB") << SizeUnit::MiB << 0 << -1L; + QTest::newRow("MiB") << SizeUnit::MiB << 1 << 1024 * 1024L; + QTest::newRow("MiB") << SizeUnit::MiB << 1000 << 1024 * 1024000L; + QTest::newRow("MiB") << SizeUnit::MiB << 1024 << 1024 * 1024 * 1024L; + QTest::newRow("MiB") << SizeUnit::MiB << -2 << -1L; - QTest::newRow("GiB") << SizeUnit::GiB << 0 << 0; - QTest::newRow("GiB") << SizeUnit::GiB << 1 << 1024 * 1024 * 1024; - QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2 * 1024 * 1024 * 1024; + QTest::newRow("GiB") << SizeUnit::GiB << 0 << -1L; + QTest::newRow("GiB") << SizeUnit::GiB << 1 << 1024 * 1024 * 1024L; + QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2048 * 1024 * 1024L; } void @@ -139,7 +139,7 @@ PartitionSizeTests::testUnitNormalisation() { QFETCH( Calamares::SizeUnit, u1 ); QFETCH( int, v ); - QFETCH( int, bytes ); + QFETCH( long, bytes ); QCOMPARE( Calamares::PartitionSize( v, u1 ).toBytes(), static_cast( bytes ) ); } From 90975b62bf167e2d08309b17935d1b3974a9916c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:44:30 +0200 Subject: [PATCH 13/36] [libcalamares] Tidy PartitionSize - Use unitsComparable where applicable - Use SizeUnit instead of unit_t -- since this is a template specialization, we have the more meaningful type name to use, instead of the generic one. --- src/libcalamares/partition/PartitionSize.cpp | 99 ++++++++++---------- src/libcalamares/partition/PartitionSize.h | 2 +- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 5c4c0e040..98db92985 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -1,6 +1,7 @@ /* === This file is part of Calamares - === * * Copyright 2019, Collabora Ltd + * Copyright 2019, Adriaan de Groot * * Calamares is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -42,23 +43,23 @@ unitSuffixes() PartitionSize::PartitionSize( const QString& s ) : NamedSuffix( unitSuffixes(), s ) { - if ( ( unit() == unit_t::Percent ) && ( value() > 100 || value() < 0 ) ) + if ( ( unit() == SizeUnit::Percent ) && ( value() > 100 || value() < 0 ) ) { cDebug() << "Percent value" << value() << "is not valid."; m_value = 0; } - if ( m_unit == unit_t::None ) + if ( m_unit == SizeUnit::None ) { m_value = s.toInt(); if ( m_value > 0 ) - m_unit = unit_t::Byte; + m_unit = SizeUnit::Byte; } if ( m_value <= 0 ) { m_value = 0; - m_unit = unit_t::None; + m_unit = SizeUnit::None; } } @@ -72,17 +73,17 @@ PartitionSize::toSectors( qint64 totalSectors, qint64 sectorSize ) const switch ( m_unit ) { - case unit_t::None: + case SizeUnit::None: return -1; - case unit_t::Percent: + case SizeUnit::Percent: if ( value() == 100 ) return totalSectors; // Common-case, avoid futzing around else return totalSectors * value() / 100; - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return CalamaresUtils::bytesToSectors ( toBytes(), sectorSize ); } @@ -97,19 +98,19 @@ PartitionSize::toBytes( qint64 totalSectors, qint64 sectorSize ) const switch ( m_unit ) { - case unit_t::None: + case SizeUnit::None: return -1; - case unit_t::Percent: + case SizeUnit::Percent: if ( totalSectors < 1 || sectorSize < 1 ) return -1; if ( value() == 100 ) return totalSectors * sectorSize; // Common-case, avoid futzing around else return totalSectors * value() / 100; - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return toBytes(); } @@ -125,19 +126,19 @@ PartitionSize::toBytes( qint64 totalBytes ) const switch ( m_unit ) { - case unit_t::None: + case SizeUnit::None: return -1; - case unit_t::Percent: + case SizeUnit::Percent: if ( totalBytes < 1 ) return -1; if ( value() == 100 ) return totalBytes; // Common-case, avoid futzing around else return totalBytes * value() / 100; - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return toBytes(); } @@ -153,16 +154,16 @@ PartitionSize::toBytes() const switch ( m_unit ) { - case unit_t::None: - case unit_t::Percent: + case SizeUnit::None: + case SizeUnit::Percent: return -1; - case unit_t::Byte: + case SizeUnit::Byte: return value(); - case unit_t::KiB: + case SizeUnit::KiB: return CalamaresUtils::KiBtoBytes( static_cast( value() ) ); - case unit_t::MiB: + case SizeUnit::MiB: return CalamaresUtils::MiBtoBytes( static_cast( value() ) ); - case unit_t::GiB: + case SizeUnit::GiB: return CalamaresUtils::GiBtoBytes( static_cast( value() ) ); } } @@ -170,19 +171,17 @@ PartitionSize::toBytes() const bool PartitionSize::operator< ( const PartitionSize& other ) const { - if ( ( m_unit == unit_t::None || other.m_unit == unit_t::None ) || - ( m_unit == unit_t::Percent && other.m_unit != unit_t::Percent ) || - ( m_unit != unit_t::Percent && other.m_unit == unit_t::Percent ) ) + if ( !unitsComparable( m_unit, other.m_unit ) ) return false; switch ( m_unit ) { - case unit_t::Percent: + case SizeUnit::Percent: return ( m_value < other.m_value ); - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return ( toBytes() < other.toBytes () ); } @@ -192,19 +191,17 @@ PartitionSize::operator< ( const PartitionSize& other ) const bool PartitionSize::operator> ( const PartitionSize& other ) const { - if ( ( m_unit == unit_t::None || other.m_unit == unit_t::None ) || - ( m_unit == unit_t::Percent && other.m_unit != unit_t::Percent ) || - ( m_unit != unit_t::Percent && other.m_unit == unit_t::Percent ) ) + if ( !unitsComparable( m_unit, other.m_unit ) ) return false; switch ( m_unit ) { - case unit_t::Percent: + case SizeUnit::Percent: return ( m_value > other.m_value ); - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return ( toBytes() > other.toBytes () ); } @@ -214,19 +211,17 @@ PartitionSize::operator> ( const PartitionSize& other ) const bool PartitionSize::operator== ( const PartitionSize& other ) const { - if ( ( m_unit == unit_t::None || other.m_unit == unit_t::None ) || - ( m_unit == unit_t::Percent && other.m_unit != unit_t::Percent ) || - ( m_unit != unit_t::Percent && other.m_unit == unit_t::Percent ) ) + if ( !unitsComparable( m_unit, other.m_unit ) ) return false; switch ( m_unit ) { - case unit_t::Percent: + case SizeUnit::Percent: return ( m_value == other.m_value ); - case unit_t::Byte: - case unit_t::KiB: - case unit_t::MiB: - case unit_t::GiB: + case SizeUnit::Byte: + case SizeUnit::KiB: + case SizeUnit::MiB: + case SizeUnit::GiB: return ( toBytes() == other.toBytes () ); } diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index da15a5e80..75ee960d9 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -49,7 +49,7 @@ class PartitionSize : public NamedSuffix { public: PartitionSize() : NamedSuffix() { } - PartitionSize( int v, unit_t u ) : NamedSuffix( v, u ) { } + PartitionSize( int v, SizeUnit u ) : NamedSuffix( v, u ) { } PartitionSize( const QString& ); bool isValid() const From 6db09f06796cf9abaa460b5c76ceabb1cee8fa42 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 13:54:09 +0200 Subject: [PATCH 14/36] [libcalamares] Handle all SizeUnit cases inside switch - Although None will be filtered out already by unitsComparable(), include it in the switch to avoid a warning .. then we can drop the post-switch return since the switch covers all possible values of the enum. --- src/libcalamares/partition/PartitionSize.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 98db92985..8eb399585 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -176,6 +176,8 @@ PartitionSize::operator< ( const PartitionSize& other ) const switch ( m_unit ) { + case SizeUnit::None: + return false; case SizeUnit::Percent: return ( m_value < other.m_value ); case SizeUnit::Byte: @@ -184,8 +186,6 @@ PartitionSize::operator< ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() < other.toBytes () ); } - - return false; } bool @@ -196,6 +196,8 @@ PartitionSize::operator> ( const PartitionSize& other ) const switch ( m_unit ) { + case SizeUnit::None: + return false; case SizeUnit::Percent: return ( m_value > other.m_value ); case SizeUnit::Byte: @@ -204,8 +206,6 @@ PartitionSize::operator> ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() > other.toBytes () ); } - - return false; } bool @@ -216,6 +216,8 @@ PartitionSize::operator== ( const PartitionSize& other ) const switch ( m_unit ) { + case SizeUnit::None: + return false; case SizeUnit::Percent: return ( m_value == other.m_value ); case SizeUnit::Byte: @@ -224,8 +226,6 @@ PartitionSize::operator== ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() == other.toBytes () ); } - - return false; } } // namespace Calamares From ed3eafbc2db8c005d5f0adcc05e996b964fd69cd Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 14:08:30 +0200 Subject: [PATCH 15/36] [oemid] Reduce warnings about vtable by adding virtual destructor --- src/modules/oemid/OEMViewStep.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/modules/oemid/OEMViewStep.cpp b/src/modules/oemid/OEMViewStep.cpp index 0f076927b..8a12bd17c 100644 --- a/src/modules/oemid/OEMViewStep.cpp +++ b/src/modules/oemid/OEMViewStep.cpp @@ -43,9 +43,15 @@ public: ) } + virtual ~OEMPage() override; + Ui_OEMPage* m_ui; } ; +OEMPage::~OEMPage() +{ +} + OEMViewStep::OEMViewStep(QObject* parent) : Calamares::ViewStep( parent ) From ba7ee445c61b9833c318b7cdeaada88bf8ae3e82 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 14:52:26 +0200 Subject: [PATCH 16/36] CMake: switch to using autouic on plugins - Use autouic so that we can also pass in --include to add a code-warning-suppression to the generated code, just like we can do with moc. --- CMakeLists.txt | 1 + CMakeModules/CalamaresAddLibrary.cmake | 10 ++++------ CMakeModules/CalamaresAutomoc.cmake | 13 ++++++++++++- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 050191c1f..843e8bc69 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -211,6 +211,7 @@ if( CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) set( CMAKE_SHARED_LINKER_FLAGS "-Wl,--no-undefined" ) set( CALAMARES_AUTOMOC_OPTIONS "-butils/moc-warnings.h" ) + set( CALAMARES_AUTOUIC_OPTIONS --include utils/moc-warnings.h ) else() set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wl,--no-undefined" ) set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wl,--fatal-warnings -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type" ) diff --git a/CMakeModules/CalamaresAddLibrary.cmake b/CMakeModules/CalamaresAddLibrary.cmake index e731e2b15..0829d919e 100644 --- a/CMakeModules/CalamaresAddLibrary.cmake +++ b/CMakeModules/CalamaresAddLibrary.cmake @@ -61,11 +61,6 @@ function(calamares_add_library) include_directories(${CMAKE_CURRENT_LIST_DIR}) include_directories(${CMAKE_CURRENT_BINARY_DIR}) - if(LIBRARY_UI) - qt5_wrap_ui(LIBRARY_UI_SOURCES ${LIBRARY_UI}) - list(APPEND LIBRARY_SOURCES ${LIBRARY_UI_SOURCES}) - endif() - # add resources from current dir if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/${LIBRARY_RESOURCES}") qt5_add_resources(LIBRARY_RC_SOURCES "${LIBRARY_RESOURCES}") @@ -83,7 +78,10 @@ function(calamares_add_library) endif() calamares_automoc(${target}) - + if(LIBRARY_UI) + calamares_autouic(${target} ${LIBRARY_UI}) + endif() + if(LIBRARY_EXPORT_MACRO) set_target_properties(${target} PROPERTIES COMPILE_DEFINITIONS ${LIBRARY_EXPORT_MACRO}) endif() diff --git a/CMakeModules/CalamaresAutomoc.cmake b/CMakeModules/CalamaresAutomoc.cmake index 0ca5cd89a..f8aa7faef 100644 --- a/CMakeModules/CalamaresAutomoc.cmake +++ b/CMakeModules/CalamaresAutomoc.cmake @@ -18,7 +18,7 @@ # ### # -# Helper function for doing automoc on a target. +# Helper function for doing automoc on a target, and autoui on a .ui file. # # Sets AUTOMOC TRUE for a target. # @@ -27,6 +27,8 @@ # libcalamares/utils/moc-warnings.h file to the moc, which in turn # reduces compiler warnings in generated MOC code. # +# If the global variable CALAMARES_AUTOUIC_OPTIONS is set, adds that +# to the options passed to uic. function(calamares_automoc TARGET) set_target_properties( ${TARGET} PROPERTIES AUTOMOC TRUE ) @@ -34,3 +36,12 @@ function(calamares_automoc TARGET) set_target_properties( ${TARGET} PROPERTIES AUTOMOC_MOC_OPTIONS "${CALAMARES_AUTOMOC_OPTIONS}" ) endif() endfunction() + +function(calamares_autouic TARGET) + set_target_properties( ${TARGET} PROPERTIES AUTOUIC TRUE ) + if ( CALAMARES_AUTOUIC_OPTIONS ) + foreach(S ${ARGN}) + set_property(SOURCE ${S} PROPERTY AUTOUIC_OPTIONS "${CALAMARES_AUTOUIC_OPTIONS}") + endforeach() + endif() +endfunction() From c44eaf107faf7e44f8b663cea95f66583ea065af Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:39:56 -0400 Subject: [PATCH 17/36] CI: When stopping the build early, log where it was left --- ci/RELEASE.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ci/RELEASE.sh b/ci/RELEASE.sh index a835ebcb3..dfb65cfeb 100644 --- a/ci/RELEASE.sh +++ b/ci/RELEASE.sh @@ -41,8 +41,8 @@ BUILDDIR=$(mktemp -d --suffix=-build --tmpdir=.) if test "x$BUILD_DEFAULT" = "xtrue" ; then rm -rf "$BUILDDIR" mkdir "$BUILDDIR" || { echo "Could not create build directory." ; exit 1 ; } - ( cd "$BUILDDIR" && cmake .. && make -j4 ) || { echo "Could not perform test-build." ; exit 1 ; } - ( cd "$BUILDDIR" && make test ) || { echo "Tests failed." ; exit 1 ; } + ( cd "$BUILDDIR" && cmake .. && make -j4 ) || { echo "Could not perform test-build in $BUILDDIR." ; exit 1 ; } + ( cd "$BUILDDIR" && make test ) || { echo "Tests failed in $BUILDDIR." ; exit 1 ; } fi ### Build with clang @@ -53,13 +53,13 @@ if test "x$BUILD_CLANG" = "xtrue" ; then # Do build again with clang rm -rf "$BUILDDIR" mkdir "$BUILDDIR" || { echo "Could not create build directory." ; exit 1 ; } - ( cd "$BUILDDIR" && CC=clang CXX=clang++ cmake .. && make -j4 ) || { echo "Could not perform test-build." ; exit 1 ; } - ( cd "$BUILDDIR" && make test ) || { echo "Tests failed." ; exit 1 ; } + ( cd "$BUILDDIR" && CC=clang CXX=clang++ cmake .. && make -j4 ) || { echo "Could not perform test-build in $BUILDDIR." ; exit 1 ; } + ( cd "$BUILDDIR" && make test ) || { echo "Tests failed in $BUILDDIR." ; exit 1 ; } fi fi if test "x$BUILD_ONLY" = "xtrue" ; then - echo "Builds completed, release stopped." + echo "Builds completed, release stopped. Build remains in $BUILDDIR." exit 1 fi From 10ba46874825b9d69bb8e856676bd1a4afc8062d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 13 May 2019 12:04:31 -0400 Subject: [PATCH 18/36] [libcalamares] Avoid warnings / errors on both gcc and clang - Clang 8 can detect that there is no need for a return if all previous paths already return. GCC 8 does not. Clang warns if the unreachable return is there, GCC errors out if it isn't. - Introduce a hack NOTREACHED that comments-out on Clang, and marks as unreachable (but still present) on GCC. - This might go away with an [[unreachable]] annotation or similar. --- CMakeLists.txt | 3 +++ src/libcalamares/geoip/Handler.cpp | 1 + src/libcalamares/partition/PartitionSize.cpp | 4 ++++ 3 files changed, 8 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 843e8bc69..a4571efba 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -193,6 +193,7 @@ if( CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) ) string( APPEND CMAKE_CXX_FLAGS " ${CLANG_WARNINGS}" ) endforeach() + set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNOTREACHED='//'" ) # Third-party code where we don't care so much about compiler warnings # (because it's uncomfortable to patch) get different flags; use @@ -218,6 +219,8 @@ else() set( SUPPRESS_3RDPARTY_WARNINGS "" ) set( SUPPRESS_BOOST_WARNINGS "" ) + + set( CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -DNOTREACHED='__builtin_unreachable();'" ) endif() # Use mark_thirdparty_code() to reduce warnings from the compiler diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 60404c2d6..192ab1a7c 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -113,6 +113,7 @@ create_interface( Handler::Type t, const QString& selector ) return nullptr; #endif } + NOTREACHED return nullptr; } static RegionZonePair diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 8eb399585..750f75393 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -166,6 +166,7 @@ PartitionSize::toBytes() const case SizeUnit::GiB: return CalamaresUtils::GiBtoBytes( static_cast( value() ) ); } + NOTREACHED return -1; } bool @@ -186,6 +187,7 @@ PartitionSize::operator< ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() < other.toBytes () ); } + NOTREACHED return false; } bool @@ -206,6 +208,7 @@ PartitionSize::operator> ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() > other.toBytes () ); } + NOTREACHED return false; } bool @@ -226,6 +229,7 @@ PartitionSize::operator== ( const PartitionSize& other ) const case SizeUnit::GiB: return ( toBytes() == other.toBytes () ); } + NOTREACHED return false; } } // namespace Calamares From bffaf47900ace8ce1a3ebe80966f74010a5b216b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:39:19 -0400 Subject: [PATCH 19/36] [partition] Reduce warnings about integer size --- src/modules/partition/gui/ResizeVolumeGroupDialog.cpp | 2 +- src/modules/partition/gui/VolumeGroupBaseDialog.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp b/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp index 1c5eef0ab..2de999360 100644 --- a/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp +++ b/src/modules/partition/gui/ResizeVolumeGroupDialog.cpp @@ -44,7 +44,7 @@ ResizeVolumeGroupDialog::ResizeVolumeGroupDialog( LvmDevice *device, for ( const Partition* p : availablePVs ) pvList()->addItem( new ListPhysicalVolumeWidgetItem( p, false ) ); - peSize()->setValue( device->peSize() / Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB) ); + peSize()->setValue( static_cast( device->peSize() / Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB) ) ); vgName()->setEnabled( false ); peSize()->setEnabled( false ); diff --git a/src/modules/partition/gui/VolumeGroupBaseDialog.cpp b/src/modules/partition/gui/VolumeGroupBaseDialog.cpp index a727fe42a..8078253b3 100644 --- a/src/modules/partition/gui/VolumeGroupBaseDialog.cpp +++ b/src/modules/partition/gui/VolumeGroupBaseDialog.cpp @@ -137,9 +137,9 @@ VolumeGroupBaseDialog::updateTotalSize() void VolumeGroupBaseDialog::updateTotalSectors() { - qint32 totalSectors = 0; + qint64 totalSectors = 0; - qint32 extentSize = ui->peSize->value() * Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB); + qint64 extentSize = ui->peSize->value() * Capacity::unitFactor(Capacity::Unit::Byte, Capacity::Unit::MiB); if ( extentSize > 0 ) totalSectors = m_totalSizeValue / extentSize; From fd4bc4bb17b2758db216204f7196c4d0356bc3ec Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:46:21 -0400 Subject: [PATCH 20/36] [partition] Avoid UB by initializing size everywhere --- src/modules/partition/core/PartitionLayout.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index a988da3f7..de5535a3e 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -150,7 +150,7 @@ PartitionLayout::execute( Device *dev, qint64 firstSector, const PartitionRole& role ) { QList< Partition* > partList; - qint64 size, minSize, maxSize, end; + qint64 minSize, maxSize, end; qint64 totalSize = lastSector - firstSector + 1; qint64 availableSize = totalSize; @@ -161,6 +161,7 @@ PartitionLayout::execute( Device *dev, qint64 firstSector, { Partition *currentPartition = nullptr; + qint64 size = -1; // Calculate partition size if ( part.partSize.isValid() ) { From 54108d2bab337dd42b10eea93658a675beae907d Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:49:53 -0400 Subject: [PATCH 21/36] [partition] Fix up logging of jobs - Logging `*it` was printing raw pointers, logging (plain) `it` needs the specialized logging `operator<<` to DTRT with temporaries. --- src/modules/partition/jobs/CreatePartitionTableJob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/partition/jobs/CreatePartitionTableJob.cpp b/src/modules/partition/jobs/CreatePartitionTableJob.cpp index 937b8437d..3465a0e2d 100644 --- a/src/modules/partition/jobs/CreatePartitionTableJob.cpp +++ b/src/modules/partition/jobs/CreatePartitionTableJob.cpp @@ -68,7 +68,7 @@ CreatePartitionTableJob::prettyStatusMessage() const static inline QDebug& -operator <<( QDebug& s, PartitionIterator& it ) +operator <<( QDebug&& s, PartitionIterator& it ) { s << ( ( *it ) ? ( *it )->deviceNode() : QString( "" ) ); return s; @@ -89,7 +89,7 @@ CreatePartitionTableJob::exec() { for ( auto it = PartitionIterator::begin( table ); it != PartitionIterator::end( table ); ++it ) - cDebug() << *it; + cDebug() << it; QProcess lsblk; lsblk.setProgram( "lsblk" ); From 81715ba1999f382a0fd201806bdb5b28021d94b8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 04:58:19 -0400 Subject: [PATCH 22/36] [partition] Warnings-- by using nullptr instead of 0 --- src/modules/partition/gui/PartitionPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/partition/gui/PartitionPage.cpp b/src/modules/partition/gui/PartitionPage.cpp index 790ff84ab..647a69d5d 100644 --- a/src/modules/partition/gui/PartitionPage.cpp +++ b/src/modules/partition/gui/PartitionPage.cpp @@ -564,7 +564,7 @@ PartitionPage::updateFromCurrentDevice() QAbstractItemModel* oldModel = m_ui->partitionTreeView->model(); if ( oldModel ) - disconnect( oldModel, 0, this, 0 ); + disconnect( oldModel, nullptr, this, nullptr ); PartitionModel* model = m_core->partitionModelForDevice( device ); m_ui->partitionBarsView->setModel( model ); From 7e12b65c94a04bfaa92331712c0886f8d825494c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:01:39 -0400 Subject: [PATCH 23/36] [partition] Silence warnings about missing vtable --- src/modules/partition/tests/PartitionJobTests.cpp | 5 +++++ src/modules/partition/tests/PartitionJobTests.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/modules/partition/tests/PartitionJobTests.cpp b/src/modules/partition/tests/PartitionJobTests.cpp index e4707accf..acb671254 100644 --- a/src/modules/partition/tests/PartitionJobTests.cpp +++ b/src/modules/partition/tests/PartitionJobTests.cpp @@ -130,6 +130,11 @@ QueueRunner::QueueRunner( JobQueue* queue ) connect( m_queue, &JobQueue::failed, this, &QueueRunner::onFailed ); } +QueueRunner::~QueueRunner() +{ + // Nothing to do. We don't own the queue, and disconnect happens automatically +} + bool QueueRunner::run() { diff --git a/src/modules/partition/tests/PartitionJobTests.h b/src/modules/partition/tests/PartitionJobTests.h index 0744cbdda..62d5924ea 100644 --- a/src/modules/partition/tests/PartitionJobTests.h +++ b/src/modules/partition/tests/PartitionJobTests.h @@ -36,6 +36,7 @@ class QueueRunner : public QObject { public: QueueRunner( Calamares::JobQueue* queue ); + virtual ~QueueRunner() override; /** * Synchronously runs the queue. Returns true on success From 80606cc38d3b67f8af70fda5a351f3cbe7c3a15c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:27:38 -0400 Subject: [PATCH 24/36] [partition] Reduce test warnings through consistent signedness --- .../partition/tests/PartitionJobTests.cpp | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/modules/partition/tests/PartitionJobTests.cpp b/src/modules/partition/tests/PartitionJobTests.cpp index acb671254..f3bd8dd13 100644 --- a/src/modules/partition/tests/PartitionJobTests.cpp +++ b/src/modules/partition/tests/PartitionJobTests.cpp @@ -77,7 +77,7 @@ static QByteArray generateTestData( qint64 size ) { QByteArray ba; - ba.resize( size ); + ba.resize( static_cast( size ) ); // Fill the array explicitly to keep Valgrind happy for ( auto it = ba.data() ; it < ba.data() + size ; ++it ) { @@ -172,7 +172,8 @@ PartitionJobTests::initTestCase() QString devicePath = qgetenv( "CALAMARES_TEST_DISK" ); if ( devicePath.isEmpty() ) { - QSKIP( "Skipping test, CALAMARES_TEST_DISK is not set. It should point to a disk which can be safely formatted" ); + // The 0 is to keep the macro parameters happy + QSKIP( "Skipping test, CALAMARES_TEST_DISK is not set. It should point to a disk which can be safely formatted", 0 ); } QVERIFY( KPMHelpers::initKPMcore() ); @@ -327,10 +328,10 @@ PartitionJobTests::testCreatePartitionExtended() void PartitionJobTests::testResizePartition_data() { - QTest::addColumn< int >( "oldStartMiB" ); - QTest::addColumn< int >( "oldSizeMiB" ); - QTest::addColumn< int >( "newStartMiB" ); - QTest::addColumn< int >( "newSizeMiB" ); + QTest::addColumn< unsigned int >( "oldStartMiB" ); + QTest::addColumn< unsigned int >( "oldSizeMiB" ); + QTest::addColumn< unsigned int >( "newStartMiB" ); + QTest::addColumn< unsigned int >( "newSizeMiB" ); QTest::newRow("grow") << 10 << 50 << 10 << 70; QTest::newRow("shrink") << 10 << 70 << 10 << 50; @@ -341,10 +342,10 @@ PartitionJobTests::testResizePartition_data() void PartitionJobTests::testResizePartition() { - QFETCH( int, oldStartMiB ); - QFETCH( int, oldSizeMiB ); - QFETCH( int, newStartMiB ); - QFETCH( int, newSizeMiB ); + QFETCH( unsigned int, oldStartMiB ); + QFETCH( unsigned int, oldSizeMiB ); + QFETCH( unsigned int, newStartMiB ); + QFETCH( unsigned int, newSizeMiB ); const qint64 sectorsPerMiB = 1_MiB / m_device->logicalSize(); From e520c66bb9c332872c5b1ccf14b67cab65a948e3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:11:07 -0400 Subject: [PATCH 25/36] [fsresizer] Give the FSResizerJob some accessors - This is primarily for the tests: then they can drop the #define private public hack and be "proper" consumers. --- src/modules/fsresizer/ResizeFSJob.h | 15 ++++++++ src/modules/fsresizer/Tests.cpp | 53 +++++++++++++---------------- 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/modules/fsresizer/ResizeFSJob.h b/src/modules/fsresizer/ResizeFSJob.h index 97696e40b..79828027f 100644 --- a/src/modules/fsresizer/ResizeFSJob.h +++ b/src/modules/fsresizer/ResizeFSJob.h @@ -54,6 +54,21 @@ public: m_size.isValid(); } + QString name() const + { + return m_fsname.isEmpty() ? m_devicename : m_fsname; + } + + Calamares::PartitionSize size() const + { + return m_size; + } + + Calamares::PartitionSize minimumSize() const + { + return m_atleast; + } + private: Calamares::PartitionSize m_size; Calamares::PartitionSize m_atleast; diff --git a/src/modules/fsresizer/Tests.cpp b/src/modules/fsresizer/Tests.cpp index 5e645a95f..ecb2fea9d 100644 --- a/src/modules/fsresizer/Tests.cpp +++ b/src/modules/fsresizer/Tests.cpp @@ -30,9 +30,7 @@ #include #include -#define private public #include "ResizeFSJob.h" -#undef private QTEST_GUILESS_MAIN( FSResizerTests ) @@ -55,10 +53,9 @@ void FSResizerTests::testConfigurationRobust() // Empty config j.setConfigurationMap( QVariantMap() ); - QVERIFY( j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); // Config is missing fs and dev, so it isn't valid YAML::Node doc0 = YAML::Load( R"(--- @@ -66,12 +63,11 @@ size: 100% atleast: 600MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_size.value(), 0 ); - QCOMPARE( j.m_atleast.value(), 0 ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().value(), 0 ); + QCOMPARE( j.minimumSize().value(), 0 ); } void FSResizerTests::testConfigurationValues() @@ -85,12 +81,11 @@ size: 100% atleast: 600MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( !j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::Percent ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.m_size.value(), 100 ); - QCOMPARE( j.m_atleast.value(), 600 ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::Percent ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.size().value(), 100 ); + QCOMPARE( j.minimumSize().value(), 600 ); // Silly config doc0 = YAML::Load( R"(--- @@ -100,12 +95,11 @@ size: 72 MiB atleast: 127 % )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( !j.m_fsname.isEmpty() ); - QVERIFY( !j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_size.value(), 72 ); - QCOMPARE( j.m_atleast.value(), 0 ); + QVERIFY( !j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().value(), 72 ); + QCOMPARE( j.minimumSize().value(), 0 ); // Silly config doc0 = YAML::Load( R"(--- @@ -115,10 +109,9 @@ size: 71MiB # atleast: 127% )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); - QVERIFY( !j.m_fsname.isEmpty() ); - QVERIFY( j.m_devicename.isEmpty() ); - QCOMPARE( j.m_size.unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.m_atleast.unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.m_size.value(), 71 ); - QCOMPARE( j.m_atleast.value(), 0 ); + QVERIFY( j.name().isEmpty() ); + QCOMPARE( j.size().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().value(), 71 ); + QCOMPARE( j.minimumSize().value(), 0 ); } From 34ffc7a20aec26ddb7f6c835a49f54b61f19236e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 05:52:58 -0400 Subject: [PATCH 26/36] [libcalamares] Consistent namespace usage in partition service - The sub-directories under libcalamares (e.g. Utils, ..) all live in namespace CalamaresUtils (well, except for Logger). The services (e.g. subdirs other than utils/) live in their own nested namespace, so partitioning should go into CalamaresUtils::Partition for consistency. --- src/libcalamares/partition/PartitionSize.cpp | 7 +- src/libcalamares/partition/PartitionSize.h | 11 ++-- src/libcalamares/partition/Tests.cpp | 65 +++++++++---------- src/modules/fsresizer/ResizeFSJob.cpp | 4 +- src/modules/fsresizer/ResizeFSJob.h | 10 +-- src/modules/fsresizer/Tests.cpp | 24 +++---- .../partition/core/PartitionActions.cpp | 2 +- .../partition/core/PartitionLayout.cpp | 6 +- src/modules/partition/core/PartitionLayout.h | 6 +- 9 files changed, 72 insertions(+), 63 deletions(-) diff --git a/src/libcalamares/partition/PartitionSize.cpp b/src/libcalamares/partition/PartitionSize.cpp index 750f75393..ee3d4c80b 100644 --- a/src/libcalamares/partition/PartitionSize.cpp +++ b/src/libcalamares/partition/PartitionSize.cpp @@ -21,7 +21,9 @@ #include "utils/Logger.h" #include "utils/Units.h" -namespace Calamares +namespace CalamaresUtils +{ +namespace Partition { static const NamedEnumTable& @@ -232,4 +234,5 @@ PartitionSize::operator== ( const PartitionSize& other ) const NOTREACHED return false; } -} // namespace Calamares +} +} // namespace diff --git a/src/libcalamares/partition/PartitionSize.h b/src/libcalamares/partition/PartitionSize.h index 75ee960d9..d8d5593a6 100644 --- a/src/libcalamares/partition/PartitionSize.h +++ b/src/libcalamares/partition/PartitionSize.h @@ -26,7 +26,9 @@ // Qt #include -namespace Calamares +namespace CalamaresUtils +{ +namespace Partition { enum class SizeUnit @@ -98,9 +100,9 @@ public: * @return the size in bytes, or -1 if it cannot be calculated. */ qint64 toBytes() const; - + /** @brief Are the units comparable? - * + * * None units cannot be compared with anything. Percentages can * be compared with each other, and all the explicit sizes (KiB, ...) * can be compared with each other. @@ -114,6 +116,7 @@ public: }; -} // namespace Calamares +} +} // namespace #endif // PARTITION_PARTITIONSIZE_H diff --git a/src/libcalamares/partition/Tests.cpp b/src/libcalamares/partition/Tests.cpp index 78d8a42a0..c0eb5206a 100644 --- a/src/libcalamares/partition/Tests.cpp +++ b/src/libcalamares/partition/Tests.cpp @@ -20,7 +20,10 @@ #include "PartitionSize.h" -Q_DECLARE_METATYPE( Calamares::SizeUnit ) +using SizeUnit = CalamaresUtils::Partition::SizeUnit; +using PartitionSize = CalamaresUtils::Partition::PartitionSize; + +Q_DECLARE_METATYPE( SizeUnit ) #include "utils/Logger.h" @@ -41,28 +44,26 @@ PartitionSizeTests::initTestCase() { } -void +void PartitionSizeTests::testUnitComparison_data() { - QTest::addColumn("u1"); - QTest::addColumn("u2"); + QTest::addColumn("u1"); + QTest::addColumn("u2"); QTest::addColumn("comparable"); - using Calamares::SizeUnit; - QTest::newRow("nones") << SizeUnit::None << SizeUnit::None << false; QTest::newRow("none+%") << SizeUnit::None << SizeUnit::Percent<< false; QTest::newRow("%+none") << SizeUnit::Percent << SizeUnit::None << false; QTest::newRow("KiB+none") << SizeUnit::KiB << SizeUnit::None << false; QTest::newRow("none+MiB") << SizeUnit::None << SizeUnit::MiB << false; - + QTest::newRow("KiB+KiB") << SizeUnit::KiB << SizeUnit::KiB << true; QTest::newRow("KiB+MiB") << SizeUnit::KiB << SizeUnit::MiB << true; QTest::newRow("KiB+GiB") << SizeUnit::KiB << SizeUnit::GiB << true; QTest::newRow("MiB+MiB") << SizeUnit::MiB << SizeUnit::MiB << true; QTest::newRow("MiB+GiB") << SizeUnit::MiB << SizeUnit::GiB << true; QTest::newRow("GiB+GiB") << SizeUnit::GiB << SizeUnit::GiB << true; - + QTest::newRow("%+None") << SizeUnit::Percent << SizeUnit::None << false; QTest::newRow("%+%") << SizeUnit::Percent << SizeUnit::Percent << true; QTest::newRow("%+KiB") << SizeUnit::Percent << SizeUnit::KiB << false; @@ -70,53 +71,51 @@ PartitionSizeTests::testUnitComparison_data() static bool -original_compare( Calamares::SizeUnit m_unit, Calamares::SizeUnit other_m_unit ) +original_compare( SizeUnit m_unit, SizeUnit other_m_unit ) { - if ( ( m_unit == Calamares::SizeUnit::None || other_m_unit == Calamares::SizeUnit::None ) || - ( m_unit == Calamares::SizeUnit::Percent && other_m_unit != Calamares::SizeUnit::Percent ) || - ( m_unit != Calamares::SizeUnit::Percent && other_m_unit == Calamares::SizeUnit::Percent ) ) + if ( ( m_unit == SizeUnit::None || other_m_unit == SizeUnit::None ) || + ( m_unit == SizeUnit::Percent && other_m_unit != SizeUnit::Percent ) || + ( m_unit != SizeUnit::Percent && other_m_unit == SizeUnit::Percent ) ) return false; return true; } -void +void PartitionSizeTests::testUnitComparison() { - QFETCH( Calamares::SizeUnit, u1 ); - QFETCH( Calamares::SizeUnit, u2 ); + QFETCH( SizeUnit, u1 ); + QFETCH( SizeUnit, u2 ); QFETCH( bool, comparable ); - + if ( comparable ) { - QVERIFY( Calamares::PartitionSize::unitsComparable( u1, u2 ) ); - QVERIFY( Calamares::PartitionSize::unitsComparable( u2, u1 ) ); + QVERIFY( PartitionSize::unitsComparable( u1, u2 ) ); + QVERIFY( PartitionSize::unitsComparable( u2, u1 ) ); } else { - QVERIFY( !Calamares::PartitionSize::unitsComparable( u1, u2 ) ); - QVERIFY( !Calamares::PartitionSize::unitsComparable( u2, u1 ) ); + QVERIFY( !PartitionSize::unitsComparable( u1, u2 ) ); + QVERIFY( !PartitionSize::unitsComparable( u2, u1 ) ); } - - QCOMPARE( original_compare( u1, u2 ), Calamares::PartitionSize::unitsComparable( u1, u2 ) ); + + QCOMPARE( original_compare( u1, u2 ), PartitionSize::unitsComparable( u1, u2 ) ); } -void +void PartitionSizeTests::testUnitNormalisation_data() { - QTest::addColumn("u1"); + QTest::addColumn("u1"); QTest::addColumn("v"); QTest::addColumn("bytes"); - - using Calamares::SizeUnit; - + QTest::newRow("none") << SizeUnit::None << 16 << -1L; QTest::newRow("none") << SizeUnit::None << 0 << -1L; QTest::newRow("none") << SizeUnit::None << -2 << -1L; - + QTest::newRow("percent") << SizeUnit::Percent << 0 << -1L; QTest::newRow("percent") << SizeUnit::Percent << 16 << -1L; QTest::newRow("percent") << SizeUnit::Percent << -2 << -1L; - + QTest::newRow("KiB") << SizeUnit::KiB << 0 << -1L; QTest::newRow("KiB") << SizeUnit::KiB << 1 << 1024L; QTest::newRow("KiB") << SizeUnit::KiB << 1000 << 1024000L; @@ -134,12 +133,12 @@ PartitionSizeTests::testUnitNormalisation_data() QTest::newRow("GiB") << SizeUnit::GiB << 2 << 2048 * 1024 * 1024L; } -void +void PartitionSizeTests::testUnitNormalisation() { - QFETCH( Calamares::SizeUnit, u1 ); + QFETCH( SizeUnit, u1 ); QFETCH( int, v ); QFETCH( long, bytes ); - - QCOMPARE( Calamares::PartitionSize( v, u1 ).toBytes(), static_cast( bytes ) ); + + QCOMPARE( PartitionSize( v, u1 ).toBytes(), static_cast( bytes ) ); } diff --git a/src/modules/fsresizer/ResizeFSJob.cpp b/src/modules/fsresizer/ResizeFSJob.cpp index 0ab583367..6b9ef9d3e 100644 --- a/src/modules/fsresizer/ResizeFSJob.cpp +++ b/src/modules/fsresizer/ResizeFSJob.cpp @@ -272,8 +272,8 @@ ResizeFSJob::setConfigurationMap( const QVariantMap& configurationMap ) return; } - m_size = Calamares::PartitionSize( configurationMap["size"].toString() ); - m_atleast = Calamares::PartitionSize( configurationMap["atleast"].toString() ); + m_size = PartitionSize( configurationMap["size"].toString() ); + m_atleast = PartitionSize( configurationMap["atleast"].toString() ); m_required = CalamaresUtils::getBool( configurationMap, "required", false ); } diff --git a/src/modules/fsresizer/ResizeFSJob.h b/src/modules/fsresizer/ResizeFSJob.h index 79828027f..76ca6c219 100644 --- a/src/modules/fsresizer/ResizeFSJob.h +++ b/src/modules/fsresizer/ResizeFSJob.h @@ -33,6 +33,8 @@ class CoreBackend; // From KPMCore class Device; // From KPMCore class Partition; +using PartitionSize = CalamaresUtils::Partition::PartitionSize; + class PLUGINDLLEXPORT ResizeFSJob : public Calamares::CppJob { Q_OBJECT @@ -59,19 +61,19 @@ public: return m_fsname.isEmpty() ? m_devicename : m_fsname; } - Calamares::PartitionSize size() const + PartitionSize size() const { return m_size; } - Calamares::PartitionSize minimumSize() const + PartitionSize minimumSize() const { return m_atleast; } private: - Calamares::PartitionSize m_size; - Calamares::PartitionSize m_atleast; + PartitionSize m_size; + PartitionSize m_atleast; QString m_fsname; // Either this, or devicename, is set, not both QString m_devicename; bool m_required; diff --git a/src/modules/fsresizer/Tests.cpp b/src/modules/fsresizer/Tests.cpp index ecb2fea9d..b825beb6e 100644 --- a/src/modules/fsresizer/Tests.cpp +++ b/src/modules/fsresizer/Tests.cpp @@ -18,6 +18,8 @@ #include "Tests.h" +#include "ResizeFSJob.h" + #include "GlobalStorage.h" #include "JobQueue.h" #include "Settings.h" @@ -30,7 +32,7 @@ #include #include -#include "ResizeFSJob.h" +using SizeUnit = CalamaresUtils::Partition::SizeUnit; QTEST_GUILESS_MAIN( FSResizerTests ) @@ -54,8 +56,8 @@ void FSResizerTests::testConfigurationRobust() // Empty config j.setConfigurationMap( QVariantMap() ); QVERIFY( j.name().isEmpty() ); - QCOMPARE( j.size().unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().unit(), SizeUnit::None ); + QCOMPARE( j.minimumSize().unit(), SizeUnit::None ); // Config is missing fs and dev, so it isn't valid YAML::Node doc0 = YAML::Load( R"(--- @@ -64,8 +66,8 @@ atleast: 600MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); QVERIFY( j.name().isEmpty() ); - QCOMPARE( j.size().unit(), Calamares::SizeUnit::None ); - QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().unit(), SizeUnit::None ); + QCOMPARE( j.minimumSize().unit(), SizeUnit::None ); QCOMPARE( j.size().value(), 0 ); QCOMPARE( j.minimumSize().value(), 0 ); } @@ -82,8 +84,8 @@ atleast: 600MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); QVERIFY( j.name().isEmpty() ); - QCOMPARE( j.size().unit(), Calamares::SizeUnit::Percent ); - QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::MiB ); + QCOMPARE( j.size().unit(), SizeUnit::Percent ); + QCOMPARE( j.minimumSize().unit(), SizeUnit::MiB ); QCOMPARE( j.size().value(), 100 ); QCOMPARE( j.minimumSize().value(), 600 ); @@ -96,8 +98,8 @@ atleast: 127 % )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); QVERIFY( !j.name().isEmpty() ); - QCOMPARE( j.size().unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().unit(), SizeUnit::MiB ); + QCOMPARE( j.minimumSize().unit(), SizeUnit::None ); QCOMPARE( j.size().value(), 72 ); QCOMPARE( j.minimumSize().value(), 0 ); @@ -110,8 +112,8 @@ size: 71MiB )" ); j.setConfigurationMap( CalamaresUtils::yamlMapToVariant( doc0 ).toMap() ); QVERIFY( j.name().isEmpty() ); - QCOMPARE( j.size().unit(), Calamares::SizeUnit::MiB ); - QCOMPARE( j.minimumSize().unit(), Calamares::SizeUnit::None ); + QCOMPARE( j.size().unit(), SizeUnit::MiB ); + QCOMPARE( j.minimumSize().unit(), SizeUnit::None ); QCOMPARE( j.size().value(), 71 ); QCOMPARE( j.minimumSize().value(), 0 ); } diff --git a/src/modules/partition/core/PartitionActions.cpp b/src/modules/partition/core/PartitionActions.cpp index 1c89a5b7f..f0d5acdd9 100644 --- a/src/modules/partition/core/PartitionActions.cpp +++ b/src/modules/partition/core/PartitionActions.cpp @@ -103,7 +103,7 @@ doAutopartition( PartitionCoreModule* core, Device* dev, Choices::AutoPartitionO { if ( gs->contains( "efiSystemPartitionSize" ) ) { - Calamares::PartitionSize part_size = Calamares::PartitionSize( + CalamaresUtils::Partition::PartitionSize part_size = CalamaresUtils::Partition::PartitionSize( gs->value( "efiSystemPartitionSize" ).toString() ); uefisys_part_sizeB = part_size.toBytes( dev->capacity() ); } diff --git a/src/modules/partition/core/PartitionLayout.cpp b/src/modules/partition/core/PartitionLayout.cpp index de5535a3e..35b90722a 100644 --- a/src/modules/partition/core/PartitionLayout.cpp +++ b/src/modules/partition/core/PartitionLayout.cpp @@ -86,10 +86,10 @@ PartitionLayout::addEntry( PartitionLayout::PartitionEntry entry ) } PartitionLayout::PartitionEntry::PartitionEntry( const QString& size, const QString& min, const QString& max ) + : partSize( size ) + , partMinSize( min ) + , partMaxSize( max ) { - partSize = Calamares::PartitionSize( size ); - partMinSize = Calamares::PartitionSize( min ); - partMaxSize = Calamares::PartitionSize( max ); } bool diff --git a/src/modules/partition/core/PartitionLayout.h b/src/modules/partition/core/PartitionLayout.h index 74bc09873..a9b8d410b 100644 --- a/src/modules/partition/core/PartitionLayout.h +++ b/src/modules/partition/core/PartitionLayout.h @@ -43,9 +43,9 @@ public: QString partLabel; QString partMountPoint; FileSystem::Type partFileSystem = FileSystem::Unknown; - Calamares::PartitionSize partSize; - Calamares::PartitionSize partMinSize; - Calamares::PartitionSize partMaxSize; + CalamaresUtils::Partition::PartitionSize partSize; + CalamaresUtils::Partition::PartitionSize partMinSize; + CalamaresUtils::Partition::PartitionSize partMaxSize; /// @brief All-zeroes PartitionEntry PartitionEntry() {} From d78bc0c5c536117f0b2b36339c3752854013add2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 13:04:24 +0200 Subject: [PATCH 27/36] [libcalamaresui] When disable-cancel is on, never confirm - This function is also reached by clicking the window-close decoration. --- src/libcalamaresui/ViewManager.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 9da71f85f..49ecfb9ad 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -364,14 +364,19 @@ ViewManager::back() bool ViewManager::confirmCancelInstallation() { + const auto* const settings = Calamares::Settings::instance(); + + if ( settings->disableCancel() ) + return false; + // If it's NOT the last page of the last step, we ask for confirmation if ( !( m_currentStep == m_steps.count() -1 && m_steps.last()->isAtEnd() ) ) { - QString title = Calamares::Settings::instance()->isSetupMode() + QString title = settings->isSetupMode() ? tr( "Cancel setup?" ) : tr( "Cancel installation?" ); - QString question = Calamares::Settings::instance()->isSetupMode() + QString question = settings->isSetupMode() ? tr( "Do you really want to cancel the current setup process?\n" "The setup program will quit and all changes will be lost." ) : tr( "Do you really want to cancel the current install process?\n" From 58686571018da86108256ec4dc182ee70be00f40 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 13:06:54 +0200 Subject: [PATCH 28/36] [calamares] Hide the window-close decoration when disable-cancel is set --- src/calamares/CalamaresWindow.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/calamares/CalamaresWindow.cpp b/src/calamares/CalamaresWindow.cpp index 157941a90..6f851513c 100644 --- a/src/calamares/CalamaresWindow.cpp +++ b/src/calamares/CalamaresWindow.cpp @@ -56,6 +56,10 @@ CalamaresWindow::CalamaresWindow( QWidget* parent ) , m_debugWindow( nullptr ) , m_viewManager( nullptr ) { + // If we can never cancel, don't show the window-close button + if ( Calamares::Settings::instance()->disableCancel() ) + setWindowFlags( Qt::Window | Qt::WindowTitleHint | Qt::CustomizeWindowHint ); + CALAMARES_RETRANSLATE( setWindowTitle( Calamares::Settings::instance()->isSetupMode() ? tr( "%1 Setup Program" ).arg( *Calamares::Branding::ProductName ) From 25099ae854aa3047d650e296b043b46d90ace946 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 13:10:36 +0200 Subject: [PATCH 29/36] [libcalamaresui] Remove duplicate setEnabled - If executing is set to true, then later setEnabled( !executing && ... ) fill be false, so we don't need to call setEnabled( false ) here as well. --- src/libcalamaresui/ViewManager.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 49ecfb9ad..4a2f62154 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -281,12 +281,8 @@ ViewManager::next() executing = qobject_cast< ExecutionViewStep* >( m_steps.at( m_currentStep ) ) != nullptr; emit currentStepChanged(); if ( executing ) - { - m_back->setEnabled( false ); - m_next->setEnabled( false ); // Enabled if there's nothing blocking it during exec m_quit->setEnabled( !( settings->dontCancel() || settings->disableCancel() ) ); - } else // Enabled unless it's also hidden m_quit->setEnabled( !settings->disableCancel() ); From ad4352b65c6ddde04b5ea995e1f23186fe40467e Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 13:18:51 +0200 Subject: [PATCH 30/36] [libcalamaresui] Make stepIsExecute() more general - Checking if the **next** step is an execute-step is a little weird, so make the API more general (and add the +1 to indexes where it was using NextWillExecute before). --- src/libcalamaresui/ViewManager.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 4a2f62154..3777ff9c1 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -227,10 +227,18 @@ ViewManager::currentStepIndex() const return m_currentStep; } +/** @brief Is the given step at @p index an execution step? + * + * Returns true if the step is an execution step, false otherwise. + * Also returns false if the @p index is out of range. + */ static inline bool -stepNextWillExecute(const ViewStepList& steps, int index) +stepIsExecute( const ViewStepList& steps, int index ) { - return ( index + 1 < steps.count() ) && qobject_cast< ExecutionViewStep* >( steps.at( index + 1 ) ); + return + ( 0 <= index ) && + ( index < steps.count() ) && + ( qobject_cast< ExecutionViewStep* >( steps.at( index ) ) != nullptr ); } void @@ -245,7 +253,7 @@ ViewManager::next() // Special case when the user clicks next on the very last page in a view phase // and right before switching to an execution phase. // Depending on Calamares::Settings, we show an "are you sure" prompt or not. - if ( settings->showPromptBeforeExecution() && stepNextWillExecute( m_steps, m_currentStep ) ) + if ( settings->showPromptBeforeExecution() && stepIsExecute( m_steps, m_currentStep+1 ) ) { QString title = settings->isSetupMode() ? tr( "Continue with setup?" ) @@ -309,7 +317,8 @@ ViewManager::updateButtonLabels() ? tr( "Cancel setup without changing the system." ) : tr( "Cancel installation without changing the system." ); - if ( stepNextWillExecute( m_steps, m_currentStep ) ) + // If we're going into the execution step / install phase, other message + if ( stepIsExecute( m_steps, m_currentStep+1 ) ) m_next->setText( next ); else m_next->setText( tr( "&Next" ) ); From 088fa5004cc5219f9c0b98d28bc474021ab77cff Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 13:21:05 +0200 Subject: [PATCH 31/36] [libcalamaresui] Disallow closing the window during execution - If the disable-cancel-during-exec setting is on, and the user clicks the window-close button, then disregard the close message. --- src/libcalamaresui/ViewManager.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 3777ff9c1..817ee346f 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -373,6 +373,8 @@ bool ViewManager::confirmCancelInstallation() if ( settings->disableCancel() ) return false; + if ( settings->dontCancel() && stepIsExecute( m_steps, m_currentStep ) ) + return false; // If it's NOT the last page of the last step, we ask for confirmation if ( !( m_currentStep == m_steps.count() -1 && From d4f4a40fa5f9dda75cbb26281ce1c7aac39ed3eb Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 13:34:14 +0200 Subject: [PATCH 32/36] [libcalamaresui] Refactor quit-enabling - Add signal for change-of-quit-enabledness - Minor tidying --- src/libcalamaresui/ViewManager.cpp | 33 ++++++++++++++++-------------- src/libcalamaresui/ViewManager.h | 4 +++- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 817ee346f..450fa18a7 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -288,12 +288,7 @@ ViewManager::next() m_steps.at( m_currentStep )->onActivate(); executing = qobject_cast< ExecutionViewStep* >( m_steps.at( m_currentStep ) ) != nullptr; emit currentStepChanged(); - if ( executing ) - // Enabled if there's nothing blocking it during exec - m_quit->setEnabled( !( settings->dontCancel() || settings->disableCancel() ) ); - else - // Enabled unless it's also hidden - m_quit->setEnabled( !settings->disableCancel() ); + updateCancelEnabled( !settings->disableCancel() && !(executing && settings->dontCancel() ) ); } else step->next(); @@ -307,13 +302,15 @@ ViewManager::next() void ViewManager::updateButtonLabels() { - QString next = Calamares::Settings::instance()->isSetupMode() + const auto* const settings = Calamares::Settings::instance(); + + QString next = settings->isSetupMode() ? tr( "&Set up" ) : tr( "&Install" ); - QString complete = Calamares::Settings::instance()->isSetupMode() + QString complete = settings->isSetupMode() ? tr( "Setup is complete. Close the setup program." ) : tr( "The installation is complete. Close the installer." ); - QString quit = Calamares::Settings::instance()->isSetupMode() + QString quit = settings->isSetupMode() ? tr( "Cancel setup without changing the system." ) : tr( "Cancel installation without changing the system." ); @@ -328,15 +325,14 @@ ViewManager::updateButtonLabels() m_quit->setText( tr( "&Done" ) ); m_quit->setToolTip( complete ); m_quit->setVisible( true ); // At end, always visible and enabled. - m_quit->setEnabled( true ); + updateCancelEnabled( true ); } else { - if ( Calamares::Settings::instance()->disableCancel() ) - { - m_quit->setVisible( false ); - m_quit->setEnabled( false ); // Can't be triggered through DBUS - } + if ( settings->disableCancel() ) + m_quit->setVisible( false ); // In case we went back from final + updateCancelEnabled( !settings->disableCancel() && !( stepIsExecute( m_steps, m_currentStep ) && settings->dontCancel() ) ); + m_quit->setText( tr( "&Cancel" ) ); m_quit->setToolTip( quit ); } @@ -403,4 +399,11 @@ bool ViewManager::confirmCancelInstallation() return true; } +void +ViewManager::updateCancelEnabled( bool enabled ) +{ + m_quit->setEnabled( enabled ); + emit cancelEnabled( enabled ); +} + } // namespace diff --git a/src/libcalamaresui/ViewManager.h b/src/libcalamaresui/ViewManager.h index 65d787e44..50e8d1dc4 100644 --- a/src/libcalamaresui/ViewManager.h +++ b/src/libcalamaresui/ViewManager.h @@ -121,6 +121,7 @@ public slots: signals: void currentStepChanged(); void enlarge( QSize enlarge ) const; // See ViewStep::enlarge() + void cancelEnabled( bool enabled ) const; private: explicit ViewManager( QObject* parent = nullptr ); @@ -128,7 +129,8 @@ private: void insertViewStep( int before, ViewStep* step ); void updateButtonLabels(); - + void updateCancelEnabled( bool enabled ); + static ViewManager* s_instance; ViewStepList m_steps; From 2208ff95fe45fbdd7b510f97bbcfbaff95661e61 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 14:03:53 +0200 Subject: [PATCH 33/36] [calamares] Simplify disable-window-close-button code --- src/calamares/CalamaresWindow.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/calamares/CalamaresWindow.cpp b/src/calamares/CalamaresWindow.cpp index 6f851513c..b2e09d961 100644 --- a/src/calamares/CalamaresWindow.cpp +++ b/src/calamares/CalamaresWindow.cpp @@ -58,7 +58,7 @@ CalamaresWindow::CalamaresWindow( QWidget* parent ) { // If we can never cancel, don't show the window-close button if ( Calamares::Settings::instance()->disableCancel() ) - setWindowFlags( Qt::Window | Qt::WindowTitleHint | Qt::CustomizeWindowHint ); + setWindowFlag( Qt::WindowCloseButtonHint, false ); CALAMARES_RETRANSLATE( setWindowTitle( Calamares::Settings::instance()->isSetupMode() From a5cba02769bd826a665b796c0b59c639e1fe5824 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 14:07:33 +0200 Subject: [PATCH 34/36] [calamares] Leave a note about changing close-window hint --- src/calamares/CalamaresWindow.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/calamares/CalamaresWindow.cpp b/src/calamares/CalamaresWindow.cpp index b2e09d961..b570a9c86 100644 --- a/src/calamares/CalamaresWindow.cpp +++ b/src/calamares/CalamaresWindow.cpp @@ -165,7 +165,15 @@ CalamaresWindow::CalamaresWindow( QWidget* parent ) m_viewManager = Calamares::ViewManager::instance( this ); if ( branding->windowExpands() ) connect( m_viewManager, &Calamares::ViewManager::enlarge, this, &CalamaresWindow::enlarge ); - + // NOTE: Although the ViewManager has a signal cancelEnabled() that + // signals when the state of the cancel button changes (in + // particular, to disable cancel during the exec phase), + // we don't connect to it here. Changing the window flag + // for the close button causes uncomfortable window flashing + // and requires an extra show() (at least with KWin/X11) which + // is too annoying. Instead, leave it up to ignoring-the-quit- + // event, which is also the ViewManager's responsibility. + mainLayout->addWidget( m_viewManager->centralWidget() ); setStyleSheet( Calamares::Branding::instance()->stylesheet() ); } From 4de703430f5d9f24c44ca3eb3197a34faa5045c1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 14:08:32 +0200 Subject: [PATCH 35/36] CMake: -O4 doesn't do anything in Clang, use -O3 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a4571efba..c18c11658 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -204,7 +204,7 @@ if( CMAKE_CXX_COMPILER_ID MATCHES "Clang" ) set( CMAKE_CXX_FLAGS_DEBUG "-g ${CMAKE_CXX_FLAGS_DEBUG}" ) set( CMAKE_CXX_FLAGS_MINSIZEREL "-Os -DNDEBUG" ) - set( CMAKE_CXX_FLAGS_RELEASE "-O4 -DNDEBUG" ) + set( CMAKE_CXX_FLAGS_RELEASE "-O3 -DNDEBUG" ) set( CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O2 -g" ) set( CMAKE_TOOLCHAIN_PREFIX "llvm-" ) From f3bfc81e5231bfefdbac3b6e277a2839d462d8db Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 14 May 2019 08:30:34 -0400 Subject: [PATCH 36/36] [libcalamares] Rename dontCancel to disableCancelDuringExec - This way the name actually refers to what it does, rather than being a somewhat ambiguous overload of disableCancel. --- src/libcalamares/Settings.cpp | 8 ++++---- src/libcalamares/Settings.h | 4 ++-- src/libcalamaresui/ViewManager.cpp | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/libcalamares/Settings.cpp b/src/libcalamares/Settings.cpp index 0089b2ef2..71825b1e5 100644 --- a/src/libcalamares/Settings.cpp +++ b/src/libcalamares/Settings.cpp @@ -185,7 +185,7 @@ Settings::Settings( const QString& settingsFilePath, , m_doChroot( true ) , m_promptInstall( false ) , m_disableCancel( false ) - , m_dontCancel( false ) + , m_disableCancelDuringExec( false ) { cDebug() << "Using Calamares settings file at" << settingsFilePath; QFile file( settingsFilePath ); @@ -207,7 +207,7 @@ Settings::Settings( const QString& settingsFilePath, m_doChroot = !requireBool( config, "dont-chroot", false ); m_isSetupMode = requireBool( config, "oem-setup", !m_doChroot ); m_disableCancel = requireBool( config, "disable-cancel", false ); - m_dontCancel = requireBool( config, "disable-cancel-during-exec", false ); + m_disableCancelDuringExec = requireBool( config, "disable-cancel-during-exec", false ); } catch ( YAML::Exception& e ) { @@ -277,9 +277,9 @@ Settings::disableCancel() const } bool -Settings::dontCancel() const +Settings::disableCancelDuringExec() const { - return m_dontCancel; + return m_disableCancelDuringExec; } diff --git a/src/libcalamares/Settings.h b/src/libcalamares/Settings.h index ca77859c3..a095ee567 100644 --- a/src/libcalamares/Settings.h +++ b/src/libcalamares/Settings.h @@ -68,7 +68,7 @@ public: /** @brief Global setting of disable-cancel: can't cancel ever. */ bool disableCancel() const; /** @brief Temporary setting of disable-cancel: can't cancel during exec. */ - bool dontCancel() const; + bool disableCancelDuringExec() const; private: static Settings* s_instance; @@ -85,7 +85,7 @@ private: bool m_isSetupMode; bool m_promptInstall; bool m_disableCancel; - bool m_dontCancel; + bool m_disableCancelDuringExec; }; } diff --git a/src/libcalamaresui/ViewManager.cpp b/src/libcalamaresui/ViewManager.cpp index 450fa18a7..bc782c1c5 100644 --- a/src/libcalamaresui/ViewManager.cpp +++ b/src/libcalamaresui/ViewManager.cpp @@ -288,7 +288,7 @@ ViewManager::next() m_steps.at( m_currentStep )->onActivate(); executing = qobject_cast< ExecutionViewStep* >( m_steps.at( m_currentStep ) ) != nullptr; emit currentStepChanged(); - updateCancelEnabled( !settings->disableCancel() && !(executing && settings->dontCancel() ) ); + updateCancelEnabled( !settings->disableCancel() && !(executing && settings->disableCancelDuringExec() ) ); } else step->next(); @@ -331,7 +331,7 @@ ViewManager::updateButtonLabels() { if ( settings->disableCancel() ) m_quit->setVisible( false ); // In case we went back from final - updateCancelEnabled( !settings->disableCancel() && !( stepIsExecute( m_steps, m_currentStep ) && settings->dontCancel() ) ); + updateCancelEnabled( !settings->disableCancel() && !( stepIsExecute( m_steps, m_currentStep ) && settings->disableCancelDuringExec() ) ); m_quit->setText( tr( "&Cancel" ) ); m_quit->setToolTip( quit ); @@ -369,7 +369,7 @@ bool ViewManager::confirmCancelInstallation() if ( settings->disableCancel() ) return false; - if ( settings->dontCancel() && stepIsExecute( m_steps, m_currentStep ) ) + if ( settings->disableCancelDuringExec() && stepIsExecute( m_steps, m_currentStep ) ) return false; // If it's NOT the last page of the last step, we ask for confirmation