Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Implement a method of retrieving the package update time #155

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions qml/components/AppInformation.qml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ MouseArea {
visible: pkg.packageName
}

ChumDetailItem {
//% "Package last updated:"
label: qsTrId("chum-pkg-package-mtime")
value: pkg.packageMTime
visible: pkg.packageMTime
}

ChumDetailItem {
//% "Download size:"
label: qsTrId("chum-pkg-download-size")
Expand Down
9 changes: 8 additions & 1 deletion qml/pages/PackagesListPage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@ Page {

allowedOrientations: Orientation.All

BusyIndicator {
size: BusyIndicatorSize.Large
anchors.centerIn: parent
running: chumModel.busy == true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be needed - whole BusyIndicator - as we have busy state in Chum

visible: running
}

SilicaListView {
id: view
anchors.fill: parent
Expand Down Expand Up @@ -78,7 +85,7 @@ Page {
}

onClicked: pageStack.push(Qt.resolvedUrl("../pages/PackagePage.qml"), {
pkg: Chum.package(model.packageId)
pkg: Chum.package(model.packageId),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
pkg: Chum.package(model.packageId),
pkg: Chum.package(model.packageId)

})

onDownChanged: {
Expand Down
95 changes: 91 additions & 4 deletions src/chum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@

#include <QDebug>
#include <QSettings>
#include <QtNetwork/QNetworkAccessManager>
#include <QtNetwork/QNetworkReply>
#include <QtNetwork/QNetworkRequest>
#include <QJsonObject>
#include <QJsonArray>

static char* apiUrl = "http://piggz.co.uk:8081";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to add that as MACRO and set it in CMakeLists.txt


using namespace PackageKit;

Expand Down Expand Up @@ -37,10 +44,7 @@ Chum::Chum(QObject *parent)
m_show_apps_by_default = (settings.value(s_config_showapps, 1).toInt() != 0);
m_manualVersion = (settings.value(s_config_manualversion, QString()).toString());

m_busy = true;
//% "Loading SailfishOS:Chum repository"
setStatus(qtTrId("chum-load-repositories"));
m_ssu.loadRepos();
getChumCache();
}

Chum* Chum::instance() {
Expand Down Expand Up @@ -450,3 +454,86 @@ void Chum::setStatus(QString status) {
m_status = status;
emit statusChanged();
}

void Chum::getChumCache()
{
if (!m_busy) {
m_busy = true;
emit busyChanged();
}

//% "Receiving the chum package cache"
setStatus(qtTrId("chum-update-cache"));

QNetworkAccessManager *netManager = new QNetworkAccessManager();

QObject::connect(netManager,&QNetworkAccessManager::finished,[=](QNetworkReply *reply) {
if (reply->error()){
qDebug() << "Unable to get package build times" << reply->errorString();
}
else {
m_chumPackageCache = QJsonDocument::fromJson(reply->readAll());
qDebug() << "Received chum package cache";
}
m_busy = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should stay on busy=true as SSU gets loaded as well. As it should be called after network access manager is finished, state busy==true is expected and we don't need to change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, aren't we expected to call reply->deleteLater()?

//% "Loading SailfishOS:Chum repository"
setStatus(qtTrId("chum-load-repositories"));
m_ssu.loadRepos();
});

// Start the network request
QNetworkRequest request=QNetworkRequest(QUrl(apiUrl));
m_busy = true;
emit busyChanged();
netManager->get(request);
}

QDateTime Chum::findPackageMTime(const QString &rpm) const
{
qDebug() << Q_FUNC_INFO << rpm << Chum::instance()->repoName() << Chum::instance()->repoVersion();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use Chum::instance if we have it in this?


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether there is a better way of doing it. In principle, we have for each package its name (ex 'harbour-advanced-camera' or 'libscrypt-devel'). At the same time, we can expose URL of active Chum repository.

For exposing Chum repo URL, we would have We would just need to add one more property (repoUrl?) to SSU and fill it up based on m_repos (see https://github.com/sailfishos-chum/sailfishos-chum-gui/blob/main/src/ssu.cpp#L60 and below). Thus, we could get current repo using m_ssu.repoUrl() and get https://repo.sailfishos.org/obs/sailfishos:/chum:/testing/4.5.0.16_aarch64/

After that, it is possible to condition URL into testing/4.5.0.16_aarch64 or chum/4.5.0.16_aarch64 and use those in JSON.

When generating JSON, we can actually forget about "projects" and immediately link those to "packages". So, that would be a task of backend solution. Ideally, we should have JSON as follows:

{
"testing/4.5.0.16_aarch64": {
"harbour-advanced-camera": { "mtime": "1634426517" },
"libscrypt-devel":  { "mtime": "1634426517" }
}
}

That way it can be extended by more details if needed and would ensure that we have data accessible easily from GUI.

QString repo = Chum::instance()->repoVersion() + "_aarch64"; //TODO need to determine device architecture
QString projectName = Chum::instance()->repoName().replace("-", ":");
QString packageName;
QDateTime mtime = QDateTime::fromMSecsSinceEpoch(0);
bool found = false;

QJsonArray projects = m_chumPackageCache.object().value("projects").toArray();

for (auto project : projects) {
//qDebug() << project.toObject()["name"];
if (project.toObject()["name"].toString() == projectName) {
QJsonArray repos = project.toObject()["repositories"].toArray();
for (auto repo : repos) {
//qDebug() << repo.toObject()["name"];
QJsonArray packages = repo.toObject()["packages"].toArray();
for (auto package : packages) {
//qDebug() << package.toObject()["name"].toString();
QJsonArray binaries = package.toObject()["binaries"].toArray();
for (auto binary : binaries) {
//qDebug() << binary.toString();
if (binary.toString().startsWith(rpm)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I suggested a bit different way of doing it, it could be irrelevant. We can get false positives with libscrypt-devel and libscrypt comparison. In this case it would be OK (same package), but what if we have osmscout and osmscout-server ?

packageName = package.toObject()["name"].toString();
found = true;
}
}
}
}

if (found) {
//loop the source projects
QJsonArray packages = project.toObject()["packages"].toArray();
for (auto package : packages) {
//qDebug() << package.toObject()["name"];
if (package.toObject()["name"].toString() == packageName) {
int mt = package.toObject()["mtime"].toString().toInt();
mtime = QDateTime::fromMSecsSinceEpoch(1000L * mt);
qDebug() << "Found binary " << rpm << " in source package " << packageName << mt << mtime;
}
}
}
}
}
return mtime;

}
8 changes: 8 additions & 0 deletions src/chum.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <QHash>
#include <QObject>
#include <QSet>
#include <QJsonDocument>

#include "chumpackage.h"
#include "ssu.h"
Expand Down Expand Up @@ -45,10 +46,14 @@ class Chum : public QObject {
void setRepoTesting(bool testing);
void setShowAppsByDefault(bool v);
void setManualVersion(const QString &v);
QString repoName() { return m_ssu.repoName();}
QString repoVersion() { return m_manualVersion.isEmpty() ? m_ssu.deviceVersion() : m_manualVersion; }
Comment on lines +49 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess those are not needed anymore


const QList<ChumPackage*> packages() const { return m_packages.values(); }
Q_INVOKABLE ChumPackage* package(const QString &id) const { return m_packages.value(id, nullptr); }

QDateTime findPackageMTime(const QString &rpm) const;

// static public methods
static Chum* instance();

Expand Down Expand Up @@ -92,6 +97,8 @@ public slots:
void startOperation(PackageKit::Transaction *pktr, const QString &pkg_id);
void setStatus(QString status);

void getChumCache();

private:
Ssu m_ssu;
bool m_busy{false};
Expand All @@ -100,6 +107,7 @@ public slots:
quint32 m_updates_count{0};
bool m_show_apps_by_default{false};
QString m_manualVersion;
QJsonDocument m_chumPackageCache;

QHash<QString, ChumPackage*> m_packages;
QSet<QString> m_packages_last_refresh;
Expand Down
9 changes: 9 additions & 0 deletions src/chumpackage.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "chumpackage.h"
#include "chum.h"

#include "projectgithub.h"
#include "projectgitlab.h"
Expand Down Expand Up @@ -122,6 +123,8 @@ void ChumPackage::setDetails(const PackageKit::Details &v) {
// derive name
QString pname = Daemon::packageName(m_pkid_latest);
m_package_name = pname;
m_package_mtime = Chum::instance()->findPackageMTime(m_package_name);

m_name = QString{};
QStringList nparts = pname.split('-');
bool is_app = false;
Expand Down Expand Up @@ -261,6 +264,12 @@ void ChumPackage::setPackagerLogin(const QString &login) {
SET_IF_EMPTY(m_packager_login, PackagePackagerRole, login);
}

void ChumPackage::setPackageMTime(const QDateTime &mtime)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not called anymore

{
m_package_mtime = mtime;
emit updated(m_id, PackageMTime);
}

void ChumPackage::setPackagerName(const QString &name) {
SET_IF_EMPTY(m_packager_name, PackagePackagerRole, name);
}
Expand Down
5 changes: 5 additions & 0 deletions src/chumpackage.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class ChumPackage : public QObject {
Q_PROPERTY(int issuesCount READ issuesCount NOTIFY updated)
Q_PROPERTY(QString license READ license NOTIFY updated)
Q_PROPERTY(QString name READ name NOTIFY updated)
Q_PROPERTY(QDateTime packageMTime READ packageMTime NOTIFY updated)
Q_PROPERTY(QString packageName READ packageName NOTIFY updated)
Q_PROPERTY(QString packager READ packager NOTIFY updated)
Q_PROPERTY(QString packagingUrl READ packagingUrl NOTIFY updated)
Expand Down Expand Up @@ -55,6 +56,7 @@ class ChumPackage : public QObject {
PackageSummaryRole,
PackageTypeRole,
PackageUpdateAvailableRole,
PackageMTime,

PackageOtherRole,
PackageRefreshRole // used for updates of many parameters
Expand Down Expand Up @@ -93,6 +95,7 @@ class ChumPackage : public QObject {
int issuesCount() const { return m_issues_count; }
QString license() const { return m_license; }
QString name() const { return m_name; }
QDateTime packageMTime() const { return m_package_mtime; }
QString packageName() const { return m_package_name; }
QString packager() const;
QString packagingUrl() const { return m_packaging_repo_url; }
Expand All @@ -119,6 +122,7 @@ class ChumPackage : public QObject {
void setForksCount(int count);
void setIssuesCount(int count);
void setPackagerLogin(const QString &login);
void setPackageMTime(const QDateTime &mtime);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed

void setPackagerName(const QString &name);
void setReleasesCount(int count);
void setStarsCount(int count);
Expand Down Expand Up @@ -161,6 +165,7 @@ class ChumPackage : public QObject {
int m_issues_count{-1};
QString m_license;
QString m_name;
QDateTime m_package_mtime;
QString m_package_name;
QString m_packager_login;
QString m_packager_name;
Expand Down
3 changes: 3 additions & 0 deletions src/chumpackagesmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ QVariant ChumPackagesModel::data(const QModelIndex &index, int role) const {
return p->type();
case ChumPackage::PackageUpdateAvailableRole:
return p->updateAvailable();
case ChumPackage::PackageMTime:
return p->packageMTime();
default:
return QVariant{};
}
Expand All @@ -62,6 +64,7 @@ QHash<int, QByteArray> ChumPackagesModel::roleNames() const {
{ChumPackage::PackageStarsCountRole, QByteArrayLiteral("packageStarsCount")},
{ChumPackage::PackageTypeRole, QByteArrayLiteral("packageType")},
{ChumPackage::PackageUpdateAvailableRole, QByteArrayLiteral("packageUpdateAvailable")},
{ChumPackage::PackageMTime, QByteArrayLiteral("packageMTime")},
};
}

Expand Down
1 change: 1 addition & 0 deletions src/chumpackagesmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class ChumPackagesModel
void filterUpdatesOnlyChanged();
void searchChanged();
void showCategoryChanged();
void busyChanged();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed


private:
void updatePackage(QString packageId, ChumPackage::Role role);
Expand Down
4 changes: 4 additions & 0 deletions src/ssu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <QDBusPendingCall>
#include <QDBusPendingReply>
#include <QDBusReply>
#include <QDebug>

static QString s_repo_regular(
Expand All @@ -25,6 +26,9 @@ Ssu::Ssu(QObject *parent) :
QDBusConnection::systemBus(),
parent )
{
QDBusReply<QString> version = call(QStringLiteral("release"), false);
m_device_version = version;
qDebug() << "Device version:" << m_device_version;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can override repo version. We should get real URL and use that, as described above

}

void Ssu::loadRepos() {
Expand Down
2 changes: 2 additions & 0 deletions src/ssu.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class Ssu : public QDBusAbstractInterface
bool repoAvailable() const { return m_manage_repo && !m_repo_name.isEmpty(); }
bool repoTesting() const { return m_manage_repo && m_repo_testing; }
QString repoName() const { return m_manage_repo ? m_repo_name : QString{}; }
QString deviceVersion() const {return m_device_version;}

void loadRepos();
void setRepo(const QString &version=QString(), bool testing=false);
Expand All @@ -30,6 +31,7 @@ class Ssu : public QDBusAbstractInterface
bool m_manage_repo{false};
bool m_repo_testing{false};
QString m_repo_name;
QString m_device_version;

QList< std::pair<QString,QString> > m_repos;
QString m_added_repo_name;
Expand Down