[Bug] Pull down entry `chum-launch` is not displayed on older Sailfish-OS releases

[Olf0]

The pull down entry chum-launch (text: "Start application") in qml/pages/PackagePage.qml lines 58-70 is not displayed on older Sailfihsh-OS releases. Currently only two test points were taken: It works fine on SailfishOS 4.5.0, but fails to show on SailfishOS 3.2.1. Curiously no warning or error message is emitted at the command line, when it does not show and this pulley menu is used, see that section "TL;DR".

[nephros]

While on the topic: Are we happy about the position of the menu entry (bottom-most one)?

I found that IF a package is updatable, the "Update" menu entry appears. Before #266, that was the bottom-most entry. Now, the menu is a bit inconsistent:
If the package is updatable but not launchable we have

[...]
[Update] 
---------

If the package is updatable and launchable we have

[...]
[Update] 
[Launch] 
---------

Maybe it's just me, but while going through several updatable packages one at a time, sometimes I updated, sometimes I launched despite wanting to update. It was annoying enough for me to wish the Launch entry was moved someplace higher, perhaps below the Remove entry.

[nephros]

On the issue at hand:

visible: pkg.installed && pkg.desktopFile.length > 0

So the only thing I can see why the entry would not appear is either pkg.installed or pkg.desktopFile not being correct in a 3.1 system.

OR, javascript evaluation of that is different. @Olf can you try changing the QML to

visible: pkg.installed && (pkg.desktopFile.length > 0)

or even

visible: pkg.installed && (pkg.desktopFile && (pkg.desktopFile.length > 0))

it shouldn't make a difference AFAICS but with javascript you never know.

Now, pkg.desktopFile is set if there is a match in the files packages for "endsWith('.desktop')'. (https://github.com/sailfishos-chum/sailfishos-chum-gui/blob/ffa557d52616bf1e4fbe57c1a0fec8e5e52af24b/src/chumpackage.cpp#L268)
This is the case for the package you are testing with?

Now, pkg.installed is a check that there is a version, and it's not empty: https://github.com/sailfishos-chum/sailfishos-chum-gui/blob/ffa557d52616bf1e4fbe57c1a0fec8e5e52af24b/src/chumpackage.cpp#L56

Again I can't see how this could be false, unless something failed earlier that caused the version not being set correctly.

[nephros]

Meh let's just debug print the whole thing.

@Olf0 please add these debug prints and see what the console sais:

visible: pkg.installed && pkg.desktopFile.length > 0
onVisibleChanged: {
    if (!visible) {
        console.debug("Launch: Not visible for package", pkg.name)
        console.debug("Launch: Package installed:", pkg.installed)
        console.debug("Launch: Desktopfile:", pkg.desktopFile)
    }
}

(Just a QML change, so no need for a rebuild. Just edit on-device.)

[Olf0]

While on the topic: Are we happy about the position of the menu entry (bottom-most one)?

I found that IF a package is updatable, the "Update" menu entry appears. Before #266, that was the bottom-most entry. Now, the menu is a bit inconsistent: If the package is updatable but not launchable we have

[...]
[Update] 
---------

If the package is updatable and launchable we have

[...]
[Update] 
[Launch] 
---------

Maybe it's just me, but while going through several updatable packages one at a time, sometimes I updated, sometimes I launched despite wanting to update. It was annoying enough for me to wish the Launch entry was moved someplace higher, perhaps below the Remove entry.

I fully agree that it should be placed above "Update", because that is the most common operation, but placing it right next to "Remove" is dangerous; I am glad that the rarely (rsp. "rareliest") used entry "Source code" separates "Remove" on the top of the menu from the rest of the entries. Because it can be hard to find an app icon on the launcher when many apps are installed, I placed it right above "Update" by PR #291. Is that O.K. for you and fixes the issue from your perspective?

[Olf0]

@Olf0 please add these debug prints and see what the console says:

visible: pkg.installed && pkg.desktopFile.length > 0
onVisibleChanged: {
    if (!visible) {
        console.debug("Launch: Not visible for package", pkg.name)
        console.debug("Launch: Package installed:", pkg.installed)
        console.debug("Launch: Desktopfile:", pkg.desktopFile)
    }
}

Originally it complained about console.debug("Launch: Not visible for package", pkg.name) by emitting: [W] unknown:64 - file:///usr/share/sailfishos-chum-gui/qml/pages/PackagePage.qml:64: TypeError: Cannot read property 'name' of null So I commented this line out.

Next try, now it complains about the second line which is supposed to generate debug output (console.debug("Launch: Package installed:", pkg.installed)): [W] unknown:65 - file:///usr/share/sailfishos-chum-gui/qml/pages/PackagePage.qml:65: TypeError: Cannot read property 'installed' of null So I commented this line out, too.

Last try, same story, now WRT console.debug("Launch: Desktopfile:", pkg.desktopFile): [W] unknown:66 - file:///usr/share/sailfishos-chum-gui/qml/pages/PackagePage.qml:66: TypeError: Cannot read property 'desktopFile' of null

[nephros]

OK lets guard against those null objects using a double-not, and be super careful.

Please try this:

visible: !!pkg && (pkg.installed && (pkg.desktopFile.length > 0))
onVisibleChanged: {
    if (!!pkg && !visible) {
        console.debug("Launch: Not visible for package", pkg.name)
        console.debug("Launch: Package installed:", pkg.installed)
        console.debug("Launch: Desktopfile:", pkg.desktopFile)
    }
}

[Olf0]

OK lets guard against those null objects using a double-not, and be super careful.

Please try this:

Now the output is … nothing! I.e. no output at all:

• None of the three expected debug messages is emitted at the CLI.
• No output WRT file:///usr/share/sailfishos-chum-gui/qml/pages/PackagePage.qml (e.g. QML warnings or errors) at the CLI.
• As expected, no visible change at the GUI.

It tried e.g. Barcode, Audioworks, Aenigma and Bugger, accessing all four via both QML pages "Applications" and "Installed packages".

P.S. / Edit: The only output at the CLI, which may be related to this is: file:///usr/share/sailfishos-chum-gui/qml/pages/PackagePage.qml:6

[Olf0]

O.K., with

                visible: pkg.installed
                // && (pkg.desktopFile.length > 0)
                onVisibleChanged: {
                    if (pkg && !visible) {
                        console.debug("Launch: Not visible for package", pkg.name)
                        console.debug("Launch: Package installed:", pkg.installed)
                        console.debug("Launch: Desktopfile:", pkg.desktopFile)
                    }
                }

I see

[W] unknown:474 - file:///usr/lib/qt5/qml/Sailfish/Silica/private/PulleyMenuBase.qml:474:13: QML State: Binding loop detected for property "when"
[D] onVisibleChanged:65 - Launch: Not visible for package Bugger!
[D] onVisibleChanged:66 - Launch: Package installed: true
[D] onVisibleChanged:67 - Launch: Desktopfile: 
[W] unknown:474 - file:///usr/lib/qt5/qml/Sailfish/Silica/private/PulleyMenuBase.qml:474:13: QML State: Binding loop detected for property "when"
[D] onVisibleChanged:65 - Launch: Not visible for package Barcode
[D] onVisibleChanged:66 - Launch: Package installed: true
[D] onVisibleChanged:67 - Launch: Desktopfile:

and the menu entry is visible. :tada:

I.e. the property pkg.desktopFile does not seem to exist, correct?

[nephros]

Cool.

Yes the property appears to be empty. Hmmm.

There must be a bug in setInstalledVersion().

[nephros]

There must be a bug in setInstalledVersion().

Lets look at it:

void ChumPackage::setInstalledVersion(const QString &v)
{
    if (v == m_installed_version) return;
    m_installed_version = v;
    emit updated(m_id, PackageInstalledVersionRole);
    emit updated(m_id, PackageInstalledRole);
    if (type() == PackageApplicationDesktop && installed()) {
        auto trfl = Daemon::getFiles(m_pkid_installed);
        connect(trfl, &Transaction::files, this, [this](
                const QString &packageID, const QStringList &filenames)
        {
            for (auto f : filenames) {
                if (f.endsWith(QStringLiteral(".desktop")))
                {
                    m_desktopFile = f;
                    emit updated(m_id, PackageDesktopFileRole);
                    break;
                }
            }
        });
    }
}

So it could return early in if (v == m_installed_version) and never collect the files.

Then we do Daemon::getFiles(m_pkid_installed), which is from PackageKit, and iterate over its results.
It could be that that is somehow empty.
It could also be that on SFOS 3.1.x, the result list returned is something that does not meet the endsWith match (like foo.desktop\n), don't know.

And, the whole thing is async, it can also be that it simply takes too long and isn't finished before the UI wants to look at the desktopFile property. (After all packagekitd tends to be busy a lot when package managers are active).

Finally, I'm not sure why the determination of package file contents is done in setInstalledVersion at all. Shouldn't that property just have its own setter method and get initialized separately from m_installed_version?

@jaimeMF any comments?

[Olf0]

Then we do Daemon::getFiles(m_pkid_installed), which is from PackageKit, and iterate over its results. It could be that that is somehow empty. It could also be that on SFOS 3.1.x, the result list returned is something that does not meet the endsWith match (like foo.desktop\n), don't know.

My experience with packagekit rsp. packagekitd is that it works most of the time, but regularly or arbitrarily shows flawed behaviour.

Is there a way I can emulate Daemon::getFiles(m_pkid_installed) by a DBUS-call issued from CLI to see what it returns?

_{Minor nitpick: It is SFOS 3.2.1 I tested on, not 3.1.x.}

[jaimeMF]

There must be a bug in setInstalledVersion().

Lets look at it:

void ChumPackage::setInstalledVersion(const QString &v)
{
    if (v == m_installed_version) return;
    m_installed_version = v;
    emit updated(m_id, PackageInstalledVersionRole);
    emit updated(m_id, PackageInstalledRole);
    if (type() == PackageApplicationDesktop && installed()) {
        auto trfl = Daemon::getFiles(m_pkid_installed);
        connect(trfl, &Transaction::files, this, [this](
                const QString &packageID, const QStringList &filenames)
        {
            for (auto f : filenames) {
                if (f.endsWith(QStringLiteral(".desktop")))
                {
                    m_desktopFile = f;
                    emit updated(m_id, PackageDesktopFileRole);
                    break;
                }
            }
        });
    }
}

So it could return early in if (v == m_installed_version) and never collect the files.

Then we do Daemon::getFiles(m_pkid_installed), which is from PackageKit, and iterate over its results. It could be that that is somehow empty. It could also be that on SFOS 3.1.x, the result list returned is something that does not meet the endsWith match (like foo.desktop\n), don't know.

And, the whole thing is async, it can also be that it simply takes too long and isn't finished before the UI wants to look at the desktopFile property. (After all packagekitd tends to be busy a lot when package managers are active).

Finally, I'm not sure why the determination of package file contents is done in setInstalledVersion at all. Shouldn't that property just have its own setter method and get initialized separately from m_installed_version?

@jaimeMF any comments?

I thought it was a reasonable place, since to determine if it is installed it checks whether the version is null or not.

[Olf0]

Well, from my perspective this is a case of (pun intended), an unspecific question results in an unspecific reply. :stuck_out_tongue_winking_eye:

More seriously:

From @nephros' statements I gather, that there a few corner-cases in which the current code may fail. Which makes it likely that more than just my XperiaX@SFOS3.2.1 is affected. I see two possible approaches:

• Try to make the code more fail safe. Use me and my XperiaX@SFOS3.2.1 as a test case. This can be an iterative process.
• Tell my how I can obtain the results for these corner cases, or provide me with Shell, Python or compiled code snippets which generate such results.

These two approaches are not mutually exclusive, i.e. they might be part of a two pronged strategy to determine the culprit.

[jaimeMF]

I have added some log in https://github.com/jaimeMF/sailfishos-chum-gui/commit/cc5e6af8dcee3efa55b88e53df53a994869c55cb to see the details of the process. It will print the version, package id, type, files and whether it finds the desktop file or not. With the package id (pkid) you can get the file list on the console with:

$ pkcon get-files "harbour-barcode;1.0.53-1.12.1.jolla;armv7hl;installed"
Operación:      Obteniendo lista de archivos
Estado:         Esperando en cola
Estado:         Comenzando
Estado:         Consultando
Estado:         Finalizado
Resultados:
Archivos del paquete
  /usr/bin
  /usr/bin/harbour-barcode
  /usr/share/applications/harbour-barcode.desktop
  /usr/share/harbour-barcode
  /usr/share/harbour-barcode/qml
  /usr/share/harbour-barcode/qml/components
  /usr/share/harbour-barcode/qml/components/Buzz.qml
....