From bf47e761b041c55aa880650759a311e225e24bbe Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Wed, 17 Jul 2019 12:15:22 +0200 Subject: [PATCH 01/16] mount: Make sure extra mounts are mounted right after / When the rootfs partition is read-only, mount points for the other partitions cannot be created, therefore they need to be created in a tmpfs, already mounted somewhere in `/`. However, the extra mounts are only mounted at the end, which causes an error as no tmpfs is currently mounted. This patch makes sure all extra mounts are mounted right after the `/` partition, allowing the use of a read-only rootfs. Signed-off-by: Arnaud Ferraris --- src/modules/mount/main.py | 184 +++++++++++++++++++------------------- 1 file changed, 94 insertions(+), 90 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index ed649aead..b093d0dfb 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -37,104 +37,109 @@ _ = gettext.translation("calamares-python", def pretty_name(): return _("Mounting partitions.") +def mount_part(root_mount_point, partition, partitions): + # Create mount point with `+` rather than `os.path.join()` because + # `partition["mountPoint"]` starts with a '/'. + raw_mount_point = partition["mountPoint"] + mount_point = root_mount_point + raw_mount_point -def mount_partitions(root_mount_point, partitions): + # Ensure that the created directory has the correct SELinux context on + # SELinux-enabled systems. + os.makedirs(mount_point, exist_ok=True) + subprocess.call(['chcon', '--reference=' + raw_mount_point, + mount_point]) + + fstype = partition.get("fs", "").lower() + + if fstype == "fat16" or fstype == "fat32": + fstype = "vfat" + + if "luksMapperName" in partition: + libcalamares.utils.debug( + "about to mount {!s}".format(partition["luksMapperName"])) + libcalamares.utils.mount( + "/dev/mapper/{!s}".format(partition["luksMapperName"]), + mount_point, + fstype, + partition.get("options", ""), + ) + + else: + libcalamares.utils.mount(partition["device"], + mount_point, + fstype, + partition.get("options", ""), + ) + + # If the root partition is btrfs, we create a subvolume "@" + # for the root mount point. + # If a separate /home partition isn't defined, we also create + # a subvolume "@home". + # Finally we remount all of the above on the correct paths. + if fstype == "btrfs" and partition["mountPoint"] == '/': + has_home_mount_point = False + for p in partitions: + if "mountPoint" not in p or not p["mountPoint"]: + continue + if p["mountPoint"] == "/home": + has_home_mount_point = True + break + + subprocess.check_call(['btrfs', 'subvolume', 'create', + root_mount_point + '/@']) + + if not has_home_mount_point: + subprocess.check_call(['btrfs', 'subvolume', 'create', + root_mount_point + '/@home']) + + subprocess.check_call(["umount", "-v", root_mount_point]) + + if "luksMapperName" in partition: + libcalamares.utils.mount( + "/dev/mapper/{!s}".format(partition["luksMapperName"]), + mount_point, + fstype, + ",".join( + ["subvol=@", partition.get("options", "")]), + ) + if not has_home_mount_point: + libcalamares.utils.mount( + "/dev/mapper/{!s}".format(partition["luksMapperName"]), + root_mount_point + "/home", + fstype, + ",".join( + ["subvol=@home", partition.get("options", "")]), + ) + else: + libcalamares.utils.mount( + partition["device"], + mount_point, + fstype, + ",".join(["subvol=@", partition.get("options", "")]), + ) + if not has_home_mount_point: + libcalamares.utils.mount( + partition["device"], + root_mount_point + "/home", + fstype, + ",".join( + ["subvol=@home", partition.get("options", "")]), + ) + +def mount_partitions(root_mount_point, partitions, extra_mounts=None): """ Pass back mount point and filesystem for each partition. :param root_mount_point: :param partitions: + :param extra_mounts: """ for partition in partitions: if "mountPoint" not in partition or not partition["mountPoint"]: continue - # Create mount point with `+` rather than `os.path.join()` because - # `partition["mountPoint"]` starts with a '/'. - raw_mount_point = partition["mountPoint"] - mount_point = root_mount_point + raw_mount_point - - # Ensure that the created directory has the correct SELinux context on - # SELinux-enabled systems. - os.makedirs(mount_point, exist_ok=True) - subprocess.call(['chcon', '--reference=' + raw_mount_point, - mount_point]) - - fstype = partition.get("fs", "").lower() - - if fstype == "fat16" or fstype == "fat32": - fstype = "vfat" - - if "luksMapperName" in partition: - libcalamares.utils.debug( - "about to mount {!s}".format(partition["luksMapperName"])) - libcalamares.utils.mount( - "/dev/mapper/{!s}".format(partition["luksMapperName"]), - mount_point, - fstype, - partition.get("options", ""), - ) - - else: - libcalamares.utils.mount(partition["device"], - mount_point, - fstype, - partition.get("options", ""), - ) - - # If the root partition is btrfs, we create a subvolume "@" - # for the root mount point. - # If a separate /home partition isn't defined, we also create - # a subvolume "@home". - # Finally we remount all of the above on the correct paths. - if fstype == "btrfs" and partition["mountPoint"] == '/': - has_home_mount_point = False - for p in partitions: - if "mountPoint" not in p or not p["mountPoint"]: - continue - if p["mountPoint"] == "/home": - has_home_mount_point = True - break - - subprocess.check_call(['btrfs', 'subvolume', 'create', - root_mount_point + '/@']) - - if not has_home_mount_point: - subprocess.check_call(['btrfs', 'subvolume', 'create', - root_mount_point + '/@home']) - - subprocess.check_call(["umount", "-v", root_mount_point]) - - if "luksMapperName" in partition: - libcalamares.utils.mount( - "/dev/mapper/{!s}".format(partition["luksMapperName"]), - mount_point, - fstype, - ",".join( - ["subvol=@", partition.get("options", "")]), - ) - if not has_home_mount_point: - libcalamares.utils.mount( - "/dev/mapper/{!s}".format(partition["luksMapperName"]), - root_mount_point + "/home", - fstype, - ",".join( - ["subvol=@home", partition.get("options", "")]), - ) - else: - libcalamares.utils.mount( - partition["device"], - mount_point, - fstype, - ",".join(["subvol=@", partition.get("options", "")]), - ) - if not has_home_mount_point: - libcalamares.utils.mount( - partition["device"], - root_mount_point + "/home", - fstype, - ",".join( - ["subvol=@home", partition.get("options", "")]), - ) + mount_part(root_mount_point, partition, partitions) + if partition["mountPoint"] is "/" and extra_mounts is not None: + mount_partitions(root_mount_point, extra_mounts) def run(): @@ -160,8 +165,7 @@ def run(): # Sort by mount points to ensure / is mounted before the rest partitions.sort(key=lambda x: x["mountPoint"]) - mount_partitions(root_mount_point, partitions) - mount_partitions(root_mount_point, extra_mounts) + mount_partitions(root_mount_point, partitions, extra_mounts) all_extra_mounts = extra_mounts if libcalamares.globalstorage.value("firmwareType") == "efi": From 257f5da1af78747759dc580eec27a6d1667f2304 Mon Sep 17 00:00:00 2001 From: Arnaud Ferraris Date: Wed, 14 Aug 2019 09:58:40 +0200 Subject: [PATCH 02/16] mount: Use a single partitions list sorted by mount point Instead of having a special case for extra mounts to be processed right after the rootfs, a better approach is to add them to the partitions list, and then sort the list by mount point. This way, we make sure every partition is mounted right when it is needed: `/` is obviously mounted first, `/run` is mounted before `/run/udev`, and so on. The overall process is therefore more generic and should suit all use-cases. Signed-off-by: Arnaud Ferraris --- src/modules/mount/main.py | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index b093d0dfb..1c684c43f 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -37,7 +37,7 @@ _ = gettext.translation("calamares-python", def pretty_name(): return _("Mounting partitions.") -def mount_part(root_mount_point, partition, partitions): +def mount_partition(root_mount_point, partition, partitions): # Create mount point with `+` rather than `os.path.join()` because # `partition["mountPoint"]` starts with a '/'. raw_mount_point = partition["mountPoint"] @@ -126,21 +126,6 @@ def mount_part(root_mount_point, partition, partitions): ["subvol=@home", partition.get("options", "")]), ) -def mount_partitions(root_mount_point, partitions, extra_mounts=None): - """ - Pass back mount point and filesystem for each partition. - - :param root_mount_point: - :param partitions: - :param extra_mounts: - """ - for partition in partitions: - if "mountPoint" not in partition or not partition["mountPoint"]: - continue - mount_part(root_mount_point, partition, partitions) - if partition["mountPoint"] is "/" and extra_mounts is not None: - mount_partitions(root_mount_point, extra_mounts) - def run(): """ @@ -163,16 +148,21 @@ def run(): if not extra_mounts and not extra_mounts_efi: libcalamares.utils.warning("No extra mounts defined. Does mount.conf exist?") - # Sort by mount points to ensure / is mounted before the rest - partitions.sort(key=lambda x: x["mountPoint"]) - mount_partitions(root_mount_point, partitions, extra_mounts) - - all_extra_mounts = extra_mounts if libcalamares.globalstorage.value("firmwareType") == "efi": - mount_partitions(root_mount_point, extra_mounts_efi) - all_extra_mounts.extend(extra_mounts_efi) + extra_mounts.extend(extra_mounts_efi) + + # Add extra mounts to the partitions list and sort by mount points. + # This way, we ensure / is mounted before the rest, and every mount point + # is created on the right partition (e.g. if a partition is to be mounted + # under /tmp, we make sure /tmp is mounted before the partition) + partitions.extend(extra_mounts) + partitions.sort(key=lambda x: x["mountPoint"]) + for partition in partitions: + if "mountPoint" not in partition or not partition["mountPoint"]: + continue + mount_partition(root_mount_point, partition, partitions) libcalamares.globalstorage.insert("rootMountPoint", root_mount_point) # Remember the extra mounts for the unpackfs module - libcalamares.globalstorage.insert("extraMounts", all_extra_mounts) + libcalamares.globalstorage.insert("extraMounts", extra_mounts) From 395c375c603f401e4c0ac780fb243c39b1b3c990 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 06:28:32 -0400 Subject: [PATCH 03/16] [mount] Winnow partition list - Simplify the iteration by first determining which partitions are mountable (at all). - This guards against the very rare case that a partition does not have a mountPoint at all (the if guarded against that) where the lambda passed to sort() would get a KeyError. --- src/modules/mount/main.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index 1c684c43f..d33b4681b 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -155,11 +155,9 @@ def run(): # This way, we ensure / is mounted before the rest, and every mount point # is created on the right partition (e.g. if a partition is to be mounted # under /tmp, we make sure /tmp is mounted before the partition) - partitions.extend(extra_mounts) - partitions.sort(key=lambda x: x["mountPoint"]) - for partition in partitions: - if "mountPoint" not in partition or not partition["mountPoint"]: - continue + mountable_partitions = [ p for p in partitions + extra_mounts if "mountPoint" in p and p["mountPoint"] ] + mountable_partitions.sort(key=lambda x: x["mountPoint"]) + for partition in mountable_partitions: mount_partition(root_mount_point, partition, partitions) libcalamares.globalstorage.insert("rootMountPoint", root_mount_point) From 52af9dbaad1775de5fa4b6f2c053ee7f73fc301b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 06:31:37 -0400 Subject: [PATCH 04/16] [mount] Add docstrings to methods --- src/modules/mount/main.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/modules/mount/main.py b/src/modules/mount/main.py index d33b4681b..b10c5c0bf 100644 --- a/src/modules/mount/main.py +++ b/src/modules/mount/main.py @@ -38,6 +38,12 @@ def pretty_name(): return _("Mounting partitions.") def mount_partition(root_mount_point, partition, partitions): + """ + Do a single mount of @p partition inside @p root_mount_point. + + The @p partitions are used to handle btrfs special-cases: + then subvolumes are created for root and home. + """ # Create mount point with `+` rather than `os.path.join()` because # `partition["mountPoint"]` starts with a '/'. raw_mount_point = partition["mountPoint"] @@ -129,9 +135,8 @@ def mount_partition(root_mount_point, partition, partitions): def run(): """ - Define mountpoints. - - :return: + Mount all the partitions from GlobalStorage and from the job configuration. + Partitions are mounted in-lexical-order of their mountPoint. """ partitions = libcalamares.globalstorage.value("partitions") From 5b4152133d5721a6dda6b2a6bba14074c05cf96b Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 06:45:05 -0400 Subject: [PATCH 05/16] [packagechooser] Look for AppStream libs --- src/modules/packagechooser/CMakeLists.txt | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/modules/packagechooser/CMakeLists.txt b/src/modules/packagechooser/CMakeLists.txt index 70a86a3bb..ec3c15826 100644 --- a/src/modules/packagechooser/CMakeLists.txt +++ b/src/modules/packagechooser/CMakeLists.txt @@ -9,7 +9,19 @@ if ( Qt5Xml_FOUND ) add_definitions( -DHAVE_XML ) list( APPEND _extra_libraries Qt5::Xml ) endif() - + +find_package(AppStreamQt) +set_package_properties( + AppStreamQt PROPERTIES + DESCRIPTION "Support for AppStream (cache) data" + URL "https://github.com/ximion/appstream" + PURPOSE "AppStream provides package data" + TYPE OPTIONAL +) +if ( AppStreamQt_FOUND ) + add_definitions( -DHAVE_APPSTREAM ) + list( APPEND _extra_libraries AppStreamQt ) +endif() calamares_add_plugin( packagechooser TYPE viewmodule From 2f20ad30bf067a1380177421b0221373702e3eac Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 06:55:36 -0400 Subject: [PATCH 06/16] [packagechooser] Refactor AppData XML support into separate file - Put the implementation entirely in a separate file, keep the not-supported one in PackageModel.cpp (but only in an #ifdef). - Makes the various optional-data-sources more similar. --- src/modules/packagechooser/CMakeLists.txt | 3 + src/modules/packagechooser/ItemAppData.cpp | 234 ++++++++++++++++++++ src/modules/packagechooser/PackageModel.cpp | 212 +----------------- 3 files changed, 239 insertions(+), 210 deletions(-) create mode 100644 src/modules/packagechooser/ItemAppData.cpp diff --git a/src/modules/packagechooser/CMakeLists.txt b/src/modules/packagechooser/CMakeLists.txt index ec3c15826..e93a5816f 100644 --- a/src/modules/packagechooser/CMakeLists.txt +++ b/src/modules/packagechooser/CMakeLists.txt @@ -1,5 +1,6 @@ find_package( Qt5 COMPONENTS Core Gui Widgets REQUIRED ) set( _extra_libraries "" ) +set( _extra_src "" ) ### OPTIONAL AppData XML support in PackageModel # @@ -8,6 +9,7 @@ find_package(Qt5 COMPONENTS Xml) if ( Qt5Xml_FOUND ) add_definitions( -DHAVE_XML ) list( APPEND _extra_libraries Qt5::Xml ) + list( APPEND _extra_src ItemAppData.cpp ) endif() find_package(AppStreamQt) @@ -30,6 +32,7 @@ calamares_add_plugin( packagechooser PackageChooserPage.cpp PackageChooserViewStep.cpp PackageModel.cpp + ${_extra_src} RESOURCES packagechooser.qrc UI diff --git a/src/modules/packagechooser/ItemAppData.cpp b/src/modules/packagechooser/ItemAppData.cpp new file mode 100644 index 000000000..7c80a1c3d --- /dev/null +++ b/src/modules/packagechooser/ItemAppData.cpp @@ -0,0 +1,234 @@ +/* === 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 . + */ + +/** @brief Loading items from AppData XML files. + * + * Only used if QtXML is found, implements PackageItem::fromAppData(). + */ +#include "PackageModel.h" + +#include "utils/Logger.h" +#include "utils/Variant.h" + +#include +#include +#include + +/** @brief try to load the given @p fileName XML document + * + * Returns a QDomDocument, which will be valid iff the file can + * be read and contains valid XML data. + */ +static inline QDomDocument +loadAppData( const QString& fileName ) +{ + QFile file( fileName ); + if ( !file.open( QIODevice::ReadOnly ) ) + { + return QDomDocument(); + } + QDomDocument doc( "AppData" ); + if ( !doc.setContent( &file ) ) + { + file.close(); + return QDomDocument(); + } + file.close(); + return doc; +} + +/** @brief gets the text of child element @p tagName + */ +static inline QString +getChildText( const QDomNode& n, const QString& tagName ) +{ + QDomElement e = n.firstChildElement( tagName ); + return e.isNull() ? QString() : e.text(); +} + +/** @brief Gets a suitable screenshot path + * + * The element contains zero or more + * elements, which can have a *type* associated with them. + * Scan the screenshot elements, return the path + * for the one labeled with type=default or, if there is no + * default, the first element. + */ +static inline QString +getScreenshotPath( const QDomNode& n ) +{ + QDomElement shotsNode = n.firstChildElement( "screenshots" ); + if ( shotsNode.isNull() ) + { + return QString(); + } + + const QDomNodeList shotList = shotsNode.childNodes(); + int firstScreenshot = -1; // Use which screenshot node? + for ( int i = 0; i < shotList.count(); ++i ) + { + if ( !shotList.at( i ).isElement() ) + { + continue; + } + QDomElement e = shotList.at( i ).toElement(); + if ( e.tagName() != "screenshot" ) + { + continue; + } + // If none has the "type=default" attribute, use the first one + if ( firstScreenshot < 0 ) + { + firstScreenshot = i; + } + // But type=default takes precedence. + if ( e.hasAttribute( "type" ) && e.attribute( "type" ) == "default" ) + { + firstScreenshot = i; + break; + } + } + + if ( firstScreenshot >= 0 ) + { + return shotList.at( firstScreenshot ).firstChildElement( "image" ).text(); + } + + return QString(); +} + +/** @brief Returns language of the given element @p e + * + * Transforms the attribute value for xml:lang to something + * suitable for TranslatedString (e.g. [lang]). + */ +static inline QString +getLanguage( const QDomElement& e ) +{ + QString language = e.attribute( "xml:lang" ); + if ( !language.isEmpty() ) + { + language.replace( '-', '_' ); + language.prepend( '[' ); + language.append( ']' ); + } + return language; +} + +/** @brief Scan the list of @p children for @p tagname elements and add them to the map + * + * Uses @p mapname instead of @p tagname for the entries in map @p m + * to allow renaming from XML to map keys (in particular for + * TranslatedString). Also transforms xml:lang attributes to suitable + * key-decorations on @p mapname. + */ +static inline void +fillMap( QVariantMap& m, const QDomNodeList& children, const QString& tagname, const QString& mapname ) +{ + for ( int i = 0; i < children.count(); ++i ) + { + if ( !children.at( i ).isElement() ) + { + continue; + } + + QDomElement e = children.at( i ).toElement(); + if ( e.tagName() != tagname ) + { + continue; + } + + m[ mapname + getLanguage( e ) ] = e.text(); + } +} + +/** @brief gets the and elements +* +* Builds up a map of the elements (which may have a *lang* +* attribute to indicate translations and paragraphs of the +* element (also with lang). Uses the +* elements to supplement the description if no description +* is available for a given language. +* +* Returns a map with keys suitable for use by TranslatedString. +*/ +static inline QVariantMap +getNameAndSummary( const QDomNode& n ) +{ + QVariantMap m; + + const QDomNodeList children = n.childNodes(); + fillMap( m, children, "name", "name" ); + fillMap( m, children, "summary", "description" ); + + const QDomElement description = n.firstChildElement( "description" ); + if ( !description.isNull() ) + { + fillMap( m, description.childNodes(), "p", "description" ); + } + + return m; +} + +PackageItem +PackageItem::fromAppData( const QVariantMap& item_map ) +{ + QString fileName = CalamaresUtils::getString( item_map, "appdata" ); + if ( fileName.isEmpty() ) + { + cWarning() << "Can't load AppData without a suitable key."; + return PackageItem(); + } + cDebug() << "Loading AppData XML from" << fileName; + + QDomDocument doc = loadAppData( fileName ); + if ( doc.isNull() ) + { + return PackageItem(); + } + + QDomElement componentNode = doc.documentElement(); + if ( !componentNode.isNull() && componentNode.tagName() == "component" ) + { + // An "id" entry in the Calamares config overrides ID in the AppData + QString id = CalamaresUtils::getString( item_map, "id" ); + if ( id.isEmpty() ) + { + id = getChildText( componentNode, "id" ); + } + if ( id.isEmpty() ) + { + return PackageItem(); + } + + // A "screenshot" entry in the Calamares config overrides AppData + QString screenshotPath = CalamaresUtils::getString( item_map, "screenshot" ); + if ( screenshotPath.isEmpty() ) + { + screenshotPath = getScreenshotPath( componentNode ); + } + + QVariantMap map = getNameAndSummary( componentNode ); + map.insert( "id", id ); + map.insert( "screenshot", screenshotPath ); + + return PackageItem( map ); + } + + return PackageItem(); +} diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index 59c6973ba..e15559552 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -21,12 +21,6 @@ #include "utils/Logger.h" #include "utils/Variant.h" -#ifdef HAVE_XML -#include -#include -#include -#endif - const NamedEnumTable< PackageChooserMode >& roleNames() { @@ -94,217 +88,15 @@ PackageItem::PackageItem::PackageItem( const QVariantMap& item_map ) } } -#ifdef HAVE_XML -/** @brief try to load the given @p fileName XML document - * - * Returns a QDomDocument, which will be valid iff the file can - * be read and contains valid XML data. - */ -static inline QDomDocument -loadAppData( const QString& fileName ) -{ - QFile file( fileName ); - if ( !file.open( QIODevice::ReadOnly ) ) - { - return QDomDocument(); - } - QDomDocument doc( "AppData" ); - if ( !doc.setContent( &file ) ) - { - file.close(); - return QDomDocument(); - } - file.close(); - return doc; -} - -/** @brief gets the text of child element @p tagName - */ -static inline QString -getChildText( const QDomNode& n, const QString& tagName ) -{ - QDomElement e = n.firstChildElement( tagName ); - return e.isNull() ? QString() : e.text(); -} - -/** @brief Gets a suitable screenshot path - * - * The element contains zero or more - * elements, which can have a *type* associated with them. - * Scan the screenshot elements, return the path - * for the one labeled with type=default or, if there is no - * default, the first element. - */ -static inline QString -getScreenshotPath( const QDomNode& n ) -{ - QDomElement shotsNode = n.firstChildElement( "screenshots" ); - if ( shotsNode.isNull() ) - { - return QString(); - } - - const QDomNodeList shotList = shotsNode.childNodes(); - int firstScreenshot = -1; // Use which screenshot node? - for ( int i = 0; i < shotList.count(); ++i ) - { - if ( !shotList.at( i ).isElement() ) - { - continue; - } - QDomElement e = shotList.at( i ).toElement(); - if ( e.tagName() != "screenshot" ) - { - continue; - } - // If none has the "type=default" attribute, use the first one - if ( firstScreenshot < 0 ) - { - firstScreenshot = i; - } - // But type=default takes precedence. - if ( e.hasAttribute( "type" ) && e.attribute( "type" ) == "default" ) - { - firstScreenshot = i; - break; - } - } - - if ( firstScreenshot >= 0 ) - { - return shotList.at( firstScreenshot ).firstChildElement( "image" ).text(); - } - - return QString(); -} - -/** @brief Returns language of the given element @p e - * - * Transforms the attribute value for xml:lang to something - * suitable for TranslatedString (e.g. [lang]). - */ -static inline QString -getLanguage( const QDomElement& e ) -{ - QString language = e.attribute( "xml:lang" ); - if ( !language.isEmpty() ) - { - language.replace( '-', '_' ); - language.prepend( '[' ); - language.append( ']' ); - } - return language; -} - -/** @brief Scan the list of @p children for @p tagname elements and add them to the map - * - * Uses @p mapname instead of @p tagname for the entries in map @p m - * to allow renaming from XML to map keys (in particular for - * TranslatedString). Also transforms xml:lang attributes to suitable - * key-decorations on @p mapname. - */ -static inline void -fillMap( QVariantMap& m, const QDomNodeList& children, const QString& tagname, const QString& mapname ) -{ - for ( int i = 0; i < children.count(); ++i ) - { - if ( !children.at( i ).isElement() ) - { - continue; - } - - QDomElement e = children.at( i ).toElement(); - if ( e.tagName() != tagname ) - { - continue; - } - - m[ mapname + getLanguage( e ) ] = e.text(); - } -} - -/** @brief gets the and elements -* -* Builds up a map of the elements (which may have a *lang* -* attribute to indicate translations and paragraphs of the -* element (also with lang). Uses the -* elements to supplement the description if no description -* is available for a given language. -* -* Returns a map with keys suitable for use by TranslatedString. -*/ -static inline QVariantMap -getNameAndSummary( const QDomNode& n ) -{ - QVariantMap m; - - const QDomNodeList children = n.childNodes(); - fillMap( m, children, "name", "name" ); - fillMap( m, children, "summary", "description" ); - - const QDomElement description = n.firstChildElement( "description" ); - if ( !description.isNull() ) - { - fillMap( m, description.childNodes(), "p", "description" ); - } - - return m; -} -#endif +#ifndef HAVE_XML PackageItem PackageItem::fromAppData( const QVariantMap& item_map ) { -#ifdef HAVE_XML - QString fileName = CalamaresUtils::getString( item_map, "appdata" ); - if ( fileName.isEmpty() ) - { - cWarning() << "Can't load AppData without a suitable key."; - return PackageItem(); - } - cDebug() << "Loading AppData XML from" << fileName; - - QDomDocument doc = loadAppData( fileName ); - if ( doc.isNull() ) - { - return PackageItem(); - } - - QDomElement componentNode = doc.documentElement(); - if ( !componentNode.isNull() && componentNode.tagName() == "component" ) - { - // An "id" entry in the Calamares config overrides ID in the AppData - QString id = CalamaresUtils::getString( item_map, "id" ); - if ( id.isEmpty() ) - { - id = getChildText( componentNode, "id" ); - } - if ( id.isEmpty() ) - { - return PackageItem(); - } - - // A "screenshot" entry in the Calamares config overrides AppData - QString screenshotPath = CalamaresUtils::getString( item_map, "screenshot" ); - if ( screenshotPath.isEmpty() ) - { - screenshotPath = getScreenshotPath( componentNode ); - } - - QVariantMap map = getNameAndSummary( componentNode ); - map.insert( "id", id ); - map.insert( "screenshot", screenshotPath ); - - return PackageItem( map ); - } - - return PackageItem(); -#else cWarning() << "Loading AppData XML is not supported."; - return PackageItem(); -#endif } +#endif PackageListModel::PackageListModel( QObject* parent ) From eaa0c02f8db46a18d1b7928ff00296d0a24d4417 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 07:10:38 -0400 Subject: [PATCH 07/16] [packagechooser] Initial support for appstream items - Use *appstream* as key in one of the items for the package- chooser to load data from the AppStream cache in the system. - Usable for some applications; for DE-selection not so much. - Currently unimplemented. --- src/modules/packagechooser/CMakeLists.txt | 1 + src/modules/packagechooser/ItemAppStream.cpp | 33 +++++++++++++++++++ .../packagechooser/PackageChooserViewStep.cpp | 4 +++ src/modules/packagechooser/PackageModel.cpp | 9 +++++ src/modules/packagechooser/PackageModel.h | 14 ++++++++ .../packagechooser/packagechooser.conf | 18 +++++++--- 6 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 src/modules/packagechooser/ItemAppStream.cpp diff --git a/src/modules/packagechooser/CMakeLists.txt b/src/modules/packagechooser/CMakeLists.txt index e93a5816f..eb84c9bab 100644 --- a/src/modules/packagechooser/CMakeLists.txt +++ b/src/modules/packagechooser/CMakeLists.txt @@ -23,6 +23,7 @@ set_package_properties( if ( AppStreamQt_FOUND ) add_definitions( -DHAVE_APPSTREAM ) list( APPEND _extra_libraries AppStreamQt ) + list( APPEND _extra_src ItemAppStream.cpp ) endif() calamares_add_plugin( packagechooser diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp new file mode 100644 index 000000000..287e5a139 --- /dev/null +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -0,0 +1,33 @@ +/* === 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 . + */ + +/** @brief Loading items from AppData XML files. + * + * Only used if QtXML is found, implements PackageItem::fromAppData(). + */ +#include "PackageModel.h" + +#include "utils/Logger.h" +#include "utils/Variant.h" + +PackageItem +PackageItem::fromAppStream( const QVariantMap& map ) +{ + cWarning() << "AppStream loading not implemented."; + return PackageItem(); +} diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index c2e849d5f..412658394 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -219,6 +219,10 @@ PackageChooserViewStep::fillModel( const QVariantList& items ) { m_model->addPackage( PackageItem::fromAppData( item_map ) ); } + else if ( item_map.contains( "appstream" ) ) + { + m_model->addPackage( PackageItem::fromAppStream( item_map ) ); + } else { m_model->addPackage( PackageItem( item_map ) ); diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index e15559552..bc80109f3 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -98,6 +98,15 @@ PackageItem::fromAppData( const QVariantMap& item_map ) } #endif +#ifndef HAVE_APPSTREAM +PackageItem +PackageItem::fromAppStream( const QVariantMap& item_map ) +{ + cWarning() << "Loading AppStream data is not supported."; + return PackageItem(); +} +#endif + PackageListModel::PackageListModel( QObject* parent ) : QAbstractListModel( parent ) diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index 869e124f0..49b4bc0dd 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -93,6 +93,20 @@ struct PackageItem * return invalid PackageItems. */ static PackageItem fromAppData( const QVariantMap& map ); + + /** @brief Loads an item from AppStream data. + * + * The @p map must have a key *appstream*. That is used as the + * primary source of information from the AppStream cache, but + * keys *id* and *screenshotPath* may be used to override parts + * of the AppStream data -- so that the ID is under the control + * of Calamares, and the screenshot can be forced to a local path + * available on the installation medium. + * + * Requires AppStreamQt, if not present will return invalid + * PackageItems. + */ + static PackageItem fromAppStream( const QVariantMap& map ); }; using PackageList = QVector< PackageItem >; diff --git a/src/modules/packagechooser/packagechooser.conf b/src/modules/packagechooser/packagechooser.conf index e9b2d9329..eebe4dfb4 100644 --- a/src/modules/packagechooser/packagechooser.conf +++ b/src/modules/packagechooser/packagechooser.conf @@ -20,14 +20,15 @@ # or one-or-more). mode: required -# Items to display in the chooser. In general, this should be a +# Items to display in the chooser. In general, this should be a # pretty short list to avoid overwhelming the UI. This is a list # of objects, and the items are displayed in list order. # # Either provide the data for an item in the list (using the keys -# below), or use existing AppData XML files as a source for the data. +# below), or use existing AppData XML files, or use AppStream cache +# as a source for the data. # -# For data provided by the list: the item has an id, which is used in +# For data provided by the list: the item has an id, which is used in # setting the value of *packagechooser_*). The following fields # are mandatory: # @@ -60,6 +61,14 @@ mode: required # be loaded and the screenshot will be missing. An item with *appdata* # **may** specify an ID or screenshot path, as above. This will override # the settings from AppData. +# +# For data provided by AppStream cache: the item has an *appstream* +# key which matches the AppStream identifier in the cache (e.g. +# *org.kde.kwrite.desktop*). Data is retrieved from the AppStream +# cache for that ID. The package name is set from the AppStream data. +# +# An item for AppStream may also contain an *id* and a *screenshot* +# key which will override the data from AppStream. items: - id: "" package: "" @@ -81,4 +90,5 @@ items: - id: calamares appdata: ../io.calamares.calamares.appdata.xml screenshot: ":/images/calamares.png" - + - id: kate + appstream: org.kde.kwrite.desktop From fa2f5763c64e486eac428d91fe9683e1571655c7 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 07:25:28 -0400 Subject: [PATCH 08/16] [packagechooser] Load AppStream data - Get the id, name, and description from AppStream data Missing: - No translations - No screenshots --- src/modules/packagechooser/ItemAppStream.cpp | 62 ++++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp index 287e5a139..769973575 100644 --- a/src/modules/packagechooser/ItemAppStream.cpp +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -25,9 +25,63 @@ #include "utils/Logger.h" #include "utils/Variant.h" -PackageItem -PackageItem::fromAppStream( const QVariantMap& map ) +#include + +static PackageItem +fromComponent( const AppStream::Component& component ) { - cWarning() << "AppStream loading not implemented."; - return PackageItem(); + cDebug() << "Loaded" << component.id(); + + QVariantMap map; + map.insert( "id", component.id() ); + map.insert( "name", component.name() ); + map.insert( "description", component.description() ); + // map.insert( "screenshot", component.screenshots() ); + return PackageItem( map ); +} + +PackageItem +PackageItem::fromAppStream( const QVariantMap& item_map ) +{ + QString appstreamId = CalamaresUtils::getString( item_map, "appstream" ); + if ( appstreamId.isEmpty() ) + { + cWarning() << "Can't load AppStream without a suitable appstreamId."; + return PackageItem(); + } + cDebug() << "Loading AppStream data for" << appstreamId; + + AppStream::Pool pool; + if ( !pool.load() ) + { + cWarning() << "AppStream load failed" << pool.lastError(); + return PackageItem(); + } + + auto itemList = pool.componentsById( appstreamId ); + if ( itemList.count() < 1 ) + { + cWarning() << "No AppStream data for" << appstreamId; + return PackageItem(); + } + if ( itemList.count() > 1 ) + { + cDebug() << "Multiple AppStream data for" << appstreamId << "using first."; + } + + auto r = fromComponent( itemList.first() ); + if ( r.isValid() ) + { + QString id = CalamaresUtils::getString( item_map, "id" ); + QString screenshotPath = CalamaresUtils::getString( item_map, "screenshot" ); + if ( !id.isEmpty() ) + { + r.id = id; + } + if ( !screenshotPath.isEmpty() ) + { + r.screenshot = screenshotPath; + } + } + return r; } From 17abbeda96737b006bd286813fb3b0c103a28d7f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Mon, 19 Aug 2019 10:02:43 -0400 Subject: [PATCH 09/16] [packagechooser] Try to load a screenshot - The smallest size image of the default (or, if there is no default, the first) screenshot is used. - Remote URLs are not supported by QPixmap, so most will not load anyway. --- src/modules/packagechooser/ItemAppStream.cpp | 63 ++++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp index 769973575..392797846 100644 --- a/src/modules/packagechooser/ItemAppStream.cpp +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -22,21 +22,76 @@ */ #include "PackageModel.h" +#include "locale/LabelModel.h" #include "utils/Logger.h" #include "utils/Variant.h" +#include #include +#include -static PackageItem -fromComponent( const AppStream::Component& component ) +/// @brief Return number of pixels in a size, for < ordering purposes +static inline quint64 +sizeOrder( const QSize& size ) { - cDebug() << "Loaded" << component.id(); + return size.width() * size.height(); +} +/// @brief Sets a screenshot in @p map from @p screenshot, if a usable one is found +static void +setScreenshot( QVariantMap& map, const AppStream::Screenshot& screenshot ) +{ + if ( screenshot.images().count() < 1 ) + { + return; + } + + // Pick the smallest + QUrl url; + quint64 size = sizeOrder( screenshot.images().first().size() ); + for ( const auto& img : screenshot.images() ) + { + if ( sizeOrder( img.size() ) <= size ) + { + url = img.url(); + } + } + + if ( url.isValid() ) + { + map.insert( "screenshot", url.toString() ); + } +} + +/// @brief Interpret an AppStream Component +static PackageItem +fromComponent( AppStream::Component& component ) +{ QVariantMap map; map.insert( "id", component.id() ); map.insert( "name", component.name() ); map.insert( "description", component.description() ); - // map.insert( "screenshot", component.screenshots() ); + map.insert( "package", component.packageNames().join(",") ); + + auto screenshots = component.screenshots(); + if ( screenshots.count() > 0 ) + { + bool done = false; + for ( const auto& s : screenshots ) + { + if ( s.isDefault() ) + { + setScreenshot( map, s ); + done = true; + break; + } + } + if ( !done ) + { + setScreenshot( map, screenshots.first() ); + } + } + return PackageItem( map ); } From 0a92ef7655f6c897ada15c75ad0b2470bdd40ce1 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 04:15:35 -0400 Subject: [PATCH 10/16] [packagechooser] Refactor fromApp*() - These don't have to be static methods of PackageItem, a free function is more convenient. - Since it's not API of PackageItem anymore, need to - update tests not to use API - do API-not-available warnings in consumers --- src/modules/packagechooser/ItemAppData.cpp | 2 +- src/modules/packagechooser/ItemAppData.h | 37 ++++++++++++++++++ src/modules/packagechooser/ItemAppStream.cpp | 4 +- src/modules/packagechooser/ItemAppStream.h | 38 +++++++++++++++++++ .../packagechooser/PackageChooserViewStep.cpp | 18 ++++++++- src/modules/packagechooser/PackageModel.cpp | 20 ---------- src/modules/packagechooser/PackageModel.h | 27 ------------- src/modules/packagechooser/Tests.cpp | 12 ++++-- 8 files changed, 102 insertions(+), 56 deletions(-) create mode 100644 src/modules/packagechooser/ItemAppData.h create mode 100644 src/modules/packagechooser/ItemAppStream.h diff --git a/src/modules/packagechooser/ItemAppData.cpp b/src/modules/packagechooser/ItemAppData.cpp index 7c80a1c3d..ed0ba9223 100644 --- a/src/modules/packagechooser/ItemAppData.cpp +++ b/src/modules/packagechooser/ItemAppData.cpp @@ -186,7 +186,7 @@ getNameAndSummary( const QDomNode& n ) } PackageItem -PackageItem::fromAppData( const QVariantMap& item_map ) +fromAppData( const QVariantMap& item_map ) { QString fileName = CalamaresUtils::getString( item_map, "appdata" ); if ( fileName.isEmpty() ) diff --git a/src/modules/packagechooser/ItemAppData.h b/src/modules/packagechooser/ItemAppData.h new file mode 100644 index 000000000..72617ff0c --- /dev/null +++ b/src/modules/packagechooser/ItemAppData.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 ITEMAPPDATA_H +#define ITEMAPPDATA_H + +#include "PackageModel.h" + +/** @brief Loads an AppData XML file and returns a PackageItem + * + * The @p map must have a key *appdata*. That is used as the + * primary source of information, but keys *id* and *screenshotPath* + * may be used to override parts of the AppData -- so that the + * ID is under the control of Calamares, and the screenshot can be + * forced to a local path available on the installation medium. + * + * Requires XML support in libcalamares, if not present will + * return invalid PackageItems. + */ +PackageItem fromAppData( const QVariantMap& map ); + +#endif diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp index 392797846..3ee49f7d0 100644 --- a/src/modules/packagechooser/ItemAppStream.cpp +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -71,7 +71,7 @@ fromComponent( AppStream::Component& component ) map.insert( "id", component.id() ); map.insert( "name", component.name() ); map.insert( "description", component.description() ); - map.insert( "package", component.packageNames().join(",") ); + map.insert( "package", component.packageNames().join( "," ) ); auto screenshots = component.screenshots(); if ( screenshots.count() > 0 ) @@ -96,7 +96,7 @@ fromComponent( AppStream::Component& component ) } PackageItem -PackageItem::fromAppStream( const QVariantMap& item_map ) +fromAppStream( const QVariantMap& item_map ) { QString appstreamId = CalamaresUtils::getString( item_map, "appstream" ); if ( appstreamId.isEmpty() ) diff --git a/src/modules/packagechooser/ItemAppStream.h b/src/modules/packagechooser/ItemAppStream.h new file mode 100644 index 000000000..7d820400f --- /dev/null +++ b/src/modules/packagechooser/ItemAppStream.h @@ -0,0 +1,38 @@ +/* === 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 ITEMAPPSTREAM_H +#define ITEMAPPSTREAM_H + +#include "PackageModel.h" + +/** @brief Loads an item from AppStream data. + * + * The @p map must have a key *appstream*. That is used as the + * primary source of information from the AppStream cache, but + * keys *id* and *screenshotPath* may be used to override parts + * of the AppStream data -- so that the ID is under the control + * of Calamares, and the screenshot can be forced to a local path + * available on the installation medium. + * + * Requires AppStreamQt, if not present will return invalid + * PackageItems. + */ +PackageItem fromAppStream( const QVariantMap& map ); + +#endif diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index 412658394..bf45c6726 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -18,6 +18,12 @@ #include "PackageChooserViewStep.h" +#ifdef HAVE_XML +#include "ItemAppData.h" +#endif +#ifdef HAVE_APPSTREAM +#include "ItemAppStream.h" +#endif #include "PackageChooserPage.h" #include "PackageModel.h" @@ -217,11 +223,19 @@ PackageChooserViewStep::fillModel( const QVariantList& items ) if ( item_map.contains( "appdata" ) ) { - m_model->addPackage( PackageItem::fromAppData( item_map ) ); +#ifdef HAVE_XML + m_model->addPackage( fromAppData( item_map ) ); +#else + cWarning() << "Loading AppData XML is not supported."; +#endif } else if ( item_map.contains( "appstream" ) ) { - m_model->addPackage( PackageItem::fromAppStream( item_map ) ); +#ifdef HAVE_APPSTREAM + m_model->addPackage( fromAppStream( item_map ) ); +#else + cWarning() << "Loading AppStream data is not supported."; +#endif } else { diff --git a/src/modules/packagechooser/PackageModel.cpp b/src/modules/packagechooser/PackageModel.cpp index bc80109f3..12995fad5 100644 --- a/src/modules/packagechooser/PackageModel.cpp +++ b/src/modules/packagechooser/PackageModel.cpp @@ -88,26 +88,6 @@ PackageItem::PackageItem::PackageItem( const QVariantMap& item_map ) } } - -#ifndef HAVE_XML -PackageItem -PackageItem::fromAppData( const QVariantMap& item_map ) -{ - cWarning() << "Loading AppData XML is not supported."; - return PackageItem(); -} -#endif - -#ifndef HAVE_APPSTREAM -PackageItem -PackageItem::fromAppStream( const QVariantMap& item_map ) -{ - cWarning() << "Loading AppStream data is not supported."; - return PackageItem(); -} -#endif - - PackageListModel::PackageListModel( QObject* parent ) : QAbstractListModel( parent ) { diff --git a/src/modules/packagechooser/PackageModel.h b/src/modules/packagechooser/PackageModel.h index 49b4bc0dd..8362bee33 100644 --- a/src/modules/packagechooser/PackageModel.h +++ b/src/modules/packagechooser/PackageModel.h @@ -80,33 +80,6 @@ struct PackageItem * A valid item has an untranslated name available. */ bool isValid() const { return !name.isEmpty(); } - - /** @brief Loads an AppData XML file and returns a PackageItem - * - * The @p map must have a key *appdata*. That is used as the - * primary source of information, but keys *id* and *screenshotPath* - * may be used to override parts of the AppData -- so that the - * ID is under the control of Calamares, and the screenshot can be - * forced to a local path available on the installation medium. - * - * Requires XML support in libcalamares, if not present will - * return invalid PackageItems. - */ - static PackageItem fromAppData( const QVariantMap& map ); - - /** @brief Loads an item from AppStream data. - * - * The @p map must have a key *appstream*. That is used as the - * primary source of information from the AppStream cache, but - * keys *id* and *screenshotPath* may be used to override parts - * of the AppStream data -- so that the ID is under the control - * of Calamares, and the screenshot can be forced to a local path - * available on the installation medium. - * - * Requires AppStreamQt, if not present will return invalid - * PackageItems. - */ - static PackageItem fromAppStream( const QVariantMap& map ); }; using PackageList = QVector< PackageItem >; diff --git a/src/modules/packagechooser/Tests.cpp b/src/modules/packagechooser/Tests.cpp index 537ecbd3c..639d06d65 100644 --- a/src/modules/packagechooser/Tests.cpp +++ b/src/modules/packagechooser/Tests.cpp @@ -18,6 +18,12 @@ #include "Tests.h" +#ifdef HAVE_XML +#include "ItemAppData.h" +#endif +#ifdef HAVE_APPSTREAM +#include "ItemAppStream.h" +#endif #include "PackageModel.h" #include "utils/Logger.h" @@ -62,8 +68,8 @@ PackageChooserTests::testAppData() QVariantMap m; m.insert( "appdata", appdataName ); - PackageItem p1 = PackageItem::fromAppData( m ); #ifdef HAVE_XML + PackageItem p1 = fromAppData( m ); QVERIFY( p1.isValid() ); QCOMPARE( p1.id, "io.calamares.calamares.desktop" ); QCOMPARE( p1.name.get(), "Calamares" ); @@ -76,12 +82,10 @@ PackageChooserTests::testAppData() m.insert( "id", "calamares" ); m.insert( "screenshot", ":/images/calamares.png" ); - PackageItem p2 = PackageItem::fromAppData( m ); + PackageItem p2 = fromAppData( m ); QVERIFY( p2.isValid() ); QCOMPARE( p2.id, "calamares" ); QCOMPARE( p2.description.get( QLocale( "nl" ) ), "Calamares is een installatieprogramma voor Linux distributies." ); QVERIFY( !p2.screenshot.isNull() ); -#else - QVERIFY( !p1.isValid() ); #endif } From d8af11adeee88b41ff210ed9e06ffcfd890cd219 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 04:38:24 -0400 Subject: [PATCH 11/16] [packagechooser] Build AppStream Pool first - Don't build a Pool for each PackageItem loaded - Do make it load all languages instead of only the current one --- src/modules/packagechooser/ItemAppStream.cpp | 9 +-------- src/modules/packagechooser/ItemAppStream.h | 7 ++++++- .../packagechooser/PackageChooserViewStep.cpp | 18 +++++++++++++++++- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp index 3ee49f7d0..fe4af95b4 100644 --- a/src/modules/packagechooser/ItemAppStream.cpp +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -96,7 +96,7 @@ fromComponent( AppStream::Component& component ) } PackageItem -fromAppStream( const QVariantMap& item_map ) +fromAppStream( AppStream::Pool& pool, const QVariantMap& item_map ) { QString appstreamId = CalamaresUtils::getString( item_map, "appstream" ); if ( appstreamId.isEmpty() ) @@ -106,13 +106,6 @@ fromAppStream( const QVariantMap& item_map ) } cDebug() << "Loading AppStream data for" << appstreamId; - AppStream::Pool pool; - if ( !pool.load() ) - { - cWarning() << "AppStream load failed" << pool.lastError(); - return PackageItem(); - } - auto itemList = pool.componentsById( appstreamId ); if ( itemList.count() < 1 ) { diff --git a/src/modules/packagechooser/ItemAppStream.h b/src/modules/packagechooser/ItemAppStream.h index 7d820400f..c44b84b06 100644 --- a/src/modules/packagechooser/ItemAppStream.h +++ b/src/modules/packagechooser/ItemAppStream.h @@ -21,6 +21,11 @@ #include "PackageModel.h" +namespace AppStream +{ +class Pool; +} + /** @brief Loads an item from AppStream data. * * The @p map must have a key *appstream*. That is used as the @@ -33,6 +38,6 @@ * Requires AppStreamQt, if not present will return invalid * PackageItems. */ -PackageItem fromAppStream( const QVariantMap& map ); +PackageItem fromAppStream( AppStream::Pool& pool, const QVariantMap& map ); #endif diff --git a/src/modules/packagechooser/PackageChooserViewStep.cpp b/src/modules/packagechooser/PackageChooserViewStep.cpp index bf45c6726..7758986ba 100644 --- a/src/modules/packagechooser/PackageChooserViewStep.cpp +++ b/src/modules/packagechooser/PackageChooserViewStep.cpp @@ -23,6 +23,8 @@ #endif #ifdef HAVE_APPSTREAM #include "ItemAppStream.h" +#include +#include #endif #include "PackageChooserPage.h" #include "PackageModel.h" @@ -209,6 +211,11 @@ PackageChooserViewStep::fillModel( const QVariantList& items ) return; } +#ifdef HAVE_APPSTREAM + std::unique_ptr< AppStream::Pool > pool; + bool poolOk = false; +#endif + cDebug() << "Loading PackageChooser model items from config"; int item_index = 0; for ( const auto& item_it : items ) @@ -232,7 +239,16 @@ PackageChooserViewStep::fillModel( const QVariantList& items ) else if ( item_map.contains( "appstream" ) ) { #ifdef HAVE_APPSTREAM - m_model->addPackage( fromAppStream( item_map ) ); + if ( !pool ) + { + pool = std::make_unique< AppStream::Pool >(); + pool->setLocale( QStringLiteral( "ALL" ) ); + poolOk = pool->load(); + } + if ( pool && poolOk ) + { + m_model->addPackage( fromAppStream( *pool, item_map ) ); + } #else cWarning() << "Loading AppStream data is not supported."; #endif From 7b699bfc762353cfd22da93ed3cfe2c09d472a1f Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 05:26:26 -0400 Subject: [PATCH 12/16] [libcalamares] Access list of locale Ids - Make it easier to obtain locale-ids (from CALAMARES_TRANSLATION_LANGUAGES) so avoid splitting that string multiple times. --- src/libcalamares/locale/LabelModel.cpp | 3 ++- src/libcalamares/locale/LabelModel.h | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libcalamares/locale/LabelModel.cpp b/src/libcalamares/locale/LabelModel.cpp index 87fca9d85..bcb8af057 100644 --- a/src/libcalamares/locale/LabelModel.cpp +++ b/src/libcalamares/locale/LabelModel.cpp @@ -29,6 +29,7 @@ namespace Locale LabelModel::LabelModel( const QStringList& locales, QObject* parent ) : QAbstractListModel( parent ) + , m_localeIds( locales ) { Q_ASSERT( locales.count() > 0 ); m_locales.reserve( locales.count() ); @@ -132,7 +133,7 @@ LabelModel::find( const QString& countryCode ) const LabelModel* availableTranslations() { - static LabelModel* model = new LabelModel( QString( CALAMARES_TRANSLATION_LANGUAGES ).split( ';' ) ); + static LabelModel* model = new LabelModel( QStringLiteral( CALAMARES_TRANSLATION_LANGUAGES ).split( ';' ) ); return model; } diff --git a/src/libcalamares/locale/LabelModel.h b/src/libcalamares/locale/LabelModel.h index 5082440d3..03daddbf3 100644 --- a/src/libcalamares/locale/LabelModel.h +++ b/src/libcalamares/locale/LabelModel.h @@ -54,6 +54,9 @@ public: */ const Label& locale( int row ) const; + /// @brief Returns all of the locale Ids (e.g. en_US) put into this model. + const QStringList& localeIds() const { return m_localeIds; } + /** @brief Searches for an item that matches @p predicate * * Returns the row number of the first match, or -1 if there isn't one. @@ -67,6 +70,7 @@ public: private: QVector< Label > m_locales; + QStringList m_localeIds; }; /** @brief Returns a model with all available translations. From ffa899b4977cde0d4525f9dad1f16a41c2572e67 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 05:37:52 -0400 Subject: [PATCH 13/16] [packagechooser] Assemble the translated name and description --- src/modules/packagechooser/ItemAppStream.cpp | 28 ++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/modules/packagechooser/ItemAppStream.cpp b/src/modules/packagechooser/ItemAppStream.cpp index fe4af95b4..83837a9ca 100644 --- a/src/modules/packagechooser/ItemAppStream.cpp +++ b/src/modules/packagechooser/ItemAppStream.cpp @@ -69,10 +69,34 @@ fromComponent( AppStream::Component& component ) { QVariantMap map; map.insert( "id", component.id() ); - map.insert( "name", component.name() ); - map.insert( "description", component.description() ); map.insert( "package", component.packageNames().join( "," ) ); + // Assume that the pool has loaded "ALL" locales, but it might be set + // to any of them; get the en_US locale as "untranslated" and then + // loop over Calamares locales (since there is no way to query for + // available locales in the Component) to see if there's anything else. + component.setActiveLocale( QStringLiteral( "en_US" ) ); + QString en_name = component.name(); + QString en_description = component.description(); + map.insert( "name", en_name ); + map.insert( "description", en_description ); + + for ( const QString& locale : CalamaresUtils::Locale::availableTranslations()->localeIds() ) + { + component.setActiveLocale( locale ); + QString name = component.name(); + if ( name != en_name ) + { + map.insert( QStringLiteral( "name[%1]" ).arg( locale ), name ); + } + QString description = component.description(); + if ( description != en_description ) + { + map.insert( QStringLiteral( "description[%1]" ).arg( locale ), description ); + } + } + + auto screenshots = component.screenshots(); if ( screenshots.count() > 0 ) { From 8c5caf9fd05278b1a0a24424808efb288c4fdc42 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 13:56:23 +0200 Subject: [PATCH 14/16] [packagechooser] Add CMake knobs to enable/disable item choices - AppData and AppStream can be disabled independently of finding their requirements (possibly useful if you want to ignore AppStream even when it's installed in your build environment). - Add a little top-level documentation about WITH_ --- CMakeLists.txt | 3 ++ src/modules/packagechooser/CMakeLists.txt | 43 ++++++++++++++--------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5461aa3c8..321989ac0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,6 +25,9 @@ # SKIP_MODULES : a space or semicolon-separated list of directory names # under src/modules that should not be built. # USE_ : fills in SKIP_MODULES for modules called - +# WITH_ : try to enable (these usually default to ON). For +# a list of WITH_ grep CMakeCache.txt after running +# CMake once. # BUILD_ : choose additional things to build # DEBUG_ : special developer flags for debugging # diff --git a/src/modules/packagechooser/CMakeLists.txt b/src/modules/packagechooser/CMakeLists.txt index eb84c9bab..eeae655c9 100644 --- a/src/modules/packagechooser/CMakeLists.txt +++ b/src/modules/packagechooser/CMakeLists.txt @@ -5,25 +5,34 @@ set( _extra_src "" ) ### OPTIONAL AppData XML support in PackageModel # # -find_package(Qt5 COMPONENTS Xml) -if ( Qt5Xml_FOUND ) - add_definitions( -DHAVE_XML ) - list( APPEND _extra_libraries Qt5::Xml ) - list( APPEND _extra_src ItemAppData.cpp ) +option( WITH_APPDATA "Support appdata: items in PackageChooser (requires QtXml)" ON ) +if ( WITH_APPDATA ) + find_package(Qt5 COMPONENTS Xml) + if ( Qt5Xml_FOUND ) + add_definitions( -DHAVE_XML ) + list( APPEND _extra_libraries Qt5::Xml ) + list( APPEND _extra_src ItemAppData.cpp ) + endif() endif() -find_package(AppStreamQt) -set_package_properties( - AppStreamQt PROPERTIES - DESCRIPTION "Support for AppStream (cache) data" - URL "https://github.com/ximion/appstream" - PURPOSE "AppStream provides package data" - TYPE OPTIONAL -) -if ( AppStreamQt_FOUND ) - add_definitions( -DHAVE_APPSTREAM ) - list( APPEND _extra_libraries AppStreamQt ) - list( APPEND _extra_src ItemAppStream.cpp ) +### OPTIONAL AppStream support in PackageModel +# +# +option( WITH_APPSTREAM "Support appstream: items in PackageChooser (requires libappstream-qt)" ON ) +if ( WITH_APPSTREAM ) + find_package(AppStreamQt) + set_package_properties( + AppStreamQt PROPERTIES + DESCRIPTION "Support for AppStream (cache) data" + URL "https://github.com/ximion/appstream" + PURPOSE "AppStream provides package data" + TYPE OPTIONAL + ) + if ( AppStreamQt_FOUND ) + add_definitions( -DHAVE_APPSTREAM ) + list( APPEND _extra_libraries AppStreamQt ) + list( APPEND _extra_src ItemAppStream.cpp ) + endif() endif() calamares_add_plugin( packagechooser From 4b35d193b743f83c6d5b6a9f01074a4f7c90e010 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 14:03:52 +0200 Subject: [PATCH 15/16] Changes: mention #1212, AppStream data loading --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index e926da175..66323ff9f 100644 --- a/CHANGES +++ b/CHANGES @@ -23,6 +23,9 @@ This release contains contributions from (alphabetically by first name): ## Modules ## +- The *packagechooser* module can load data from the config-file, + from AppData XML files referred by the config-file, and (new) also + from AppStream caches by referring to an application's AppStream id. #1212 - The *partition* module now understands the units *KB*, *MB*, *GB* which are powers-of-ten sizes, alongside the powers-of-two sizes that it already used. (thanks to Arnaud) From 82622373bc8778e5b4f4563723871b35b5d7e1a8 Mon Sep 17 00:00:00 2001 From: Adriaan de Groot Date: Tue, 20 Aug 2019 16:05:22 +0200 Subject: [PATCH 16/16] [libcalamares] Remove superfluous ; (warnings--) --- src/libcalamares/utils/PluginFactory.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcalamares/utils/PluginFactory.h b/src/libcalamares/utils/PluginFactory.h index 84694a83a..e8236a888 100644 --- a/src/libcalamares/utils/PluginFactory.h +++ b/src/libcalamares/utils/PluginFactory.h @@ -58,8 +58,8 @@ public: template < class impl, class ParentType > static QObject* createInstance( QWidget* parentWidget, QObject* parent, const QVariantList& args ) { - Q_UNUSED( parentWidget ); - Q_UNUSED( args ); + Q_UNUSED( parentWidget ) + Q_UNUSED( args ) ParentType* p = nullptr; if ( parent ) {