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

CHANGES

@@ -11,6 +11,7 @@ website will have to do for older versions.
 
 This release contains contributions from (alphabetically by first name):
  - Erik Dubois
+ - Joe Kamprad
  - Lisa Vitolo
 
 ## Core ##
@@ -32,6 +33,15 @@ This release contains contributions from (alphabetically by first name):
    has been revived and merged.
 
 
+# 3.2.39.3 (2021-04-14) #
+
+A minor bugfix tweak release. Since this contains yet **another**
+autologin-related fix, and there is nothing large enough to justify
+a 3.2.40 release yet, add it to the growing tail of 3.2.39. (Reported
+by Joe Kamprad, #1672). Also fixes a regression from 3.2.28 in
+localized packages (e.g. *package-LOCALE* did not work).
+
+
 # 3.2.39.2 (2021-04-02) #
 
 This is **another** hotfix release for issues around autologin ..

src/modules/initcpiocfg/main.py

@@ -92,6 +92,7 @@ def write_mkinitcpio_lines(hooks, modules, files, root_mount_point):
     with open(path, "w") as mkinitcpio_file:
         mkinitcpio_file.write("\n".join(mklins) + "\n")
 
+
 def detect_plymouth():
     """
     Checks existence (runnability) of plymouth in the target system.
@@ -99,10 +100,8 @@ def detect_plymouth():
     @return True if plymouth exists in the target, False otherwise
     """
     # Used to only check existence of path /usr/bin/plymouth in target
-    isPlymouth = target_env_call(["sh", "-c", "which plymouth"])
-    debug("which plymouth exit code: {!s}".format(isPlymouth))
+    return target_env_call(["sh", "-c", "which plymouth"]) == 0
 
-    return isPlymouth == 0
 
 def modify_mkinitcpio_conf(partitions, root_mount_point):
     """

src/modules/netinstall/CMakeLists.txt

@@ -26,6 +26,8 @@ calamares_add_test(
     netinstalltest
     SOURCES
         Tests.cpp
+        Config.cpp
+        LoaderQueue.cpp
         PackageTreeItem.cpp
         PackageModel.cpp
     LIBRARIES

src/modules/netinstall/Config.cpp

@@ -97,7 +97,6 @@ Config::loadGroupList( const QVariantList& groupData )
     {
         setStatus( Status::Ok );
     }
-    emit statusReady();
 }
 
 void
@@ -108,6 +107,7 @@ Config::loadingDone()
         m_queue->deleteLater();
         m_queue = nullptr;
     }
+    emit statusReady();
 }
 
 
@@ -136,25 +136,23 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     // Lastly, load the groups data
     const QString key = QStringLiteral( "groupsUrl" );
     const auto& groupsUrlVariant = configurationMap.value( key );
+    m_queue = new LoaderQueue( this );
     if ( groupsUrlVariant.type() == QVariant::String )
     {
-        m_queue = new LoaderQueue( this );
         m_queue->append( SourceItem::makeSourceItem( groupsUrlVariant.toString(), configurationMap ) );
     }
     else if ( groupsUrlVariant.type() == QVariant::List )
     {
-        m_queue = new LoaderQueue( this );
         for ( const auto& s : groupsUrlVariant.toStringList() )
         {
             m_queue->append( SourceItem::makeSourceItem( s, configurationMap ) );
         }
     }
-    if ( m_queue && m_queue->count() > 0 )
-    {
-        cDebug() << "Loading netinstall from" << m_queue->count() << "alternate sources.";
-        connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
-        m_queue->load();
-    }
+
+    setStatus( required() ? Status::FailedNoData : Status::Ok );
+    cDebug() << "Loading netinstall from" << m_queue->count() << "alternate sources.";
+    connect( m_queue, &LoaderQueue::done, this, &Config::loadingDone );
+    m_queue->load();
 }
 
 void

src/modules/netinstall/Config.h

@@ -49,10 +49,12 @@ public:
         FailedNetworkError,
         FailedBadData,
         FailedNoData
-
     };
 
+    /// Human-readable, translated representation of the status
     QString status() const;
+    /// Internal code for the status
+    Status statusCode() const { return m_status; }
     void setStatus( Status s );
 
     bool required() const { return m_required; }

src/modules/netinstall/LoaderQueue.cpp

@@ -25,6 +25,10 @@
  * On destruction, a new call to fetchNext() is queued, so that
  * the queue continues loading. Calling release() before the
  * destructor skips the fetchNext(), ending the queue-loading.
+ *
+ * Calling done(b) is a conditional release: if @p b is @c true,
+ * queues a call to done() on the queue and releases it; otherwise,
+ * does nothing.
  */
 class FetchNextUnless
 {
@@ -41,6 +45,17 @@ public:
         }
     }
     void release() { m_q = nullptr; }
+    void done( bool b )
+    {
+        if ( b )
+        {
+            if ( m_q )
+            {
+                QMetaObject::invokeMethod( m_q, "done", Qt::QueuedConnection );
+            }
+            release();
+        }
+    }
 
 private:
     LoaderQueue* m_q = nullptr;
@@ -83,7 +98,6 @@ LoaderQueue::fetchNext()
 {
     if ( m_queue.isEmpty() )
     {
-        m_config->setStatus( Config::Status::FailedBadData );
         emit done();
         return;
     }
@@ -138,7 +152,7 @@ LoaderQueue::fetch( const QUrl& url )
 void
 LoaderQueue::dataArrived()
 {
-    FetchNextUnless finished( this );
+    FetchNextUnless next( this );
 
     if ( !m_reply || !m_reply->isFinished() )
     {
@@ -170,16 +184,14 @@ LoaderQueue::dataArrived()
 
         if ( groups.IsSequence() )
         {
-            finished.release();
             m_config->loadGroupList( CalamaresUtils::yamlSequenceToVariant( groups ) );
-            emit done();
+            next.done( m_config->statusCode() == Config::Status::Ok );
         }
         else if ( groups.IsMap() )
         {
-            finished.release();
             auto map = CalamaresUtils::yamlMapToVariant( groups );
             m_config->loadGroupList( map.value( "groups" ).toList() );
-            emit done();
+            next.done( m_config->statusCode() == Config::Status::Ok );
         }
         else
         {

src/modules/netinstall/Tests.cpp

@@ -7,13 +7,17 @@
  *
  */
 
+#include "Config.h"
 #include "PackageModel.h"
 #include "PackageTreeItem.h"
 
 #include "utils/Logger.h"
+#include "utils/NamedEnum.h"
 #include "utils/Variant.h"
 #include "utils/Yaml.h"
 
+#include <KMacroExpander>
+
 #include <QtTest/QtTest>
 
 class ItemTests : public QObject
@@ -40,6 +44,9 @@ private Q_SLOTS:
     void testCompare();
     void testModel();
     void testExampleFiles();
+
+    void testUrlFallback_data();
+    void testUrlFallback();
 };
 
 ItemTests::ItemTests() {}
@@ -326,6 +333,93 @@ ItemTests::testExampleFiles()
     }
 }
 
+void
+ItemTests::testUrlFallback_data()
+{
+    QTest::addColumn< QString >( "filename" );
+    QTest::addColumn< int >( "status" );
+    QTest::addColumn< int >( "count" );
+
+    using S = Config::Status;
+
+    QTest::newRow( "bad" ) << "1a-single-bad.conf" << smash( S::FailedBadConfiguration ) << 0;
+    QTest::newRow( "empty" ) << "1a-single-empty.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "error" ) << "1a-single-error.conf" << smash( S::FailedBadData ) << 0;
+    QTest::newRow( "two" ) << "1b-single-small.conf" << smash( S::Ok ) << 2;
+    QTest::newRow( "five" ) << "1b-single-large.conf" << smash( S::Ok ) << 5;
+    QTest::newRow( "none" ) << "1c-none.conf" << smash( S::FailedNoData ) << 0;
+    QTest::newRow( "unset" ) << "1c-unset.conf" << smash( S::FailedNoData ) << 0;
+    // Finds small, then stops
+    QTest::newRow( "fallback-small" ) << "1d-fallback-small.conf" << smash( S::Ok ) << 2;
+    // Finds large, then stops
+    QTest::newRow( "fallback-large" ) << "1d-fallback-large.conf" << smash( S::Ok ) << 5;
+    // Finds empty, finds small
+    QTest::newRow( "fallback-mixed" ) << "1d-fallback-mixed.conf" << smash( S::Ok ) << 2;
+    // Finds empty, then bad
+    QTest::newRow( "fallback-bad" ) << "1d-fallback-bad.conf" << smash( S::FailedBadConfiguration ) << 0;
+}
+
+void
+ItemTests::testUrlFallback()
+{
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+    QFETCH( QString, filename );
+    QFETCH( int, status );
+    QFETCH( int, count );
+
+    cDebug() << "Loading" << filename;
+
+    // BUILD_AS_TEST is the source-directory path
+    QString testdir = QString( "%1/tests" ).arg( BUILD_AS_TEST );
+    QFile fi( QString( "%1/%2" ).arg( testdir, filename ) );
+    QVERIFY( fi.exists() );
+
+    Config c;
+
+    QFile yamlFile( fi.fileName() );
+    if ( yamlFile.exists() && yamlFile.open( QFile::ReadOnly | QFile::Text ) )
+    {
+        QString ba( yamlFile.readAll() );
+        QVERIFY( ba.length() > 0 );
+        QHash< QString, QString > replace;
+        replace.insert( "TESTDIR", testdir );
+        QString correctedDocument = KMacroExpander::expandMacros( ba, replace, '$' );
+
+        try
+        {
+            YAML::Node yamldoc = YAML::Load( correctedDocument.toUtf8() );
+            auto map = CalamaresUtils::yamlToVariant( yamldoc ).toMap();
+            QVERIFY( map.count() > 0 );
+            c.setConfigurationMap( map );
+        }
+        catch ( YAML::Exception& e )
+        {
+            bool badYaml = true;
+            QVERIFY( !badYaml );
+        }
+    }
+    else
+    {
+        QCOMPARE( QStringLiteral( "not found" ), fi.fileName() );
+    }
+
+    // Each of the configs sets required to **true**, which is not the default
+    QVERIFY( c.required() );
+
+    // Now give the loader time to complete
+    QEventLoop loop;
+    connect( &c, &Config::statusReady, &loop, &QEventLoop::quit );
+    QSignalSpy spy( &c, &Config::statusReady );
+    QTimer::singleShot( std::chrono::seconds(1), &loop, &QEventLoop::quit );
+    loop.exec();
+
+    // Check it didn't time out
+    QCOMPARE( spy.count(), 1 );
+    // Check YAML-loading results
+    QCOMPARE( smash( c.statusCode() ), status );
+    QCOMPARE( c.model()->rowCount(), count );
+}
+
 
 QTEST_GUILESS_MAIN( ItemTests )
 

src/modules/netinstall/tests/1a-single-bad.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/bad.yaml

src/modules/netinstall/tests/1a-single-empty.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-empty.yaml

src/modules/netinstall/tests/1a-single-error.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-error.yaml

src/modules/netinstall/tests/1b-single-large.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-large.yaml

src/modules/netinstall/tests/1b-single-small.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-small.yaml

src/modules/netinstall/tests/1c-none.conf

@@ -0,0 +1,6 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl: []

src/modules/netinstall/tests/1c-unset.conf

@@ -0,0 +1,5 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true

src/modules/netinstall/tests/1d-fallback-bad.conf

@@ -0,0 +1,10 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-bad.yaml

src/modules/netinstall/tests/1d-fallback-large.conf

@@ -0,0 +1,10 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-bad.yaml
+    - file://$TESTDIR/data-large.yaml
+    - file://$TESTDIR/data-small.yaml

src/modules/netinstall/tests/1d-fallback-mixed.conf

@@ -0,0 +1,13 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-bad.yaml
+    - file://$TESTDIR/data-empty.yaml
+    - file://$TESTDIR/data-small.yaml
+    - file://$TESTDIR/data-large.yaml
+    - file://$TESTDIR/data-bad.yaml

src/modules/netinstall/tests/1d-fallback-small.conf

@@ -0,0 +1,10 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+required: true
+groupsUrl:
+    - file://$TESTDIR/data-nonexistent.yaml
+    - file://$TESTDIR/data-bad.yaml
+    - file://$TESTDIR/data-small.yaml
+    - file://$TESTDIR/data-large.yaml

src/modules/netinstall/tests/data-empty.yaml

@@ -0,0 +1,6 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+bogus: true
+

src/modules/netinstall/tests/data-error.yaml

@@ -0,0 +1,5 @@
+derp
+derp
+herpa-derp: no
+--
+# This file is not valid YAML

src/modules/netinstall/tests/data-large.yaml

@@ -0,0 +1,38 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+- name: "Default"
+  description: "Default group"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - base
+- name: "Two"
+  description: "group 2"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-two
+- name: "Three"
+  description: "group 3"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-three
+- name: "Four"
+  description: "group 4"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-four
+- name: "Five"
+  description: "group 5"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-five

src/modules/netinstall/tests/data-small.yaml

@@ -0,0 +1,17 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+- name: "Default"
+  description: "Default group"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - base
+- name: "Second"
+  description: "Second group"
+  hidden: false
+  selected: true
+  critical: false
+  packages:
+    - chakra-live-skel

src/modules/plymouthcfg/main.py

@@ -27,6 +27,16 @@ def pretty_name():
     return _("Configure Plymouth theme")
 
 
+def detect_plymouth():
+    """
+    Checks existence (runnability) of plymouth in the target system.
+
+    @return True if plymouth exists in the target, False otherwise
+    """
+    # Used to only check existence of path /usr/bin/plymouth in target
+    return target_env_call(["sh", "-c", "which plymouth"]) == 0
+
+
 class PlymouthController:
 
     def __init__(self):
@@ -42,14 +52,8 @@ class PlymouthController:
                          plymouth_theme + '|', "-i",
                          "/etc/plymouth/plymouthd.conf"])
 
-    def detect(self):
-        isPlymouth = target_env_call(["sh", "-c", "which plymouth"])
-        debug("which plymouth exit code: {!s}".format(isPlymouth))
-
-        return isPlymouth
-
     def run(self):
-        if self.detect() == 0:
+        if detect_plymouth():
             if (("plymouth_theme" in libcalamares.job.configuration) and
                (libcalamares.job.configuration["plymouth_theme"] is not None)):
                 self.setTheme()

src/modules/users/Config.cpp

@@ -803,6 +803,29 @@ addPasswordCheck( const QString& key, const QVariant& value, PasswordCheckList&
     return true;
 }
 
+/** @brief Returns a value of either key from the map
+ *
+ * Takes a function (e.g. getBool, or getString) and two keys,
+ * returning the value in the map of the one that is there (or @p defaultArg)
+ */
+template < typename T, typename U >
+T
+either( T ( *f )( const QVariantMap&, const QString&, U ),
+        const QVariantMap& configurationMap,
+        const QString& oldKey,
+        const QString& newKey,
+        U defaultArg )
+{
+    if ( configurationMap.contains( oldKey ) )
+    {
+        return f( configurationMap, oldKey, defaultArg );
+    }
+    else
+    {
+        return f( configurationMap, newKey, defaultArg );
+    }
+}
+
 void
 Config::setConfigurationMap( const QVariantMap& configurationMap )
 {
@@ -814,7 +837,8 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
     // Now it might be explicitly set to empty, which is ok
     setUserShell( shell );
 
-    setAutoLoginGroup( CalamaresUtils::getString( configurationMap, "autoLoginGroup" ) );
+    setAutoLoginGroup( either< QString, const QString& >(
+        CalamaresUtils::getString, configurationMap, "autologinGroup", "autoLoginGroup", QString() ) );
     setSudoersGroup( CalamaresUtils::getString( configurationMap, "sudoersGroup" ) );
 
     m_hostNameActions = getHostNameActions( configurationMap );
@@ -823,16 +847,11 @@ Config::setConfigurationMap( const QVariantMap& configurationMap )
 
     // Renaming of Autologin -> AutoLogin in 4ffa79d4cf also affected
     // configuration keys, which was not intended. Accept both.
-    const auto oldKey = QStringLiteral( "doAutologin" );
-    const auto newKey = QStringLiteral( "doAutoLogin" );
-    if ( configurationMap.contains( oldKey ) )
-    {
-        m_doAutoLogin = CalamaresUtils::getBool( configurationMap, oldKey, false );
-    }
-    else
-    {
-        m_doAutoLogin = CalamaresUtils::getBool( configurationMap, newKey, false );
-    }
+    m_doAutoLogin = either( CalamaresUtils::getBool,
+                            configurationMap,
+                            QStringLiteral( "doAutologin" ),
+                            QStringLiteral( "doAutoLogin" ),
+                            false );
 
     m_writeRootPassword = CalamaresUtils::getBool( configurationMap, "setRootPassword", true );
     Calamares::JobQueue::instance()->globalStorage()->insert( "setRootPassword", m_writeRootPassword );

src/modules/users/Tests.cpp

@@ -44,6 +44,9 @@ private Q_SLOTS:
     void testHostActions();
     void testPasswordChecks();
     void testUserPassword();
+
+    void testAutoLogin_data();
+    void testAutoLogin();
 };
 
 UserTests::UserTests() {}
@@ -339,6 +342,43 @@ UserTests::testUserPassword()
     }
 }
 
+void
+UserTests::testAutoLogin_data()
+{
+    QTest::addColumn< QString >( "filename" );
+    QTest::addColumn< bool >( "autoLoginIsSet" );
+    QTest::addColumn< QString >( "autoLoginGroupName" );
+
+    QTest::newRow( "old, old" ) << "tests/6a-issue-1672.conf" << true << "derp";
+    QTest::newRow( "old, new" ) << "tests/6b-issue-1672.conf" << true << "derp";
+    QTest::newRow( "new, old" ) << "tests/6c-issue-1672.conf" << true << "derp";
+    QTest::newRow( "new, new" ) << "tests/6d-issue-1672.conf" << true << "derp";
+    QTest::newRow( "default" ) << "tests/6e-issue-1672.conf" << false << QString();
+}
+
+void
+UserTests::testAutoLogin()
+{
+    QFETCH( QString, filename );
+    QFETCH( bool, autoLoginIsSet );
+    QFETCH( QString, autoLoginGroupName );
+
+    // BUILD_AS_TEST is the source-directory path
+    QFile fi( QString( "%1/%2" ).arg( BUILD_AS_TEST, filename ) );
+    QVERIFY( fi.exists() );
+
+    bool ok = false;
+    const auto map = CalamaresUtils::loadYaml( fi, &ok );
+    QVERIFY( ok );
+    QVERIFY( map.count() > 0 );
+
+    Config c;
+    c.setConfigurationMap( map );
+
+    QCOMPARE( c.doAutoLogin(), autoLoginIsSet );
+    QCOMPARE( c.autoLoginGroup(), autoLoginGroupName );
+}
+
 
 QTEST_GUILESS_MAIN( UserTests )
 

src/modules/users/tests/6a-issue-1672.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+autologinGroup:  derp
+doAutologin:     true
+

src/modules/users/tests/6b-issue-1672.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+autologinGroup:  derp
+doAutoLogin:     true
+

src/modules/users/tests/6c-issue-1672.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+autoLoginGroup:  derp
+doAutologin:     true
+

src/modules/users/tests/6d-issue-1672.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+autoLoginGroup:  derp
+doAutoLogin:     true
+

src/modules/users/tests/6e-issue-1672.conf

@@ -0,0 +1,7 @@
+# SPDX-FileCopyrightText: no
+# SPDX-License-Identifier: CC0-1.0
+#
+---
+doautologin: true
+autologingroup: wheel
+