Merge pull request #2007 from calamares/issue-1438

Use `${}` everywhere instead of `@@`

CHANGES-3.3

@@ -25,6 +25,9 @@ that the distribution configuration files follow the current schema.
 
 Pre-release versions:
  - 3.3.0-alpha1 (2022-06-27)
+   Initial 3.3.0 release to check the release scripts &c.
+ - 3.3.0-alpha2 (unreleased)
+   Incompatible module-configuration changes, see #1438.
 
 ## Project ##
  - The C++ code in the project is now formatted with clang-format 12 or 13,
@@ -49,6 +52,8 @@ Pre-release versions:
 ## Modules ##
  - *bootloader* now supports more options when building the kernel
    command-line. (Thanks Evan)
+ - *bootloader* no longer supports `@@`-style suffixes for unique-EFI-id
+   generation. Use `${}` instead.
  - *displaymanager* no longer supports the discontinued *kdm* display manager.
  - *fstab* configuration has been completely re-done. Many configuration
    options have moved to the *mount* module. See #1993
@@ -56,7 +61,10 @@ Pre-release versions:
    Please update configurations.
  - *mount* now does most of the mounting; options that were in *fstab*
    have moved here. See #1993
+ - *oemid* now uses consistent variable replacement (e.g. KMacroExpander)
+   and does not support `@@DATE@@` anymore (use `${DATE}`).
  - *partition* requires KPMCore 21.12 (e.g. KPMCore 4.2 API, or later).
  - *partition* can now skip installing the bootloader in more scenarios.
    #1632 (Thanks Anubhav)
+ - *preservefiles* follows `${}` variable syntax instead of `@@`.
 

src/libcalamares/utils/StringExpander.h

@@ -48,6 +48,14 @@ public:
     virtual ~DictionaryExpander() override;
 
     void insert( const QString& key, const QString& value );
+    /** @brief As insert(), but supports method-chaining.
+     *
+     */
+    DictionaryExpander& add( const QString& key, const QString& value )
+    {
+        insert( key, value );
+        return *this;
+    }
 
     void clearErrors();
     bool hasErrors() const;

src/modules/bootloader/bootloader.conf

@@ -53,10 +53,10 @@ efiBootMgr: "efibootmgr"
 # (problematic characters, see above, are replaced).
 #
 # There are some special words possible at the end of *efiBootloaderId*:
-#   @@SERIAL@@ can be used to obtain a uniquely-numbered suffix
+#   ${SERIAL} can be used to obtain a uniquely-numbered suffix
 #       that is added to the Id (yielding, e.g., `dirname1` or `dirname72`)
-#   @@RANDOM@@ can be used to obtain a unique 4-digit hex suffix
-#   @@PHRASE@@ can be used to obtain a unique 1-to-3-word suffix
+#   ${RANDOM} can be used to obtain a unique 4-digit hex suffix
+#   ${PHRASE} can be used to obtain a unique 1-to-3-word suffix
 #       from a dictionary of space-themed words
 # These words must be at the **end** of the *efiBootloaderId* value.
 # There must also be at most one of them. If there is none, no suffix-

src/modules/bootloader/main.py

@@ -393,28 +393,28 @@ class phraseEfi(object):
 
 def get_efi_suffix_generator(name):
     """
-    Handle EFI bootloader Ids with @@<something>@@ for suffix-processing.
+    Handle EFI bootloader Ids with ${<something>} for suffix-processing.
     """
-    if "@@" not in name:
-        raise ValueError("Misplaced call to get_efi_suffix_generator, no @@")
-    parts = name.split("@@")
-    if len(parts) != 3:
-        raise ValueError("EFI Id {!r} is malformed".format(name))
-    if parts[2]:
-        # Supposed to be empty because the string ends with "@@"
-        raise ValueError("EFI Id {!r} is malformed".format(name))
-    if parts[1] not in ("SERIAL", "RANDOM", "PHRASE"):
-        raise ValueError("EFI suffix {!r} is unknown".format(parts[1]))
+    if "${" not in name:
+        raise ValueError("Misplaced call to get_efi_suffix_generator, no ${}")
+    if not name.endswith("}"):
+        raise ValueError("Misplaced call to get_efi_suffix_generator, no trailing ${}")
+    if name.count("${") > 1:
+        raise ValueError("EFI ID {!r} contains multiple generators".format(name))
+    import re
+    prefix, generator_name = re.match("(.*)\${([^}]*)}$", name).groups()
+    if generator_name not in ("SERIAL", "RANDOM", "PHRASE"):
+        raise ValueError("EFI suffix {!r} is unknown".format(generator_name))
 
     generator = None
-    if parts[1] == "SERIAL":
-        generator = serialEfi(parts[0])
-    elif parts[1] == "RANDOM":
-        generator = randomEfi(parts[0])
-    elif parts[1] == "PHRASE":
-        generator = phraseEfi(parts[0])
+    if generator_name == "SERIAL":
+        generator = serialEfi(prefix)
+    elif generator_name == "RANDOM":
+        generator = randomEfi(prefix)
+    elif generator_name == "PHRASE":
+        generator = phraseEfi(prefix)
     if generator is None:
-        raise ValueError("EFI suffix {!r} is unsupported".format(parts[1]))
+        raise ValueError("EFI suffix {!r} is unsupported".format(generator_name))
 
     return generator
 
@@ -422,10 +422,10 @@ def get_efi_suffix_generator(name):
 def change_efi_suffix(efi_directory, bootloader_id):
     """
     Returns a label based on @p bootloader_id that is usable within
-    @p efi_directory. If there is a @@<something>@@ suffix marker
+    @p efi_directory. If there is a ${<something>} suffix marker
     in the given id, tries to generate a unique label.
     """
-    if bootloader_id.endswith("@@"):
+    if bootloader_id.endswith("}") and "${" in bootloader_id:
         # Do 10 attempts with any suffix generator
         g = suffix_iterator(10, get_efi_suffix_generator(bootloader_id))
     else:

src/modules/bootloader/tests/test-bootloader-efiname.py

@@ -10,7 +10,7 @@ libcalamares.globalstorage.insert("testing", True)
 from src.modules.bootloader import main
 
 # Specific Bootloader test
-g = main.get_efi_suffix_generator("derp@@SERIAL@@")
+g = main.get_efi_suffix_generator("derp${SERIAL}")
 assert g is not None
 assert g.next() == "derp"  # First time, no suffix
 for n in range(9):
@@ -18,13 +18,13 @@ for n in range(9):
 # We called next() 10 times in total, starting from 0
 assert g.next() == "derp10"
 
-g = main.get_efi_suffix_generator("derp@@RANDOM@@")
+g = main.get_efi_suffix_generator("derp${RANDOM}")
 assert g is not None
 for n in range(10):
     print(g.next())
 # it's random, nothing to assert
 
-g = main.get_efi_suffix_generator("derp@@PHRASE@@")
+g = main.get_efi_suffix_generator("derp${PHRASE}")
 assert g is not None
 for n in range(10):
     print(g.next())
@@ -38,19 +38,19 @@ except ValueError as e:
     pass
 
 try:
-    g = main.get_efi_suffix_generator("derp@@HEX@@")
+    g = main.get_efi_suffix_generator("derp${HEX}")
     raise TypeError("Shouldn't get generator (unknown indicator)")
 except ValueError as e:
     pass
 
 try:
-    g = main.get_efi_suffix_generator("derp@@SERIAL@@x")
+    g = main.get_efi_suffix_generator("derp${SERIAL}x")
     raise TypeError("Shouldn't get generator (trailing garbage)")
 except ValueError as e:
     pass
 
 try:
-    g = main.get_efi_suffix_generator("derp@@SERIAL@@@@RANDOM@@")
+    g = main.get_efi_suffix_generator("derp${SERIAL}${RANDOM}")
     raise TypeError("Shouldn't get generator (multiple indicators)")
 except ValueError as e:
     pass
@@ -59,9 +59,9 @@ except ValueError as e:
 # Try the generator (assuming no calamares- test files exist in /tmp)
 import os
 assert "calamares-single" == main.change_efi_suffix("/tmp", "calamares-single")
-assert "calamares-serial" == main.change_efi_suffix("/tmp", "calamares-serial@@SERIAL@@")
+assert "calamares-serial" == main.change_efi_suffix("/tmp", "calamares-serial${SERIAL}")
 try:
     os.makedirs("/tmp/calamares-serial", exist_ok=True)
-    assert "calamares-serial1" == main.change_efi_suffix("/tmp", "calamares-serial@@SERIAL@@")
+    assert "calamares-serial1" == main.change_efi_suffix("/tmp", "calamares-serial${SERIAL}")
 finally:
     os.rmdir("/tmp/calamares-serial")

src/modules/oemid/OEMViewStep.cpp

@@ -14,6 +14,7 @@
 #include "IDJob.h"
 
 #include "utils/Retranslator.h"
+#include "utils/StringExpander.h"
 #include "utils/Variant.h"
 
 #include <QDate>
@@ -82,14 +83,10 @@ OEMViewStep::isAtEnd() const
 static QString
 substitute( QString s )
 {
-    QString t_date = QStringLiteral( "@@DATE@@" );
-    if ( s.contains( t_date ) )
-    {
-        auto date = QDate::currentDate();
-        s = s.replace( t_date, date.toString( Qt::ISODate ) );
-    }
+    Calamares::String::DictionaryExpander d;
+    d.insert( QStringLiteral( "DATE" ), QDate::currentDate().toString( Qt::ISODate ) );
 
-    return s;
+    return d.expand( s );
 }
 
 void

src/modules/oemid/oemid.conf

@@ -5,12 +5,12 @@
 ---
 # The batch-identifier is written to /var/log/installer/oem-id.
 # This value is put into the text box as the **suggested**
-# OEM ID. If @@DATE@@ is included in the identifier, then
+# OEM ID. If ${DATE} is included in the identifier, then
 # that is replaced by the current date in yyyy-MM-dd (ISO) format.
 #
-# it is ok for the identifier to be empty.
+# It is ok for the identifier to be empty.
 #
 # The identifier is written to the file as UTF-8 (this will be no
 # different from ASCII, for most inputs) and followed by a newline.
 # If the identifier is empty, only a newline is written.
-batch-identifier: neon-@@DATE@@
+batch-identifier: neon-${DATE}

src/modules/preservefiles/PreserveFiles.cpp

@@ -15,6 +15,7 @@
 #include "utils/CalamaresUtilsSystem.h"
 #include "utils/CommandList.h"
 #include "utils/Logger.h"
+#include "utils/StringExpander.h"
 #include "utils/Units.h"
 
 #include <QFile>
@@ -37,7 +38,8 @@ atReplacements( QString s )
         user = gs->value( "username" ).toString();
     }
 
-    return s.replace( "@@ROOT@@", root ).replace( "@@USER@@", user );
+    Calamares::String::DictionaryExpander d;
+    return d.add( QStringLiteral( "ROOT" ), root ).add( QStringLiteral( "USER" ), user ).expand( s );
 }
 
 PreserveFiles::PreserveFiles( QObject* parent )

src/modules/preservefiles/preservefiles.conf

@@ -40,11 +40,11 @@
 #     not exist in the host system (e.g. nvidia configuration files that
 #     are created in some boot scenarios and not in others).
 #
-# The target path (*dest*) is modified as follows:
-#   - `@@ROOT@@` is replaced by the path to the target root (may be /).
+# The target path (*dest*) is modified by expanding variables in `${}`:
+#   - `ROOT` is replaced by the path to the target root (may be /).
 #     There is never any reason to use this, since the *dest* is already
 #     interpreted in the target system.
-#   - `@@USER@@` is replaced by the username entered by on the user
+#   - `USER` is replaced by the username entered by on the user
 #       page (may be empty, for instance if no user page is enabled)
 #
 #
@@ -53,6 +53,9 @@ files:
   - from: log
     dest: /var/log/Calamares.log
     perm: root:wheel:600
+  - from: log
+    dest: /home/${USER}/installation.log
+    optional: true
   - from: config
     dest: /var/log/Calamares-install.json
     perm: root:wheel:600

src/modules/users/Config.cpp

@@ -18,10 +18,9 @@
 #include "JobQueue.h"
 #include "utils/Logger.h"
 #include "utils/String.h"
+#include "utils/StringExpander.h"
 #include "utils/Variant.h"
 
-#include <KMacroExpander>
-
 #include <QCoreApplication>
 #include <QFile>
 #include <QMetaProperty>
@@ -415,20 +414,20 @@ invalidEmpty( const QString& s )
 STATICTEST QString
 makeHostnameSuggestion( const QString& templateString, const QStringList& fullNameParts, const QString& loginName )
 {
-    QHash< QString, QString > replace;
+    Calamares::String::DictionaryExpander d;
     // User data
-    replace.insert( QStringLiteral( "first" ),
-                    invalidEmpty( fullNameParts.isEmpty() ? QString() : cleanupForHostname( fullNameParts.first() ) ) );
-    replace.insert( QStringLiteral( "name" ), invalidEmpty( cleanupForHostname( fullNameParts.join( QString() ) ) ) );
-    replace.insert( QStringLiteral( "login" ), invalidEmpty( cleanupForHostname( loginName ) ) );
-    // Hardware data
-    replace.insert( QStringLiteral( "product" ), guessProductName() );
-    replace.insert( QStringLiteral( "product2" ), cleanupForHostname( QSysInfo::prettyProductName() ) );
-    replace.insert( QStringLiteral( "cpu" ), cleanupForHostname( QSysInfo::currentCpuArchitecture() ) );
-    // Hostname data
-    replace.insert( QStringLiteral( "host" ), invalidEmpty( cleanupForHostname( QSysInfo::machineHostName() ) ) );
+    d.add( QStringLiteral( "first" ),
+           invalidEmpty( fullNameParts.isEmpty() ? QString() : cleanupForHostname( fullNameParts.first() ) ) )
+        .add( QStringLiteral( "name" ), invalidEmpty( cleanupForHostname( fullNameParts.join( QString() ) ) ) )
+        .add( QStringLiteral( "login" ), invalidEmpty( cleanupForHostname( loginName ) ) )
+        // Hardware data
+        .add( QStringLiteral( "product" ), guessProductName() )
+        .add( QStringLiteral( "product2" ), cleanupForHostname( QSysInfo::prettyProductName() ) )
+        .add( QStringLiteral( "cpu" ), cleanupForHostname( QSysInfo::currentCpuArchitecture() ) )
+        // Hostname data
+        .add( QStringLiteral( "host" ), invalidEmpty( cleanupForHostname( QSysInfo::machineHostName() ) ) );
 
-    QString hostnameSuggestion = KMacroExpander::expandMacros( templateString, replace, '$' );
+    QString hostnameSuggestion = d.expand( templateString );
 
     // RegExp for valid hostnames; if the suggestion produces a valid name, return it
     static const QRegExp HOSTNAME_RX( "^[a-zA-Z0-9][-a-zA-Z0-9_]*$" );