Differential D8636

Add support for downloading the 2nd or 3rd download link from a kde store product when fetching lookandfeel dependencies
ClosedPublic
Actions

Authored by Zren on Nov 3 2017, 5:54 AM.

Details

Reviewers
apol
Commits
R252:35656828f12c: Support downloading the 2nd or 3rd download link from a KNS product
Summary

BUG: 385429

Implementing the feature request in https://bugs.kde.org/show_bug.cgi?id=385429

LookAndFeels introduced the ability to set dependencies, that are downloaded first before installing the look and feel package.
https://userbase.kde.org/Plasma/Create_a_Look_and_Feel_Package

Eg: X-KPackage-Dependencies=kns://plasmoids.knsrc/api.kde-look.org/1160672

It will call:

/usr/lib/x86_64-linux-gnu/libexec/kf5/kpackagehandlers/knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672

Previously this file called engine.install(entry);
the second argument (linkId) isn't supplied so it defaults to 1.

void install(KNSCore::EntryInternal entry, int linkId = 1);

This means it downloads the first link, which for me is the oldest version of the widget typically.

https://api.kde-look.org/ocs/v1/content/download/1160672/1
tells it to download tiledmenu-v05-kde5.5.plasmoid

while
https://api.kde-look.org/ocs/v1/content/download/1160672/3
tells it to download the latest version tiledmenu-v18-kde5.9.plasmoid

This patch adds the ability to specify which link to download with the ?linkId=3 query parameter.

Eg: kns://plasmoids.knsrc/api.kde-look.org/1160672?linkId=3

Test Plan

Shorten the command call for testing since knshandler isn't in $PATH.

alias knshandler='/usr/lib/x86_64-linux-gnu/libexec/kf5/kpackagehandlers/knshandler'
knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672
# Installs tiledmenu-v05-kde5.5.plasmoid

knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672?linkId=1
# Should error, since we didn't uninstall the previous version. Should log:
# Command ' "kpackagetool5 --install /tmp/tiledmenu-v05-kde5.5.plasmoid --type Plasma/Applet" ' failed with code 4

knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672?linkId=2
# Should error, since we didn't uninstall the previous version. Should log:
# Command ' "kpackagetool5 --install /tmp/tiledmenu-v11-kde5.6.plasmoid --type Plasma/Applet" ' failed with code 4

knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672?linkId=3
# Should error, since we didn't uninstall the previous version. Should log:
# Command ' "kpackagetool5 --install /tmp/tiledmenu-v18-kde5.9.plasmoid --type Plasma/Applet" ' failed with code 4

knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672?rawr=blarg
# Should error, since we didn't uninstall the previous version. Should log:
# Command ' "kpackagetool5 --install /tmp/tiledmenu-v05-kde5.5.plasmoid --type Plasma/Applet" ' failed with code 4

knshandler kns://plasmoids.knsrc/api.kde-look.org/1160672?linkId=test
# linkId is not an integer QUrl("kns://plasmoids.knsrc/api.kde-look.org/1160672?linkId=test") ("api.kde-look.org", "1160672")

knshandler kns://plasmoids.knsrc/api.kde-look.org/
# wrong format in the url path QUrl("kns://plasmoids.knsrc/api.kde-look.org/") ("api.kde-look.org")

Diff Detail

Repository
R252 Framework Integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
Zren created this revision.Nov 3 2017, 5:54 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptNov 3 2017, 5:54 AM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
Zren edited the test plan for this revision. (Show Details)Nov 3 2017, 5:56 AM
Zren edited the test plan for this revision. (Show Details)Nov 3 2017, 6:00 AM
Zren added a reviewer: apol.Nov 3 2017, 6:05 AM
apol added a comment.Nov 3 2017, 11:43 AM

Hi,
Thanks for your patch! It's something I pondered doing at some point but then got busy with other stuff.

I was thinking that instead of adding yet another part, maybe we could have it as a URI query, this way we don't break the components that are parsing kns: uri. It would look like this: kns://plasmoids.knsrc/api.kde-look.org/1160672?linkid=3

What do you think?

ngraham edited the summary of this revision. (Show Details)Nov 3 2017, 1:10 PM
Zren updated this revision to Diff 21841.Nov 3 2017, 3:28 PM
Zren edited the summary of this revision. (Show Details)
Zren edited the test plan for this revision. (Show Details)

Good idea. This will future proof us in case we ever want ?linkName=tiledmenu-v18-kde5.9.plasmoid or something as well.

apol accepted this revision.Nov 3 2017, 5:18 PM
This revision is now accepted and ready to land.Nov 3 2017, 5:18 PM
Zren added a comment.Nov 5 2017, 6:55 PM

Quick note since I just notice you suggested a lowercase i in ?linkid=3, should I change it to lowercase or stick with camelcase?

apol added a comment.Nov 5 2017, 7:06 PM
In D8636#164605, @Zren wrote:

Quick note since I just notice you suggested a lowercase i in ?linkid=3, should I change it to lowercase or stick with camelcase?

I don't really mind, maybe leave it as link?

Closed by commit R252:35656828f12c: Support downloading the 2nd or 3rd download link from a KNS product (authored by Zren). · Explain WhyNov 7 2017, 2:12 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 22014

View Options

src/kpackage-install-handlers/kns/main.cpp

Loading...