From d7602df51e8c12112cbc16ceea9ceceb6900af60 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 09:25:37 -0400 Subject: [PATCH 01/26] [libcalamares] Introduce networking service - The networking service is intended to wrap up use of QNetworkAccessManager and others for consumption within Calamares, and to provide some convenience functions for internet access. - Medium term, it may also monitor network access, so that we can respond to changes in network availability during installation. Currently very minimal and undocumented. --- src/libcalamares/CMakeLists.txt | 3 + src/libcalamares/network/Manager.cpp | 93 ++++++++++++++++++++++++++++ src/libcalamares/network/Manager.h | 55 ++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 src/libcalamares/network/Manager.cpp create mode 100644 src/libcalamares/network/Manager.h diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index f2063813c..ac9b2df08 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -36,6 +36,9 @@ set( libSources locale/Lookup.cpp locale/TranslatableConfiguration.cpp + # Network service + network/Manager.cpp + # Partition service partition/PartitionSize.cpp diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp new file mode 100644 index 000000000..c229ab036 --- /dev/null +++ b/src/libcalamares/network/Manager.cpp @@ -0,0 +1,93 @@ +/* === 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 "Manager.h" + +#include +#include +#include +#include + +namespace CalamaresUtils +{ +namespace Network +{ +struct Manager::Private +{ + std::unique_ptr< QNetworkAccessManager > m_nam; + QUrl m_hasInternetUrl; + bool m_hasInternet; + + Private(); +}; +} // namespace Network +} // namespace CalamaresUtils +CalamaresUtils::Network::Manager::Private::Private() + : m_nam( std::make_unique< QNetworkAccessManager >() ) + , m_hasInternet( false ) +{ +} + +CalamaresUtils::Network::Manager::Manager() + : d( std::make_unique< Private >() ) +{ +} + +CalamaresUtils::Network::Manager::~Manager() {} + +CalamaresUtils::Network::Manager& +CalamaresUtils::Network::Manager::instance() +{ + static auto* s_manager = new CalamaresUtils::Network::Manager(); + return *s_manager; +} + +bool +CalamaresUtils::Network::Manager::hasInternet() +{ + return d->m_hasInternet; +} + +bool +CalamaresUtils::Network::Manager::checkHasInternet() +{ + bool b = false; + if ( d->m_hasInternetUrl.isValid() ) + { + b = synchronousPing( d->m_hasInternetUrl ); + } + d->m_hasInternet = b; + return b; +} + +void +CalamaresUtils::Network::Manager::setCheckHasInternetUrl( const QUrl& url ) +{ + d->m_hasInternetUrl = url; +} + +bool +CalamaresUtils::Network::Manager::synchronousPing( const QUrl& url ) +{ + QNetworkRequest req = QNetworkRequest( url ); + QNetworkReply* reply = d->m_nam->get( req ); + QEventLoop loop; + connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); + loop.exec(); + return reply->bytesAvailable(); +} diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h new file mode 100644 index 000000000..a9903e115 --- /dev/null +++ b/src/libcalamares/network/Manager.h @@ -0,0 +1,55 @@ +/* === 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_NETWORK_MANAGER_H +#define LIBCALAMARES_NETWORK_MANAGER_H + +#include "DllMacro.h" + +#include +#include + +#include + +namespace CalamaresUtils +{ +namespace Network +{ +class DLLEXPORT Manager : QObject +{ + Q_OBJECT + + Manager(); + +public: + static Manager& instance(); + virtual ~Manager(); + + bool synchronousPing( const QUrl& url ); + + void setCheckHasInternetUrl( const QUrl& url ); + bool checkHasInternet(); + bool hasInternet(); + +private: + struct Private; + std::unique_ptr< Private > d; +}; +} // namespace Network +} // namespace CalamaresUtils +#endif // LIBCALAMARES_NETWORK_MANAGER_H From b8d56bb4a63100cfc5fa1d0b7b284be9d6deeebc Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 09:51:39 -0400 Subject: [PATCH 02/26] [libcalamares] Add tests for network service --- src/libcalamares/CMakeLists.txt | 10 +++++++ src/libcalamares/network/Tests.cpp | 43 ++++++++++++++++++++++++++++++ src/libcalamares/network/Tests.h | 37 +++++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 src/libcalamares/network/Tests.cpp create mode 100644 src/libcalamares/network/Tests.h diff --git a/src/libcalamares/CMakeLists.txt b/src/libcalamares/CMakeLists.txt index ac9b2df08..87338ae6c 100644 --- a/src/libcalamares/CMakeLists.txt +++ b/src/libcalamares/CMakeLists.txt @@ -197,6 +197,16 @@ if ( ECM_FOUND AND BUILD_TESTING ) Qt5::Test ) calamares_automoc( libcalamareslocaletest ) + + ecm_add_test( + network/Tests.cpp + TEST_NAME + libcalamaresnetworktest + LINK_LIBRARIES + calamares + Qt5::Test + ) + calamares_automoc( libcalamaresnetworktest ) endif() if( BUILD_TESTING ) diff --git a/src/libcalamares/network/Tests.cpp b/src/libcalamares/network/Tests.cpp new file mode 100644 index 000000000..a13a74c86 --- /dev/null +++ b/src/libcalamares/network/Tests.cpp @@ -0,0 +1,43 @@ +/* === 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 "Manager.h" + +#include + +QTEST_GUILESS_MAIN( NetworkTests ) + +NetworkTests::NetworkTests() +{ +} + +NetworkTests::~NetworkTests() +{ +} + +void NetworkTests::initTestCase() +{ +} + +void NetworkTests::testInstance() +{ + auto& nam = CalamaresUtils::Network::Manager::instance(); + QVERIFY( !nam.hasInternet() ); +} diff --git a/src/libcalamares/network/Tests.h b/src/libcalamares/network/Tests.h new file mode 100644 index 000000000..56f87cc67 --- /dev/null +++ b/src/libcalamares/network/Tests.h @@ -0,0 +1,37 @@ +/* === 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_NETWORK_TESTS_H +#define LIBCALAMARES_NETWORK_TESTS_H + +#include + +class NetworkTests : public QObject +{ + Q_OBJECT +public: + NetworkTests(); + ~NetworkTests() override; + +private Q_SLOTS: + void initTestCase(); + + void testInstance(); +}; + +#endif From 8d3530154f007793a7e50223512a155ef68395e5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 09:57:04 -0400 Subject: [PATCH 03/26] [libcalamares] Expand network service test - Do an actual ping (also to check for memory leaks) --- src/libcalamares/network/Tests.cpp | 25 +++++++++++++++---------- src/libcalamares/network/Tests.h | 1 + 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libcalamares/network/Tests.cpp b/src/libcalamares/network/Tests.cpp index a13a74c86..559a955fe 100644 --- a/src/libcalamares/network/Tests.cpp +++ b/src/libcalamares/network/Tests.cpp @@ -24,20 +24,25 @@ QTEST_GUILESS_MAIN( NetworkTests ) -NetworkTests::NetworkTests() +NetworkTests::NetworkTests() {} + +NetworkTests::~NetworkTests() {} + +void +NetworkTests::initTestCase() { } -NetworkTests::~NetworkTests() -{ -} - -void NetworkTests::initTestCase() -{ -} - -void NetworkTests::testInstance() +void +NetworkTests::testInstance() { auto& nam = CalamaresUtils::Network::Manager::instance(); QVERIFY( !nam.hasInternet() ); } + +void +NetworkTests::testPing() +{ + auto& nam = CalamaresUtils::Network::Manager::instance(); + QVERIFY( nam.synchronousPing( QUrl( "https://www.kde.org" ) ) ); +} diff --git a/src/libcalamares/network/Tests.h b/src/libcalamares/network/Tests.h index 56f87cc67..b63f7eff0 100644 --- a/src/libcalamares/network/Tests.h +++ b/src/libcalamares/network/Tests.h @@ -32,6 +32,7 @@ private Q_SLOTS: void initTestCase(); void testInstance(); + void testPing(); }; #endif From 11d52df04c64a7af54db5634e78a89bf2f95f3fa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 10:18:49 -0400 Subject: [PATCH 04/26] [libcalamares] Add API docs to network service --- src/libcalamares/network/Manager.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index a9903e115..f357d8d70 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -37,13 +37,36 @@ class DLLEXPORT Manager : QObject Manager(); public: + /** @brief Gets the single Manager instance. + * + * Typical code will use `auto& nam = Manager::instance();` + * to keep the reference. + */ static Manager& instance(); virtual ~Manager(); + /** @brief Checks if the given @p url returns data. + * + * Returns @c true if it does; @c false means no data, typically + * because of an error or no network access. + */ bool synchronousPing( const QUrl& url ); + /// @brief Set the URL which is used for the general "is there internet" check. void setCheckHasInternetUrl( const QUrl& url ); + /** @brief Do an explicit check for internet connectivity. + * + * This **may** do a ping to the configured check URL, but can also + * use other mechanisms. + */ bool checkHasInternet(); + /** @brief Is there internet connectivity? + * + * This returns the result of the last explicit check, or if there + * is other information about the state of the internet connection, + * whatever is known. @c true means you can expect (all) internet + * connectivity to be present. + */ bool hasInternet(); private: From 4389c254df1165bbb524e819db766aeea71d8ea2 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 10:22:03 -0400 Subject: [PATCH 05/26] [libcalamares] Rely directly on QNAM's networkAccessible() --- src/libcalamares/network/Manager.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index c229ab036..716c93f6a 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -66,8 +66,9 @@ CalamaresUtils::Network::Manager::hasInternet() bool CalamaresUtils::Network::Manager::checkHasInternet() { - bool b = false; - if ( d->m_hasInternetUrl.isValid() ) + bool b = d->m_nam->networkAccessible() == QNetworkAccessManager::Accessible; + + if ( !b && d->m_hasInternetUrl.isValid() ) { b = synchronousPing( d->m_hasInternetUrl ); } From e06500863101b6d3506c5710b81ab35e948fe2f6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 10:57:02 -0400 Subject: [PATCH 06/26] [welcome] Switch to the network service - simplify configuration - use existing ping- and hasInternet() --- .../welcome/checker/GeneralRequirements.cpp | 41 ++++++++----------- .../welcome/checker/GeneralRequirements.h | 1 - 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/modules/welcome/checker/GeneralRequirements.cpp b/src/modules/welcome/checker/GeneralRequirements.cpp index 0e6133d3f..4787fe51d 100644 --- a/src/modules/welcome/checker/GeneralRequirements.cpp +++ b/src/modules/welcome/checker/GeneralRequirements.cpp @@ -25,12 +25,14 @@ #include "partman_devices.h" #include "modulesystem/Requirement.h" +#include "network/Manager.h" #include "widgets/WaitingWidget.h" #include "utils/CalamaresUtilsGui.h" #include "utils/Logger.h" #include "utils/Retranslator.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Units.h" +#include "utils/Variant.h" #include "Settings.h" #include "JobQueue.h" @@ -245,16 +247,16 @@ GeneralRequirements::setConfigurationMap( const QVariantMap& configurationMap ) incompleteConfiguration = true; } - if ( configurationMap.contains( "internetCheckUrl" ) && - configurationMap.value( "internetCheckUrl" ).type() == QVariant::String ) + QUrl checkInternetUrl; + QString checkInternetSetting = CalamaresUtils::getString( configurationMap, "internetCheckUrl" ); + if ( !checkInternetSetting.isEmpty() ) { - m_checkHasInternetUrl = configurationMap.value( "internetCheckUrl" ).toString().trimmed(); - if ( m_checkHasInternetUrl.isEmpty() || - !QUrl( m_checkHasInternetUrl ).isValid() ) + checkInternetUrl = QUrl( checkInternetSetting.trimmed() ); + if ( !checkInternetUrl.isValid() ) { - cWarning() << "GeneralRequirements entry 'internetCheckUrl' is invalid in welcome.conf" << m_checkHasInternetUrl + cWarning() << "GeneralRequirements entry 'internetCheckUrl' is invalid in welcome.conf" << checkInternetSetting << "reverting to default (http://example.com)."; - m_checkHasInternetUrl = "http://example.com"; + checkInternetUrl = QUrl( "http://example.com" ); incompleteConfiguration = true; } } @@ -262,10 +264,13 @@ GeneralRequirements::setConfigurationMap( const QVariantMap& configurationMap ) { cWarning() << "GeneralRequirements entry 'internetCheckUrl' is undefined in welcome.conf," "reverting to default (http://example.com)."; - - m_checkHasInternetUrl = "http://example.com"; + checkInternetUrl = "http://example.com"; incompleteConfiguration = true; } + if ( checkInternetUrl.isValid() ) + { + CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( checkInternetUrl ); + } if ( incompleteConfiguration ) { @@ -357,22 +362,8 @@ GeneralRequirements::checkHasPower() bool GeneralRequirements::checkHasInternet() { - // default to true in the QNetworkAccessManager::UnknownAccessibility case - QNetworkAccessManager qnam; - bool hasInternet = qnam.networkAccessible() == QNetworkAccessManager::Accessible; - - if ( !hasInternet && qnam.networkAccessible() == QNetworkAccessManager::UnknownAccessibility ) - { - QNetworkRequest req = QNetworkRequest( QUrl( m_checkHasInternetUrl ) ); - QNetworkReply* reply = qnam.get( req ); - QEventLoop loop; - connect( reply, &QNetworkReply::finished, - &loop, &QEventLoop::quit ); - loop.exec(); - if( reply->bytesAvailable() ) - hasInternet = true; - } - + auto& nam = CalamaresUtils::Network::Manager::instance(); + bool hasInternet = nam.checkHasInternet(); Calamares::JobQueue::instance()->globalStorage()->insert( "hasInternet", hasInternet ); return hasInternet; } diff --git a/src/modules/welcome/checker/GeneralRequirements.h b/src/modules/welcome/checker/GeneralRequirements.h index 1efe118a6..8e5a6cd54 100644 --- a/src/modules/welcome/checker/GeneralRequirements.h +++ b/src/modules/welcome/checker/GeneralRequirements.h @@ -48,7 +48,6 @@ private: qreal m_requiredStorageGiB; qreal m_requiredRamGiB; - QString m_checkHasInternetUrl; }; #endif // REQUIREMENTSCHECKER_H From eae931f2ed21e82abb1461042251a8e9dcd370c6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Wed, 21 Aug 2019 03:43:51 -0400 Subject: [PATCH 07/26] [libcalamares] Ping only when accessibility is unknown - Restores exact functionality of previous version (noted by Kevin Kofler) - Short-circuit ping if the URL is bad. --- src/libcalamares/network/Manager.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 716c93f6a..b0c068c6e 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -66,14 +66,14 @@ CalamaresUtils::Network::Manager::hasInternet() bool CalamaresUtils::Network::Manager::checkHasInternet() { - bool b = d->m_nam->networkAccessible() == QNetworkAccessManager::Accessible; + bool hasInternet = d->m_nam->networkAccessible() == QNetworkAccessManager::Accessible; - if ( !b && d->m_hasInternetUrl.isValid() ) + if ( !hasInternet && ( d->m_nam->networkAccessible() == QNetworkAccessManager::UnknownAccessibility ) ) { - b = synchronousPing( d->m_hasInternetUrl ); + hasInternet = synchronousPing( d->m_hasInternetUrl ); } - d->m_hasInternet = b; - return b; + d->m_hasInternet = hasInternet; + return hasInternet; } void @@ -85,6 +85,11 @@ CalamaresUtils::Network::Manager::setCheckHasInternetUrl( const QUrl& url ) bool CalamaresUtils::Network::Manager::synchronousPing( const QUrl& url ) { + if ( !url.isValid() ) + { + return false; + } + QNetworkRequest req = QNetworkRequest( url ); QNetworkReply* reply = d->m_nam->get( req ); QEventLoop loop; From 8ea1ea666237de767eddd09bc5596b719589d733 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 22 Aug 2019 16:03:21 +0200 Subject: [PATCH 08/26] [libcalamares] Add synchronousGet() to network service - Synchronous download of a given URL; not something to do from the GUI thread. - Use it from the GeoIP service, which downloads in a separate thread to do GeoIP lookups. - Drop now-unused headers. - Adjust tests for GeoIP to use network service --- src/libcalamares/geoip/GeoIPTests.cpp | 26 +++----------------------- src/libcalamares/geoip/Handler.cpp | 25 +++---------------------- src/libcalamares/network/Manager.cpp | 18 ++++++++++++++++++ src/libcalamares/network/Manager.h | 8 ++++++++ 4 files changed, 32 insertions(+), 45 deletions(-) diff --git a/src/libcalamares/geoip/GeoIPTests.cpp b/src/libcalamares/geoip/GeoIPTests.cpp index c24b6d98f..3400bb24f 100644 --- a/src/libcalamares/geoip/GeoIPTests.cpp +++ b/src/libcalamares/geoip/GeoIPTests.cpp @@ -24,9 +24,7 @@ #endif #include "Handler.h" -#include -#include -#include +#include "network/Manager.h" #include @@ -197,27 +195,9 @@ GeoIPTests::testSplitTZ() } -static QByteArray -synchronous_get( const char* urlstring ) -{ - QUrl url( urlstring ); - QNetworkAccessManager manager; - QEventLoop loop; - - qDebug() << "Fetching" << url; - - QObject::connect( &manager, &QNetworkAccessManager::finished, &loop, &QEventLoop::quit ); - - QNetworkRequest request( url ); - QNetworkReply* reply = manager.get( request ); - loop.exec(); - reply->deleteLater(); - return reply->readAll(); -} - #define CHECK_GET( t, selector, url ) \ { \ - auto tz = GeoIP##t( selector ).processReply( synchronous_get( url ) ); \ + auto tz = GeoIP##t( selector ).processReply( CalamaresUtils::Network::Manager::instance().synchronousGet( QUrl( url ) ) ); \ qDebug() << tz; \ QCOMPARE( default_tz, tz ); \ auto tz2 = CalamaresUtils::GeoIP::Handler( "" #t, url, selector ).get(); \ @@ -236,7 +216,7 @@ GeoIPTests::testGet() GeoIPJSON default_handler; // Call the KDE service the definitive source. - auto default_tz = default_handler.processReply( synchronous_get( "https://geoip.kde.org/v1/calamares" ) ); + auto default_tz = default_handler.processReply( CalamaresUtils::Network::Manager::instance().synchronousGet( QUrl( "https://geoip.kde.org/v1/calamares" ) ) ); // This is bogus, because the test isn't always run by me // QCOMPARE( default_tz.first, QStringLiteral("Europe") ); diff --git a/src/libcalamares/geoip/Handler.cpp b/src/libcalamares/geoip/Handler.cpp index 6017c404f..99e55e926 100644 --- a/src/libcalamares/geoip/Handler.cpp +++ b/src/libcalamares/geoip/Handler.cpp @@ -23,14 +23,11 @@ #include "GeoIPXML.h" #endif +#include "network/Manager.h" #include "utils/Logger.h" #include "utils/NamedEnum.h" #include "utils/Variant.h" -#include -#include -#include - #include static const NamedEnumTable< CalamaresUtils::GeoIP::Handler::Type >& @@ -87,22 +84,6 @@ Handler::Handler( const QString& implementation, const QString& url, const QStri Handler::~Handler() {} -static QByteArray -synchronous_get( const QString& urlstring ) -{ - QUrl url( urlstring ); - QNetworkAccessManager manager; - QEventLoop loop; - - QObject::connect( &manager, &QNetworkAccessManager::finished, &loop, &QEventLoop::quit ); - - QNetworkRequest request( url ); - QNetworkReply* reply = manager.get( request ); - loop.exec(); - reply->deleteLater(); - return reply->readAll(); -} - static std::unique_ptr< Interface > create_interface( Handler::Type t, const QString& selector ) { @@ -131,7 +112,7 @@ do_query( Handler::Type type, const QString& url, const QString& selector ) return RegionZonePair(); } - return interface->processReply( synchronous_get( url ) ); + return interface->processReply( CalamaresUtils::Network::Manager::instance().synchronousGet( url ) ); } static QString @@ -143,7 +124,7 @@ do_raw_query( Handler::Type type, const QString& url, const QString& selector ) return QString(); } - return interface->rawReply( synchronous_get( url ) ); + return interface->rawReply( CalamaresUtils::Network::Manager::instance().synchronousGet( url ) ); } RegionZonePair diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index b0c068c6e..c98863835 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -95,5 +95,23 @@ CalamaresUtils::Network::Manager::synchronousPing( const QUrl& url ) QEventLoop loop; connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); loop.exec(); + reply->deleteLater(); return reply->bytesAvailable(); } + +QByteArray +CalamaresUtils::Network::Manager::synchronousGet( const QUrl& url ) +{ + if ( !url.isValid() ) + { + return QByteArray(); + } + + QNetworkRequest request( url ); + QNetworkReply* reply = d->m_nam->get( request ); + QEventLoop loop; + connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); + loop.exec(); + reply->deleteLater(); + return reply->readAll(); +} diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index f357d8d70..1c965d27e 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -21,6 +21,7 @@ #include "DllMacro.h" +#include #include #include @@ -52,6 +53,13 @@ public: */ bool synchronousPing( const QUrl& url ); + /** @brief Downloads the data from a given @p url + * + * Returns the data as a QByteArray, or an empty + * array if any error occurred. + */ + QByteArray synchronousGet( const QUrl& url ); + /// @brief Set the URL which is used for the general "is there internet" check. void setCheckHasInternetUrl( const QUrl& url ); /** @brief Do an explicit check for internet connectivity. From b8bad1c0b4b25c20b4254e4d292ec1c6639d5acf Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Thu, 22 Aug 2019 16:16:29 +0200 Subject: [PATCH 09/26] [welcome] Drop unused includes --- src/modules/welcome/checker/GeneralRequirements.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/modules/welcome/checker/GeneralRequirements.cpp b/src/modules/welcome/checker/GeneralRequirements.cpp index 4787fe51d..1881558b8 100644 --- a/src/modules/welcome/checker/GeneralRequirements.cpp +++ b/src/modules/welcome/checker/GeneralRequirements.cpp @@ -44,13 +44,9 @@ #include #include #include -#include #include #include #include -#include -#include -#include #include #include From af1aa701bc2c22b79049c8ba44dfc4b417c2cbf5 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Aug 2019 12:09:30 +0200 Subject: [PATCH 10/26] [libcalamares] Shuffle namespace lines around - put all the definitions inside namespace {} to avoid needlessly long source lines. --- src/libcalamares/network/Manager.cpp | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index c98863835..66721c812 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -35,36 +35,35 @@ struct Manager::Private Private(); }; -} // namespace Network -} // namespace CalamaresUtils -CalamaresUtils::Network::Manager::Private::Private() + +Manager::Private::Private() : m_nam( std::make_unique< QNetworkAccessManager >() ) , m_hasInternet( false ) { } -CalamaresUtils::Network::Manager::Manager() +Manager::Manager() : d( std::make_unique< Private >() ) { } -CalamaresUtils::Network::Manager::~Manager() {} +Manager::~Manager() {} -CalamaresUtils::Network::Manager& -CalamaresUtils::Network::Manager::instance() +Manager& +Manager::instance() { - static auto* s_manager = new CalamaresUtils::Network::Manager(); + static auto* s_manager = new Manager(); return *s_manager; } bool -CalamaresUtils::Network::Manager::hasInternet() +Manager::hasInternet() { return d->m_hasInternet; } bool -CalamaresUtils::Network::Manager::checkHasInternet() +Manager::checkHasInternet() { bool hasInternet = d->m_nam->networkAccessible() == QNetworkAccessManager::Accessible; @@ -77,13 +76,13 @@ CalamaresUtils::Network::Manager::checkHasInternet() } void -CalamaresUtils::Network::Manager::setCheckHasInternetUrl( const QUrl& url ) +Manager::setCheckHasInternetUrl( const QUrl& url ) { d->m_hasInternetUrl = url; } bool -CalamaresUtils::Network::Manager::synchronousPing( const QUrl& url ) +Manager::synchronousPing( const QUrl& url ) { if ( !url.isValid() ) { @@ -100,7 +99,7 @@ CalamaresUtils::Network::Manager::synchronousPing( const QUrl& url ) } QByteArray -CalamaresUtils::Network::Manager::synchronousGet( const QUrl& url ) +Manager::synchronousGet( const QUrl& url ) { if ( !url.isValid() ) { @@ -115,3 +114,6 @@ CalamaresUtils::Network::Manager::synchronousGet( const QUrl& url ) reply->deleteLater(); return reply->readAll(); } + +} // namespace Network +} // namespace CalamaresUtils From 1f2b3b734d683e1ed2f89bc575179703dae0eec0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Aug 2019 12:18:59 +0200 Subject: [PATCH 11/26] [libcalamares] Extend synchronous API with options --- src/libcalamares/network/Manager.cpp | 27 +++++++++++++++++--- src/libcalamares/network/Manager.h | 38 ++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 66721c812..857a93b69 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -27,6 +27,23 @@ namespace CalamaresUtils { namespace Network { +void +RequestOptions::applyToRequest( QNetworkRequest* request ) const +{ + if ( m_flags & Flag::FollowRedirect ) + { + // Follows all redirects except unsafe ones (https to http). + request->setAttribute( QNetworkRequest::FollowRedirectsAttribute, true ); + } + + if ( m_flags & Flag::FakeUserAgent ) + { + // Not everybody likes the default User Agent used by this class (looking at you, + // sourceforge.net), so let's set a more descriptive one. + request->setRawHeader( "User-Agent", "Mozilla/5.0 (compatible; Calamares)" ); + } +} + struct Manager::Private { std::unique_ptr< QNetworkAccessManager > m_nam; @@ -82,16 +99,17 @@ Manager::setCheckHasInternetUrl( const QUrl& url ) } bool -Manager::synchronousPing( const QUrl& url ) +Manager::synchronousPing( const QUrl& url, const RequestOptions& options ) { if ( !url.isValid() ) { return false; } - QNetworkRequest req = QNetworkRequest( url ); - QNetworkReply* reply = d->m_nam->get( req ); + QNetworkRequest request = QNetworkRequest( url ); + QNetworkReply* reply = d->m_nam->get( request ); QEventLoop loop; + options.applyToRequest( &request ); connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); loop.exec(); reply->deleteLater(); @@ -99,7 +117,7 @@ Manager::synchronousPing( const QUrl& url ) } QByteArray -Manager::synchronousGet( const QUrl& url ) +Manager::synchronousGet( const QUrl& url, const RequestOptions& options ) { if ( !url.isValid() ) { @@ -109,6 +127,7 @@ Manager::synchronousGet( const QUrl& url ) QNetworkRequest request( url ); QNetworkReply* reply = d->m_nam->get( request ); QEventLoop loop; + options.applyToRequest( &request ); connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); loop.exec(); reply->deleteLater(); diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index 1c965d27e..7f12925ef 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -25,12 +25,46 @@ #include #include +#include #include +class QNetworkRequest; + namespace CalamaresUtils { namespace Network { +class DLLEXPORT RequestOptions +{ +public: + enum Flag + { + FollowRedirect = 0x1, + FakeUserAgent = 0x100 + }; + Q_DECLARE_FLAGS( Flags, Flag ) + + RequestOptions() + : m_flags( 0 ) + , m_timeout( -1 ) + { + } + + RequestOptions( Flags f, std::chrono::milliseconds timeout = std::chrono::milliseconds( -1 ) ) + : m_flags( f ) + , m_timeout( timeout ) + { + } + + void applyToRequest( QNetworkRequest* ) const; + +private: + Flags m_flags; + std::chrono::milliseconds m_timeout; +}; + +Q_DECLARE_OPERATORS_FOR_FLAGS( RequestOptions::Flags ); + class DLLEXPORT Manager : QObject { Q_OBJECT @@ -51,14 +85,14 @@ public: * Returns @c true if it does; @c false means no data, typically * because of an error or no network access. */ - bool synchronousPing( const QUrl& url ); + bool synchronousPing( const QUrl& url, const RequestOptions& options = RequestOptions() ); /** @brief Downloads the data from a given @p url * * Returns the data as a QByteArray, or an empty * array if any error occurred. */ - QByteArray synchronousGet( const QUrl& url ); + QByteArray synchronousGet( const QUrl& url, const RequestOptions& options = RequestOptions() ); /// @brief Set the URL which is used for the general "is there internet" check. void setCheckHasInternetUrl( const QUrl& url ); From 85f0d386980b5aec0539a442cde608adfbc68107 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Fri, 23 Aug 2019 14:59:35 +0200 Subject: [PATCH 12/26] [libcalamares] Refactor synchronous get - Add timeout support - Refactor into a static helper method --- src/libcalamares/network/Manager.cpp | 39 ++++++++++++++++++---------- src/libcalamares/network/Manager.h | 9 +++++-- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index 857a93b69..d5e27e611 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -22,6 +22,7 @@ #include #include #include +#include namespace CalamaresUtils { @@ -98,6 +99,28 @@ Manager::setCheckHasInternetUrl( const QUrl& url ) d->m_hasInternetUrl = url; } +static QNetworkReply* +synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& url, const RequestOptions& options ) +{ + QNetworkRequest request = QNetworkRequest( url ); + QNetworkReply* reply = nam->get( request ); + QEventLoop loop; + QTimer timer; + + options.applyToRequest( &request ); + if ( options.hasTimeout() ) + { + timer.setSingleShot( true ); + QObject::connect( &timer, &QTimer::timeout, &loop, &QEventLoop::quit ); + timer.start( options.timeout() ); + } + + QObject::connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); + loop.exec(); + reply->deleteLater(); + return reply; +} + bool Manager::synchronousPing( const QUrl& url, const RequestOptions& options ) { @@ -106,13 +129,7 @@ Manager::synchronousPing( const QUrl& url, const RequestOptions& options ) return false; } - QNetworkRequest request = QNetworkRequest( url ); - QNetworkReply* reply = d->m_nam->get( request ); - QEventLoop loop; - options.applyToRequest( &request ); - connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); - loop.exec(); - reply->deleteLater(); + auto reply = synchronousRun( d->m_nam, url, options ); return reply->bytesAvailable(); } @@ -124,13 +141,7 @@ Manager::synchronousGet( const QUrl& url, const RequestOptions& options ) return QByteArray(); } - QNetworkRequest request( url ); - QNetworkReply* reply = d->m_nam->get( request ); - QEventLoop loop; - options.applyToRequest( &request ); - connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); - loop.exec(); - reply->deleteLater(); + auto reply = synchronousRun( d->m_nam, url, options ); return reply->readAll(); } diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index 7f12925ef..fef75ffb8 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -37,6 +37,8 @@ namespace Network class DLLEXPORT RequestOptions { public: + using milliseconds = std::chrono::milliseconds; + enum Flag { FollowRedirect = 0x1, @@ -50,7 +52,7 @@ public: { } - RequestOptions( Flags f, std::chrono::milliseconds timeout = std::chrono::milliseconds( -1 ) ) + RequestOptions( Flags f, milliseconds timeout = milliseconds( -1 ) ) : m_flags( f ) , m_timeout( timeout ) { @@ -58,9 +60,12 @@ public: void applyToRequest( QNetworkRequest* ) const; + bool hasTimeout() const { return m_timeout > milliseconds( 0 ); } + auto timeout() const { return m_timeout; } + private: Flags m_flags; - std::chrono::milliseconds m_timeout; + milliseconds m_timeout; }; Q_DECLARE_OPERATORS_FOR_FLAGS( RequestOptions::Flags ); From ededebbc6c399395e61351fb7ffc56fb1c8b96b7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Sat, 24 Aug 2019 15:23:07 +0200 Subject: [PATCH 13/26] [libcalamares] Return reply early if the request is bad --- src/libcalamares/network/Manager.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index d5e27e611..b4e37d895 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -107,6 +107,12 @@ synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& QEventLoop loop; QTimer timer; + if ( reply->error() ) + { + reply->deleteLater(); + return reply; + } + options.applyToRequest( &request ); if ( options.hasTimeout() ) { From 87ea14f68abf0a04189f8f91f0e7ffa926334f5c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 10:15:22 +0200 Subject: [PATCH 14/26] [libcalamares] Drop INTERFACES again - The compile failure came from bad #include paths, so restoring this interface declaration wasn't a fix. - Reported to cause runtime failures on both KaOS and Manjaro. --- src/libcalamares/utils/PluginFactory.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libcalamares/utils/PluginFactory.h b/src/libcalamares/utils/PluginFactory.h index 958a7858a..65ee6eee9 100644 --- a/src/libcalamares/utils/PluginFactory.h +++ b/src/libcalamares/utils/PluginFactory.h @@ -101,7 +101,4 @@ public: #define CALAMARES_PLUGIN_FACTORY_DEFINITION( name, pluginRegistrations ) \ K_PLUGIN_FACTORY_DEFINITION_WITH_BASEFACTORY( name, CalamaresPluginFactory, pluginRegistrations ) - -Q_DECLARE_INTERFACE( CalamaresPluginFactory, CalamaresPluginFactory_iid ) - #endif From f0be7fd4aa01ff9247ab1413ffaff973902d92b3 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 14:43:41 +0200 Subject: [PATCH 15/26] [libcalamares] Make failures in the internal methods obvious - internally, timeout and error will return nullptr --- src/libcalamares/network/Manager.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index b4e37d895..fd58c5408 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -99,6 +99,14 @@ Manager::setCheckHasInternetUrl( const QUrl& url ) d->m_hasInternetUrl = url; } +/** @brief Does a request synchronously, returns the request itself + * + * The extra options for the request are taken from @p options, + * including the timeout setting. + * + * On failure, returns nullptr (e.g. bad URL, timeout). The request + * is marked for later automatic deletion, so don't store the pointer. + */ static QNetworkReply* synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& url, const RequestOptions& options ) { @@ -107,10 +115,11 @@ synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& QEventLoop loop; QTimer timer; + // Bail out early if the request is bad if ( reply->error() ) { reply->deleteLater(); - return reply; + return nullptr; } options.applyToRequest( &request ); @@ -123,6 +132,12 @@ synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& QObject::connect( reply, &QNetworkReply::finished, &loop, &QEventLoop::quit ); loop.exec(); + if ( options.hasTimeout() && !timer.isActive() ) + { + reply->deleteLater(); + return nullptr; + } + reply->deleteLater(); return reply; } @@ -135,8 +150,8 @@ Manager::synchronousPing( const QUrl& url, const RequestOptions& options ) return false; } - auto reply = synchronousRun( d->m_nam, url, options ); - return reply->bytesAvailable(); + auto* reply = synchronousRun( d->m_nam, url, options ); + return reply && reply->bytesAvailable(); } QByteArray @@ -147,8 +162,8 @@ Manager::synchronousGet( const QUrl& url, const RequestOptions& options ) return QByteArray(); } - auto reply = synchronousRun( d->m_nam, url, options ); - return reply->readAll(); + auto* reply = synchronousRun( d->m_nam, url, options ); + return reply ? reply->readAll() : QByteArray(); } } // namespace Network From e2c6591a77c36dd7d6aea62aa66fb3946f53f716 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 15:24:58 +0200 Subject: [PATCH 16/26] [libcalamares] Refactor request internals - distinguish timeouts from other failures - git synchronousPing() a more detailed result, which is still bool-compatible. --- src/libcalamares/network/Manager.cpp | 27 +++++++++++++++++---------- src/libcalamares/network/Manager.h | 28 ++++++++++++++++++++++++---- 2 files changed, 41 insertions(+), 14 deletions(-) diff --git a/src/libcalamares/network/Manager.cpp b/src/libcalamares/network/Manager.cpp index fd58c5408..6c2d6caab 100644 --- a/src/libcalamares/network/Manager.cpp +++ b/src/libcalamares/network/Manager.cpp @@ -107,7 +107,7 @@ Manager::setCheckHasInternetUrl( const QUrl& url ) * On failure, returns nullptr (e.g. bad URL, timeout). The request * is marked for later automatic deletion, so don't store the pointer. */ -static QNetworkReply* +static QPair< RequestStatus, QNetworkReply* > synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& url, const RequestOptions& options ) { QNetworkRequest request = QNetworkRequest( url ); @@ -119,7 +119,7 @@ synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& if ( reply->error() ) { reply->deleteLater(); - return nullptr; + return qMakePair( RequestStatus( RequestStatus::Failed ), nullptr ); } options.applyToRequest( &request ); @@ -135,23 +135,30 @@ synchronousRun( const std::unique_ptr< QNetworkAccessManager >& nam, const QUrl& if ( options.hasTimeout() && !timer.isActive() ) { reply->deleteLater(); - return nullptr; + return qMakePair( RequestStatus( RequestStatus::Timeout ), nullptr ); } reply->deleteLater(); - return reply; + return qMakePair( RequestStatus( RequestStatus::Ok ), reply ); } -bool +RequestStatus Manager::synchronousPing( const QUrl& url, const RequestOptions& options ) { if ( !url.isValid() ) { - return false; + return RequestStatus::Failed; } - auto* reply = synchronousRun( d->m_nam, url, options ); - return reply && reply->bytesAvailable(); + auto reply = synchronousRun( d->m_nam, url, options ); + if ( reply.first ) + { + return reply.second->bytesAvailable() ? RequestStatus::Ok : RequestStatus::Empty; + } + else + { + return reply.first; + } } QByteArray @@ -162,8 +169,8 @@ Manager::synchronousGet( const QUrl& url, const RequestOptions& options ) return QByteArray(); } - auto* reply = synchronousRun( d->m_nam, url, options ); - return reply ? reply->readAll() : QByteArray(); + auto reply = synchronousRun( d->m_nam, url, options ); + return reply.first ? reply.second->readAll() : QByteArray(); } } // namespace Network diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index fef75ffb8..33a1ba350 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -70,6 +70,22 @@ private: Q_DECLARE_OPERATORS_FOR_FLAGS( RequestOptions::Flags ); +struct RequestStatus +{ + enum State + { + Ok, + Timeout, // Timeout exceeded + Failed, // bad Url + Empty // for ping(), response is empty + }; + + RequestStatus( State s = Ok ); + operator bool() const { return m_s == Ok; } + + State m_s; +}; + class DLLEXPORT Manager : QObject { Q_OBJECT @@ -87,15 +103,19 @@ public: /** @brief Checks if the given @p url returns data. * - * Returns @c true if it does; @c false means no data, typically - * because of an error or no network access. + * Returns a RequestStatus, which converts to @c true if the ping + * was successful. Other status reasons convert to @c false, + * typically because of no data, a Url error or no network access. + * + * May return Empty if the request was successful but returned + * no data at all. */ - bool synchronousPing( const QUrl& url, const RequestOptions& options = RequestOptions() ); + RequestStatus synchronousPing( const QUrl& url, const RequestOptions& options = RequestOptions() ); /** @brief Downloads the data from a given @p url * * Returns the data as a QByteArray, or an empty - * array if any error occurred. + * array if any error occurred (or no data was returned). */ QByteArray synchronousGet( const QUrl& url, const RequestOptions& options = RequestOptions() ); From a1b0049bbfa16655441acc24a2d8cf4c3c6f2caa Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 15:41:30 +0200 Subject: [PATCH 17/26] [libcalamares] Use more readable names --- src/libcalamares/network/Manager.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index 33a1ba350..0f2d0c651 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -80,10 +80,13 @@ struct RequestStatus Empty // for ping(), response is empty }; - RequestStatus( State s = Ok ); - operator bool() const { return m_s == Ok; } + RequestStatus( State s = Ok ) + : status( s ) + { + } + operator bool() const { return status == Ok; } - State m_s; + State status; }; class DLLEXPORT Manager : QObject From f7215393d1332f91f9e72578137274269c578417 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 15:45:10 +0200 Subject: [PATCH 18/26] [tracking] Use the network service - drop own NAM handling - use timeout mechanism - report timeout as fatal error (like it already did), other errors are ignored. --- src/modules/tracking/TrackingJobs.cpp | 47 +++++---------------------- src/modules/tracking/TrackingJobs.h | 8 ----- 2 files changed, 9 insertions(+), 46 deletions(-) diff --git a/src/modules/tracking/TrackingJobs.cpp b/src/modules/tracking/TrackingJobs.cpp index bf71b541a..f101c8e6c 100644 --- a/src/modules/tracking/TrackingJobs.cpp +++ b/src/modules/tracking/TrackingJobs.cpp @@ -18,12 +18,10 @@ #include "TrackingJobs.h" +#include "network/Manager.h" #include "utils/CalamaresUtilsSystem.h" #include "utils/Logger.h" -#include -#include -#include #include #include @@ -31,13 +29,11 @@ TrackingInstallJob::TrackingInstallJob( const QString& url ) : m_url( url ) - , m_networkManager( nullptr ) { } TrackingInstallJob::~TrackingInstallJob() { - delete m_networkManager; } QString @@ -61,48 +57,23 @@ TrackingInstallJob::prettyStatusMessage() const Calamares::JobResult TrackingInstallJob::exec() { - m_networkManager = new QNetworkAccessManager(); + using CalamaresUtils::Network::Manager; + using CalamaresUtils::Network::RequestOptions; + using CalamaresUtils::Network::RequestStatus; - QNetworkRequest request; - request.setUrl( QUrl( m_url ) ); - // Follows all redirects except unsafe ones (https to http). - request.setAttribute( QNetworkRequest::FollowRedirectsAttribute, true ); - // Not everybody likes the default User Agent used by this class (looking at you, - // sourceforge.net), so let's set a more descriptive one. - request.setRawHeader( "User-Agent", "Mozilla/5.0 (compatible; Calamares)" ); - - QTimer timeout; - timeout.setSingleShot( true ); - - QEventLoop loop; - - connect( m_networkManager, &QNetworkAccessManager::finished, this, &TrackingInstallJob::dataIsHere ); - connect( m_networkManager, &QNetworkAccessManager::finished, &loop, &QEventLoop::quit ); - connect( &timeout, &QTimer::timeout, &loop, &QEventLoop::quit ); - - m_networkManager->get( request ); // The semaphore is released when data is received - timeout.start( std::chrono::milliseconds( 5000 ) ); - - loop.exec(); - - if ( !timeout.isActive() ) + auto result = Manager::instance().synchronousPing( + QUrl( m_url ), + RequestOptions( RequestOptions::FollowRedirect | RequestOptions::FakeUserAgent, + RequestOptions::milliseconds( 5000 ) ) ); + if ( result.status == RequestStatus::Timeout ) { cWarning() << "install-tracking request timed out."; return Calamares::JobResult::error( tr( "Internal error in install-tracking." ), tr( "HTTP request timed out." ) ); } - timeout.stop(); - return Calamares::JobResult::ok(); } -void -TrackingInstallJob::dataIsHere( QNetworkReply* reply ) -{ - cDebug() << "Installation feedback request OK"; - reply->deleteLater(); -} - QString TrackingMachineNeonJob::prettyName() const { diff --git a/src/modules/tracking/TrackingJobs.h b/src/modules/tracking/TrackingJobs.h index a379441c9..7351c3346 100644 --- a/src/modules/tracking/TrackingJobs.h +++ b/src/modules/tracking/TrackingJobs.h @@ -21,13 +21,10 @@ #include "Job.h" -class QNetworkAccessManager; -class QNetworkReply; class QSemaphore; class TrackingInstallJob : public Calamares::Job { - Q_OBJECT public: TrackingInstallJob( const QString& url ); ~TrackingInstallJob() override; @@ -37,13 +34,8 @@ public: QString prettyStatusMessage() const override; Calamares::JobResult exec() override; -public slots: - void dataIsHere( QNetworkReply* ); - private: const QString m_url; - - QNetworkAccessManager* m_networkManager; }; class TrackingMachineNeonJob : public Calamares::Job From ee29c45433b1549e7923ec00a168cf95987282c7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 16:01:14 +0200 Subject: [PATCH 19/26] [netinstall] Point documentation towards netinstall README.md --- src/modules/netinstall/netinstall.conf | 7 +++++++ src/modules/netinstall/netinstall.yaml | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf index fe99eb2be..fa160d34c 100644 --- a/src/modules/netinstall/netinstall.conf +++ b/src/modules/netinstall/netinstall.conf @@ -4,6 +4,13 @@ # groupsUrl: http://example.org/netinstall.php # or it can be a locally installed file: # groupsUrl: file:///usr/share/calamares/netinstall.yaml +# +# Note that the contents of the groups file is the **inmportant** +# part of the configuration of this module. It specifies what +# the user may select and what commands are to be run. +# +# The format of the groups file is documented in `README.md`. +# # groupsUrl: file:///usr/share/calamares/netinstall.yaml # If the installation can proceed without netinstall (e.g. the Live CD diff --git a/src/modules/netinstall/netinstall.yaml b/src/modules/netinstall/netinstall.yaml index 8e9037f4d..e00f06c73 100644 --- a/src/modules/netinstall/netinstall.yaml +++ b/src/modules/netinstall/netinstall.yaml @@ -1,3 +1,7 @@ +# Example configuration with groups and packages, taken from Chakra Linux. +# +# This example is rather limited. See `README.md` for full documentation +# on the configuration format. - name: "Default" description: "Default group" hidden: true From 56792fdcb578f41e9e2285c6918afb042b815a01 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 16:26:06 +0200 Subject: [PATCH 20/26] [netinstall] Polish the README - describe the format better - more consistent typography - refer to `packages.conf` for details on shell commands --- src/modules/netinstall/README.md | 134 +++++++++++++++++++------------ 1 file changed, 82 insertions(+), 52 deletions(-) diff --git a/src/modules/netinstall/README.md b/src/modules/netinstall/README.md index 6478a844e..855754822 100644 --- a/src/modules/netinstall/README.md +++ b/src/modules/netinstall/README.md @@ -1,86 +1,116 @@ # Netinstall module -The netinstall module allows distribution maintainers to ship minimal ISOs with only a basic set of preinstall packages. -At installation time, the user is presented with the choice to install groups of packages from a predefined list. +The netinstall module allows distribution maintainers to ship minimal ISOs with +only a basic set of preinstall packages. At installation time, the user is +presented with the choice to install groups of packages from a predefined list. Calamares will then invoke the correct backend to install the packages. -## Configuration of the packages -Every distribution can choose which groups to display and which packages should be in the groups. +## Module Configuration -The *netinstall.conf* file should have this format: +The `netinstall.conf` file is self-describing, and at the very +lease should contain a *groupsUrl* key: +``` ---- groupsUrl: +``` -The URL must point to a YAML file. Here is a short example of how the YAML file should look. +The URL must point to a YAML file, the *groups* file. See below for +the format of that groups file. The URL may be a local file. - - name: "Group name" - description: "Description of the group" - packages: - - lsb-release - - avahi - - grub - - name: "Second group name" + +## Groups Configuration + + Here is a short example +of how the YAML file should look. + +``` + - name: "Group name" + description: "Description of the group" + packages: + - lsb-release + - avahi + - grub + - name: "Second group name" ... +``` -The file is composed of a list of entry, each describing one group. The keys *name*, *description* and *packages* are required. +The file is composed of a list of entries, each describing one group. The +keys *name*, *description* and *packages* are required for each group. -More keys are supported: +More keys (per group) are supported: - - hidden: if true, do not show the group on the page. Defaults to false. - - selected: if true, display the group as selected. Defaults to false. - - critical: if true, make the installation process fail if installing - any of the packages in the group fails. Otherwise, just log a warning. - Defaults to false. - - subgroups: if present this follows the same structure as the top level - of the YAML file, allowing there to be sub-groups of packages to an - arbitary depth - - pre-install: an optional command to run within the new system before - the group's packages are installed. It will run before each package in - the group is installed. - - post-install: an optional command to run within the new system after - the group's packages are installed. It will run after each package in - the group is installed. + - *hidden*: if true, do not show the group on the page. Defaults to false. + - *selected*: if true, display the group as selected. Defaults to false. + - critical*: if true, make the installation process fail if installing + any of the packages in the group fails. Otherwise, just log a warning. + Defaults to false. + - *subgroups*: if present this follows the same structure as the top level + of the YAML file, allowing there to be sub-groups of packages to an + arbitary depth + - *pre-install*: an optional command to run within the new system before + the group's packages are installed. It will run before each package in + the group is installed. + - *post-install*: an optional command to run within the new system after + the group's packages are installed. It will run after each package in + the group is installed. -If you set both *hidden* and *selected* for a group, you are basically creating a "default" group of packages -which will always be installed in the user's system. +If you set both *hidden* and *selected* for a group, you are basically creating +a "default" group of packages which will always be installed in the user's +system. -## Configuration of the module +> The note below applies to Calamares up-to-and-including 3.2.13, but will +> change in a later release. -Here is the set of instructions to have the module work in your Calamares. As of July 2016, this has been successfully -tested using the live installation of Chakra Fermi. +The *pre-install* and *post-install* commands are **not** passed to +a shell; see the **packages** module configuration (i.e. `packages.conf`) +for details. To use a full shell pipeline, call the shell explicitly. -First, if the module is used, we need to require a working Internet connection, otherwise the module will be -unable to fetch the package groups and to perform the installation. Requirements for the Calamares instance -are configured in the **welcome.conf** file (configuration for the **welcome** module). Make sure *internet* -is listed below *required*. -In the *settings.conf* file, decide where the **netinstall** page should be displayed. I put it just after the -**welcome** page, but any position between that and just before **partition** should make no difference. -If not present, add the **packages** job in the **exec** list. This is the job that calls the package manager -to install packages. Make sure it is configured to use the correct package manager for your distribution; this -is configured in src/modules/packages/packages.conf. +## Overall Configuration -The **exec** list in *settings.conf* should contain the following items in +Here is the set of instructions to have the module work in your Calamares. + +First, if the module is used, we need to require a working Internet connection, +otherwise the module will be unable to fetch the package groups and to perform +the installation. Requirements for the Calamares instance are configured in the +`welcome.conf` file (configuration for the **welcome** module). Make sure +*internet* is listed under the *required* checks. + +In the `settings.conf` file, decide where the **netinstall** page should be +displayed. I put it just after the **welcome** page, but any position between +that and just before **partition** should make no difference. + +If not present, add the **packages** job in the *exec* list. This is the job +that calls the package manager to install packages. Make sure it is configured +to use the correct package manager for your distribution; this is configured in +`packages.conf`. + +The *exec* list in `settings.conf` should contain the following items in order (it's ok for other jobs to be listed inbetween them, though): +``` - unpackfs - networkcfg - packages +``` -**unpackfs** creates the chroot where the installation is performed, and unpacks the root image with the filesystem -structure; **networkcfg** set ups a working network in the chroot; and finally **packages** can install packages -in the chroot. +**unpackfs** creates the chroot where the installation is performed, and unpacks +the root image with the filesystem structure; **networkcfg** set ups a working +network in the chroot; and finally **packages** can install packages in the +chroot. ## Common issues -If launching the package manager command returns you negative exit statuses and nothing is actually invoked, this -is likely an error in the setup of the chroot; check that the parameter **rootMountPoint** is set to the correct -value in the Calamares configuration. +If launching the package manager command returns you negative exit statuses and +nothing is actually invoked, this is likely an error in the setup of the chroot; +check that the parameter **rootMountPoint** is set to the correct value in the +Calamares configuration. -If the command is run, but exits with error, check that the network is working in the chroot. Make sure /etc/resolv.conf -exists and that it's not empty. +If the command is run, but exits with error, check that the network is +working in the chroot. Make sure `/etc/resolv.conf` exists and that +it's not empty. From d063d60e37c25eff193ed3d3a96bd7aef0a168d7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 16:42:05 +0200 Subject: [PATCH 21/26] [packages] Explain pre-script isn't actually a shell script --- src/modules/packages/packages.conf | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/modules/packages/packages.conf b/src/modules/packages/packages.conf index 94f3cfdb6..e42e8e9b8 100644 --- a/src/modules/packages/packages.conf +++ b/src/modules/packages/packages.conf @@ -31,9 +31,9 @@ backend: dummy # # Set "update_db" to 'true' for refreshing the database on the # target system. On target installations, which got installed by -# unsquashing, a full system update may be needed. Otherwise +# unsquashing, a full system update may be needed. Otherwise # post-installing additional packages may result in conflicts. -# Therefore set also "update_system" to 'true'. +# Therefore set also "update_system" to 'true'. # skip_if_no_internet: false update_db: true @@ -87,9 +87,25 @@ update_system: false # pre-script: touch /tmp/installing-vi # post-script: rm -f /tmp/installing-vi # -# The pre- and post-scripts are optional, but you cannot leave both out: using -# "package: vi" with neither script option will trick Calamares into -# trying to install a package named "package: vi", which is unlikely to work. +# The pre- and post-scripts are optional, but you cannot leave both out +# if you do use the *package* key: using "package: vi" with neither script +# option will trick Calamares into trying to install a package named +# "package: vi", which is unlikely to work. +# +# The pre- and post-scripts are **not** executed by a shell unless you +# explicitly invoke `/bin/sh` in them. The command-lines are passed +# to exec(), which does not understand shell syntax. In other words: +# +# pre-script: ls | wc -l +# +# Will fail, because `|` is passed as a command-line argument to ls, +# as are `wc`, and `-l`. No shell pipeline is set up, and ls is likely +# to complain. Invoke the shell explicitly: +# +# pre-script: /bin/sh -c \"ls | wc -l\" +# +# The above note on shell-expansion applies to versions up-to-and-including +# Calamares 3.2.12, but will change in future. # # Any package name may be localized; this is used to install localization # packages for software based on the selected system locale. By including @@ -104,7 +120,7 @@ update_system: false # # Take care that just plain `LOCALE` will not be replaced, so `foo-LOCALE` will # be left unchanged, while `foo-$LOCALE` will be changed. However, `foo-LOCALE` -# **will** be removed from the list of packages (i.e. not installed), if +# **will** be removed from the list of packages (i.e. not installed), if # English is selected. If a non-English locale is selected, then `foo-LOCALE` # will be installed, unchanged (no language-name-substitution occurs). # From 5868f102f25c3f4e96936d2419871cfc24fa97a0 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 16:49:58 +0200 Subject: [PATCH 22/26] [packagechooser] Less-bad "no selection" image - It's only "less bad", not actually a good image, composed from Breeze "empty" and "generic packages" icon. --- .../packagechooser/images/no-selection.png | Bin 4618 -> 1709 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/src/modules/packagechooser/images/no-selection.png b/src/modules/packagechooser/images/no-selection.png index f9dbbbac45d0b03667f66be54664fdc5dce71983..437c97051cb9d0d16049ebff1bdc6cffe1ddb8a1 100644 GIT binary patch literal 1709 zcmd^9`BT$J6yI+Mm;eGwXF}Bmt1XNpBzR(_K&+I2q~Mi7DuE~pB4VhN5+g^zY7a)x zVW`mn83v2+MFcD01kJ$!MwF{k2#|vY6ha_`1QG~G(jdx@{RiBco%i-#`}yp=eZL(J zC9bvIU<&|P8ypmH6adJgLI7oB5s{b5V=V%m6cmvRfJ5{0h6=VjYy`k2Iym6KaeARz zCP<)+ekRgtZl#^wbnIYo@RlT1W9~M2Vav^+WA^G^|IM=3^dAyMGIj}$VoTl{caF{2 zc1Gh$ZFsTm3vw{JrK`ED-7)4nTbGwDm^V;A_07PrCZ=#V3^B9hqOy$mvhv`Z6vZ1|E1k zUcQk=i60rCJ05~$i2ceXM2GgJ3yF#9i{bBRcdm0cw= z(mEm5B~PRlaWa0&JgZl=JNh@IEXwKQhsZ^hx%t90F=l^y>=PJas*=^vHCPJLl`ErN?+r{u4f*Yc!51T1rLO3sU2ot2l9IO2(h{z9-2k z#-VG+?Cj)*rkfrf9{;?}!Gw}~FjwkwI-owjB3xAL%Scl7Ks-r5+s9&^bQT(985-j$ z7QfcpCr{ap1asHd0hAkXB7w~;K=6W?R0L>5f^0NEI)5~dzN`p5leo}5IN8BK=T*hS z8FPz64{kSZMG2Wp+D`L?nj0QLXu*gQK1RxeKyCaWawCxinCh3d85$ z7GNr2mCi8C6VJ>%o3M6XiI5C}!vdqWDc@Y@Ka--uCg&>c)h| zr@fVxl2kK(pRx3P-2p5OvCmkso_Qkt3C*N-`k93AqNZp5ZgQ8fP3_4ceve#V`o6oX z#?ODvVGDx6YVm0n36yl0C!dQ=Y!5f)=aTA}ey!^(Bxoc7tKM0z)YKWSLlJ7G1r)Y^ zJ!=OIddES^E~TyDuJJR+IF;{&2)mtyOzs-OAZfL)W`WIvPJgrw*$w<^Vd!NA(sw}w zFH78~K*>gf>^s1wjRvi%#<1n&Hl){?K){Gy;W>@TTJ?)8S-`$8b5dHUUvGHf7|R3L z1Hk6O(m+@o56M?Ztv<>|{=Y9=6OZt!fH&lc62lT8Y}0MS$i4ABMOIz4bw|j0?R}1n zbBQAK>-#V%2;Gs@@b?isM)JPckJiJ`BU wydCia1Lb90lj*6q{2^Qwfk%B9@1PHFwr;kvMwtJ0mo6KFzYPs|_zgAtKayIY1ONa4 literal 4618 zcmdUz`9IX_-^Z`d3}Xx?OF4F<(?XW0oJca3?4d~|q!E>jecxwDp#^n>BZPfr!@*sl;kBG`?sSKf7Y!|h>s$r}K4^Y;gOERPlkK%mA*Punsel`($FTihU# zllHrbqLrb^qGx^Q&W^vNP?;g@{A0>V#|uyb_uB-U1HR0!62~x6B2Z&YldXFS&KMGS z;Yi)$^VY5ZxBtsPhczBf;?0q8w^zN#F=-m$gA37{ES+KB3FILUi zH-cW#k(v3gTebJ4jV-}6F5WxTRBouXlOv=2C@4@ZZgGk08Za<&dE=!;qdxEkRhC;H&E?~_&HV@1!I z2Bg5#4Rr~4^mZ?Ig48| zyoGCT?4mWvUz3D6n#xQb<2;%AODTjmb)^j^Gb*)1pBldyr`Q^YzFbxeD&@lS($chN z6p_hb`%N6@14eRGAgo9@Ut4H-falatL%s7lo}MvVErb3A_~?I+C)vN zOp~2gtieiJlTtvZ%CFOVy_AgXK!Gl4|K!cP0m-ndkJhyoZ3 zp2HjjaY|M?ErCV>DG_A!V)^*!&{Sc=u*L#cobF1roHv^GtV7fn$v^L2x-9WIAT}CR zUEY<6YB@3QQ_)wuF;3NpTEwP10bo zYOH%&;a@`z7sX$y(4R_nbv0&m{h^>TRnwpTh`h!+Qw}G8P}5IMirnwm+$=+m^4H{2 zz8&2sxTJg-dG-njxKS)F5!+r4KuwV@U@F|_#K~FV8UtejQiYKVlVN>@U@rAdU#~st zb1$5K0};da8p(Fl0`p5Vve}A1t7gZOibKsi8|Im0(c|a(H99lXAIrL?Rd?|PWLw4? zYr6^A(Hjo6yQMSaI8_E8Kianu%D)HroL$%3-I-~2XYCYO{gy3CDmS|F-RAnlq}x!F z*scA86X%?w@)E7O&$qd3awvo5LwQfKp2vHn{FqcXCIq%M3^p< zRCT7Ji6$9ajovvCTdW+d;$Xcuy~-+|=7Ged)BstmHe{^b5Y&6-2HegcKDstn5MD0q zz?{6SNs+>hqu5#kQekE$&AaCp%dIb}qWTg!We>&*HEg9qTw{z8B%vRd(6Of4 zdcO~6j!2)4?!)EFrRg^IqRDe1J3b}So)GqN<%u1l$jQYu%~R4rW6a>xnP_kMg2K~Id=-ItNr>hqR;l=x4KkV7%@fQT-cz7 zVJe*8+zOQZLGe1jZuaRA7NVIJ-YBO_qIILTLgTulAek9ge|)rPt{iP$7&SyK8o6-r zRp_2f2gn}6J}HYMx;=i_;-<}1FovYeAQM9@wAnRTL4Rnhc`$zT;~}}BX9HsSayJ{< z(EyNvnmqip+He(w%8>7l`o7)$O<4bG^u(o3R~8Yyn!mW$R04r)0sPLI=##f2-C~Pd zO2nWSgQiZWb?$U!Eq`C+T=fsfxKd}TY?MpyYNw(SO#Fb0H^gM=YzJ0KveI;FLXyul z27XRDGFdpo+MJ!RQ6nf#+7sS^2h1y#&Ki%1AtO-6$ZuSD3$FtEVyly1qnqYDB{X~n z9hT5hJyXhlbG5g_M;3Xk2wabBc|qpj6gj6KrRDeH2CPQa@EXYiK};gc;H@Y&yp%DYKVfgBbe#K= zO*fQ<{kT)J7*KY7!(NV()eo?gpK&JEvNNmwl=BLpq#W#lYo0Xd>}Jd4!d>1Og1Zalz&#!nHN|)YeL|b2t6i86nlRHRhat z_}NPI5^n3^hEg;iU<`I>&+^-wDlV#&ZxZRg_@;rO;YwEne63`j%R%nbWg##!d*5mF z-m|6>QKAQv%JLhEi>f(-nI84|Z*)TQMM^HqpIUZSX0K$#K(C#zS9gZr*3&C|7nDn? zp&j4jjmfLVpQCd}2CAVz)num+unbxt#8b$r4ain-g3`rXg{|QlSIoB{Y^Sg8`x3p* zLd#)6f+9JA=k>-q^P}{lm*CRO6#;EqU~vuj4E7Tms#fn72b~$VUtk}-&rAA9xun$a z!?~PS%pmh~%_SvI!X1g50}cRr-W+bt%L>{*{L#6%ltaXHTp+odb9Vm+Pn0T{A*t)x z@n!)yR^hKSM2#)q;H2`V_3mRsuJzuAYTW{rczM)jzY_1H6!^*xQO#;Gmz4^wl?|V~ zgRpKB*88VP!*7W>41F3y{<<*7S$1&rF9)%U7_lXGT9P(jJ+ZV5>y&Z7c&pY&1sM!p ztAOkyrMrjUmrIfB(mB38t$yXVo)n}yEFmam#;n{*wsl}q^-^>IRyh= zh56U`@-9AJDgWx1O01knk)t2{RGjpUIaTCCm`gRXP9q4NFB3fMd|q^vR6a95NE;oG zf}Vf$QXLjNa(!QRV}8Rv2O#^`W${HqPDZK;SAk9y+cH4P~WQI!@ch<6a)!a=kz; zh3iNztZ9~pSdBF{l-(k|rq+wALJgGgPdEI|3(R?>(_*`h+l=^*gvgDW^sFlGo*m^)w-{m zakHe>7hz{i^a4IIAMx&mlR8LP{gJm3;tumEVQ(qIw4~IeVKn#tR9zjz%C7z2ZWV1k zdvC$Nn_moYWQ_CR6Z^xKhyGyIvCq;+`wv7`B=U8iL>I8bC9m|5$K^r7wFc|JVjC|d z!yK1V*pVW{Zu9G)z;~gpZo=pf%lC<_ggS`WF0Tr83=BByPGKWO6140t)@ zdaMZEOMR9-Ud;73Xn5+9g6VOW|u43a_;v36O0Q)L+b-#C(7Aw#}<53Kk* zsav#2{R2zdQ{cmiJ4*!^*(B&w;oA!WC8d&?XQDO#rj~Xw7%kBJ%%&ikr6!gLpfwO} z`JG}8I3q1c7n&GV>EaimIo0DY5!H3Vrc0~%1?YbmC^=7^RCLp-B~#i!x7vQ!-$3-? z4(R?fLcU(?{`TFWB1C>E)FDq?p8a2{VxGW3dK^og3rn3Ec}%+)*Zb4dKwVP4FPT0{ z(sTbj`%&u;*PmMdO<2hl*T}A)b}68YsYM~@Z7&>Ri~lbKV~h8oTPs&es%@TqNqI2+ zTk}6X5|aYA;&P^hx+x%asGmm0Nu*&T@N*~u>2bS*wBlb=!Xf)g^3^{UAforW$6+q@ z0Fmg4Kg*fz#R>Ur?_Rk zSM;?WC6+b)P}s}|DXjt%4bir1*q~~YYPqDUN3bUXKZXjaa%@!jiFZWwtsnaM^_k_r zmIDCFS|L3@Ejw;%uAUjR?et_<05xev_osK+fmC$dg?pTQS}Qrx@8_)F0hap-yyE^? zSwt+u5z}Mia$0+c82OvbPsg|*EnXeKur6DVqJN0_U6n(YaE1(X7$sWtJbcFVCEU&{ z$ZdP+3^(3)2%u2$M~%(eRI-{$62jB*6I(ewQLA6LG=8}!0XM!YtDZTn@Wtvy9>}|y zcGgR;T|k|4TJ2W@1gJf8C!cZfbV4ej`YH(VumO$*&y?+anpRc#QE?Xxvv@ zL@cz1fA2A2Ur(P*ee>R*{gra^JA*qY`$*|B%MD^6{M`(513Avh-P2=I6z?piePOsN xBVY5l{veK&2QXRX_CK=1zjxaI)n&&PvHiG~mG*{#Gy8G}7@aoLd#Qtu_z$sg6Py45 From 9dbd3a765bf76aff56ddcdce0943004bccc365bb Mon Sep 17 00:00:00 2001 From: Kevin Kofler Date: Mon, 26 Aug 2019 19:02:15 +0200 Subject: [PATCH 23/26] [netinstall] Fix typo in netinstall.conf comment s/inmportant/important/ --- src/modules/netinstall/netinstall.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/netinstall/netinstall.conf b/src/modules/netinstall/netinstall.conf index fa160d34c..fd59c24c6 100644 --- a/src/modules/netinstall/netinstall.conf +++ b/src/modules/netinstall/netinstall.conf @@ -5,7 +5,7 @@ # or it can be a locally installed file: # groupsUrl: file:///usr/share/calamares/netinstall.yaml # -# Note that the contents of the groups file is the **inmportant** +# Note that the contents of the groups file is the **important** # part of the configuration of this module. It specifies what # the user may select and what commands are to be run. # From 6035a74a93c070c960c0cd4701a6c6f810bdb30c Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 20:34:32 +0200 Subject: [PATCH 24/26] [packagechooser] Align the screenshot - hcenter + vcenter the screenshot - make it expand as necessary - fill in some sample text --- src/modules/packagechooser/page_package.ui | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/modules/packagechooser/page_package.ui b/src/modules/packagechooser/page_package.ui index 17a960549..496c478a1 100644 --- a/src/modules/packagechooser/page_package.ui +++ b/src/modules/packagechooser/page_package.ui @@ -37,21 +37,36 @@ - TextLabel + Product Name + + + 1 + 0 + + TextLabel + + Qt::AlignCenter + + + + 0 + 2 + + - TextLabel + Long Product Description true From d8a587e16e3b45cb99abf289a6c7ec021c0b62c4 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 21:40:16 +0200 Subject: [PATCH 25/26] [packagechooser] Scale screenshot - if the screenshot is too large, scale it down - (doesn't react to window resizes though) --- .../packagechooser/PackageChooserPage.cpp | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/src/modules/packagechooser/PackageChooserPage.cpp b/src/modules/packagechooser/PackageChooserPage.cpp index 6f565c914..f0a6579b4 100644 --- a/src/modules/packagechooser/PackageChooserPage.cpp +++ b/src/modules/packagechooser/PackageChooserPage.cpp @@ -52,6 +52,37 @@ PackageChooserPage::PackageChooserPage( PackageChooserMode mode, QWidget* parent } } +/** @brief size the given @p pixmap to @p size + * + * This is "smart" in the sense that it tries to keep the image un-scaled + * (if it's just a little too big) and otherwise scales as needed. + * + * Returns a copy if any modifications are done. + */ +static QPixmap +smartClip( const QPixmap& pixmap, QSize size ) +{ + auto pixSize = pixmap.size(); + if ( ( pixSize.width() <= size.width() ) && ( pixSize.height() <= size.height() ) ) + { + return pixmap; + } + + // only slightly bigger? Trim the edges + constexpr int margin = 16; + if ( ( pixSize.width() <= size.width() + margin ) && ( pixSize.height() <= size.height() + margin ) ) + { + int x = pixSize.width() <= size.width() ? 0 : ( pixSize.width() - size.width() / 2 ); + int new_width = pixSize.width() <= size.width() ? pixSize.width() : size.width(); + int y = pixSize.height() <= size.height() ? 0 : ( pixSize.height() - size.height() / 2 ); + int new_height = pixSize.height() <= size.height() ? pixSize.height() : size.height(); + + return pixmap.copy( x, y, new_width, new_height ); + } + + return pixmap.scaled( size, Qt::KeepAspectRatio ); +} + void PackageChooserPage::currentChanged( const QModelIndex& index ) { @@ -66,8 +97,17 @@ PackageChooserPage::currentChanged( const QModelIndex& index ) const auto* model = ui->products->model(); ui->productName->setText( model->data( index, PackageListModel::NameRole ).toString() ); - ui->productScreenshot->setPixmap( model->data( index, PackageListModel::ScreenshotRole ).value< QPixmap >() ); ui->productDescription->setText( model->data( index, PackageListModel::DescriptionRole ).toString() ); + + QPixmap currentScreenshot = model->data( index, PackageListModel::ScreenshotRole ).value< QPixmap >(); + if ( currentScreenshot.isNull() ) + { + ui->productScreenshot->setPixmap( m_introduction.screenshot ); + } + else + { + ui->productScreenshot->setPixmap( smartClip( currentScreenshot, ui->productScreenshot->size() ) ); + } } } From eb21c90861f399236357b1f68cd22bb04a1111c6 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 26 Aug 2019 16:54:35 +0200 Subject: [PATCH 26/26] [libcalamares] Avoid implicit 0-to-flags conversion - clang complains about using 0 as a Flags value, so make the default (empty) initialization explicit. --- src/libcalamares/network/Manager.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcalamares/network/Manager.h b/src/libcalamares/network/Manager.h index 0f2d0c651..041314f13 100644 --- a/src/libcalamares/network/Manager.h +++ b/src/libcalamares/network/Manager.h @@ -47,7 +47,7 @@ public: Q_DECLARE_FLAGS( Flags, Flag ) RequestOptions() - : m_flags( 0 ) + : m_flags( Flags() ) , m_timeout( -1 ) { }