Bug 418466

Summary: Failures installing and uninstalling content using new QtQuick dialog
Product: [Frameworks and Libraries] frameworks-knewstuff Reporter: hardcodecoder
Component: generalAssignee: Marco Martin <notmart>
Status: RESOLVED FIXED    
Severity: normal CC: admin, bugseforuns, kde, kdelibs-bugs, nate, notmart, pvdvegte, viru.rks
Priority: VHI Keywords: regression
Version: 5.69.0   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In: 5.71
Sentry Crash Report:
Attachments: Theme installed yet not showing up in global themes section

Description hardcodecoder 2020-03-04 13:30:33 UTC
Created attachment 126593 [details]
Theme installed yet not showing up in global themes section

SUMMARY
Global themes not showing up in Setting's Global theme section.

STEPS TO REPRODUCE
1. Open System settings
2. Navigate to Global themes under Appearance section
3. Click on "Get New Global Themes" and install any theme (I tried with McMojave LAF or ChromeOS theme by vinceliuice)

OBSERVED RESULT
After installing the new theme, it does not show up in the Global theme section


EXPECTED RESULT
Installed theme should show up in the Global theme section after installing it.


SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: 5.18.2
(available in About System)
KDE Plasma Version: 5.18.2
KDE Frameworks Version: 5.67.0
Qt Version: 5.14.1

ADDITIONAL INFORMATION
Comment 1 Nate Graham 2020-03-05 14:39:07 UTC
Can confirm.
Comment 2 David Edmundson 2020-03-05 14:52:39 UTC
Does it appear if you close the window and reopen it?
Comment 3 Nate Graham 2020-03-05 14:56:53 UTC
No.
Comment 4 hardcodecoder 2020-03-12 13:29:02 UTC
(In reply to Nate Graham from comment #3)
> No.

Fixed yet?
Comment 5 Nate Graham 2020-03-12 13:38:15 UTC
If it was fixed, the bug would be closed. :)
Comment 6 Marco Martin 2020-04-06 09:49:52 UTC
i can't reproduce neither on rleased 5.18 nor master, themes appear fine here
Comment 7 Nate Graham 2020-04-06 14:26:01 UTC
This is heavily related to https://phabricator.kde.org/D28532; there are a set of deeper problems here but the lack of error reporting is masking them. With that patch installed, things become clearer.

For example with Arc KDE, I now see an error message when trying to remove it:

An error occurred during the installation process:
The uninstallation process failed to successfully run the command kpackagetool5 -t Plasma/LookAndFeel -r /tmp/com.github.varlesh.arc-dark.tar.gz

However the GHNS window seems to think that it was removed as it no longer shows up as installed. And thereafter, I see an error message when trying to reinstall it:

Error: Installation of /tmp/com.github.varlesh.arc-dark.tar.gz failed: /home/nate/.local/share/plasma/look-and-feel/com.github.varlesh.arc-dark already exists

Seems like an issue with KNewstuff that's unrelated to Global Themes; moving there.
Comment 8 Dan Leinir Turthra Jensen 2020-04-06 14:43:44 UTC
Git commit eb23f549ea9b6fc893b73c29784c67a163aa47b3 by Dan Leinir Turthra Jensen.
Committed on 06/04/2020 at 14:43.
Pushed by leinir into branch 'master'.

Introduce more user-visible error reporting for installations

Summary:
Prior to this, we did have error reporting, but only in the way of
writing errors out on the command line (through qCCritical). While
this is fine for debugging purposes, it is unfortunate when errors
can be expected to occur in day to day use (as we are dealing with
a wide range of things that can (and often do) go wrong), and giving
proper feedback to the user as to why, for example, their new icons
aren't showing up when they seem to have installed just fine would
seem a reasonable thing to do.

* Add a signal to Installation fired when there's a critical error
* Forward the installer errors through KNSCore::Engine
* Add the MessageBoxSheet component from kaccounts-integration
* Add a component for displaying errors from the engine
* Use the ErrorDisplayer component (only show on the current page)

Test Plan:
An error displayey thing:
{F8220509}

Reviewers: #knewstuff, #plasma, ngraham, #frameworks

Reviewed By: ngraham

Subscribers: kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D28532

M  +1    -0    src/core/engine.cpp
M  +8    -1    src/core/installation.cpp
M  +6    -0    src/core/installation.h
M  +1    -0    src/qtquick/qml/EntryDetails.qml
M  +1    -0    src/qtquick/qml/Page.qml
M  +1    -0    src/qtquick/qml/private/EntryCommentsPage.qml
A  +47   -0    src/qtquick/qml/private/ErrorDisplayer.qml     [License: LGPL (v2+)]
A  +60   -0    src/qtquick/qml/private/MessageBoxSheet.qml     [License: LGPL (v2+)]

https://commits.kde.org/knewstuff/eb23f549ea9b6fc893b73c29784c67a163aa47b3
Comment 9 Dan Leinir Turthra Jensen 2020-04-07 10:02:22 UTC
(In reply to Nate Graham from comment #7)
> This is heavily related to https://phabricator.kde.org/D28532; there are a
> set of deeper problems here but the lack of error reporting is masking them.
> With that patch installed, things become clearer.
> 
> For example with Arc KDE, I now see an error message when trying to remove
> it:
> 
> An error occurred during the installation process:
> The uninstallation process failed to successfully run the command
> kpackagetool5 -t Plasma/LookAndFeel -r
> /tmp/com.github.varlesh.arc-dark.tar.gz
> 
> However the GHNS window seems to think that it was removed as it no longer
> shows up as installed. And thereafter, I see an error message when trying to
> reinstall it:
> 
> Error: Installation of /tmp/com.github.varlesh.arc-dark.tar.gz failed:
> /home/nate/.local/share/plasma/look-and-feel/com.github.varlesh.arc-dark
> already exists
> 
> Seems like an issue with KNewstuff that's unrelated to Global Themes; moving
> there.

Right, so the problem is...

Open dialogs as in the steps to reproduce (using mcmohave as a test)
kpackagetool5 installation succeeds on the first go (and incidentally i also have a successful refresh of the kcm's contents, resulting in the theme showing up there, which means i can't confirm this bug's issue as described)
Open the ghns dialog again, and click uninstall on mcmohave
An error is displayed as you describe upon removal, and knewstuff now thinks the entry is uninstalled, but stuff has been left behind on the file system

Two things to consider here:
kpackagetool5 is failing to remove the thing we installed (presumably because the file it installs is deleted?)
knewstuff cannot differentiate between uninstall being requested, and that uninstallation failing in an external tool (it works fine if it fails internally, that is to say, if there is no external tool used for the installation management)
Comment 10 Dan Leinir Turthra Jensen 2020-05-04 10:45:34 UTC
Git commit 3f38da8a70d8101d8fda586c28c4c8cd64d68daa by Dan Leinir Turthra Jensen.
Committed on 04/05/2020 at 10:45.
Pushed by leinir into branch 'master'.

Add KPackage support to KNewStuffCore

Summary:
Adding support for KPackage directly to KNewStuff means that we are
able to deal more gracefully with things like Plasma's Global Themes
(and indeed any other kpackage based thing).

This is done by adding another archive specialisation to the installer
class, and by also adding a check to the cache to ensure that even
when a kpackage is removed from the system outside of KNewStuff,
it does not remain seemingly installed in the KNS lists.

* Make sure the cache gets written periodically
* Add KPackage support to KNSCore::Installation
* Introduce a getter (and enum) for the uncompress Installation setting
* Add a redirection to the knsrc documentation location
* Add a function to clean the cache of functionally stale entries
* Clean the cache when the uncompression method is set to kpackage
* Add a fallback for unconverted kpackage based knsrc files
* Clean up some of the error reporting, and reset the entry's state
* Check if installedFile is a file, if so bypass KPackage and delete
* Add a KPackageType property to Installation, for fallback purposes
* Add documentation for the new knsrc bits
* Handle adopting an already installed kpackage item
* Also uninstall not-adopted-but-there possibly installed kpackage bits
* Add a simple async job wrapper for KPackage operations (and use it for the installation handling tasks in Installation)

Test Plan:
There are two options for testing out this patch:

1) Use an existing knsrc file which uses kpackage installation (such as plasma themes), which will use the fallback
2) Manually convert such a knsrc file, by removing the uninstall and installation commands from the knsrc file, and adding in an "Uncompress=kpackage" line instead

Both these should result in the KPackage path being used. You should see this on the command line when attempting to install an item, resulting in lines like "Using KPackage for installation", as well as more pleasant error reporting in the UI in the cases where something goes wrong.

To turn on debug output for KNewStuffCore, add QT_LOGGING_RULES="org.kde.knewstuff*=true" to your command line. For example, you can launch the test dialogue directly by launching the following from your build directory:

  QT_LOGGING_RULES="org.kde.knewstuff*=true" ./bin/khotnewstuff-dialog plasma-themes.knsrc

Reviewers: #plasma, #knewstuff, #frameworks, ngraham, mart, davidedmundson, broulik, bshah

Reviewed By: #plasma, mart

Subscribers: alex, ngraham, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D28701

M  +1    -0    CMakeLists.txt
M  +4    -0    src/core/CMakeLists.txt
M  +21   -0    src/core/cache.cpp
M  +13   -0    src/core/cache.h
M  +3    -0    src/core/engine.cpp
M  +1    -0    src/core/engine.h
M  +429  -190  src/core/installation.cpp
M  +15   -0    src/core/installation.h
A  +182  -0    src/core/jobs/kpackagejob.cpp     [License: LGPL (v2.1+)]
A  +82   -0    src/core/jobs/kpackagejob.h     [License: LGPL (v2.1+)]
M  +19   -0    src/downloaddialog.h

https://commits.kde.org/knewstuff/3f38da8a70d8101d8fda586c28c4c8cd64d68daa
Comment 11 Alexander Lohnau 2020-07-15 19:25:04 UTC
*** Bug 407593 has been marked as a duplicate of this bug. ***