storeman-developers / harbour-storeman

OpenRepos client app for SailfishOS
https://openrepos.net/content/olf/storeman-installer
Other
52 stars 15 forks source link

[Bug] Page "Local RPM files" does no longer work #461

Closed citronalco closed 6 months ago

citronalco commented 6 months ago

SailfishOS VERSION (Settings → About product → Build): 4.5.0.24
HARDWARE (Settings → About product → Manufacturer & Product name): Sony Xperia X Compact
Storeman VERSION (Storeman → \<Top pulley> → About Storeman): 0.3.1

BUG DESCRIPTION

Page "Local RPM files" does no longer work.

STEPS TO REPRODUCE

Open "Local RPM files". "Page could not get loaded" gets displayed.

ADDITIONAL INFORMATION

Console output:

QML AnimatedLoader: (file:///usr/share/harbour-storeman/qml/pages/LocalRpmsPage.qml:3:1: module "QtSparql" is not installed
    import QtSparql 1.0
    ^)

libqtsparql is installed.

Seems to be caused by this change: https://github.com/sailfishos/libqtsparql/commit/f543a9d332df50d66fbeac18dcded34cca92a2a0

Olf0 commented 6 months ago

@citronalco, thank you very much for this issue report and analysis.

I reported it to @pvuorela here: https://github.com/sailfishos/libqtsparql/commit/f543a9d332df50d66fbeac18dcded34cca92a2a0#commitcomment-139687738

If I missed some detail, feel free to add it here or as another commit-comment.

AFAICS, this is the (only) affected piece of code: lines 17 to 23 of qml/pages/LocalRpmsPage.qml.

pvuorela commented 6 months ago

Thanks for noting. It wouldn't be totally out of the question to enable again the qml plugin. It does need some love and the next time I would put it in a subpackage so not included on images by default, but the SparqlListModel isn't too bad there. Could still note that it's not supported api and not (maybe ever) used by sailfish os itself, somewhat decreasing the priority for maintenance.

However that's not enough to make the LocalRpmsPage.qml work. The query it's doing hasn't been migrated to Tracker 3.0 with separate graphs for different content. In other words, it was broken even before I disabled the qml plugin.

If you want to make it work quickly and not bother about sparql queries, I could suggest using QtDocGallery. E.g. something like

import QtDocGallery 5.0

[...]
    DocumentGalleryModel {
        id: queryModel
        properties: ["url", "fileName"]
        sortProperties: ["+fileName"]
        rootType: DocumentGallery.File
        filter: GalleryEqualsFilter { property: "fileExtension"; value: "rpm" }
    }

Disclaimer: didn't actually test running.

Olf0 commented 6 months ago

@pvuorela, thank you very much for your prompt reply and even more so for taking a look at the QML code and suggesting an alternative. I sure do not mind how the functionality of qml/pages/LocalRpmsPage.qml is restored, only that it is fully restored.

Does the QtDocGallery way work on older SailfishOS releases ≥ 3.1.0? If not, do you remember which SailfishOS release introduced to Tracker 3 and which omitted driver: "QTRACKER_DIRECT" (or was that a switch, so both happened with the same release?) as well as which SailfishOS release omitted libqtsparql's QML API?

Background is that Storeman ≥ 0.3.0 supports all SailfishOS releases ≥ 3.1.0, and it shall stay this way. But there are already very slightly different codebases used for SailfishOS releases 3.1.0 to 3.2.1, 3.3.0 to 4.1.0 and ≥ 4.2.0. Hence I want to understand, if this adaption works on all these releases or if I have to make a switch, and if so, if I can use the extant switch points or have to introduce a new one.


@citronalco, I am not a QML coder, I became maintainer of Storeman because I earned Petr Tsymbarovich's (mentaljam / osetr) trust as packager. Hence I would appreciate very much if you also pose a PR for Storeman (as you did for ISOdrive), because I can only copy & paste Pekka's code snippet, but will fail to see flaws I might create when doing so.

pvuorela commented 6 months ago

QtDocGallery might work on older releases. There were some adjustments, but think it was more on needing to define proper root type to match the graph. Above should be fine. I guess the new tracker version came with 4.3.0.

Olf0 commented 6 months ago

Thank you @pvuorela, out of curiosity I looked up the changes in SailfishOS, which was quick and easy thanks to your hints:

So trying the QtDocGallery way seems to be the easiest solution. Side note: I can test on SailfishOS 3.2.1, but unfortunately have no 3.1.0 accessible.


@pvuorela, one concluding remark: This is one of many examples how changes in SailfishOS break applications.

Unfortunately the app ecosystem for SailfishOS is dysfunctional: No paid apps in the Jolla Store, but lots of restrictions, consequently it is dead for long; SailfishOS being an unattractive target for FLOSS-developers, because it is proprietary despite utilising many FLOSS-components, and as dire consequence uses a completely outdated Qt, which imposes a massive hurdle for cross-Linux-distribution-development rsp. porting modern QT-based software to SailfishOS. This made many developers turn away from SailfishOS, because there are alternatives which are fully FLOSS and (barely) usable (UBports, PostmarketOS etc.), leaving behind many abandoned apps. Because of abstraction layers providing backward compatibility (QML, Qt etc.) these abandoned apps often run surprisingly well on later releases of SaifishOS, until Jolla decides to introduce a breaking change (backward compatibility does not seem to be an aspect Jolla takes into account): These can be minor changes (as this one) affecting a few apps (up to now we have identified Storeman and ISOdrive) or major changes (SailJail falls into this category) which break almost every app. The consequence is that SailfishOS' app ecosystem is continuously declining, specifically disruptive actions as introducing SailJail being on by default constitute a large step downwards.

TL;DR: Please do take backward compatibility into account in the future, when altering extant APIs in SailfishOS (no matter if they are Jolla Store compliant, because the Jolla Store is dysfunctional and hence irrelevant), in order to cease thinning out the SailfishOS app ecosystem further and further.

pvuorela commented 6 months ago

It's a balancing act. On one hand making changes can break things elsewhere, but on the other hand keeping backward compatibility can also be an accumulating maintenance burden, eventually slowing down the development. Also often the changes are done because the old api was just somehow bad, perhaps being good enough for the internal feature it was developed for, or maybe done or inherited from somewhere without actually having real-life application proving it's fit for the purpose.

But I'm noting your comment and perhaps we can still bring back the qml api.

Olf0 commented 6 months ago

It's a balancing act.

Sure, I only want to point out the scope and lasting, dire consequences this regularly has, because I repeatedly had the impression that Jolla rsp. its sailors do not take the SailfishOS' app ecosystem (i.e. outside of the Jolla Store) seriously into account when considering breaking changes.

On one hand …

All agreed, but this is a purely technical and Jolla-internal perspective. It does not take into account frustration of users (apps do not work any longer after an SailfishOS upgrade), developers ("Jolla broke my app again, so I have to invest time"), developer brain drain (there are less and less around due to aforementioned reasons, hence broken apps often stay broken forever) and the ultimate consequence of all this: less and less working apps for SailfishOS.

But I'm noting your comment and perhaps we can still bring back the qml api.

Well, I do not think this is necessary any longer due to your kind help. And the breakage is already done by SFOS 4.5.0 WRT libqtsparcl's QML API and SFOS 4.3.0 WRT the tracker API.

I would really appreciate if you mind the big picture I tried to draw with that comment when performing the "balancing act" of introducing breaking changes and / or investing effort into designing APIs: IMO one should always try hard to design APIs as thoroughly as if they have to last forever (e.g. extensible; there always will be users soon, you do not know about), because non-backwards-compatible API changes will likely break some software.

citronalco commented 6 months ago

Guys, I'm also no programmer. Yesterday was the first time I ever installed Sailfish SDK and saw a QML file.

Install the Sailfish SDK (if you want to test the app on older Sailfish OS versions, simply enable that versions as "Target" and "Emulator", too). Then clone the harbour-storeman repo, open the "harbour-storeman.pro" in Sailfish SDK and click the green play button on the lower left. You can switch to another Sailfish OS version with the "Debug" button on the lower left and pick the wanted "Device".

You can even connect with ssh (given you've installed Sailfish SDK in ~/SailfishOS):

ssh -p 2223 -i ~/SailfishOS/vmshare/ssh/private_keys/sdk defaultuser@localhost
ssh -p 2223 -i ~/SailfishOS/vmshare/ssh/private_keys/sdk root@localhost

If you have more than one emulator VMs, the next VM listens on port 2232 (at least that's what it did here)

It's not that difficult, pvuorela nearly posted the solution. The rest is searching on Stackoverflow and Github for similar code. And do not be afraid to break anything: It is already broken.

I had to increase the RAM size of the "Sailfish SDK Build Engine" VM in VirtualBox to get rid of "out of memory" errors while building.

Olf0 commented 6 months ago

Guys, I'm also no programmer.

Oh, sorry; because you posed the PRs based on @pvuorela's suggestion for the two ISOdrive forks so promptly and apparently profoundly, I expected you to be way ahead in the learning curve.

Well, decades ago I was a mediocre C-for-microcontrollers programmer and know UNIX shell programming quite well (and still like it). But employments have led me first to teaching UNIX inside out (including shell & C: Solaris, HP-UX, AIX, IRIX and Linux) and later away from hands on technical stuff to IT project management and IT security, then ultimately to IT policies and legal aspects of IT, data processing and FLOSS. I forgot so many things over the years and was only able to keep my shell scripting skills up.

Yesterday was the first time I ever installed Sailfish SDK and saw a QML file.

Oh, wow! While I have never installed the SFOS-SDK (and probably never will), I have looked at many QML files over the past decade (i.e. due to using SailfishOS), but still do not comprehend much, which is likely caused by the fact that I do not like JavaScript.

Install the Sailfish SDK …

Here I do refrain, I employ GitHub workflows with Coderus' SFOS-SDK docker images for compiling stuff. You know … "modern" … "in the Cloud" and … "with containers". :wink:

When looking through the changes @citronalco's PR #462 brings, two things came to my mind:

  1. I realised that the old query line, which uses libqtsparql's QML API, looks much alike an SQL SELECT statement: query: "SELECT str… as ?filePath WHERE { …:mimeType 'application/x-rpm' }" Hence I asked myself, "Which database does this use?" and searched the WWW for sparql: Wikipedia nicely provides a structured explanation of SparQL, basically stating that it allows to query RDF information by SQL-like statements. But with libqtsparql the structured information which can be queried are Qt objects: That is indeed very cool!

    @pvuorela, as denoted before, the breakage is done, hence restoring libqtsparql's QML API cannot reverse that for the SailfishOS releases made meanwhile (and you did not like its old QML API: "the qml options usage from c++ was strange"), but now I see how versatile and elegant (and impressively cool) the capabilities of libqtsparql are: Hence it really might be worth considering to expose libqtsparql to QML again.

  2. Following @pvuorela's advice, @citronalco's implementation now looks for the file extensions rpm and RPM. But IMO querying for files whose mimeType is application/x-rpm is much more elegant, as the original code did. So I tried to determine how this might be achieved, unfortunately to no avail: There is no documentation for QtDocGallery at doc.qt.io, because it is marked as "experimental", and the doc directory in the source tree contains nothing useful (only stubs). The furthest I could get was via a code browser, which resolves the header files, but there my knowledge how to proceed ends. Side note: I was surprised to see that QtDocGallery's development basically ceased 10 years ago, but then I searched for libqtsparql's documentation an all I found was once on "NOKIA developers" for MeeGo 1.2 Harmattan: [1] and [2]! O.K., now I see why QtDocGallery ranks better and basically it is you @pvuorela who maintains both, AFAICS. Kudos!

    If you @pvuorela can spontaneously think of a way how to query files for mimeType: 'application/x-rpm', please denote that, if not, do not invest much effort. Even though relying on file extensions is "so 90ties" rsp. (worse) "so Windows", this basically works, despite being ugly.

Thank you both for all your research and efforts as well as @pvuorela's kind advice.

citronalco commented 6 months ago

It works at least in Sailfish OS 4.5 and 3.4. Tested that in the emulator VMs provided by the SDK. Moved to https://github.com/storeman-developers/harbour-storeman/pull/462#issuecomment-1998706287, because the corresponding question was moved there.

pvuorela commented 6 months ago

@pvuorela, as denoted before, the breakage is done, hence restoring libqtsparql's QML API cannot reverse that for the SailfishOS releases made meanwhile (and you did not like its old QML API: "the qml options usage from c++ was strange"), but now I see how versatile and elegant (and impressively cool) the capabilities of libqtsparql are: Hence it really might be worth considering to expose libqtsparql to QML again.

Still considering that indeed. Tracker sparql allows to search for many things, but it can be also tricky to get right sometimes, and the data, query details etc have been changing so QtDocGallery has been there for easier to use model.

Side note: I was surprised to see that QtDocGallery's development basically ceased 10 years ago, but then I searched for libqtsparql's documentation an all I found was once on "NOKIA developers" for MeeGo 1.2 Harmattan: [1] and [2]! O.K., now I see why QtDocGallery ranks better and basically it is you @pvuorela who maintains both, AFAICS. Kudos!

Thanks. Yea, basically the upstream development stopped at some point and ours if the de facto upstream now.

If you @pvuorela can spontaneously think of a way how to query files for mimeType: 'application/x-rpm', please denote that, if not, do not invest much effort. Even though relying on file extensions is "so 90ties" rsp. (worse) "so Windows", this basically works, despite being ugly.

I think it should work just by using 'mimeType' property instead of the 'fileExtension'. In practice the result should be the same and don't recall even seeing that much capital RPM extensions ever.

Olf0 commented 6 months ago

I have found documentation for DocumentGalleryModel in the "Qt Mobility 1.2.2 Project Reference Documentation":

Olf0 commented 6 months ago

Thank you both, @citronalco and @pvuorela very much, both variants are working fine: As @citronalco already tested on SailfishOS 4.5.0 and 3.4.0, I just additionally tested both variants on SailfishOS 3.2.1.

For the diff between the two variants, see commit 3198be2 (or equivalently 56eabea).

Olf0 commented 6 months ago

@citronalco, if you would like to perform a final test of the release version of Storeman 0.3.8, you can either use the one(s) compiled by the GitHub workflow(s) or the "testing" ones at the SailfishOS-OBS. Actually I am also interested to know if the RPM files compiled at GitHub with the Sailfish-SDK for SailfishOS 4.2 run on SailfishOS 4.5.0; if these unpacked RPM files do not work, the ones inside the ZIP archive compiled for 4.5.0 should.

Note that Storeman's self-updating mechanism and the Storeman Installer currently still fetch Storeman 0.3.7 from the SailfishOS-OBS, so one has to download a Storeman 0.3.8 RPM file and install it with devel-su pkcon install-local <RPM file name> or via the "Local RPM files" page of a Storeman 0.3.8 pre-release.