Merge branch 'master' of https://github.com/calamares/calamares into development

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
 
@@ -194,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 )

src/libcalamares/geoip/GeoIPTests.cpp

@@ -24,9 +24,7 @@
 #endif
 #include "Handler.h"
 
-#include <QNetworkAccessManager>
+#include "network/Manager.h"
-#include <QNetworkReply>
-#include <QNetworkRequest>
 
 #include <QtTest/QtTest>
 
@@ -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") );

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 <QEventLoop>
-#include <QNetworkReply>
-#include <QNetworkRequest>
-
 #include <memory>
 
 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

src/libcalamares/network/Manager.cpp

@@ -0,0 +1,177 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *
+ *   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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "Manager.h"
+
+#include <QEventLoop>
+#include <QNetworkAccessManager>
+#include <QNetworkReply>
+#include <QNetworkRequest>
+#include <QTimer>
+
+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;
+    QUrl m_hasInternetUrl;
+    bool m_hasInternet;
+
+    Private();
+};
+
+Manager::Private::Private()
+    : m_nam( std::make_unique< QNetworkAccessManager >() )
+    , m_hasInternet( false )
+{
+}
+
+Manager::Manager()
+    : d( std::make_unique< Private >() )
+{
+}
+
+Manager::~Manager() {}
+
+Manager&
+Manager::instance()
+{
+    static auto* s_manager = new Manager();
+    return *s_manager;
+}
+
+bool
+Manager::hasInternet()
+{
+    return d->m_hasInternet;
+}
+
+bool
+Manager::checkHasInternet()
+{
+    bool hasInternet = d->m_nam->networkAccessible() == QNetworkAccessManager::Accessible;
+
+    if ( !hasInternet && ( d->m_nam->networkAccessible() == QNetworkAccessManager::UnknownAccessibility ) )
+    {
+        hasInternet = synchronousPing( d->m_hasInternetUrl );
+    }
+    d->m_hasInternet = hasInternet;
+    return hasInternet;
+}
+
+void
+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 QPair< RequestStatus, 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;
+
+    // Bail out early if the request is bad
+    if ( reply->error() )
+    {
+        reply->deleteLater();
+        return qMakePair( RequestStatus( RequestStatus::Failed ), nullptr );
+    }
+
+    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();
+    if ( options.hasTimeout() && !timer.isActive() )
+    {
+        reply->deleteLater();
+        return qMakePair( RequestStatus( RequestStatus::Timeout ), nullptr );
+    }
+
+    reply->deleteLater();
+    return qMakePair( RequestStatus( RequestStatus::Ok ), reply );
+}
+
+RequestStatus
+Manager::synchronousPing( const QUrl& url, const RequestOptions& options )
+{
+    if ( !url.isValid() )
+    {
+        return RequestStatus::Failed;
+    }
+
+    auto reply = synchronousRun( d->m_nam, url, options );
+    if ( reply.first )
+    {
+        return reply.second->bytesAvailable() ? RequestStatus::Ok : RequestStatus::Empty;
+    }
+    else
+    {
+        return reply.first;
+    }
+}
+
+QByteArray
+Manager::synchronousGet( const QUrl& url, const RequestOptions& options )
+{
+    if ( !url.isValid() )
+    {
+        return QByteArray();
+    }
+
+    auto reply = synchronousRun( d->m_nam, url, options );
+    return reply.first ? reply.second->readAll() : QByteArray();
+}
+
+}  // namespace Network
+}  // namespace CalamaresUtils

src/libcalamares/network/Manager.h

@@ -0,0 +1,148 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *
+ *   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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef LIBCALAMARES_NETWORK_MANAGER_H
+#define LIBCALAMARES_NETWORK_MANAGER_H
+
+#include "DllMacro.h"
+
+#include <QByteArray>
+#include <QObject>
+#include <QUrl>
+
+#include <chrono>
+#include <memory>
+
+class QNetworkRequest;
+
+namespace CalamaresUtils
+{
+namespace Network
+{
+class DLLEXPORT RequestOptions
+{
+public:
+    using milliseconds = std::chrono::milliseconds;
+
+    enum Flag
+    {
+        FollowRedirect = 0x1,
+        FakeUserAgent = 0x100
+    };
+    Q_DECLARE_FLAGS( Flags, Flag )
+
+    RequestOptions()
+        : m_flags( Flags() )
+        , m_timeout( -1 )
+    {
+    }
+
+    RequestOptions( Flags f, milliseconds timeout = milliseconds( -1 ) )
+        : m_flags( f )
+        , m_timeout( timeout )
+    {
+    }
+
+    void applyToRequest( QNetworkRequest* ) const;
+
+    bool hasTimeout() const { return m_timeout > milliseconds( 0 ); }
+    auto timeout() const { return m_timeout; }
+
+private:
+    Flags m_flags;
+    milliseconds m_timeout;
+};
+
+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 )
+        : status( s )
+    {
+    }
+    operator bool() const { return status == Ok; }
+
+    State status;
+};
+
+class DLLEXPORT Manager : QObject
+{
+    Q_OBJECT
+
+    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 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.
+     */
+    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 (or no data was returned).
+     */
+    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 );
+    /** @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:
+    struct Private;
+    std::unique_ptr< Private > d;
+};
+}  // namespace Network
+}  // namespace CalamaresUtils
+#endif  // LIBCALAMARES_NETWORK_MANAGER_H

src/libcalamares/network/Tests.cpp

@@ -0,0 +1,48 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *
+ *   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 <http://www.gnu.org/licenses/>.
+ */
+
+#include "Tests.h"
+
+#include "Manager.h"
+
+#include <QtTest/QtTest>
+
+QTEST_GUILESS_MAIN( NetworkTests )
+
+NetworkTests::NetworkTests() {}
+
+NetworkTests::~NetworkTests() {}
+
+void
+NetworkTests::initTestCase()
+{
+}
+
+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" ) ) );
+}

src/libcalamares/network/Tests.h

@@ -0,0 +1,38 @@
+/* === This file is part of Calamares - <https://github.com/calamares> ===
+ *
+ *   Copyright 2019, Adriaan de Groot <groot@kde.org>
+ *
+ *   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 <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef LIBCALAMARES_NETWORK_TESTS_H
+#define LIBCALAMARES_NETWORK_TESTS_H
+
+#include <QObject>
+
+class NetworkTests : public QObject
+{
+    Q_OBJECT
+public:
+    NetworkTests();
+    ~NetworkTests() override;
+
+private Q_SLOTS:
+    void initTestCase();
+
+    void testInstance();
+    void testPing();
+};
+
+#endif

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

src/modules/netinstall/README.md

@@ -1,21 +1,32 @@
 # Netinstall module
 
-The netinstall module allows distribution maintainers to ship minimal ISOs with only a basic set of preinstall packages.
+The netinstall module allows distribution maintainers to ship minimal ISOs with
-At installation time, the user is presented with the choice to install groups of packages from a predefined list.
+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: <URL to YAML file>
+```
 
-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.
 
+
+## Groups Configuration
+
+ Here is a short example
+of how the YAML file should look.
+
+```
     - name: "Group name"
       description: "Description of the group"
       packages:
@@ -24,63 +35,82 @@ The URL must point to a YAML file. Here is a short example of how the YAML file
         - 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.
+ - *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.
+ - *selected*: if true, display the group as selected. Defaults to false.
-      - critical: if true, make the installation process fail if installing
+ - 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
+ - *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
+ - *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
+ - *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
+If you set both *hidden* and *selected* for a group, you are basically creating
-which will always be installed in the user's system.
+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
+The *pre-install* and *post-install* commands are **not** passed to
-tested using the live installation of Chakra Fermi.
+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
+## Overall Configuration
-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.
 
-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
+**unpackfs** creates the chroot where the installation is performed, and unpacks
-structure; **networkcfg** set ups a working network in the chroot; and finally **packages** can install packages
+the root image with the filesystem structure; **networkcfg** set ups a working
-in the chroot.
+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
+If launching the package manager command returns you negative exit statuses and
-is likely an error in the setup of the chroot; check that the parameter **rootMountPoint** is set to the correct
+nothing is actually invoked, this is likely an error in the setup of the chroot;
-value in the Calamares configuration.
+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
+If the command is run, but exits with error, check that the network is
-exists and that it's not empty.
+working in the chroot. Make sure `/etc/resolv.conf` exists and that
+it's not empty.

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 **important**
+# 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

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

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() ) );
+        }
     }
 }
 

src/modules/packagechooser/page_package.ui

@@ -37,21 +37,36 @@
        <item>
         <widget class="QLabel" name="productName">
          <property name="text">
-          <string>TextLabel</string>
+          <string>Product Name</string>
          </property>
         </widget>
        </item>
        <item>
         <widget class="QLabel" name="productScreenshot">
+         <property name="sizePolicy">
+          <sizepolicy hsizetype="Expanding" vsizetype="Expanding">
+           <horstretch>1</horstretch>
+           <verstretch>0</verstretch>
+          </sizepolicy>
+         </property>
          <property name="text">
           <string>TextLabel</string>
          </property>
+         <property name="alignment">
+          <set>Qt::AlignCenter</set>
+         </property>
         </widget>
        </item>
        <item>
         <widget class="QLabel" name="productDescription">
+         <property name="sizePolicy">
+          <sizepolicy hsizetype="Preferred" vsizetype="Expanding">
+           <horstretch>0</horstretch>
+           <verstretch>2</verstretch>
+          </sizepolicy>
+         </property>
          <property name="text">
-          <string>TextLabel</string>
+          <string>Long Product Description</string>
          </property>
          <property name="wordWrap">
           <bool>true</bool>

src/modules/packages/packages.conf

@@ -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
+# The pre- and post-scripts are optional, but you cannot leave both out
-# "package: vi" with neither script option will trick Calamares into
+# if you do use the *package* key: using "package: vi" with neither script
-# trying to install a package named "package: vi", which is unlikely to work.
+# 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

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 <QEventLoop>
-#include <QNetworkAccessManager>
-#include <QNetworkReply>
 #include <QSemaphore>
 #include <QTimer>
 
@@ -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;
+    auto result = Manager::instance().synchronousPing(
-    request.setUrl( QUrl( m_url ) );
+        QUrl( m_url ),
-    // Follows all redirects except unsafe ones (https to http).
+        RequestOptions( RequestOptions::FollowRedirect | RequestOptions::FakeUserAgent,
-    request.setAttribute( QNetworkRequest::FollowRedirectsAttribute, true );
+                        RequestOptions::milliseconds( 5000 ) ) );
-    // Not everybody likes the default User Agent used by this class (looking at you,
+    if ( result.status == RequestStatus::Timeout )
-    // 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() )
     {
         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
 {

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

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"
@@ -42,13 +44,9 @@
 #include <QDBusInterface>
 #include <QDesktopWidget>
 #include <QDir>
-#include <QEventLoop>
 #include <QFile>
 #include <QFileInfo>
 #include <QLabel>
-#include <QNetworkAccessManager>
-#include <QNetworkRequest>
-#include <QNetworkReply>
 #include <QProcess>
 #include <QTimer>
 
@@ -245,16 +243,16 @@ GeneralRequirements::setConfigurationMap( const QVariantMap& configurationMap )
         incompleteConfiguration = true;
     }
 
-    if ( configurationMap.contains( "internetCheckUrl" ) &&
+    QUrl checkInternetUrl;
-         configurationMap.value( "internetCheckUrl" ).type() == QVariant::String )
+    QString checkInternetSetting = CalamaresUtils::getString( configurationMap, "internetCheckUrl" );
+    if ( !checkInternetSetting.isEmpty() )
     {
-        m_checkHasInternetUrl = configurationMap.value( "internetCheckUrl" ).toString().trimmed();
+        checkInternetUrl = QUrl( checkInternetSetting.trimmed() );
-        if ( m_checkHasInternetUrl.isEmpty() ||
+        if ( !checkInternetUrl.isValid() )
-             !QUrl( m_checkHasInternetUrl ).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 +260,13 @@ GeneralRequirements::setConfigurationMap( const QVariantMap& configurationMap )
     {
         cWarning() << "GeneralRequirements entry 'internetCheckUrl' is undefined in welcome.conf,"
                     "reverting to default (http://example.com).";
-
+        checkInternetUrl = "http://example.com";
-        m_checkHasInternetUrl = "http://example.com";
         incompleteConfiguration = true;
     }
+    if ( checkInternetUrl.isValid() )
+    {
+        CalamaresUtils::Network::Manager::instance().setCheckHasInternetUrl( checkInternetUrl );
+    }
 
     if ( incompleteConfiguration )
     {
@@ -357,22 +358,8 @@ GeneralRequirements::checkHasPower()
 bool
 GeneralRequirements::checkHasInternet()
 {
-    // default to true in the QNetworkAccessManager::UnknownAccessibility case
+    auto& nam = CalamaresUtils::Network::Manager::instance();
-    QNetworkAccessManager qnam;
+    bool hasInternet = nam.checkHasInternet();
-    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;
-    }
-
     Calamares::JobQueue::instance()->globalStorage()->insert( "hasInternet", hasInternet );
     return hasInternet;
 }

src/modules/welcome/checker/GeneralRequirements.h

@@ -48,7 +48,6 @@ private:
 
     qreal m_requiredStorageGiB;
     qreal m_requiredRamGiB;
-    QString m_checkHasInternetUrl;
 };
 
 #endif // REQUIREMENTSCHECKER_H