Re: application not responding (ANR) handling

[Harald Sitter]

This has been implemented more or less as described. First ANR report
arrived on sentry already :)

On Sun, Jul 14, 2024 at 7:27 PM Harald Sitter  wrote:
>
> Ciao!
>
> A while ago I was thinking about ANR handling but then forgot about it
> again, some malfunction reminded me of it again so I thought I should
> write down some musings. Maybe y'all have some input as well.
>
> Right now we don't really know when our applications deadlock because
> kwin somewhat gracefully kills the process when it detects no answer
> to window actions, leaving no trace of the malfunction for debugging.
> Even outside that feature it's exceptionally hard for a user to
> generate an ANR report because the user either needs to SEGV the app
> manually (at which point kcrash and drkonqi kick in), or attach a
> debugger (requiring basically developer-level knowledge). All in all a
> garbage situation.
>
> It's actually a bit tricky to solve because currently it seems neither
> POSIX nor Linux have a concept of ANR defects so we need some custom
> metadata on top.
>
> Here's my thinking...
>
> - The way kwin does the killing is in a helper binary that more or
> less simply calls kill() on the stuck pid
> - The kill helper could write some trivial metadata to
> .cache/kwin/anr/$exe.$bootid.$pid.$time_at_time_of_crash.json (the
> name format is the one used by coredumpd as well FWIW)
> - It could then send ABRT instead of KILL as first signal
> - It should probably also make sure the pid actually shoved off in
> some timeout or else send KILL
> - KCrash kicks in and does the handover dance with drkonqi
> - DrKonqi can check for the ANR metadata and then mark the report ANR
> for sentry and bugzilla
>
> 3rd party software would still get ABRT and if they have a crash
> handler they'll be able to handle it, it will look like a random ABRT
> at a glance but they'll have at least the possibility of noticing that
> something is deadlocking in their software. I don't think there's a
> better solution for them right now, seeing as we have no platform way
> to tell them this ABRT was ANR. Of course if they are running outside
> a sandbox they are free to also pick up the kwin ANR metadata.
>
> When no crash handlers of any sort are installed ABRT will by default
> cause a core dump, which then ideally goes into a crash handler daemon
> like coredumpd. In fact, with coredumpd the user is then able to
> excavate such deadlock traces via drkonqi's crashed process viewer.
> Improving the debugging UX of deadlocks in general.
>
> Since systemd also sends ABRT when a service watchdog barks this also
> allows us to notice daemon deadlocks on systems where drkonqi covers
> all software (e.g. plasma-mobile).
>
> Any further thoughts on this?
>
> HS

application not responding (ANR) handling

[Harald Sitter]

Ciao!

A while ago I was thinking about ANR handling but then forgot about it
again, some malfunction reminded me of it again so I thought I should
write down some musings. Maybe y'all have some input as well.

Right now we don't really know when our applications deadlock because
kwin somewhat gracefully kills the process when it detects no answer
to window actions, leaving no trace of the malfunction for debugging.
Even outside that feature it's exceptionally hard for a user to
generate an ANR report because the user either needs to SEGV the app
manually (at which point kcrash and drkonqi kick in), or attach a
debugger (requiring basically developer-level knowledge). All in all a
garbage situation.

It's actually a bit tricky to solve because currently it seems neither
POSIX nor Linux have a concept of ANR defects so we need some custom
metadata on top.

Here's my thinking...

- The way kwin does the killing is in a helper binary that more or
less simply calls kill() on the stuck pid
- The kill helper could write some trivial metadata to
.cache/kwin/anr/$exe.$bootid.$pid.$time_at_time_of_crash.json (the
name format is the one used by coredumpd as well FWIW)
- It could then send ABRT instead of KILL as first signal
- It should probably also make sure the pid actually shoved off in
some timeout or else send KILL
- KCrash kicks in and does the handover dance with drkonqi
- DrKonqi can check for the ANR metadata and then mark the report ANR
for sentry and bugzilla

3rd party software would still get ABRT and if they have a crash
handler they'll be able to handle it, it will look like a random ABRT
at a glance but they'll have at least the possibility of noticing that
something is deadlocking in their software. I don't think there's a
better solution for them right now, seeing as we have no platform way
to tell them this ABRT was ANR. Of course if they are running outside
a sandbox they are free to also pick up the kwin ANR metadata.

When no crash handlers of any sort are installed ABRT will by default
cause a core dump, which then ideally goes into a crash handler daemon
like coredumpd. In fact, with coredumpd the user is then able to
excavate such deadlock traces via drkonqi's crashed process viewer.
Improving the debugging UX of deadlocks in general.

Since systemd also sends ABRT when a service watchdog barks this also
allows us to notice daemon deadlocks on systems where drkonqi covers
all software (e.g. plasma-mobile).

Any further thoughts on this?

HS

Re: KDE Builder: request for review

[Harald Sitter]

On Tue, Apr 9, 2024 at 11:26 AM Kevin Kofler  wrote:
>
> ash...@linuxcomp.ru wrote:
> > I've been working on KDE Builder project. This is a reimplementation of
> > kdesrc-build in Python.
>
> What is the point of rewriting a tool originally written in a stable
> language that guarantees long-term backwards compatibility (since the
> originally planned incompatible "Perl 6" was renamed, as it should) and with
> a syntax close to C/C++ (i.e., what KDE software is mostly written in),
> rewriting it in a programming language that releases completely backwards-
> incompatible major versions every few years and whose syntax depends on
> whitespace (!)? Do we not have enough pointless porting busywork for the
> incompatible Qt major releases yet, so now we need the same for KDE Builder?

Yes.

Re: KDE Builder: request for review

[Harald Sitter]

On Mon, Apr 8, 2024 at 10:05 AM  wrote:
>
> Hi all!
>
> I've been working on KDE Builder project. This is a reimplementation of 
> kdesrc-build in Python. At this point, the tool is rather mature, but some 
> minor bugs may probably be unnoticed. I'd like to officially request a review 
> of KDE Builder for declaring it official tool, and deprecating the 
> kdesrc-build. That way we can move forward with new tool.
>
> You can reach me on Matrix at @ashark:kde.org for more instant communications.

^ quoting so it becomes readable hopefully

relevant issue https://invent.kde.org/sdk/kde-builder/-/issues/84

Re: Retirement of Binary Factory

[Harald Sitter]

On Sun, Feb 18, 2024 at 10:23 AM Ben Cooksley  wrote:
>
> On Sun, Feb 18, 2024 at 2:58 PM Loren Burkholder 
>  wrote:
>>
>> On Saturday, February 17, 2024 8:35:48 PM EST Ben Cooksley wrote:
>> > On Sun, Feb 4, 2024 at 10:26 AM Ben Cooksley  wrote:
>> >
>> > The Binary Factory, alongside it's two build servers, and the Flatpak
>> > repository it provided at https://distribute.kde.org/ has now been
>> > decommissioned.
>>
>> Hi all,
>>
>> Just last evening, I was downloading Filelight on a Windows machine. Due to 
>> the machine being rather ancient and slow, I ended up going for the direct 
>> binary download instead of the Microsoft Store download. The apps.kde.org 
>> page had me download from a Binary Factory link. As of right now, that link 
>> is still on https://apps.kde.org/filelight/, but it obviously doesn't work. 
>> I haven't checked the apps.kde.org source, but it seems that perhaps those 
>> URLs are automatically generated for each app, so it should be trivial to 
>> change or remove them.
>
>
> It would appear that Filelight has not yet enabled themselves for any form of 
> continuous delivery builds aside from Flatpak.
> See 
> https://invent.kde.org/utilities/filelight/-/blob/master/.gitlab-ci.yml?ref_type=heads
>
> If Filelight contributors are still interested in supporting other platforms 
> those builds will need to be added.

I'm curious, why didn't you enable stuff for the things that were
previously building on binary factory?

On Sun, Feb 18, 2024 at 10:23 AM Ben Cooksley  wrote:
>
> On Sun, Feb 18, 2024 at 2:58 PM Loren Burkholder 
>  wrote:
>>
>> On Saturday, February 17, 2024 8:35:48 PM EST Ben Cooksley wrote:
>> > On Sun, Feb 4, 2024 at 10:26 AM Ben Cooksley  wrote:
>> >
>> > The Binary Factory, alongside it's two build servers, and the Flatpak
>> > repository it provided at https://distribute.kde.org/ has now been
>> > decommissioned.
>>
>> Hi all,
>>
>> Just last evening, I was downloading Filelight on a Windows machine. Due to 
>> the machine being rather ancient and slow, I ended up going for the direct 
>> binary download instead of the Microsoft Store download. The apps.kde.org 
>> page had me download from a Binary Factory link. As of right now, that link 
>> is still on https://apps.kde.org/filelight/, but it obviously doesn't work. 
>> I haven't checked the apps.kde.org source, but it seems that perhaps those 
>> URLs are automatically generated for each app, so it should be trivial to 
>> change or remove them.
>
>
> It would appear that Filelight has not yet enabled themselves for any form of 
> continuous delivery builds aside from Flatpak.
> See 
> https://invent.kde.org/utilities/filelight/-/blob/master/.gitlab-ci.yml?ref_type=heads
>
> If Filelight contributors are still interested in supporting other platforms 
> those builds will need to be added.
>
>>
>>
>> Are there other places that need this URL replaced as well? 
>> https://lxr.kde.org/search?%21v=kf6-qt6&_filestring=&_string=binary-factory.kde.org
>>  shows that there are a number of READMEs that still link to Binary Factory, 
>> and Krita has some CI stuff that still references binary-factory.kde.org, 
>> but I have a sneaking suspicion that there are binary-factory.kde.org links 
>> in KDE webpages and other repositories that aren't indexed by lxr.kde.org.
>
>
> I have a checkout of most website repositories on my local system and did a 
> quick grep which showed a variety of hits. Most of them were on README files 
> that were intended to show build status of the website itself.
> Those links would have been broken for some time as websites were converted 
> over a while ago.
>
> Affected sites content wise includes:
> - develop.kde.org
> - digikam.org
> - haruna.kde.org
> - kaidan.im
> - kate-editor.org
> - kdeconnect.kde.org
> - kde.ru
> - kdevelop.org
> - kirogi.org
> - kmymoney.org
> - konversation.kde.org
> - krita.org
> - okular.kde.org
> - plasma-mobile.org
> - rkward.kde.org
> - umbrello.kde.org
>
>
>>
>>
>> - Loren
>
>
> Cheers,
> Ben

Re: revisiting the sycoca

[Harald Sitter]

On Tue, Feb 13, 2024 at 1:13 PM Sune Vuorela  wrote:
>
> On 2024-02-08, Harald Sitter  wrote:
> > It occurs to me that we should ponder sycoca a bit.
> >
> > Currently the sycoca contains 3 types of caches:
>
> At some point I remember David Faure, iirc as part of his QMimeType
> work, removed part of sycoca, but left the remaining bits as a per
> user cache *because* we allow the user to modify things.
>
> I don't know how much of it is still relevant, but I do think we should
> be testing it, both on modern ssd systems, but given our occasional
> marketing drive about keeping older systems running, especially also
> on older systems with a spinning metal disk, rather than a bit handwavy
> "This is probably going to be fine"

Do you have a testing plan in mind?

HS

revisiting the sycoca

[Harald Sitter]

It occurs to me that we should ponder sycoca a bit.

Currently the sycoca contains 3 types of caches:

- the mime cache: should in fact be unnecessary because there is
already a mime.cache in /usr/share/mime?

- the menu structure cache: for the most part only loaded once by
plasmashell so the caching seems a bit questionable (caveat: kopenwith
also uses the menu structure)

- the applications desktop file cache: with most metadata having moved
to json, the desktop file caching sycoca offers seems not particularly
useful any longer as it is basically just caching
/usr/share/applications and variants thereof. In practice we have very
few apps that actually start other apps (notably plasmashell) so they
could just always hold the desktop files in memory and refresh on
inotifies. There is also a new-ish mimeinfo.cache which we can hold on
to.

All in all I am not convinced we still need the sycoca. Indeed one
huge question is why we are holding on to a bespoke cache. If files
need caching then shouldn't we cache them on an xdg level? There is of
course also the point that nobody else seems to need them cached,
calling the sycoca yet more into question.

With all that in mind... how about we deprecated the sycoca? It'd
shave some 5k lines off of kservice in the long run.

HS

Re: Proposal for using gitlab for kdereview process

[Harald Sitter]

https://community.kde.org/Goals/All_about_the_Apps

On Wed, Feb 7, 2024 at 11:16 PM Friedrich W. H. Kossebau
 wrote:
>
> Long time ago, but perhaps there are still memories...
>
> Am Dienstag, 4. Juli 2023, 14:32:59 CET schrieb Jonathan Riddell:
> > I've gone ahead an updated the Sanity checklist
>
> Having come across the checklist items
>
> * Passing KDE neon build
> * App packages in Flatpak, Snap, AppImages and Windows etc as appropriate
>
> I tried to understand the background & motivation, but only found this "I've
> gone ahead an updated" here and the diff from your edit on 4 July 2023:
> https://community.kde.org/index.php?
> title=ReleasingSoftware=next=97120
>
> I assume this was the result of some community discussion. If so I failed to
> witness and now fail to find it. Could you help me to get more insight into
> the previous discussion here?
>
> Cheers
> Friedrich
>
>

Re: FileCopyrightText...

[Harald Sitter]

On Mon, Jan 29, 2024 at 10:49 AM Carl Schwan  wrote:
>
> On Monday, January 29, 2024 10:43:04 AM CET Harald Sitter wrote:
> > do we really need it?
> >
> > systemd for example only has a spdx license header, resulting in much
> > tidier file headers.
> >
> > I entirely fail to understand why we need to slap a FileCopyrightText
> > on files. The copyright surely applies whether or not I put the
> > FileCopyrightText there. The list is also just about always
> > incomplete, further calling its use into question. Not to mention that
> > it is annoying book keeping of information that is already available
> > in git.
> >
> > Can someone shed some light on this?
>
> The reuse FAQ has an entry about why only keeping the copyright information in
> git is a bad idea: https://reuse.software/faq/#vcs-copyright

Yes, that makes no sense to me. Whether I put my copyright stamp on a
file or not has no impact on whether I actually have copyright. That
is to say when someone doesn't add their stamp they would still be a
copyright holder. Which means that unless everyone who ever touched a
file puts their stamp on it, which is something we didn't and don't
enforce, the list is inherently incomplete and by extension useless
... So, authorship data is in fact more relevant here because you get
all would-be copyright holders, not just the ones that bothered to put
their stamp on the file. No? ... Obviously the authors list may still
be incomplete but I'm willing to put money on the fact that 9/10 it is
more complete than whatever the license headers proclaim to be the
case.

> Also when copying file between repo, the git history for that file is often
> lost.

That is a fair point.

TBH I am more arguing against our current policy of requiring this
field. If someone wants to put their stamp on a file I have no
interest in stopping them. But, I for one would be content with just
putting the license there, hence my ponderings.

HS

FileCopyrightText...

[Harald Sitter]

do we really need it?

systemd for example only has a spdx license header, resulting in much
tidier file headers.

I entirely fail to understand why we need to slap a FileCopyrightText
on files. The copyright surely applies whether or not I put the
FileCopyrightText there. The list is also just about always
incomplete, further calling its use into question. Not to mention that
it is annoying book keeping of information that is already available
in git.

Can someone shed some light on this?

HS

Re: Kandalf: request for review

[Harald Sitter]

This isn't going to be very useful, but I'm sure I'm not the only one
who has heard of the story that the Tolkien Estate wasn't too happy
with our original kandalf. Consequently I'm not sure we should
actually name this thing kandalf. But then I wasn't around back then
so I don't know if there is any truth to it or if it's just an urban
legend. Regardless it's confusing to recycle the name for something
entirely different and I'd reconsider the name here. Food for thought.

On Thu, Dec 7, 2023 at 7:10 PM Loren Burkholder
 wrote:
>
> Hi all!
>
> I've been working towards getting Kandalf ready for integration into KDE (and 
> I'd like to thank everyone who has pitched in to help me, mainly Laurent and 
> redstrate). At this point, while the app is not necessarily feature-complete 
> or fully polished, it's getting close to that point. Therefore, I'd like to 
> officially request a review of Kandalf for inclusion in the KDE apps.
>
> You can reach me on Matrix at @lorendb:nheko.im for more instant 
> communications.
>
> Thanks,
> Loren Burkholder

Re: per-issue notifications from sentry

[Harald Sitter]

I've removed all of the default alert rules. Everyone should be able
to setup rules as they see fit though, don't spam other people though
:)

HS

On Sun, Nov 26, 2023 at 3:38 PM Harald Sitter  wrote:
>
> does anyone else find the emails from sentry when a new issue appears
> to be rather useless? they are very noisy and often times not
> interesting since a single crash often doesn't have enough context to
> act on anyway.
>
> should I look into disabling them?
>
> HS

per-issue notifications from sentry

[Harald Sitter]

does anyone else find the emails from sentry when a new issue appears
to be rather useless? they are very noisy and often times not
interesting since a single crash often doesn't have enough context to
act on anyway.

should I look into disabling them?

HS

CI log verbosity

[Harald Sitter]

What are your thoughts on having the CI be less verbose by default and
instead have an env var or some other toggle to switch into verbose
mode?

Specifically I'm talking about the qtlogging rules that are currently
enabling everything and the kitchen sink. To my mind we should just
use the default rules by default.
I find that 99% of the time the output is entirely useless in finding
what is wrong, if anything it gets in the way because I first have to
find where the test failure is and then instead of reading walls of qt
plugin info I will just proceed to reproduce the problem locally
anyway.

Thoughts?

HS

Re: drkonqi's many debuggers

[Harald Sitter]

On Tue, Aug 29, 2023 at 5:23 AM Thiago Macieira  wrote:
>
> On Monday, 28 August 2023 12:59:01 PDT Adriaan de Groot wrote:
> > (puts on FreeBSD hat) On the non-GNU side of the world, lldb is the thing
> > that is "installed anyway" and gdb takes extra effort. Though I don't know
> > if the lldb integration works -- I have both installed, and I don't know if
> > I ever bother to interact with DrKonqi (sorry).
>
> True, but the majority of our user base is still Linux, so if we had to choose
> only one, it would have to be gdb.

I am not quite following. We can depend on any debugger we want,
surely? I mean, there are practical implications to consider for sure,
but gdb being the dominant debugger on Linux doesn't prevent us from
depending on lldb instead.

HS

drkonqi's many debuggers

[Harald Sitter]

I am wondering: does anyone actually use anything besides the default
gdb debugger? We have a lot of code just for supporting multiple
debuggers and I am wondering if we couldn't just focus on one debugger
and get less code with a better experience (both in writing and
reading it).

On a related note: does anyone have opinions on using lldb instead of
gdb? It seems much faster to me and should be just as scriptable as
gdb from a quick glance. The trace format would change though, so
that's a challenge. OTOH texty traces are a thing of the past with
sentry anyway.

HS

Re: sentry evaluation

[Harald Sitter]

The problem you are describing is because of the evaluative nature of
the current setup, where I was asked to force the entire system to
have limited exposure. In full rollout every report that the user
manually writes in drkonqi will also end up on sentry. Every
additional crash also, so long as the user enabled auto-submission
(that's the current plan anyway). i.e. Sentry would be the superset of
all crashes. We can look into tighter linking between bugzilla and
sentry but, again, that hasn't happened because of the evaluative
nature of things.

HS

On Thu, Jun 1, 2023 at 10:44 PM Nate Graham  wrote:
>
> To be honest, I haven't found Sentry to be that useful in its current
> implementation. The primary issue is that it represents a second source
> of truth for where crash reports live. As a result, developers who
> already struggle to notice Bugzilla-based crash reports have to look in
> a second place, further diving their scarce attention. I haven't seen
> evidence of people regularly interacting with it or looking at its
> crashes beyond the excitement of the initial rollout, so now it's
> largely just a new graveyard of issues being ignored due to insufficient
> developer time.
>
> I think Sentry could make sense to keep if it was implemented for us
> more as a kind of automatic symbolication service that can take a bad
> backtrace, add debug symbols, retrace it, and then pass *that* off to
> DrKonqi so that the resulting Bugzilla ticket is guaranteed to have a
> *good* backtrace in it. That's a real problem we currently have that
> could benefit from being solved in a targeted way.
>
> If we can't or don't want to do that, then Sentry might make sense if it
> were possible for us to bypass the "multiple sources of crash truth"
> issue by completely disabling Bugzilla for crash reports and migrating
> old Bugzilla crashes into sentry. But Then we'd run into the new issue
> that Sentry doesn't permit discussion with the person experiencing the
> crash to ask for more details if needed. This isn't always needed, but
> it sometimes is. Sentry also isn't integrated into our issue priority
> tracking system in Bugzilla, but that's a more minor issue.
>
> Nate
>
>
>
> On 6/1/23 04:35, Harald Sitter wrote:
> > Hey,
> >
> > We've had almost a year, albeit in a super limited capacity for git
> > builds, with sentry (https://errors-eval.kde.org) and we should
> > probably render a final verdict on the evaluation.
> >
> > Did you like it?
> > What could be improved?
> > Should we move ahead with a more permanent setup?
> >
> > The overall game plan would be to have drkonqi ask the user to opt
> > into automatic crash submission when they open drkonqi so we get close
> > to all crashes (the ones caught by kcrash) automatically filed into
> > sentry from then on out. To increase exposure of this feature I'd also
> > like to glue it into the feedback KCM but I'm not yet sure if it
> > should be a separate setting or linked to the regular feedback
> > categories, the former sounds more accessible. The current bugzilla
> > based workflow would still be available for when the user actually
> > wants to write a description.
> >
> > (previous discussion: https://markmail.org/thread/6y5paczdposz3aoj)
> >
> > HS

Re: KSvg in kdereview

[Harald Sitter]

LGTM now +2

On Wed, Jun 21, 2023 at 10:04 AM Marco Martin  wrote:
>
> I fixed CI, passes now
>
> On Tue, Jun 20, 2023 at 8:44 PM Ben Cooksley  wrote:
> >
> > Hi all,
> >
> > Sysadmin has just received a request to move this repository to Frameworks, 
> > however after seeing some of the comments raised here regarding the 
> > repository I took a look myself to see if they had been corrected.
> >
> > At this time CI is still broken in KSvg, for both platforms - something 
> > that was mentioned in an earlier email here.
> > This does not inspire confidence that the code is up to scratch.
> >
> > I've therefore declined to move it to Frameworks at this time.
> >
> > Would be appreciated, given this is looking to be promoted to Frameworks, 
> > if people could please have a further look at this repository and comment 
> > as appropriate.
> >
> > Thanks,
> > Ben
> >
> > On Wed, Jun 14, 2023 at 9:12 PM Marco Martin  wrote:
> >>
> >> Hi all,
> >> Some time has passes, and changes have been done in the repo to
> >> address some of the points.
> >> Now there are review requests in plasma-framework which depends on
> >> this repo (and accompanying plasma-workspace, plasma-desktop etc)
> >> It would probably be better to have this in frameworks to have the
> >> rest depending from it?
> >>
> >> On Thu, Apr 20, 2023 at 10:25 AM Marco Martin  wrote:
> >> >
> >> > Hi all,
> >> > A part of plasma-framewrok, which is the one to do SVG-based themes,
> >> > has now been splitted in a standalone library which is intended to be
> >> > a new framework in KF6 (all usages of the plasma-framework version
> >> > will be ported once this officially lands, and then those classes will
> >> > be removed)
> >> > The repo for now lives in
> >> > https://invent.kde.org/libraries/plasmasvg
> >> >
> >> > In the end it will be renamed in ksvg
> >> >
> >> > Comments? reviews?
> >> >
> >> >
> >> > --
> >> > Marco Martin
> >>
> >> --
> >> Marco Martin

Re: sentry evaluation

[Harald Sitter]

On Mon, Jun 12, 2023 at 11:39 PM David Edmundson
 wrote:
> I'm not sure the teams and groups really work out right now. Do we
> need to request access to groups and have that approved?

That is how it used to be, yes. I've just sprinkled some code at the
instance that should allow us to forgo the approval system though,
only KDE developers can now log in (which is what the approval was
based on anyway).

The way this works in general is that "issues" get sent to a "project"
and that project can be part of multiple teams but to get access to
the data the developer has to opt into a team with access to a given
project first. Alas, I don't think we'll get around the opt-in.
Someone who's only interested in e.g. kwin would get annoyed by/ignore
the noise caused by e.g. ktuberling. That said, projects can be part
of multiple teams and we can have as many teams as we feel like.

> There was also a minor issue that a plasma bug might have a frameworks
> or Qt fix, but I couldn't reliably say "fixed in next version", it's
> not a case which sentry had a good infra for. (bugzilla also isn't
> amazing at that)

Indeed. It's a complicated problem in particular because we are not
the binary distributor for regular linuxy builds. I don't really have
a good answer here unfortunately.

> >Should we move ahead with a more permanent setup?
>
> 100% has my vote. Can we do things on a per-project basis? We can
> surface things in the Plasma UI right now, and there's still a while
> before release to pivot if needed.

I suppose we could do things per-project. What advantage do you think
we gain from this? Everything routes through drkonqi anyway and at
that point it makes no difference if we are inspecting kate or
kinfocenter.

HS

Re: sentry evaluation

[Harald Sitter]

On Sun, Jun 11, 2023 at 7:13 PM Albert Vaca Cintora
 wrote:
>
> I didn't know we had Sentry! How can we get crash reports from KDE
> Connect through it?

Because this isn't widely rolled out yet, due to evaluation period,
there's only limited data for kdeconnect. Here's one:
https://errors-eval.kde.org/organizations/kde/issues/119/

(currently kdeconnect goes into the fallthrough project, which is
because of insufficient configuration on my part)

HS

Re: sentry evaluation

[Harald Sitter]

On Mon, Jun 5, 2023 at 12:20 AM Albert Astals Cid  wrote:
>
> El dijous, 1 de juny de 2023, a les 12:35:41 (CEST), Harald Sitter va
> escriure:
> > Hey,
> >
> > We've had almost a year, albeit in a super limited capacity for git
> > builds, with sentry (https://errors-eval.kde.org) and we should
> > probably render a final verdict on the evaluation.
> >
> > Did you like it?
>
> Did I miss the email announcing this?

There may have been no email. I've announced it at akademy and blogged about it
https://apachelog.wordpress.com/2022/10/13/kde-crash-tracking-system-%f0%9f%92%a3/

> By clicking on https://errors-eval.kde.org i can't really seem to see anything
> to evaluate either, all it deos is it asks me to join a team.

Yep. To get access to projects you need to be member of a team with
access to that project. E.g. if you join the #plasma team you get
access to the plasma projects. #community has access to everything.

HS

sentry evaluation

[Harald Sitter]

Hey,

We've had almost a year, albeit in a super limited capacity for git
builds, with sentry (https://errors-eval.kde.org) and we should
probably render a final verdict on the evaluation.

Did you like it?
What could be improved?
Should we move ahead with a more permanent setup?

The overall game plan would be to have drkonqi ask the user to opt
into automatic crash submission when they open drkonqi so we get close
to all crashes (the ones caught by kcrash) automatically filed into
sentry from then on out. To increase exposure of this feature I'd also
like to glue it into the feedback KCM but I'm not yet sure if it
should be a separate setting or linked to the regular feedback
categories, the former sounds more accessible. The current bugzilla
based workflow would still be available for when the user actually
wants to write a description.

(previous discussion: https://markmail.org/thread/6y5paczdposz3aoj)

HS

Re: Moving FutureSQL to KDE review

[Harald Sitter]

On Fri, Mar 24, 2023 at 12:23 PM Kevin Kofler  wrote:
>
> Jonah Brüchert wrote:
> > I would like to maintain it as an independent library for now, to be
> > able to use C++20 features […]
>
> What is the point of allowing to build KDE Frameworks with an older C++
> standard (what is the minimum these days?) if "many KDE projects" are going
> to depend on a library that requires C++20 to build? Either we can use C++20
> or we cannot.

KDE frameworks is used by people other than us.

what to do with phonon & why aren't you using it?

[Harald Sitter]

Hola!

Why don't you use phonon?

What would it make it useful?

What do you reckon we should do with it?

A bit of background perhaps: wy back when phonon was conceived as
multimedia abstraction library to serve as multimedia pillar. While it
is certainly abstract and a library I'm not so sure about the pillar
aspect ;) In fact, phonon is barely even an abstraction since I only
maintain the vlc backend and nobody else maintains any other backend.
So, one has to wonder if it'd not make more sense to put the bit of
energy that flows into phonon to instead go into qtvlc and not have to
deal with the abstraction element at all. Indeed all these
abstractions seem a bit silly because the multimedia libraries
gstreamer and vlc are already abstractions... it's nesting dolls all
the way down.
With all that in mind I was prototyping a revisit of the API many
years ago. Away from complicated mediagraph pipeline building
abstraction to simple player abstraction (you have a player, you give
it a url, it plays the url, that's it), as indeed that seems to be
what most remaining users want to do these days - hassle free
playback. Perhaps that is a path to take? But then the question
remains, do we need the abstraction at all?

Any and all thoughts welcome.

HS

kde-inotify-survey in kdereview

[Harald Sitter]

https://invent.kde.org/system/kde-inotify-survey

simple kded that watches inotify resources and informs the user when
limits are getting hit (along with a way to bump them slightly)

thanks for your time!

HS

Re: kio-admin in kdereview

[Harald Sitter]

On Sat, Oct 15, 2022 at 9:29 PM Albert Astals Cid  wrote:
>
> El divendres, 14 d’octubre de 2022, a les 10:34:04 (CEST), Harald Sitter va
> escriure:
> > On Thu, Oct 13, 2022 at 10:32 PM Albert Astals Cid  wrote:
> > > El dijous, 13 d’octubre de 2022, a les 1:03:53 (CEST), Harald Sitter va
> > >
> > > escriure:
> > > > On Thu, Oct 13, 2022 at 12:46 AM Albert Astals Cid 
> wrote:
> > > > > Did I misunderstood the code? It looks like this run all of kio with
> > > > > root
> > > > > powers?
> > > >
> > > > That is correct
> > >
> > > That feels like a reasonably big no no with my security hat.
> > >
> > > I'm relatively sure we have not audited all of KIO and it's dependencies
> > > to be "running as root"-safe.
> >
> > It is scary to be sure, but then the user has to opt into shooting in the
> > foot.
>
> How much of that opt in message mentions potential security issues?

None. Just like with kdesu and kdesudo it's merely by virtue of the
authentication dialog that the user opts into any security concerns.

HS

Re: kio-admin in kdereview

[Harald Sitter]

On Thu, Oct 13, 2022 at 10:32 PM Albert Astals Cid  wrote:
>
> El dijous, 13 d’octubre de 2022, a les 1:03:53 (CEST), Harald Sitter va
> escriure:
> > On Thu, Oct 13, 2022 at 12:46 AM Albert Astals Cid  wrote:
> > > Did I misunderstood the code? It looks like this run all of kio with root
> > > powers?
> >
> > That is correct
>
> That feels like a reasonably big no no with my security hat.
>
> I'm relatively sure we have not audited all of KIO and it's dependencies to be
> "running as root"-safe.

It is scary to be sure, but then the user has to opt into shooting in the foot.

> What's the use case of this against the kauth support in file_unix.cpp ?

The latter doesn't exist :(

HS

Re: kio-admin in kdereview

[Harald Sitter]

On Thu, Oct 13, 2022 at 12:46 AM Albert Astals Cid  wrote:
> Did I misunderstood the code? It looks like this run all of kio with root
> powers?

That is correct

kio-admin in kdereview

[Harald Sitter]

Hola

kio-admin implements an admin worker that gives root level access to
the file system

https://invent.kde.org/system/kio-admin

HS

Re: KJournald in KDE-Review

[Harald Sitter]

Neat!

- clazy has some hints
- you could probably use
https://api.kde.org/ecm/module/ECMQtDeclareLoggingCategory.html for
loggincategory.cpp/h?
- for qml you should probably use Kirigami.Units.gridUnit*N instead of
hardcoding sizes
- the hardcoded colors also seem a bit of an odd choice
- they are done already but for future reference you might want to
prefer i18nc over i18n, in particular for single word string such as
`i18n("Url:")` (which btw is also incorrectly capitalized)
- talking about... TopMenuBar doesn't consistently use Title Capitalization
- may be of interest to shove the raw journal pointer into a
unique_ptr asap:
https://invent.kde.org/plasma/drkonqi/-/blob/master/src/coredump/memory.h

On Sun, Oct 9, 2022 at 7:18 PM Andreas Cord-Landwehr
 wrote:
>
> Hi, after a few releases over the past year, I would like to get KJournald
> included in KDE application releases. This project is about providing both an
> QItemModel abstraction library for the C-style journald API and providing an
> efficient graphical browser for journald logs.
>
> Sysadmins moved the repository to KDE Review today, you can find the source
> code here:
>
> https://invent.kde.org/libraries/kjournald
>
> (flatpack packaging is also available for easy trying it out)
>
> Even though KJournald is currently contained in the "libraries" playground
> module, I would like to get it included in the "utilities" module after
> passing KDE Review. The reason is that at the moment I more focus on the
> application part and that is the most user-facing part. Having it in
> "utilities" thus will avoid confusion.
>
> Looking forward for your review comments!
>
> Best regards,
> Andreas
>
>

Re: Plasma Welcome Center on KDEReview

[Harald Sitter]

plasma-disks is a fairly exhaustive example code base

On Tue, Sep 20, 2022 at 2:19 AM Nate Graham  wrote:
>
> On 9/18/22 11:03, Harald Sitter wrote:
> > Not all code is licensing policy compliant (e.g. appdata is missing
> > spdx tags). I'd suggest enabling the reuse linter pipeline.
>
> How do I do this? Is there an example I can copy from elsewhere (which
> in case people haven't noticed, is basically how I do everything)?
>
> Nate

Re: Plasma Welcome Center on KDEReview

[Harald Sitter]

Not all code is licensing policy compliant (e.g. appdata is missing
spdx tags). I'd suggest enabling the reuse linter pipeline.

On Fri, Sep 16, 2022 at 6:00 PM Nate Graham  wrote:
>
> Hello folks!
>
> I've been working with Aleix on an onboarding wizard for Plasma, based
> on work originally started by Felipe Kinoshita last year.
>
> You can see some screenshots at
> https://invent.kde.org/websites/product-screenshots/-/commit/e300be66e62263c63d8a85ba391bbcc1de691148.
>
> I'd like to target Plasma 5.27 for release and am submitting it for
> KDEReview. The repo is at https://invent.kde.org/plasma/welcome-app.
>
>
> Nate

Re: Putting Flatpak KCM through kdereview

[Harald Sitter]

Amazing!

For future reference: there is a sanity check list linked at
https://community.kde.org/Policies/Application_Lifecycle#kdereview
that you can follow to do a pre-review yourself and find most common
issues.

- Messages.sh missing (I see there is an MR pending)
- you should enable CI and in particular also enable the reuse
template to ensure reuse compliance moving forward
- the KCM name has a "Settings" suffix, that seems rather uncoming,
I'd remove it
- clazy is not happy with some stuff, I suggest you do a compile run
with it and fix its complaints
- missing a bugzilla product (request one via sysadmin ticket)
- missing an appstream file
- I have a rather large margin on the right hand side of the permissions view :O
- please always use i18nc() rather than i18n() for new strings and
provide suitable context. in particular for one-word strings... to a
translator it won't at all be obvious in what context "features"
appears or what "pcsc" refers to

All in all really good stuff.

HS

Re: Plasma Remote Controllers in KDE Review

[Harald Sitter]

- src dir is missing i18n setup (messages.sh and define in cmakelists)
- not reuse covered
- metainfo.xml missing (appstream)
- doesn't have gitlab ci builds apparently?
- doesn't have bugzilla product
- clazy is not happy

On Fri, Aug 26, 2022 at 12:21 PM Aditya Mehra  wrote:
>
> Hi,
>
> Plasma remote controllers is in KDE Review and would like to release it along 
> with Plasma Bigscreen
>
> Plasma remote controllers allows translating various input device events into 
> keyboard and pointer events send from remote control type devices like CEC 
> and Gamepads to provide key navigation support in applications. It also 
> allows re mapping of keys to specific events.
>
> You can find the repository here: 
> https://invent.kde.org/plasma-bigscreen/plasma-remotecontrollers
>
> Request to please review Plasma remote controllers.
>
> Regards,
> Aditya

Re: Plasma Bigscreen is in kdereview again

[Harald Sitter]

- not fully reuse compliant. much sad but probably can't be helped
this far into the project :((
- kdeconnect.h has include guards for wifi.h
- kcm_mediacenter_bigscreen_settings may be missing a -DTRANSLATION_DOMAIN
- I would suggest you build with clazy. there are numerous possible
container detachments you could prevent. also many other reasonable
warnings
- you appear to not have an appstream file for the product. is that intentional?

I've only glanced over the code but it all seems fairly reasonable.
HS


On Fri, Jun 24, 2022 at 2:00 PM Aditya Mehra  wrote:
>
> Bump!
>
> Plasma Bigscreen has been in review for a while, can it please be reviewed or 
> released assuming no problems have been detected ? Request to please take 
> this forward as we would like to get proper stable packaging for it with 
> distributions with the next plasma release.
>
> Regards,
> Aditya
> 
> From: Aditya Mehra
> Sent: Friday, May 6, 2022 1:53 AM
> To: kde-core-devel 
> Subject: Plasma Bigscreen is in kdereview again
>
> Hello,
>
> Plasma Bigscreen is in KDE Review again, it was in review the last time and 
> as development went on, we did not leave playground and the review was 
> dropped on our part, sorry. I would like to request you to please review 
> Plasma Bigscreen again as we would like to make a release for it. The 
> repository url is https://invent.kde.org/plasma/plasma-bigscreen
>
> Plasma Bigscreen project consist of a containment for Plasma aimed at a Smart 
> TV interface, it is host to a grid based tile launcher for applications and 
> some specific kcms like wifi, audio devices, launcher settings, and kde 
> connect which can all be navigated with just a usb based remote, hdmi cec 
> connected tv remote, kde connect bigscreen plugin or arrow key navigation on 
> a keyboard.
>
> The project is also integrated with the Mycroft voice assistant open source 
> project, to supports voice commands and Mycroft skills QML interface for 
> providing graphical voice applications.
>
> Regards,
> Aditya

Re: Eloquens now on KDEREVIEW)

[Harald Sitter]

On Wed, Jun 22, 2022 at 12:07 AM Felipe Kinoshita  wrote:
>
> > Could you elaborate why your config.kcfg uses Ints for everything when
> > you clearly want booleans (e.g. `Config.code == 1 ? true : false`)
>
> The API expects ones and zeros for its params, I chose to convert them to
> booleans to make the API call easier to write and change.

Ah! I would suggest moving the conversion into the Controller then. As
far as kcfg, your Config object and your Settings.qml are concerned
they can be proper bools, it's only in the Controller that you have
the presentation requirement that bools must be 0/1. This saves you
the two-way conversion, in the Controller you only need to convert
bool=>int and the rest of the app can treat them as proper bools.

HS

Re: Eloquens now on KDEREVIEW

[Harald Sitter]

On Mon, Jun 20, 2022 at 10:30 PM Felipe Kinoshita  wrote:
>
> For those who don't know, Eloquens is a simple application targeted at 
> developers/designers, it generates the lorem ipsum text and allows you to 
> customize it a little bit like adding heading, bullet lists, etc...
>
> I would also like to ask if it would be possible for it to be released along 
> with KDE Gear.

Neat!

Link for would be reviewers https://invent.kde.org/sdk/eloquens

I mostly have some nit picks:

Your saveWindowGeometryTimer  seems like a huge hack. Why not simply
disable the Connections object when the window is not visible (or only
turn it enabled when the window is fully initialized). There is
absolutely no guarantee that the window will be in a good state after
1 second. It may be 5 it may be 10 it may be 30.

In main.qml you really shouldn't hardcode font.family: "Liberation
Serif";. If you want a serif font then simply use `serif`.

Could you elaborate why your config.kcfg uses Ints for everything when
you clearly want booleans (e.g. `Config.code == 1 ? true : false`)

In Controller::fetch() you can declare and initialize `reply` in the
same line. For the connect in there should pass `this` as third
argument (not passing a context object is usually bad practice cause
the lambda may get called past the captured variable's life)

In App your destructor is unnecessary. You can remove it entirely - it
violates the rule of five. There's also a dangling private: in the
header.

You might want to consider using the reuse-lint template to assert
that reuse compliance doesn't regress
https://invent.kde.org/plasma/plasma-disks/-/blob/master/.gitlab-ci.yml

some desktop file validation: (the last point is because
SingleMainWindow isn't actually a valid key, you should remove it I
guess)

org.kde.eloquens.desktop: hint: value "Qt;KDE;Development;Utility;"
for key "Categories" in group "Desktop Entry" contains more than one
ma
in category; application might appear more than once in the application menu
org.kde.eloquens.desktop: error: file contains key "SingleMainWindow"
in group "Desktop Entry", but keys extending the format should start
with "X-"

Re: Git merge workflow: reverse it?

[Harald Sitter]

I'm sure we can, it's one of the technical pillars of the manifesto :)

https://manifesto.kde.org/commitments/

On Tue, May 10, 2022 at 10:59 AM Ingo Klöcker  wrote:
>
> On Dienstag, 10. Mai 2022 09:36:17 CEST Kai Uwe Broulik wrote:
> > can we get an update on this?
> >
> > Plasma is full on cherry-pick now but in KDE Gear it's inconsistent and
> > frustrating when some projects use cherry-pick (e.g. Kate) and some
> > don't (e.g. Dolphin).
> >
> > I don't really mind what the outcome is but I need it to be consistent
> > and predictable where to target my MRs, at least for every domain
> > (Plasma, Gear, ..).
>
> I can imagine a consistent rule for Frameworks (if that's what you mean by
> "domain"). Apart from that I can only imagine a consistent rule per
> development team, e.g. Plasma team or PIM team, but not for Gear as a whole
> which is just a random mix of the products of different teams with different
> workflows. Of course, this doesn't stop the different development teams from
> adopting the same merge workflow. But this cannot be forced on all development
> teams.
>
> Regards,
> Ingo

Re: KSANECore in KDE review

[Harald Sitter]

On Sun, Mar 27, 2022 at 6:29 PM Alexander Stippich  wrote:
>
> Hello everyone,
>
> KSANECore is now in KDE review. Kåre and I mentioned it in previous emails
> before, but as a short summary:
> KSANECore is a Qt interface to the SANE scanner library. It is stripped out of
> the KSaneWidget of libksane without any QWidget dependency. It is currently
> located inside the libksane repository as KSaneCore and basically just copied
> into the new repository.
>
> Due to breaking API anyway, the code was cleaned up, better named as well as
> smaller API fixes made on top. Also, KSANECore is fully REUSE compliant.
> KSaneWidget of libksane will remain ABI compatible.
>
> I don't know if it is strictly required to move the new repo with already
> existing code through KDE review, but I guessed it is better to be on the safe
> side :)
>
> The plan is to switch libksane and Skanpage over to the new library during the
> KDE Gear 22.08 release cycle. The adaptions are located at
> https://invent.kde.org/astippich/skanpage/-/commits/ksanecore
> and
> https://invent.kde.org/astippich/libksane/-/commits/ksanecoreSplit

amazing!

for everyone's convenience here's the url to the lib ;)
https://invent.kde.org/libraries/ksanecore


some comments from scrolling through the code:

you may want to reconsider how stringEnumTranslation works
https://github.com/KDE/clazy/blob/master/docs/checks/README-non-pod-global-static.md
either use q_global_static, or - IMHO neater - move it into the
function loadDeviceOptions as function local static since we don't
need it outside that function anyway.

CoreInterface, CoreInterfacePrivate, InternalOption, ScanThread and
CoreOption should have their constructors marked explicit

the typedefs in coreoption.h are super old school maybe modernize them? ;)

some of the API documentation still refers to libksane, should that be changed?

on a similar note, you still use the KSane namespace and include
guards. It may make sense to rename them KSaneCore and KSANECORE
respectively? for consistency if nothing else

if you havent considered this: you might want to use the clang-format
rules from ECM to join the common formatting we have (and apply that
via commit hooks)

I would argue that reloadDevicesList and getOption(const OptionName
optionEnum); should have their const dropped. the const there doesn't
impact the signature and as such is confusing from an API POV

on a matter of less code noise I would use std::make_unique when
creating new unique ptrs instead of manually passing a raw pointer to
the ctor.

in CoreInterfacePrivate m_batchMode + Delay are uninitialized in the
ctor. please initialize them nullptr

since you have Qt6 ifdefs you may want to enable qt6 CI as well

the shebang line in your Messages.sh appears to have lost its position
and is no longer at the top of the file

you appear to have an autotests dir and cmakelists but no actual tests? :O

Re: Should we consider project basenames unique?

[Harald Sitter]

On Wed, Nov 24, 2021 at 10:02 AM Ben Cooksley  wrote:
> Which means you either provide the path (plasma-mobile/plasma-dialer) or you 
> need to go look in the metadata anyway.

If names are unique (not persistent, just unique) and plasma-dialer is
what I want to release then I know plasma-dialer is called
plasma-dialer because I'm a plasma-dialer dev. I can then call
releasme with the argument  'plasma-dialer' and releaseme can work out
the path from that because the name is global unique so there is only
one plasma-dialer and that will be what I want to release.

HS

Re: Should we consider project basenames unique?

[Harald Sitter]

On Wed, Nov 24, 2021 at 6:00 AM Ben Cooksley  wrote:
>
> On Wed, Nov 24, 2021 at 2:29 PM Aleix Pol  wrote:
>>
>> On Wed, Nov 24, 2021 at 2:10 AM ash...@linuxcomp.ru  
>> wrote:
>> >
>> > Hello.
>> > There is currently a problem that projects are identified by path in 
>> > invent.kde.org. But in pkgbuilds in Arch Linux the makepkg clones repo 
>> > twice, first time to the dir with pkgbuild, and second time to the srcdir 
>> > for build package. And that second repo has first repo as origin, i.e. its 
>> > origin is path to the first repo. And this leads to breakage of 
>> > determining project identifier in cmake module. To workaround, I either 
>> > need to clone to the path that matches the regexp, such as 
>> > crutch.kde.org:pim/zanshin.git or fixing the origin of the problem.
>> >
>> > I wanted to fix it by identifying the project by its name. See 
>> > https://invent.kde.org/sdk/releaseme/-/merge_requests/13.
>> >
>> > Now, the question is, could we consider project names unique? In that 
>> > case, we can merge my mr, and path crutch will not be needed anymore.
>>
>> I agree it would be a problem to have different repositories with the
>> same name. I'm not sure how we can enforce it though.
>
>
> This is something that Sysadmin already encountered and resolved.
>
> Please see the 'identifier' key in the repository metadata (located at 
> https://invent.kde.org/sysadmin/repo-metadata/ in the projects/ subfolder) 
> which we guarantee to be unique.
> This is already used and relied upon by scripty as well as our internal 
> systems (such as commits.kde.org) when referencing repositories.

That is the problem. We, the humans, do not know the identifier nor
should we care. When I want to make a release of plasma-dialer I
should be able to pass plasma-dialer as argument because I know the
repo/project is called plasma-dialer.

> The repository name itself (stripped of any folders) is not guaranteed to be 
> unique.

That is what is proposed to get changed.

HS

Towards Excellent Defect Management

[Harald Sitter]

For many years now I've been unhappy with both the quality and volume
of crash reports we get for our software. The barrier for crash report
submissions is incredibly high because we've never really had tech to
help elevate "insufficient" reports to become sufficient, outside the
client on which the crash occurred. Out of the very few people that
might want to report a crash even fewer will get beyond the first set
of questions from drkonqi, once they've managed they still have to
fight with their distro for debug symbols and quite possibly lose, and
even if they win there is a good chance the report will either get a
"this isn't very useful. install more symbols" comment or get marked
as dupe. Meanwhile we are spending our days looking at duplicated
crashes, or finding the right blurb to copy paste to ask for a better
trace, or try to find out why software crashes that hasn't actually
crashed for a year because the bug had already been fixed in the
meantime.

We are wasting our users' time. We are wasting our time. This waste
needs to stop.

The good news is that we have all the technical building blocks to fix
it today. In fact, it's even getting better in the future still. All
it takes is a bit of code and a bit of flexibility on our part.

A while ago I started looking into improving the drkonqi experience.
Specifically: submitting crash reports into a purpose built crash
tracking system rather than a bug tracking system. The advantages are
kind of obvious and ranging from server-side de-duplication to
server-side retracing. I've spent many afternoons reading up on and
poking demo instances of every somewhat suitable software I could
find, and Sentry looks like the best option for what we need. It is
practically free software as far as we are concerned, scales
tremendously, has systems for server-side deduplication, server-side
cross-distro/platform retracing (which might also help with some of
the open questions of richer tracing for windows and android), data
scrubbing (what with privacy concerns), client and server-side tags,
can try to figure out when a crash first appeared if supplied with
commit data, can track the quality of specific releases, when a given
crash was fixed, health reports, performance tracking, warning rules,
health report emails, ... I've been playing with it for a month and
still find amazing new things!

One of the best things about Sentry is that it has native support for
debuginfod, enabling us to get debug symbols directly from
distributions, solving the entire cross-distro aspect of crash tracing
in just about the neatest way possible. We get the (incomplete) trace
with lots of metadata, and Senty then uses the metadata to resolve the
symbols through the distros' debuginfod instances to give us a
complete trace.

Even better: with relatively minor adjustments to drkonqi we could use
it right now and get immediate advantage of server-side retracing! I
already have a blob of prototype code for drkonqi that piggybacks
Sentry submission onto the existing code such that we can have both
bugzilla and Sentry.

I am proposing that we roll out a Sentry instance for testing so we
can see if we want to fully embrace it.

You can get a general sense of the features at Sentry's demo instance
https://sentry.io/demo/sandbox/
Here's a code dump for drkonqi
https://invent.kde.org/sitter/drkonqi/-/commits/work/sentry

HS

Re: Skanpage moved to kdereview

[Harald Sitter]

On Wed, Aug 11, 2021 at 9:46 PM Alexander Stippich  wrote:
> > - getUnitString -> can that maybe use
> > https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html
>
> What do you have in mind here? getUnitString just matches the enum of libksane
> to a unit suffix string, it does not format the values in any way.

I was rather thinking that you can get the localized suffix out of
kformat. but I suppose there is actually no API for that. So...
nevermind.

> > - something is wonky with the "Scan area size" label in the options
> > window. it appears to be constructed from the string "Scan area size"
> > and the string ":". they should be one string though. indeed the other
> > options are one string :shrug:
>
> Hmm, how does this look? I don't see a difference.
> There actually is a subtle difference between how this string and the others
> are obtained: Scan area size is a custom option of libksane and is translated
> there, whereas the others are coming from SANE directly. However, they are all
> appended the same way with "%1:" in Skanpage.

You can only see that sort of thing with the x-test language. If the
label is already translated in libksane I guess that explains it and
is fine.

> > - not really a fan of the comboboxes and long options label in the
> > toolbar. it feels very crowded and with long scanner names the window
> > easily takes up more than half my screen width at minimum size
>
> There has been a discussion already to move the scanner name to the window
> title like it is done in Skanlite. This would save some space. Also, the
> comboboxes are currently not scaled to their content width. Then there would
> also be some possible savings.
> Personally I find the quick access that the comboboxes provide for the most
> important options very useful.

I totally agree that having stuff at the fingertips is ever so handy.
Perhaps also ask the VDG for input?
Dolphin for example has some view properties in the (smaller) bottom
bar rather than the regular toolbar. Perhaps that is something that
could work here.
In any event this isn't anything that holds up the review, just my 2 cents.

HS

Re: Skanpage moved to kdereview

[Harald Sitter]

it works amazingly well. good job! I mostly only have some nitpicky stuff

- since the code base is really close to
complete reuse coverage, it might be nice to push it over the finishing
line and then `reuse lint` it to not have it fall behind again
- your cmake code is GPL but policy-wise it should be BSD-ish. any
chance this can be relicensed?
- for stylistic consistency with other repos you should probably use
https://cmake.org/cmake/help/v3.16/module/FeatureSummary.html
- testconfig.h.in -> I think you want
https://doc.qt.io/qt-5/qtest.html#QFINDTESTDATA ;)
- getUnitString -> can that maybe use
https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html
- the scanner options window is not high enough by default on my
system (with Noto Sans 10pt as font)
- the options really ought to use an apply/cancel pattern. applying
changes on the fly feels very strange for a KDE app
- something is wonky with the "Scan area size" label in the options
window. it appears to be constructed from the string "Scan area size"
and the string ":". they should be one string though. indeed the other
options are one string :shrug:
- not really a fan of the comboboxes and long options label in the
toolbar. it feels very crowded and with long scanner names the window
easily takes up more than half my screen width at minimum size
- I'm not sure if that is the code's fault (it certainly looks
correct) but in french the documentlist.qml bottom label reads `1
page` when there are no pages

HS

Re: Haruna in kdereview

[Harald Sitter]

On Wed, Jul 21, 2021 at 4:38 PM George Florea Banus
 wrote:
>
> On 21.07.2021 13:53, Harald Sitter wrote:
> > - the color-schemes dir seems to do nothing. it installs files that
> > don't actually exist in the source. fix it or rm -rf?
>
> It had the Breeze color schemes, but I removed them because I don't know
> their licenses
>
> https://bugs.kde.org/show_bug.cgi?id=434465

Good call. Maybe add a comment to the commented out option in the
cmakelists? There is also the question whether it is a good idea to
copy the schemes to begin with though. What's the use case behind
this?

> > - for the screenshots.md and the metainfo.xml you should consider
> > using our screenshot CDN instead
> > https://invent.kde.org/websites/product-screenshots
> Already submitted a merge request
> > - not sure how big of a concern that will be in practice, but you
> > should be careful with calling mimeTypeForFile, it is potentially
> > costly and a local path may still be backed by a network'd mount. when
> > in doubt it's probably better to check KFileItem::isSlow() first and
> > use mimeTypeForUrl when isSlow is true (or resolve the mimetype in a
> > qfuture/thread)
>
> mimeTypeForUrl says it will use mimeTypeForFile for local files. Is
> checking with KFileItem::isSlow() still necessary?

You are right, I am misremember. What you want to do is match on
extension only when the file is slow
https://doc.qt.io/qt-5/qmimedatabase.html#MatchMode-enum

Mind you, putting the mimetype resolution into a QFuture (and not
change the matchmode) still might be better choice over all since you
eventually create previews that will read form the file anyway. So, if
you first resolve a mimetype no harm is done, so long as you don't do
it on the qApp thread.

> Or is it still needed because "a local path may still be backed by a
> network'd mount"?

Yep, that is what KFileItem::isSlow() is telling us. If you always
resolve in a QFuture (even for local files) you don't really need to
check isSlow.

> > - the way static singletons are managed looks a bit old school.
> > function local statics might be neater
> > https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables
> >
> > Single *instance() { static Single s; return  }
> >
> > HS
>
> I searched a little about this and people also recommend disabling the
> copying and moving for singletons.
>
> Any opinions on that?

Sounds like a good idea ->
https://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY_MOVE in private
section of class should get that sorted

On Wed, Jul 21, 2021 at 4:38 PM George Florea Banus
 wrote:
>
> On 21.07.2021 13:53, Harald Sitter wrote:
> > It's an amazing app! Really awesome.
>
> Thank you.
>
> > - you might want to consider using a newer version in your
> > find_package call for ECM. ECM does enable more modern/useful features
> > but only when used with a suitably high version. so, I'd put this at
> > the actual minimal version you want to support
>
> Done.
>
> > - the color-schemes dir seems to do nothing. it installs files that
> > don't actually exist in the source. fix it or rm -rf?
>
> It had the Breeze color schemes, but I removed them because I don't know
> their licenses
>
> https://bugs.kde.org/show_bug.cgi?id=434465
>
> > - for the screenshots.md and the metainfo.xml you should consider
> > using our screenshot CDN instead
> > https://invent.kde.org/websites/product-screenshots
> Already submitted a merge request
> > - not sure how big of a concern that will be in practice, but you
> > should be careful with calling mimeTypeForFile, it is potentially
> > costly and a local path may still be backed by a network'd mount. when
> > in doubt it's probably better to check KFileItem::isSlow() first and
> > use mimeTypeForUrl when isSlow is true (or resolve the mimetype in a
> > qfuture/thread)
>
> mimeTypeForUrl says it will use mimeTypeForFile for local files. Is
> checking with KFileItem::isSlow() still necessary?
>
> Or is it still needed because "a local path may still be backed by a
> network'd mount"?
>
> > - Application::aboutApplication: we do have an AboutPage in klirigami
> > these days, maybe check that out so you can get rid of the qwidget
> > dialog
>
> Done.
>
> > - you should codify the runtime dep on youtube-dl via cmake. make a
> > dummy finder and set_package_properties it as type RUNTIME e.g.
> > https://invent.kde.org/plasma/plasma-disks/-/blob/master/cmake/Findsmartctl.cmake
> Done.
> > some further nitpicks:
> > - _debug.h should actually be superfluous as qdebug can inject context
> > on its own https://doc.

Re: Haruna in kdereview

[Harald Sitter]

It's an amazing app! Really awesome.

- you might want to consider using a newer version in your
find_package call for ECM. ECM does enable more modern/useful features
but only when used with a suitably high version. so, I'd put this at
the actual minimal version you want to support
- the color-schemes dir seems to do nothing. it installs files that
don't actually exist in the source. fix it or rm -rf?
- for the screenshots.md and the metainfo.xml you should consider
using our screenshot CDN instead
https://invent.kde.org/websites/product-screenshots
- not sure how big of a concern that will be in practice, but you
should be careful with calling mimeTypeForFile, it is potentially
costly and a local path may still be backed by a network'd mount. when
in doubt it's probably better to check KFileItem::isSlow() first and
use mimeTypeForUrl when isSlow is true (or resolve the mimetype in a
qfuture/thread)
- Application::aboutApplication: we do have an AboutPage in klirigami
these days, maybe check that out so you can get rid of the qwidget
dialog
- you should codify the runtime dep on youtube-dl via cmake. make a
dummy finder and set_package_properties it as type RUNTIME e.g.
https://invent.kde.org/plasma/plasma-disks/-/blob/master/cmake/Findsmartctl.cmake

some further nitpicks:
- _debug.h should actually be superfluous as qdebug can inject context
on its own https://doc.qt.io/qt-5/qtglobal.html#qSetMessagePattern
- Application::formatTime might want to use kformat::formatduration
https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html
- the way static singletons are managed looks a bit old school.
function local statics might be neater
https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables

Single *instance() { static Single s; return  }

HS

Re: KHealthCertificate in kdereview

[Harald Sitter]

My only gripe, besides what Albert already pointed out, is that all
the properties are WRITEable but have no NOTIFY signal nor are they
CONSTANT. One of those things ought to change. Considering only the
certificate objects have properties and they are always constructed by
the parsers I guess you could just drop the WRITE and make them
CONSTANT?

There is a somewhat different approach, that I only bring up because I
do enjoy meta programming way too much: Currently the parsers have
exhaustive if-else constructs to map incoming fields to setters. What
you could do is make static maps from incoming keys to property key
and then write the property filing in an abstract fashion e.g. to
illustrate:

> static QMap propertyMap{{"tg", "disease"}, {"fr", 
> "dateOfPositiveTest"}, ...};
> while (reader.hasNext()) {
> cert.setProperty(propertyMap.value(key), CborUtils::readString(reader));
> // needs some error handling when key mapping fails and possibly type 
> conversion helpers
> }

Then the WRITE would make sense and I'd use a single "changed()"
signal to NOTIFY on all the properties. drkonqi's bugzilla library
does something like that to model API objects for example.

HS

Re: AudioTube in KDEReview

[Harald Sitter]

On 16.06.21 21:27, Jonah Brüchert wrote:
>> - something is also wonky with the playlist. it doesn't cover the width
>> (when data fails to load). it is overlapped by the scrollbar. clear and
>> shuffle don't seem to have labels though there would be enough space
>> https://i.imgur.com/Q3IoaBj.png
> The left side should display the album cover, do you think the playlist
> should fill the whole page if there is none?

That would be an option. Alternatively what might make sense is show a
placeholdermessage [1] instead of the album cover and/or possibly an
icon as generic placeholder artwork. Not sure which makes more sense.
With a placeholder it'd certainly be more consistent from a visual POV.

https://api.kde.org/frameworks/kirigami/html/classorg_1_1kde_1_1kirigami_1_1PlaceholderMessage.html

>> - on some artists I get `TypeError: 'NoneType' object is not iterable `
>> https://i.imgur.com/U8BKHbQ.png
> Which exactly? I can't reproduce it with Daft Punk.

Ah sorry. It was the Thomas Bangalter entry. Curiously the search is
different today ^^. If you search for 'Bangalter' it happens with the
artist entry still.

HS

Re: AudioTube in KDEReview

[Harald Sitter]

Hola

On 16.06.21 00:44, Jonah Brüchert wrote:
> I would like to move AudioTube to KDEReview.

- the source is almost reuse compliant but not quite. needs some extra
data files equipped with CC-0 or the like [1][2]. You could then also
add an include on
https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-reuse.yml
to your gitlab-ci.yml to ensure it stays at complete coverage

- l10n is not correctly set up. You need to set a translation domain on
the KLocalizedContext

- app crashes when ytmusicapi isn't installed ;) maybe add a
find_package that simply tries to run a simple script importing it? tag
the package RUNTIME. that way the runtime dep is properly codified

- same for youtube_dl which seems to be required behind the scenes

- app isn't kcrash enabled :((

- seeking doesn't seem to do anything. I get `audiotube(103398)/(qml)
onMoved: Value: 71454.30987821381` as output but then the seek doesn't
actually happen

- something is also wonky with the playlist. it doesn't cover the width
(when data fails to load). it is overlapped by the scrollbar. clear and
shuffle don't seem to have labels though there would be enough space
https://i.imgur.com/Q3IoaBj.png

- on some artists I get `TypeError: 'NoneType' object is not iterable `
https://i.imgur.com/U8BKHbQ.png

- on the subject of errors. I'm not sure the passive popup overlapping
the controls at the bottom is all that nice

- appdata should probably describe the 

- ytmusic.cpp forces LC_ALL to en_us, that seems a bit fishy and it
doesn't explain why either

- VideoInfoExtractor ctor should be explicit

- PlaylistUtils looks like it maybe should be a namespace not a class.
feel free to ignore me if you disagree

- in asyncytmusic.h there's a typo in 'ininitalized'

- ~YTMusicThread be tagged `override`

- example has a type in 'excaustive'

- you may want to do a pass over all strings. I'm seeing some action
strings not using title capitalization (e.g. i18n("Add to playlist")
i18n("Play next")) [3]

[1] https://apachelog.wordpress.com/2021/04/06/reuse-licensing-helper/
[2]
https://community.kde.org/Guidelines_and_HOWTOs/Licensing#License_Statements_in_Non-Source-Code_Files
[3] https://develop.kde.org/hig/style/writing/capitalization/

Re: New repo in kdereview: kasts

[Harald Sitter]

On Sat, May 29, 2021 at 10:20 PM Bart De Vries  wrote:
>
> On 27/05/2021 15:09, Harald Sitter wrote:
> > - the source is almost reuse compliant but not quite. needs some extra
> > data files equipped with CC-0 or the like [1][2]. You could then also
> > add an include on
> > https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-reuse.yml
> > to your gitlab-ci.yml to ensure it stays at complete coverage
>
> We've made all source files reuse compliant with one exception: the dbus
> interface files.  We couldn't figure out which license to apply.
> Any pointers are welcome.

org.freedesktop.PowerManagement.Inhibit.xml can be simply CC0 with no
copyright holder. It's essentially auto-generated introspection data.
example at [1]

org.gnome.SessionManager.xml is a bit more complicated because it
contains exhaustive documentation that could possibly be considered
copyrightable.
I'm guessing it was excavated from gnome-session [2]? If so it's
probably GPL-2+ with gnome as copyright holder, **but** it may make
sense to send a mail to the maintainers to clarify the situation. You
could also replace this file with a rewrite: You really only need a
super trivial definition like
org.freedesktop.PowerManagement.Inhibit.xml, modeling merely the
Inhibit and Uninhibit methods. Neither the documentation nor the other
methods actually serve any purpose here anyway.

[1] 
https://community.kde.org/Guidelines_and_HOWTOs/Licensing#License_Statements_in_Non-Source-Code_Files
[2] 
https://gitlab.gnome.org/GNOME/gnome-session/-/blob/master/gnome-session/org.gnome.SessionManager.xml

> > - AudioManager deals a lot with time numbers. it'd aid readability
> > (and probably type safety) if you used std::chrono type and in
> > particular also chrono literals
>
> I'll have to have a closer look at this.  AudioManager is a wrapper
> around QMediaPlayer which is using qint64 for durations and positions.
> As such, the current implementation avoids type conversions by using
> qint64 everywhere for time numbers.
> While I agree that std::chrono might be a more logical choice here, I'm
> afraid it would also introduce a lot more type conversions to the
> underlying QMediaPlayer.

Fair point. I thought there's only the two setters where you interact
with QMP but since you also pass through signals from QMP it gets a
bit more complicated. I would still convert the types to get the
advantages of chrono but I fully understand that this isn't nearly as
neat as I had thought originally. Entirely up to you.

I now noticed your hack in AudioManagerPrivate::prepareAudioStream
though. I would strongly urge you to find another way of dealing with
this. Nested event loops (QEventLoop::exec) have shown to be fairly
crashy in the past, double so for QML application. If any event fires
(e.g. through a timer) that deletes an object or otherwise mutates
expected states out from under the current stack the application will
crash. QEventLoop::exec is really an anti-pattern most of the time
IMO.

HS

Re: New repo in kdereview: kasts

[Harald Sitter]

On Thu, May 27, 2021 at 12:20 PM Bart De Vries  wrote:
>
> Hello everyone!
>
> I would like to move kasts to kdereview.
>
> https://invent.kde.org/plasma-mobile/kasts  
> 
>
> Kasts is a podcast app for Plasma Mobile. It was started as a fork of 
> Alligator, the Plasma Mobile feed reader. It currently features a play queue, 
> play position resume/persistence, MPRIS2 support, and suspend inhibition.

Hey.
It's amazing. Also in incredible shape, well done!

Some stuff I've noticed:

- the source is almost reuse compliant but not quite. needs some extra
data files equipped with CC-0 or the like [1][2]. You could then also
add an include on
https://invent.kde.org/sysadmin/ci-tooling/raw/master/invent/ci-reuse.yml
to your gitlab-ci.yml to ensure it stays at complete coverage

- your appdata file has no screenshots defined

- since you require qt 5.15 you can use Qt::Core etc. targets instead
of Qt5::Core. this will make porting to qt6 less work

- the page stack between Settings and About is a bit wonky. they both
push onto layer stack rather the regular stack (I guess?) which seems
not exactly ideal considering from the way these two are presented in
the UI they should behave same as Queue and Episodes etc..
particularly problematic is however that if you switch between
settings and about you keep growing the page stack and the only way to
get back to the "real page" is to manually unwind the stack again

- also on the subject of the navigation pane: I believe the current
page ought to be highlighted in the navigation pane. when I'm drilled
down into a subpage of e.g. the Episodes page I wouldn't know that
this is where I came from other than through unwinding the stack
manually again

- something is wonky with the colors on desktop. in all listviews it
looks like the delegates are inactive/disabled colors for some reason.
e.g. https://i.imgur.com/xkgigs4.png

- I'm not super certain but also on that same page you might need to
turn the entire "by %1" sequence into a translatable string rather
than just "by". I don't think you can presume that every language has
that particular order? you might want to ask on the kde-i18n-doc
mailing list.

- the generic entry delegate hardcodes size formatting, this results
in my system usually talking about MiB everywhere but kasts is talking
about MB (yet indeed it still means MiB). You'll want to use kformat
[3]

- you may want to do a pass over all strings. I'm seeing some action
strings not using title capitalization (e.g. the actions in
GenericEntryDelegate.qml  - "Add to queue") [4]

- I see you are hardcoding a color in GenericHeader.qml, I'm not super
sure that will work correctly across different themes but then I also
don't really see the color actively used :shrug:

- there is a `// TODO: implement` in the main.qml. you might want to do that ;)

- AudioManager::timeString probably can be replaced by KFormat::formatDuration

- AudioManager has lots of commented out qdebugs. I suggest to either
remove them, or turn them into qCDebugs (so they may be
enabled/disabled at runtime)

- AudioManager deals a lot with time numbers. it'd aid readability
(and probably type safety) if you used std::chrono type and in
particular also chrono literals

- AudioManager uses some "old style" stringy connects
`loop.connect(, SIGNAL(timeout()), , SLOT(quit()));` these
SIGNAL() and SLOT() bits are only evaluated at runtime. To have the
compiler ensure the functions are valid I would very strongly suggest
to change them to function pointers like in the other connect calls.

- AudioManager also has some singleShot(0, lambda) calls. I believe
these are kind of unsafe since `this` might get deleted between the
call and the execution. you'll want to contextualize them
`singleShot(0, this, lambda)`

- QueueModel has an m_audio member that isn't ever set or used from
what I can tell

[1] https://apachelog.wordpress.com/2021/04/06/reuse-licensing-helper/
[2] 
https://community.kde.org/Guidelines_and_HOWTOs/Licensing#License_Statements_in_Non-Source-Code_Files
[3] 
https://invent.kde.org/plasma/plasma-nm/-/blob/9005f2580c37d5dbf32f1d264eb87d73ca723829/applet/contents/ui/ConnectionItem.qml#L258
[4] https://develop.kde.org/hig/style/writing/capitalization/

HS

Re: Qt5PatchCollection versions (broken QWE)

[Harald Sitter]

On Wed, May 26, 2021 at 7:39 PM Antonio Rojas  wrote:
>
> El miércoles, 26 de mayo de 2021 13:19:34 (CEST), Harald Sitter escribió:
> > Hola
> >
> > QWE and QtScript in our kde/5.15 branches currently have unaligned versions.
> >
> > QWE 5.15.4:
> > https://invent.kde.org/qt/qt/qtwebengine/-/blob/kde/5.15/.qmake.conf
> >
> > QtBase 5.15.3:
> > https://invent.kde.org/qt/qt/qtbase/-/blob/kde/5.15/.qmake.conf
> >
> > This then breaks version alignment expectations. For example QWECore
> > requires QtWebChannel at the same version but since the versions are
> > misaligned the requirement check changes.
>
> This is because the upstream QWE and QtScripts 5.15 branches are public,
> unlike the other Qt repos. The versioning issue is discussed here:
>
> https://www.qt.io/blog/building-qt-webengine-against-other-qt-versions
>
> Lowering the version number would be misleading since this is the real,
> upstream 5.15 code, which is post-5.15.4 already.

I suppose then we need to fix the cmake check?

Right now building the lot of our patch collection branches leads to
skrooge not being able to build because of that defect. That seems
silly at the best of times.

HS

Qt5PatchCollection versions (broken QWE)

[Harald Sitter]

Hola

QWE and QtScript in our kde/5.15 branches currently have unaligned versions.

QWE 5.15.4:
https://invent.kde.org/qt/qt/qtwebengine/-/blob/kde/5.15/.qmake.conf

QtBase 5.15.3:
https://invent.kde.org/qt/qt/qtbase/-/blob/kde/5.15/.qmake.conf

This then breaks version alignment expectations. For example QWECore
requires QtWebChannel at the same version but since the versions are
misaligned the requirement check changes.

CMake Error at 
/usr/lib/x86_64-linux-gnu/cmake/Qt5WebEngineCore/Qt5WebEngineCoreConfig.cmake:111
(find_package):
  Could not find a configuration file for package "Qt5WebChannel" that is
  compatible with requested version "5.15.4".
  The following configuration files were considered but not accepted:
/usr/lib/x86_64-linux-gnu/cmake/Qt5WebChannel/Qt5WebChannelConfig.cmake,
version: 5.15.3

I would think that we should simply align the versions at whatever
version seems appropriate. To that end I would propose that we lower
the versions of QWE and QtScript to .3.

Thoughts? Should I start an MR?

(Also, kinda unrelated but I couldn't find a good venue to discuss
things with all curators as gitlab issues are disabled for everything
but the backport tracker :-( similarly I'm not sure where one would
file a regression bug report if there were one - might need sorting
out TBH)

HS

Re: Koko in KDEReview

[Harald Sitter]

On 04.05.21 17:53, Carl Schwan wrote:
> Le mardi, mai 4, 2021 4:53 PM, Adriaan de Groot  a écrit :
> 
>> On Tuesday, 4 May 2021 15:50:27 CEST Halla Rempt wrote:
>>
>>> On Tuesday, 4 May 2021 15:28:30 CEST Nate Graham wrote:
>>>
 I don't see anyone really trying to argue otherwise.
>>>
>>> I have certainly made that argument many times. Since only developers can
>>> add tags, it will be impossible for ordinary users to provide enough
>>> information to classify the bug. Tagging systems suck big-time. Looking it
>>> GIMP's gitlab issues shows that not even the OS is reliably tagged!
>>
>> What Halla said. Every time we have this conversation, Krita is the special
>> case, because it is a special case -- many many users, diverse platforms,
>> non-technical bug-reports. We must not discount Krita's experiences and needs
>> -- conversely not ignore the needs of some obscure edge-case tool that is 
>> only
>> going to get FreeBSD sysadmins to file bug reports (which might be fine in
>> GitLab issues, because there's only ever 3 users).
> 
> If Bugzilla was working so well for Krita, I'm wondering why they are 
> redirecting
> their users to write their bug reports first in an external tool 
> (krita-artists.org)
> instead of Bugzilla.

Passive aggression really doesn't move the debate forward :(

> Some documentation website aren't using docs.kde.org but their own tooling
> (e.g. docs.krita.org, docs.plasma-mobile.org, ...), should we also force them
> to use docs.kde.org? Should we force every project to use forum.kde.org?

... neither do straw men.

> Any migration has to be able to handle huge numbers of issues, and also
>> provide maintainers tools to manage them, and to handle the diversity of 
>> issue
>> meta-data that bugzilla handles.
>>
>> To move this conversation forward, we'd need a concrete example of "these are
>> the tools used to manage issue lifecycle, similar to how bugs lifecycle in
>> bugzilla works". I know Harald has built tools and views and things to help
>> out there, but we do need a .. well, a concrete proposal for how things would
>> work.
> 
> Currently, the extra tooling we maintain for closing bugs and adding comments 
> in Bugzilla
> when an MR is opened is currently natively supported by GitLab. For bug 
> triagging,
> there is also a very helpful tool made by GitLab: 
> https://gitlab.com/gitlab-org/gitlab-triage
> 
> This tool allows for example to add special labels when an issue/MR, older 
> than a
> few days, didn't get any comments yet or adding automatically OS labels when
> Windows/Arch/FreeBSD is mentioned in the issue.

Yep. I think to drive the point home: We have solved a great many things
with tooling, we may be able to get rid of some, we may need to add some
more, but we first need to know what the great many things (not just
tooling I guess) are that we do and rely on. Only then can we actually
a) go "yes, let's move" b) "here's the migration plan". Gitlab
insufficiencies may well be solved by some extra magic put on top.

So, I suggest that someone who cares to migrate sits down with the major
stake holders and figures out what we have come to rely on in bugz that
isn't available in gitlab (OS, versions, version supportedness, personal
tags, moving bugs between projects, global search etc.), and also what
gitlab does better or might improve. Write all the stuff down on a wiki
page. Then we can think about how to deal with the various problems. And
maybe in the end the answer is not everyone can have the same BTS.
Martin has raised a really good point there. But we really, really,
really do not want a 50/50 split and everyone picks the system that
takes their fancy and then we need twice the
tooling/solutions/clientsoftware AND on top of everything figure out how
to bounce bugs between two systems. It'd not be sustainable.

> Also, it's important to note that different teams have different needs and 
> finding
> a tool that fits everyone will be very difficult. For example in NeoChat, I 
> have
> special needs like asking the bug reporter to add some information about the
> matrix instance used, the libQuotient version used, ... and for that, I need a
> special bug reports template. Something that Bugzilla doesn't provide and 
> without
> it, the bugs report are in many cases worthless.

Side note: this isn't a special need TBH ;)
Pretty much everyone has that need. So much so that I kind of have a
general game plan that there ought to be a kbugreport utility which is
able to gobble up all the very specific data $product needs through
scripts or something and either attaches the data to an existing report
or files a new one. Cause a filing template also only gets you so far  +
for the causal user it's still a fairly meh experience.

HS

Re: Koko in KDEReview

[Harald Sitter]

On 04.05.21 15:28, Nate Graham wrote:
> On 5/4/21 1:16 AM, Harald Sitter wrote:
>> Every time the bugz vs gitlab schism comes up I eventually tune out with
>> the distinct feeling that there is strong opposition to moving. What I,
>> what we all, do not want is to end up with two systems that require us
>> to maintain two client libraries with double the bugs for the next 10
>> years, and everyone gets to roll a dice which platform a given product
>> tracks bugs on. So what we need is community agreement which BTS to use
>> long term and then we can drive the technical changes to make that
>> happen.
>>
>> Short to mid term bugz is the reality we have to live with.
> 
> There is policy, and there is reality. Policy is not self-enforcing, and
> if it attempts to prescribe a reality too different from the actual one,
> it breaks and looks somewhat silly.
> 
> Since we have no way of actually *disabling* the Issues feature in our
> GitLab instance, certain projects are inevitably going to start using it
> despite all the policy we can come up with. Why? Because it's generally
> better for most of the things that most of us care about (it does not
> have to better for literally everything to still be a net win) . I don't
> see anyone really trying to argue otherwise.
> 
> Therefore, I think we need to re-focus the conversation towards "how do
> we migrate to GitLab issues in a coordinated and supported manner."

Sure, I suppose, I mean, not this conservation, this conversation was
about koko and how it needs crash tracking because quite clearly there's
opportunity for crashing. The reality right here right now is that
drkonqi only sports bugz support and will not gain gitlab support until
at least October. Which is why I said that this is the reality we live
in short to mid term.

HS



OpenPGP_signature
Description: OpenPGP digital signature

Re: Koko in KDEReview

[Harald Sitter]

On 04.05.21 00:07, Carl Schwan wrote:
> Le lundi, mai 3, 2021 4:35 PM, Harald Sitter  a écrit :
> 
>> On 23.04.21 01:00, Carl Schwan wrote:
>>
> 
>>>>>>> -   please get a bugzilla produce created for it
>>>>>
> 
>>>>> Not a fan of that. This will ends up exactly like the www bugs, something
>>>>> that I look into every 6 months. We already have many issues opened in
>>>>> invent and it's working fine for us.
>>>>
> 
>>>> Did I miss something? The last agreement I recall was that we are using
>>>> bugzilla for bugs and gitlab for tasks for the time being. If even as
>>>> developer I have to go hunting where $project tracks their bugs I'm sure
>>>> not going to be a happy camper.
>>>>
> 
>>>>> Also we don't use KCrash.
>>>>
> 
>>>> Shouldn't you?
>>>
> 
>>> The problem is that DrKonqi doesn't really work on Plasma Mobile and I 
>>> don't think
>>> this justify getting locked up in using Bugzilla. If having a bugzilla 
>>> product
>>> is really required, we can request one but I can't guarantee that I will 
>>> look at it
>>> more often than I look at the kde-www bug reports in bugzilla (every 6 
>>> months).
>>
> 
>> Let's consider it required then.
>>
> 
>> If you don't care about crash reports that's your choice I guess, but it
>> does kinda call into question the product quality since you also don't
>> have an alternative system to the kcrash-drkonqi-bugzilla caravan. Quite
>> clearly koko would have benefited from crash tracking and since the only
>> solution we presently have for that is the aforementioned stack it quite
>> clearly also would have benefited from being on bugzilla to receive
>> those crash reports and potentially move to other products if the crash
>> is not in koko. After all, crashes get submitted to the product that
>> crashed, not the library of the top most frame.
>>
> 
>> I have to be honest, it is a bit surreal to even have to argue this. I
>> get thorough enjoyment out of throwing tomatoes at the current system
>> but to actively pretend that the problem-domain of crashing software
>> doesn't exist so you don't have to look at bugzilla sure as heck doesn't
>> solve anything. In particular since you pitched koko as convergent and
>> useful on the desktop.
>>
> 
>> If all that's stopping you from embracing crash tracking is drkonqi then
>> I am happy to tell you that sticking a mobile-suitable UI on top
>> shouldn't be all that difficult ;)
> 
> Would you be open to an MR adding GitLab support to DrKonqi?

Once there's actually agreement on moving to gitlab. The code is not the
problem here. Code is easy. We are hackers after all.

Every time the bugz vs gitlab schism comes up I eventually tune out with
the distinct feeling that there is strong opposition to moving. What I,
what we all, do not want is to end up with two systems that require us
to maintain two client libraries with double the bugs for the next 10
years, and everyone gets to roll a dice which platform a given product
tracks bugs on. So what we need is community agreement which BTS to use
long term and then we can drive the technical changes to make that happen.

Short to mid term bugz is the reality we have to live with.

HS



OpenPGP_signature
Description: OpenPGP digital signature

Re: Koko in KDEReview

[Harald Sitter]

On 23.04.21 01:00, Carl Schwan wrote:
> -   please get a bugzilla produce created for it
>>>
>>> Not a fan of that. This will ends up exactly like the www bugs, something
>>> that I look into every 6 months. We already have many issues opened in
>>> invent and it's working fine for us.
>>
>> Did I miss something? The last agreement I recall was that we are using
>> bugzilla for bugs and gitlab for tasks for the time being. If even as
>> developer I have to go hunting where $project tracks their bugs I'm sure
>> not going to be a happy camper.
>>
>>> Also we don't use KCrash.
>>
>> Shouldn't you?
> 
> The problem is that DrKonqi doesn't really work on Plasma Mobile and I don't 
> think
> this justify getting locked up in using Bugzilla. If having a bugzilla product
> is really required, we can request one but I can't guarantee that I will look 
> at it
> more often than I look at the kde-www bug reports in bugzilla (every 6 
> months).

Let's consider it required then.

If you don't care about crash reports that's your choice I guess, but it
does kinda call into question the product quality since you also don't
have an alternative system to the kcrash-drkonqi-bugzilla caravan. Quite
clearly koko would have benefited from crash tracking and since the only
solution we presently have for that is the aforementioned stack it quite
clearly also would have benefited from being on bugzilla to receive
those crash reports and potentially move to other products if the crash
is not in koko. After all, crashes get submitted to the product that
crashed, not the library of the top most frame.

I have to be honest, it is a bit surreal to even have to argue this. I
get thorough enjoyment out of throwing tomatoes at the current system
but to actively pretend that the problem-domain of crashing software
doesn't exist so you don't have to look at bugzilla sure as heck doesn't
solve anything. In particular since you pitched koko as convergent and
useful on the desktop.

If all that's stopping you from embracing crash tracking is drkonqi then
I am happy to tell you that sticking a mobile-suitable UI on top
shouldn't be all that difficult ;)

HS



OpenPGP_signature
Description: OpenPGP digital signature

Re: Is there (going to be) an auto-retracer service for KDE?

[Harald Sitter]

On Tue, Apr 27, 2021 at 2:16 PM Halla Rempt  wrote:
>
> On Monday, 26 April 2021 13:07:17 CEST Harald Sitter wrote:
> > Also going off on a tangent: On windows I understand the store already
> > has crash tracking and all that stuff implemented, I expect the same
> > is true for OSX?
>
> Yes, but without proper debug symbols the traces are not all that useful -- 
> though still good enough to tell me that display drivers on Windows are the 
> main source of crashes.

Our apps that I have access to all seem to have symbols uploaded. That
being said the "stack trace" isn't too hot in general:

> FrameImageFunctionOffset
> 0Qt5Core.dllqt_message_fatal0x0018
> 1Qt5Core.dllQMessageLogger::fatal0x0071
> 2Qt5Gui.dllinit_platform0x0861

Which, I mean, is a call trace, but that's all it is. Fortunately one
can actually get a hold of the minidump with a huge caveat:

https://docs.microsoft.com/en-us/windows/uwp/publish/health-report

> CAB files will only be available when the failure occurred on a computer 
> using a Windows Insider build, so not all failures will include the CAB 
> download option.

With that in mind I guess the answer is that crash reports from
systems that aren't in insider mode aren't terribly useful in general
outside the general sense that something may be crashing a whole lot
and which functions are involved.
Crash reports from insiders OTOH have this zip file available which
contains the WER [1] metadata and the minidump which one can load into
VS [2] or windbg [3]. Combined with auto-retrieval [4] that surely
should give all the building blocks to generate rich(er) traces.

This really doesn't seem quite as useless as you make it sound. It's
still a bit meh to need insider users to get viable data though. What
might be interesting there is if an app could opt into WER local
dumping and then have a dialog on next run "yo, krita crashed last
time, how about you send us the crash" which would then behave like
what I described for desktop linux (upload minidump to us, we trace
server-side) thereby bypassing microsoft entirely. Albeit this has all
the same challenges as desktop linux too then. Someone needed to write
the weby-techy bits of receiving dumps and then tracing them and
manage a crash record database.

[1] https://docs.microsoft.com/en-us/windows/win32/wer/windows-error-reporting
[2] https://docs.microsoft.com/en-us/visualstudio/debugger/using-dump-files
[3] 
https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-download-tools
[4] 
https://docs.microsoft.com/en-us/windows/uwp/monetize/access-analytics-data-using-windows-store-services

HS

Re: Is there (going to be) an auto-retracer service for KDE?

[Harald Sitter]

On Mon, Apr 26, 2021 at 12:34 PM Halla Rempt  wrote:
>
> On Monday, 26 April 2021 11:26:38 CEST Harald Sitter wrote:
> > * get a core instead of a trace - sounds simpler than it is because
> > cores can be huge and network speed may be limited. google's breakpad
> > has a special minidump format that seeks to deal with that, I think
> > apport also has its own reduction format.
>
> I've twice in my life integrated breakpad into software, and it was hell both 
> times.

I did not mean to suggest  that we should use breakpad. In fact my
ephemeral plan, as far as desktop Linux goes, would be to rely on
systemd's coredumpd (drkonqi integration for that is actually what I'm
working on right now). Regardless of "how" the crash gets intercepted
we need a way to send cores over the wire and for that we may need a
better format. Plasma cores are upwards of 50MiB for me, even when
compressed.

> Generating and updating and managing the symbol files was a continuous chore, 
> too.

Luckily distros have that sorted out already ;)

Also going off on a tangent: On windows I understand the store already
has crash tracking and all that stuff implemented, I expect the same
is true for OSX? No idea about android, flatpak and snap though. In
general I would argue that we should rely on the platform crash
tracking systems more. On desktop linux I'd let cores fall into
coredumpd and then fish them out of there. On freebsd and linuxes
without systemd we can simply do what we always did - to drkonqi it
makes no diff if it traces the process or generates a core. Of course
in either case we'd need a way to send and then trace crashes
server-side on our end.

HS

Re: Is there (going to be) an auto-retracer service for KDE?

[Harald Sitter]

On Sun, Apr 25, 2021 at 10:38 PM Albert Astals Cid  wrote:
> KDE can't do retracing because KDE hasn't built the packages people use 
> (except KDE Neon).

I've mused on this in the past and I think it can be done, it's just a
lot of work that in particular also will need to involve some web
software to upload to, and manage crashes in.

In broad strokes what needs to happen is:

* get a core instead of a trace - sounds simpler than it is because
cores can be huge and network speed may be limited. google's breakpad
has a special minidump format that seeks to deal with that, I think
apport also has its own reduction format.
* get all the metadata we can find - this likely needs distro
integration depending on the data. this will need (e.g.) a list of
either all installed binary packages and their version, or at least
the ones supplying the files loaded in memory
* upload all the data to a web service and put it in a processing queue
* send all the data off to a distro-specific retracing service - the
distros would need to supply/maintain this. it'd setup an env as
similar to the crashing system as possible, then run a series of gdb
commands to then send the output back to us (this could conceivably be
as simple as a dockerfile)
* we parse the output (which would include a trace) to then either
record a new crash or attach the data to an existing crash

All of this ideally without falling for alluring NIH solutions.

It'd be a huge undertaking and is a bit undermined by the rather
excellent debuginfod [1] that makes our existing way of doing things
more accessible, albeit network-wise just as expensive.

[1] https://sourceware.org/elfutils/Debuginfod.html

For further reference:

- https://abrt.readthedocs.io/en/latest/
- https://launchpad.net/errors

HS

Re: Including LayerShellQt in Plasma in time for 5.22

[Harald Sitter]

- I'm like a broken record at this point: some of the cmake files and
the json file lack licensing details. please run `reuse lint` to check
the list. it's incredibly close to being fully licensed
- you include KDEClangFormat but don't actually seem to enable it
- along a similar note: the code style is inconsistent at times, you
might want to also enable the githooks when built with a new enough
ECM
- CheckIncludeFiles is also included but not used
- you sometimes use the Qt5::Foo targets rather than the Qt::Foo
targets, I believe the latter is preferred for easier porting to qt6
- the library code calls qputenv and ignores its return value, might
make sense to qcwarning when the env variable failed to set

other than that it looks lovely 

HS

Re: KQuickImageEditor in KDEReview

[Harald Sitter]

- really close to 100% reuse cover. plz consider adding data to
KQuickImageEditorConfig.cmake.in and src/controls/plugins.qmltypes
- in fact... shouldn't the qmltypes file be generated at build time?
- there's commented out KDEFrameworkCompilerSettings in CMakeLists,
please remove it, Friedrich asked us to not use it outside frameworks
and having it commented out only tempts people into using it ;)
- as with koko, it might make sense to bump the kf5 requirement to
5.79 and use clang format+hooks. there are some stilistic
inconsistency within the code already
- you should consider adding an option() definition for the
BUILD_SHARED_LIBS variable so it is documented and expicitly
initialized to a default value, I guess
- ResizeRectangle has raw pointers that aren't initialized to nullptr by default
- resizerectangle.cpp includes moc_resizerectangle.cpp is there a
reason for that? shouldn't automoc magic do this on its own?
- same for resizehandle.cpp

some typos in imagedocument.h:
  Mirrror
  horizonal (actually in mirrocommand.{h,cpp} as well)
  operattion

HS

Re: Koko in KDEReview

[Harald Sitter]

On 16.03.21 20:10, Carl Schwan wrote:
> Le mardi, mars 16, 2021 12:55 PM, Harald Sitter  a écrit :
> 
>> On Tue, Mar 16, 2021 at 12:49 PM Harald Sitter sit...@kde.org wrote:
>>
> 
>>> Yay!
> 
> Thanks for the big review :)
> 
>>>
> 
>>> -   please get a bugzilla produce created for it
> 
> Not a fan of that. This will ends up exactly like the www bugs, something
> that I look into every 6 months. We already have many issues opened in
> invent and it's working fine for us. 

Did I miss something? The last agreement I recall was that we are using
bugzilla for bugs and gitlab for tasks for the time being. If even as
developer I have to go hunting where $project tracks their bugs I'm sure
not going to be a happy camper.

> Also we don't use KCrash.

Shouldn't you?

>>> While koko is running, if I copy a file with geodata to the pictures
>>> dir it crashes on `kd_insert3 (tree=0x0,` supposedly because the
>>> geocoder had been deinit()'d while it was still running. most notably
>>> Processor (which is the gui thread) calls deinit on the geocoder
>>> (which is used from various runner threads). in point of fact,
>>> glancing over the code I'm not convinced this is actually correctly
>>> threading
>>> ImageProcessorRunnable is a runnable that is put into the thread pool.
>>> Inside its run there's
>>>
> 
>>> if (!m_geoCoder->initialized()) {
>>> m_geoCoder->init();
>>> }
>>> 
> 
>>>
> 
>>> This is racing code. In between the call to initialized() and the
>>> init() another thread might have done init() already. At best that is
>>> leaking kd_create instances, at worst that crashes on one of the many
>>> asserts on members. On top of that Processor then also calls into
>>> deinit() from its thread which might be at any point in time while the
>>> Runnable's run() runs. The entire construct is lacking any sort of
>>> appreciation for thread synchronization. This needs at least a mutex
>>> to synchronize the init/deinit and possibly lookup if kd_* is not
>>> thread safe.
>>> I am not sure if the init-deinit dance is really adding much value
>>> here. If it isn't I might just do away with the deinit TBH.
>>> HS
> 
> This code is now using a ReadWriteLock. This should fix the racing code. 

Still crashes when I move an image with geodata into the picture dir :(

at ../src/reversegeocoder.cpp:151

>> Oh! Three follow ups:
>>
> 
>> -   is it intentional that this isn't a uniqueapp [1]? do multiple
>> concurrent koko instances even work with the database?
> 
> It is intentional since users can open image from Dolphin while Koko is
> already running. Maybe I could look into using KDSingleApplication for
> this usecase. Also multiple koko instances seems to be working in my
> experience but I didn't really test that in details.

That's what [1] does.
You register on kdbusservice register as org.kde.koko and connect to
activateRequested so when the user clicks on an image while koko is
already running that opening request is sent to the running instance
instead of starting a completely new one.

>> -   the geodata magic doesn't seem to be working for me. it correctly
>> maps the geodata in ReverseGeoCoder but in the UI nothing is displayed
>> under locations
> 
> Could you take a look into `~/.local/share/koko/imageData.sqlite3` with
> sqlitebrowser and tell me if the locations and image are correctly tagged?
> Also try to remove `~/.local/share/koko/imageData.sqlite3` and 
> `~/.local/share/koko/fstracker.sqlite3`
> after pulling the project new, this might have been caused by the racing
> code from above.

Works after I wiped config and data to start with a fresh set of data
:shrug:

HS

Re: Koko in KDEReview

[Harald Sitter]

On Tue, Mar 16, 2021 at 12:49 PM Harald Sitter  wrote:
>
> Yay!
>
> - please get a bugzilla produce created for it
> - COPYING-CMAKE-SCRIPTS seems unused
> - the find_package calls on qt5 probably should be versioned on
> whatever version you actually tested with, currently it's unversioned
> which I doubt is correct
> - same for kquickimageeditor
> - kquickimageeditor is found with an empty COMPONENTS list. is that 
> intentional?
> - unless you specifically target kf5.70 it might make sense to bump to
> .79 and use KDEGitCommitHooks so clang-format is more consistently
> applied
> - kdtree.{c,h} references BSD-3-Clause but that license isn't in the source
> - on the topic of licensing: since the code base is really close to
> complete reuse coverage it might be nice to push it over the finishing
> line and then `reuse lint` it to not have it fall behind again
> - some classes have trivial constructors/destructors and could use
> =default (e.g. roles.cpp, types.cpp, processor.cpp, openfilemodel.cpp,
> probably others)
> - some of those are further QObjects that have a parent signature but
> don't delegate construction correctly (i.e. the parent arg is never
> forwarded to QObject). since they are also trivial the constructors
> could be removed and replaced by `using QObject::QObject;` to adopt
> QObject's ctors (e.g. roles.cpp, types.cpp)
> - some of the .h's have `parent = 0`, it'd be nice to have them use
> nullptr instead
> - some functions take references when they should take const refs
> (e.g. `void setPath(QString )` or the ImageProcessorRunnable ctor)
> this suggests they mutate the object, but really the mentioned
> functions don't
> - for future reference: .appdata.xml is a legacy suffix and
> .metainfo.xml ought to be used instead. alas, no point changing this
> now to avoid extra work for the i18n team
> - the appstream file must use the CDN not a wiki for screenshots
> https://invent.kde.org/websites/product-screenshots
> - also generally speaking please don't generate or use thumbs for
> appstream files, appstream-generator will thumbnail the raw images
> during assembling stage on the distro side of things
> - appstream data has a help link that goes nowhere
>
> While koko is running, if I copy a file with geodata to the pictures
> dir it crashes on `kd_insert3 (tree=0x0,` supposedly because the
> geocoder had been deinit()'d while it was still running. most notably
> Processor (which is the gui thread) calls deinit on the geocoder
> (which is used from various runner threads). in point of fact,
> glancing over the code I'm not convinced this is actually correctly
> threading
>
> ImageProcessorRunnable is a runnable that is put into the thread pool.
> Inside its run there's
>
> if (!m_geoCoder->initialized()) {
> m_geoCoder->init();
> }
>
>
> This is racing code. In between the call to initialized() and the
> init() another thread might have done init() already. At best that is
> leaking kd_create instances, at worst that crashes on one of the many
> asserts on members. On top of that Processor then also calls into
> deinit() from its thread which might be at any point in time while the
> Runnable's run() runs. The entire construct is lacking any sort of
> appreciation for thread synchronization. This needs at least a mutex
> to synchronize the init/deinit and possibly lookup if kd_* is not
> thread safe.
>
> I am not sure if the init-deinit dance is really adding much value
> here. If it isn't I might just do away with the deinit TBH.
>
> HS

Oh! Three follow ups:

- is it intentional that this isn't a uniqueapp [1]? do multiple
concurrent koko instances even work with the database?
- the geodata magic doesn't seem to be working for me. it correctly
maps the geodata in ReverseGeoCoder but in the UI nothing is displayed
under locations
- since it doesn't appear in the UI, I can't check: is the geodata
actually getting localized? at least the geocoder seems to spit out
non-localized values

[1] https://api.kde.org/frameworks/kdbusaddons/html/classKDBusService.html

Re: Koko in KDEReview

[Harald Sitter]

Yay!

- please get a bugzilla produce created for it
- COPYING-CMAKE-SCRIPTS seems unused
- the find_package calls on qt5 probably should be versioned on
whatever version you actually tested with, currently it's unversioned
which I doubt is correct
- same for kquickimageeditor
- kquickimageeditor is found with an empty COMPONENTS list. is that intentional?
- unless you specifically target kf5.70 it might make sense to bump to
.79 and use KDEGitCommitHooks so clang-format is more consistently
applied
- kdtree.{c,h} references BSD-3-Clause but that license isn't in the source
- on the topic of licensing: since the code base is really close to
complete reuse coverage it might be nice to push it over the finishing
line and then `reuse lint` it to not have it fall behind again
- some classes have trivial constructors/destructors and could use
=default (e.g. roles.cpp, types.cpp, processor.cpp, openfilemodel.cpp,
probably others)
- some of those are further QObjects that have a parent signature but
don't delegate construction correctly (i.e. the parent arg is never
forwarded to QObject). since they are also trivial the constructors
could be removed and replaced by `using QObject::QObject;` to adopt
QObject's ctors (e.g. roles.cpp, types.cpp)
- some of the .h's have `parent = 0`, it'd be nice to have them use
nullptr instead
- some functions take references when they should take const refs
(e.g. `void setPath(QString )` or the ImageProcessorRunnable ctor)
this suggests they mutate the object, but really the mentioned
functions don't
- for future reference: .appdata.xml is a legacy suffix and
.metainfo.xml ought to be used instead. alas, no point changing this
now to avoid extra work for the i18n team
- the appstream file must use the CDN not a wiki for screenshots
https://invent.kde.org/websites/product-screenshots
- also generally speaking please don't generate or use thumbs for
appstream files, appstream-generator will thumbnail the raw images
during assembling stage on the distro side of things
- appstream data has a help link that goes nowhere

While koko is running, if I copy a file with geodata to the pictures
dir it crashes on `kd_insert3 (tree=0x0,` supposedly because the
geocoder had been deinit()'d while it was still running. most notably
Processor (which is the gui thread) calls deinit on the geocoder
(which is used from various runner threads). in point of fact,
glancing over the code I'm not convinced this is actually correctly
threading

ImageProcessorRunnable is a runnable that is put into the thread pool.
Inside its run there's

if (!m_geoCoder->initialized()) {
m_geoCoder->init();
}


This is racing code. In between the call to initialized() and the
init() another thread might have done init() already. At best that is
leaking kd_create instances, at worst that crashes on one of the many
asserts on members. On top of that Processor then also calls into
deinit() from its thread which might be at any point in time while the
Runnable's run() runs. The entire construct is lacking any sort of
appreciation for thread synchronization. This needs at least a mutex
to synchronize the init/deinit and possibly lookup if kd_* is not
thread safe.

I am not sure if the init-deinit dance is really adding much value
here. If it isn't I might just do away with the deinit TBH.

HS

Re: New repo in kdereview: plasma-settings

[Harald Sitter]

On 25.01.21 08:05, Bhushan Shah wrote:
> Hello everyone!
> 
> I want to move plasma-settings repo to kdereview, it is settings
> application used by the Plasma Mobile, it is based on Kirigami.
> 
> https://invent.kde.org/plasma-mobile/plasma-settings
> 
> Main difference between plasma-settings and original system settings
> application used in plasma desktop is, this code is much more lean and
> modern, it is my understanding that long term idea is to replace the
> original systemsettings code base with plasma-settings. But for now this
> is mobile specific.

kpackage is very grumpy during cmake stage

no appstream data :(

Not sure if intentional but ModulesModel actually doesn't constrain to
settings KCMS. It'd also display info center kcms as well as (I think)
plugin KCMS (e.g. krunner's)

not installing a hicolor icon

I am pretty sure l10n isn't working. I can't see the translations domain
set anywhere (mind you, kcms likely work; --help might now)

Along a similar note the package/ dir isn't covered by a Messages.sh and
consequently isn't actually i18n'd. I suggest moving package into src/
and the main Message.sh into src/ as well and let it cover everything
inside. It'd clarify extraction responsibility somewhat.

might be worth making the source reuse compliant

some qml file lack license information (KCMContainer.qml, info's
main.qml at least - would be easier to check if the source was compliant ;))

I am very certain that lots of code inside modules/info/ was copied from
kinfocenter's about-distro but lacks any attribution to its original
authors *cough* including your's truly

password module has copies of org.freesktop.Accounts*xml at a glance
they are installed by accountsservice, you could just use the installed
versions instead of holding a copy

HS



signature.asc
Description: OpenPGP digital signature

Re: New repo in kdereview: plasma-angelfish

[Harald Sitter]

On 25.01.21 06:55, Bhushan Shah wrote:
> Hello everyone!
> 
> Yet another Plasma Mobile repository in kdereview: 
> 
> https://invent.kde.org/plasma-mobile/plasma-angelfish
> 
> Plasma Angelfish is browser written in Kirigami which uses the
> QtWebengine for rendering web pages. It is optimized for the mobile
> usecase.
> 
> Current feature list is available in README.md of the repository.

Not sure I am liking the InitialPreference in the desktop file.

Encoding in desktop files has been deprecated for like a million years ^^

Inconsistent spelling between desktop file "Web Browser" and appstream
file "Webbrowser". I also think the former is the way one generally
spells it.

.appdata.xml is a legacy suffix, you might want to use .metainfo.xml

Same as with phonebook, I don't see the translation domain getting set
anywhere and by extension l10n is likely kaput.

Inside SettingsNavigationBarPage.qml there's a concatenated multi line
string, I'm not sure if that'd get extracted correctly for l10n. Best check.

Many files don't have license info, see point 15 of the license policy
[1] :(

You may want to move from the incomplete COPYING file to reuse LICENSES/
folder.

DesktopFileGenerator manually calls kbuildsycoca :O - This should just
work and if not get fixed properly. Super alternatively
KSycoca::self()->ensureCacheValid() is exactly made for this use case.

The flatpak run inside DesktopFileGenerator seems super opinionated, but
with the only target system being plasma-mobile I guess it doesn't matter?

The message("warning:" in src/CMakeLists.txt ought to be
add_feature_info() really [2]

[1] https://community.kde.org/Policies/Licensing_Policy
[2] https://cmake.org/cmake/help/v3.8/module/FeatureSummary.html

> Before moving it to extragear I would like to drop plasma- prefix from
> the repository as this repo have nothing much to do with "Plasma". And
> it is otherwise already called just "Angelfish" in its desktop files and
> stuff.
+1




signature.asc
Description: OpenPGP digital signature

Re: New repo in kdereview: plasma-phonebook

[Harald Sitter]

On 25.01.21 06:48, Bhushan Shah wrote:
> Hello everyone!
> 
> I am back with more Plasma Mobile related repositories in kdereview. I
> want to move plasma-phonebook in kdereview. plasma-phonebook is kirigami
> based phonebook application, it uses kpeople backends like kpeoplesink,
> kpeoplevcard, kpeople akonadi backend etc to fetch and store contacts on
> your system.

I am pretty sure l10n isn't working. I can't see the translations domain
set anywhere.

On the subject of strings I'd suggest checking them all for HIG
compliance. In main.qml alone all strings are wrong as titles and
buttons are supposed to use Title Capitalization
https://hig.kde.org/style/writing/capitalization.html

The desktop file seems to be lacking an actual main category (it's in
the lost section in the launcher for me)

What's the use case for the unregistered x-plasma-phonebook mimetype
(registered for by the desktop file)? Perhaps the more relevant question
is why would it should be unregistered instead of inside our vendor tree
(vnd.kde.phone.book or some such)?

Many files don't have license info, see point 15 of the license policy
[1] :(

Actual qml **source** files have no license info!

DetailListItem.qml isn't used.

Are we quite sure that hardcoding colors is the way to go? :O

Header.qml actually has a crash for me because ColorOverlay is inside a
FastBlur that is both parent and source (qt docs about source: "Note: It
is not supported to let the effect include itself, for instance by
setting source to the effect's parent.")

Similarly the FastBlur looks to be falling into that trap of
parent===source, albeit not crashing for me.

Spelling is inconsistent. The desktop file spells it "Phone Book" the
appstream file spells it "Plasma Phonebook".

.appdata.xml is a legacy suffix, you might want to use .metainfo.xml
instead.

On the subject of the appstream file. Missing
KDE :(

# Probably a bit nitpicky

Failing to store contact changes in AddContactPage lacks actual UI
backing, it merely qwarns.

Various single argument ctors aren't tagged `explicit`.

[1] https://community.kde.org/Policies/Licensing_Policy

> Another thing I want to discuss is, branding, currently it is named as
> Plasma Phonebook which is bit awkward because Plasma is not related to
> app at all and even android port for app exists. Maybe we should name
> this just phonebook?

Is this about the repo/tarball name or the display string?

The former already lacks the plasma prefix in the .desktop file, I'd
argue the appstream name should do the same. System Settings is a
precedent for something similar.

As for tarballs: some distros get grumpy over overly generic source
names, so while I think phonebook is fine it may be less fine for
distros. It probably matters little what the repo/tarball is called though.

HS



signature.asc
Description: OpenPGP digital signature

Re: PSA: Please do not use KDEFrameworkCompilerSettings for non-KF projects

[Harald Sitter]

On 19.01.21 16:57, Friedrich W. H. Kossebau wrote:
> Hi,
> 
> the docs of the module tell us:
> "KDEFrameworkCompilerSettings - Set stricter compile and link flags for KDE 
> Frameworks modules."
> https://api.kde.org/ecm/kde-module/KDEFrameworkCompilerSettings.html
> 
> And it has been meant that way. E.g. because extending those settings with 
> more strict settings or new features would be done with other KF-specific 
> requirements or policies in mind, always matching those of the current 
> version 
> (given KF & ECM are released in-sync).
> Having to take care of backward-compatibility with non-KF usage and to do 
> proper documentation adds additional burden not wanted here.
> 
> As convenient it is to not have to write out e.g. all the 
> add_definitions(
> -DQT_NO_CAST_TO_ASCII
> -DQT_NO_CAST_FROM_ASCII
> -DQT_NO_URL_CAST_FROM_STRING
> -DQT_NO_CAST_FROM_BYTEARRAY
> -DQT_NO_SIGNALS_SLOTS_KEYWORDS
> -DQT_USE_QSTRINGBUILDER
> -DQT_NO_NARROWING_CONVERSIONS_IN_CONNECT
> )
> that needs some other solution.
> 
> For ECM/KF5 times the train has left, we need to handle those existing 
> abuses. 
> But please fix your projects now already by changing back to 
> KDECompilerSettings and the manual boilerplate, so in ECM/KF6 times that 
> burden is gone.

Perhaps there should be a unit test asserting this somehow?
LXR reports `247 occurences found.` so this seems a fairly wide spread
issue :(

Perhaps hardcode a list of all cmake project names that are frameworks
in ECM and then test that the settings are only used with one of them?
Just an idea.

HS

Re: CMake kauth_install_* with system kauth and user CMAKE_INSTALL_PREFIX

[Harald Sitter]

On 19.10.20 17:44, Ingo Klöcker wrote:
> On Montag, 19. Oktober 2020 17:22:22 CEST Milian Wolff wrote:
>> On Montag, 19. Oktober 2020 17:21:28 CEST Harald Sitter wrote:
>>> I'm not sure it's a good idea, but we could treat this like
>>> KDE_INSTALL_USE_QT_SYS_PATHS. By default we'll simply install to
>>> $CMAKE_PREFIX/share/polkit-1/actions/ unless explicitly configured with
>>> KDE_INSTALL_USE_POLKIT_PATHS in which case we'll use whatever polkit uses.
>>> What sets this apart from qt_syspath of course is that not using the
>>> polkit path basically never makes sense as it'd be broken. So maybe the
>>> default state should be ON and when one is sure that polkit isn't needed
>>> one could turn it OFF.
>>
>> Installing something that's never going to be used - why? I would much
>> rather prefer making it explicit by saying:
>>
>> Either you install it to the correct path, or you don't install it at all.

Fair point

> FWIW, for a very similar problem (udev rules instead of polkit rules) 
> bluez-qt 
> has the CMAKE flag INSTALL_UDEV_RULE to disable installation of udev rules.
> 
> If have this in my .kdesrc-buildrc:
> =
> options bluez-qt
> cmake-options -DINSTALL_UDEV_RULE=0
> end options
> =
> 
> So, maybe add a KDE_INSTALL_POLKIT_RULE[S] (or ..._ACTION[S]) flag to allow 
> disabling the installation of polkit stuff.

+1

HS



signature.asc
Description: OpenPGP digital signature

Re: CMake kauth_install_* with system kauth and user CMAKE_INSTALL_PREFIX

[Harald Sitter]

On 19.10.20 17:06, Milian Wolff wrote:
> On Montag, 19. Oktober 2020 16:56:27 CEST Harald Sitter wrote:
>> On 19.10.20 12:30, Milian Wolff wrote:
>>> On Montag, 19. Oktober 2020 11:27:29 CEST Harald Sitter wrote:
>>>> Yo
>>>>
>>>> On 18.10.20 10:11, Milian Wolff wrote:
>>>>> Hey all,
>>>>>
>>>>> since some time now I'm only compiling parts of KF5 selectively into a
>>>>> custom prefix. Most notably, I'm only interested in ktexteditor and
>>>>> syntax- highlighting, and would like to obtain all other frameworks
>>>>> through my distribution packages.
>>>>>
>>>>> Sadly, this doesn't work as easily, as ktexteditor is trying to do this:
>>>>>
>>>>> ```
>>>>> install(TARGETS kauth_ktexteditor_helper DESTINATION $
>>>>> {KAUTH_HELPER_INSTALL_DIR} )
>>>>> kauth_install_helper_files(kauth_ktexteditor_helper
>>>>> org.kde.ktexteditor.katetextbuffer root)
>>>>> kauth_install_actions(org.kde.ktexteditor.katetextbuffer buffer/
>>>>> org.kde.ktexteditor.katetextbuffer.actions)
>>>>> ```
>>>>>
>>>>> Because my KAuth is a system-provided installation, and KTextEditor
>>>>> should
>>>>> be installed into a user-writable prefix, I get this error on `ninja
>>>>> install`:
>>>>>
>>>>> ```
>>>>>
>>>>> CMake Error at src/cmake_install.cmake:143 (file):
>>>>>   file INSTALL cannot copy file
>>>>>   "/home/milian/projects/kf5/build/frameworks/ktexteditor/src/
>>>>>
>>>>> org.kde.ktexteditor.katetextbuffer.policy"
>>>>>
>>>>>   to
>>>>>   "/usr/share/polkit-1/actions/org.kde.ktexteditor.katetextbuffer.policy
>>>>>   "
>>>>>   
>>>>>   Permission denied.
>>>>>
>>>>> Call Stack (most recent call first):
>>>>>   cmake_install.cmake:77 (include)
>>>>>
>>>>> ```
>>>>>
>>>>> I can workaround this by either commenting out the kauth install bits in
>>>>> ktexteditor or by installing kauth seperately too. But both are in my
>>>>> opinion not ideal.
>>>>>
>>>>> I don't think it's currently possible to overwrite the KAuth install
>>>>> directory at cmake configure time. I would like to change that, unless
>>>>> there is something I'm missing which would prevent this? I am hoping
>>>>> that
>>>>> any folder works as long as it's listed in the `XDG_DATA_DIRS` env var -
>>>>> can someone confirm this?
>>>>
>>>> Alas, the path is hardcoded at buildtime from what I see:
>>>> https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/src/polkitback
>>>> end /polkitbackendinteractiveauthority.c#L302
>>>>
>>>> Relevant: https://bugs.kde.org/show_bug.cgi?id=425272
>>>
>>> So the path that needs to be used is actually the polkit path?
>>
>> That's my understanding of the code, yes.
>>
>>> Meaning the
>>> question where KAuth is being installed to doesn't really matter? That
>>> would imply that KAuth is broken anyways when it's not installed to the
>>> same prefix that polkit is being installed to, no?
>>
>> Nope. If the policy was installed to a different path it would be
>> broken, but the very error you posted is because kauth is insisting on
>> putting the policy in the correct path rather than the prefix the rest
>> of the build is installing to.
> 
> No, it's using the install location of kauth, it doesn't query polkit itself. 
> I.e. if I hand-compile KAuth and install into a custom prefix, then I will 
> have:
> 
> ```
> $ grep INSTALL /home/milian/projects/compiled/kf5-dbg/lib/cmake/KF5Auth/
> KF5AuthConfig.cmake
> set(KAUTH_POLICY_FILES_INSTALL_DIR "/home/milian/projects/compiled/kf5/share/
> polkit-1/actions")
> set(KAUTH_HELPER_INSTALL_DIR "lib/libexec/kauth")
> set(KAUTH_HELPER_INSTALL_ABSOLUTE_DIR "/home/milian/projects/compiled/kf5/lib/
> libexec/kauth")
> ```

Ohh! This is indeed plain silly. I had thought it would use the value
from pkgconfig :S

> Now I can happily compile e.g. ktexteditor against this, but it would be just 
> as broken as the polkit files it installed will never be found anyways.
> 
> So, I guess there are two things to solve here:
> 
> a) make it easy to opt-out / disable kauth polkit file installation via a 
> cache var in the `kauth_install_*` cmake macros, such that I can just disable 
> that when building ktexteditor.

I'm not sure it's a good idea, but we could treat this like
KDE_INSTALL_USE_QT_SYS_PATHS. By default we'll simply install to
$CMAKE_PREFIX/share/polkit-1/actions/ unless explicitly configured with
KDE_INSTALL_USE_POLKIT_PATHS in which case we'll use whatever polkit uses.
What sets this apart from qt_syspath of course is that not using the
polkit path basically never makes sense as it'd be broken. So maybe the
default state should be ON and when one is sure that polkit isn't needed
one could turn it OFF.

> b) find a way to query polkit for the right path to use and use that by 
> default instead of the kauth install prefix.

polkit-gobject-1's pkgconfig encodes the path. So that's easy.

HS



signature.asc
Description: OpenPGP digital signature

Re: CMake kauth_install_* with system kauth and user CMAKE_INSTALL_PREFIX

[Harald Sitter]

On 19.10.20 12:30, Milian Wolff wrote:
> On Montag, 19. Oktober 2020 11:27:29 CEST Harald Sitter wrote:
>> Yo
>>
>> On 18.10.20 10:11, Milian Wolff wrote:
>>> Hey all,
>>>
>>> since some time now I'm only compiling parts of KF5 selectively into a
>>> custom prefix. Most notably, I'm only interested in ktexteditor and
>>> syntax- highlighting, and would like to obtain all other frameworks
>>> through my distribution packages.
>>>
>>> Sadly, this doesn't work as easily, as ktexteditor is trying to do this:
>>>
>>> ```
>>> install(TARGETS kauth_ktexteditor_helper DESTINATION $
>>> {KAUTH_HELPER_INSTALL_DIR} )
>>> kauth_install_helper_files(kauth_ktexteditor_helper
>>> org.kde.ktexteditor.katetextbuffer root)
>>> kauth_install_actions(org.kde.ktexteditor.katetextbuffer buffer/
>>> org.kde.ktexteditor.katetextbuffer.actions)
>>> ```
>>>
>>> Because my KAuth is a system-provided installation, and KTextEditor should
>>> be installed into a user-writable prefix, I get this error on `ninja
>>> install`:
>>>
>>> ```
>>>
>>> CMake Error at src/cmake_install.cmake:143 (file):
>>>   file INSTALL cannot copy file
>>>   "/home/milian/projects/kf5/build/frameworks/ktexteditor/src/
>>>
>>> org.kde.ktexteditor.katetextbuffer.policy"
>>>
>>>   to
>>>   "/usr/share/polkit-1/actions/org.kde.ktexteditor.katetextbuffer.policy"
>>>   :
>>>   Permission denied.
>>>
>>> Call Stack (most recent call first):
>>>   cmake_install.cmake:77 (include)
>>>
>>> ```
>>>
>>> I can workaround this by either commenting out the kauth install bits in
>>> ktexteditor or by installing kauth seperately too. But both are in my
>>> opinion not ideal.
>>>
>>> I don't think it's currently possible to overwrite the KAuth install
>>> directory at cmake configure time. I would like to change that, unless
>>> there is something I'm missing which would prevent this? I am hoping that
>>> any folder works as long as it's listed in the `XDG_DATA_DIRS` env var -
>>> can someone confirm this?
>>
>> Alas, the path is hardcoded at buildtime from what I see:
>> https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/src/polkitbackend
>> /polkitbackendinteractiveauthority.c#L302
>>
>> Relevant: https://bugs.kde.org/show_bug.cgi?id=425272
> 
> So the path that needs to be used is actually the polkit path?

That's my understanding of the code, yes.

> Meaning the 
> question where KAuth is being installed to doesn't really matter? That would 
> imply that KAuth is broken anyways when it's not installed to the same prefix 
> that polkit is being installed to, no?

Nope. If the policy was installed to a different path it would be
broken, but the very error you posted is because kauth is insisting on
putting the policy in the correct path rather than the prefix the rest
of the build is installing to.

HS



signature.asc
Description: OpenPGP digital signature

Re: CMake kauth_install_* with system kauth and user CMAKE_INSTALL_PREFIX

[Harald Sitter]

Yo

On 18.10.20 10:11, Milian Wolff wrote:
> Hey all,
> 
> since some time now I'm only compiling parts of KF5 selectively into a custom 
> prefix. Most notably, I'm only interested in ktexteditor and syntax-
> highlighting, and would like to obtain all other frameworks through my 
> distribution packages.
> 
> Sadly, this doesn't work as easily, as ktexteditor is trying to do this:
> 
> ```
> install(TARGETS kauth_ktexteditor_helper DESTINATION $
> {KAUTH_HELPER_INSTALL_DIR} )
> kauth_install_helper_files(kauth_ktexteditor_helper 
> org.kde.ktexteditor.katetextbuffer root)
> kauth_install_actions(org.kde.ktexteditor.katetextbuffer buffer/
> org.kde.ktexteditor.katetextbuffer.actions)
> ```
> 
> Because my KAuth is a system-provided installation, and KTextEditor should be 
> installed into a user-writable prefix, I get this error on `ninja install`:
> 
> ```
> CMake Error at src/cmake_install.cmake:143 (file):
>   file INSTALL cannot copy file
>   "/home/milian/projects/kf5/build/frameworks/ktexteditor/src/
> org.kde.ktexteditor.katetextbuffer.policy"
>   to "/usr/share/polkit-1/actions/org.kde.ktexteditor.katetextbuffer.policy":
>   Permission denied.
> Call Stack (most recent call first):
>   cmake_install.cmake:77 (include)
> ```
> 
> I can workaround this by either commenting out the kauth install bits in 
> ktexteditor or by installing kauth seperately too. But both are in my opinion 
> not ideal.
> 
> I don't think it's currently possible to overwrite the KAuth install 
> directory 
> at cmake configure time. I would like to change that, unless there is 
> something I'm missing which would prevent this? I am hoping that any folder 
> works as long as it's listed in the `XDG_DATA_DIRS` env var - can someone 
> confirm this?

Alas, the path is hardcoded at buildtime from what I see:
https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/src/polkitbackend/polkitbackendinteractiveauthority.c#L302

Relevant: https://bugs.kde.org/show_bug.cgi?id=425272

HS



signature.asc
Description: OpenPGP digital signature

Re: stale MR triaging - you can help!

[Harald Sitter]

On 16.10.20 10:00, Albert Astals Cid wrote:
> El dimarts, 13 d’octubre de 2020, a les 15:53:10 CEST, Harald Sitter va 
> escriure:
>> Moin
>>
>> As promised in a previous mail I've set up a system that helps us find
>> stale MRs to ensure patches don't fall by the wayside.
>>
>> Lists are filed as issues on invent. To help out you can hit the bell to
>> subscribe to `New issue` events at
>> https://invent.kde.org/sysadmin/gitlab-triaging
>>
>> First list of stale MRs here:
>> https://invent.kde.org/sysadmin/gitlab-triaging/-/issues
>>
>> To deal with a stale MR you'll want to check the box and either make
>> sure someone takes a look at the MR (e.g. by @mentioning someone who may
>> have experience) or doing the review yourself.
>>
>> Unlike previously described I've refined the behavior a bit:
>>
>> a) MRs are not linked but listed with verbatim url, it sucks a bit but
>> it prevents MRs from updating due to the back-reference an actual
>> mention would cause
>> b) stale MRs are now MRs that have not seen activity within 4 weeks (up
>> from 2 - was woefully too short a time frame)
>> c) MRs without subscribers after 2+ weeks are recorded separately
>> d) MRs without subscribers after 1+ week where the author is not a KDE
>> dev are also recorded (currently a bit bugged as it turns out)
>>
>> The motivation with c and d is to prevent MRs from going stale to begin
>> with, ideally anyway.
> 
> Can we get the issues in each of those 4 categories in different sections of 
> the issue you create?
> 
> It makes more sense for me to look at d) than at some MR whose author is 
> Aleix (for example), if it's stale, he knows how to do how to un-stale it, 
> while a newbie may not know how to.

Indeed. That's actually how it works, we've already dealt with the
important ones this week though. So all that's left is the mountain of
otherwise randomly stale MRs ;)

https://invent.kde.org/sysadmin/gitlab-triaging/-/issues?scope=all=%E2%9C%93=closed

HS

stale MR triaging - you can help!

[Harald Sitter]

Moin

As promised in a previous mail I've set up a system that helps us find
stale MRs to ensure patches don't fall by the wayside.

Lists are filed as issues on invent. To help out you can hit the bell to
subscribe to `New issue` events at
https://invent.kde.org/sysadmin/gitlab-triaging

First list of stale MRs here:
https://invent.kde.org/sysadmin/gitlab-triaging/-/issues

To deal with a stale MR you'll want to check the box and either make
sure someone takes a look at the MR (e.g. by @mentioning someone who may
have experience) or doing the review yourself.

Unlike previously described I've refined the behavior a bit:

a) MRs are not linked but listed with verbatim url, it sucks a bit but
it prevents MRs from updating due to the back-reference an actual
mention would cause
b) stale MRs are now MRs that have not seen activity within 4 weeks (up
from 2 - was woefully too short a time frame)
c) MRs without subscribers after 2+ weeks are recorded separately
d) MRs without subscribers after 1+ week where the author is not a KDE
dev are also recorded (currently a bit bugged as it turns out)

The motivation with c and d is to prevent MRs from going stale to begin
with, ideally anyway.

HS

Re: KDiagram - Persistent FTBFS for stable branch on Windows

[Harald Sitter]

On 12.10.20 11:23, Milian Wolff wrote:
> On Montag, 12. Oktober 2020 11:11:22 CEST Ben Cooksley wrote:
>> Hi KDiagram Developers,
>>
>> The stable branch of KDiagram has been persistently failing to build from
>> source now on Windows for a period greater than 5 days.
> 
> Hey Ben,
> 
> can you link me to such a CI failure? Then I could try to blind-fix it, as I 
> don't have a KDE-on-windows setup here. But maybe it's simple enough to fix 
> even without that.

Just FTR for everyone's benefit:
https://community.kde.org/Guidelines_and_HOWTOs/Build_from_source/Windows#Virtual_Machines

Ever so useful to have if one deals with windows breakage more than once
a year ;)

HS



signature.asc
Description: OpenPGP digital signature

Re: plasma-systemmonitor in kdereview

[Harald Sitter]

On 01.10.20 14:36, Arjen Hiemstra wrote:
> On Thursday, 1 October 2020 14:11:16 CEST Harald Sitter wrote:
>> On 01.10.20 11:36, Arjen Hiemstra wrote:
>>> Hello,
>>>
>>> I'd hereby like to announce that plasma-systemmonitor is in kdereview. It
>>> can be found at https://invent.kde.org/plasma/plasma-systemmonitor .
>>>
>>> plasma-systemmonitor is a new system monitor UI built with Kirigami. It
>>> makes use of the ksystemstats daemon and the faces system for system
>>> monitor plasmoids that were both introduced in Plasma 5.19.
>>>
>>> Our current plan is to do a "preview release" alongside Plasma 5.20, then
>>> have it be an official part of Plasma with 5.21.
>>
>> Cool stuff.
>>
>> L10n is currently a bit incomplete.
>> Notably
>> - the pages files lack any localization at all and I'm also not sure how
>> those could be best localized.
> 
> Yeah that is a good point. If we can somehow extract the strings from these 
> files I think we can make it work. But that may need some custom scripting.
> 
>> - the 'configure columns' feature doesn't load the translated strings
>> into the checkboxes
> 
> Huh, weird. They are being populated by i18n calls, wonder what's going on 
> there.

I also couldn't spot what's going wrong FWIW. There was a problem with
strings getting loaded before the domain was set up or something with
qml plugins. Maybe it's that problem? OTOH why would that affect the
combobox but not the labels. It's very odd indeed.

>>
>> On the subject of configure columns I feel like word wrapping would look
>> better than eliding there https://i.imgur.com/RbAeUhH.png
>>
>> Something is astray with memory calculation. See konsole which is
>> somehow listed as 2.6gib https://i.imgur.com/MjIpj3O.png but its actual
>> children aren't getting nowhere near that number. I'm guessing it's
>> because even detached processes (notably vscode here) are in the same
>> cgroup? If so we probably can't do much about this?
>>
> 
> There's two things going on here I expect: everything started from konsole 
> will be part of the konsole cgroup unless started through kstart5 or a 
> similar 
> tool that explicitly moves things to a new cgroup. I doubt this is solvable 
> without patching all major shells. 
> 
> The other thing is that the "memory" column here is what is called "Total 
> Memory" in KSysGuard, which is the process' private memory with the shared 
> memory added divided over the processes that share that memory. This provides 
> a more accurate number for the actual memory usage of the process, but does 
> mean that it will most likely seem like it suddenly uses more memory, since 
> the "memory" in KSysGuard is only the private memory.

Makes sense.
Maybe we should blacklist konsole or something ^^
I'm almost certain people are going to look at it and go "gosh, konsole
is fat" and then turn to their forum of choice and lament how terribly
inefficient our software is.

>> Sometimes the CPU column shows nan% for new apps. Happens a lot with
>> bustle for me. In fact, sometimes I have two entries one of which is
>> nan% for all eternity https://i.imgur.com/HLavLsI.png
> 
> The lingering one is odd, but may have something to do with cgroups not 
> getting cleaned up properly. If you can reproduce and provide the output of 
> systemd-cgls that would be helpful.

│   ├─user@1000.service
│   │ ├─app.slice
│   │ │ ├─app-org.freedesktop.Bustle-c200626490444a60b898eb65b179d6f7.scope
│   │ │ │ ├─216216 bwrap --args 31 bustle
│   │ │ │ ├─216224 bwrap --args 31 bustle
│   │ │ │ └─216225 bustle
│   │ ├─flatpak-org.freedesktop.Bustle-216216.scope
│   │ │ ├─216221 bwrap --args 31 xdg-dbus-proxy --args=33
│   │ │ └─216222 xdg-dbus-proxy --args=33

Mind you, both are cleaned up on exit and the second one doesn't always
appear to begin with.

>> Also the columns in the processes page lack labels.
> 
> Gah. And applications works correctly?

Yup.

HS

Re: plasma-systemmonitor in kdereview

[Harald Sitter]

On 01.10.20 11:36, Arjen Hiemstra wrote:
> Hello,
> 
> I'd hereby like to announce that plasma-systemmonitor is in kdereview. It can 
> be found at https://invent.kde.org/plasma/plasma-systemmonitor .
> 
> plasma-systemmonitor is a new system monitor UI built with Kirigami. It makes 
> use of the ksystemstats daemon and the faces system for system monitor 
> plasmoids that were both introduced in Plasma 5.19.
> 
> Our current plan is to do a "preview release" alongside Plasma 5.20, then 
> have 
> it be an official part of Plasma with 5.21.

Cool stuff.

L10n is currently a bit incomplete.
Notably
- the pages files lack any localization at all and I'm also not sure how
those could be best localized.
- the 'configure columns' feature doesn't load the translated strings
into the checkboxes

On the subject of configure columns I feel like word wrapping would look
better than eliding there https://i.imgur.com/RbAeUhH.png

Something is astray with memory calculation. See konsole which is
somehow listed as 2.6gib https://i.imgur.com/MjIpj3O.png but its actual
children aren't getting nowhere near that number. I'm guessing it's
because even detached processes (notably vscode here) are in the same
cgroup? If so we probably can't do much about this?

The table highlights are getting messed up when new apps appear/disappear.
New app appears: https://i.imgur.com/tacWeoI.png mind that suddenly the
ksysguard memory cell is selected as well. More or less the same happens
when a process disappears. The cells seem somewhat random though Memory
is almost always a highlighted one.

Sometimes the CPU column shows nan% for new apps. Happens a lot with
bustle for me. In fact, sometimes I have two entries one of which is
nan% for all eternity https://i.imgur.com/HLavLsI.png

Possibly related: when editing the Processes page and discarding all
changes the R/W columns show negative values for a second or two.
https://i.imgur.com/amrfewG.png

Also the columns in the processes page lack labels.

HS

Re: stale MR triage

[Harald Sitter]

On 19.09.20 02:18, Albert Astals Cid wrote:
> Not really, the thing is, for Okular i knew that all things that were old 
> didn't need my attention, it was either on the patch author to come back and 
> fix something or someone else should be the one reviewing, so just looking at 
> the last updated MRs gave me an overview if something had changed that needed 
> my attention, now suddenly that's not true anymore.

Hm. I'm thinking we maybe should filter out okular from the triage in
general then. In the scenarios you describe I'd expect the triage team
to most likely write ping comments of some sort, so the MRs would jump
to the top of the list even without the help of gitlab backreference.

Thoughts?

HS

Re: stale MR triage

[Harald Sitter]

On Fri, Sep 18, 2020 at 3:09 AM Ben Cooksley  wrote:
>
> On Fri, 18 Sep 2020, 2:58 am Harald Sitter,  wrote:
>>
>> Griaß eich!
>
>
> Hi Harald,
>
>>
>> In the KF6 BOF we were chatting about merge requests not being nearly
>> as actively watched because people didn't necessarily subscribe to all
>> projects. While that is a solvable problem by asking people to kindly
>> subscribe, it got us thinking that we should have a way to deal with
>> stale MRs in general. For all projects.
>
>
> Please note that from my reading of the code this would also trigger for 
> people's personal projects as well?

It only lists !personal projects:
https://invent.kde.org/sitter/triage/-/blob/master/plugin.rb#L34 takes
care fo that.

> I assume the token it currently uses is your personal one?

Yep, Bhushan was thinking we should use a bot account, and I agree.

>>
>> So here's the proposal:
>>
>> We'll setup a new triage project (prototype at [1]; going to move)
>> that project runs a pipeline once a week that runs the existing
>> gitlab-triage tool [1] to collect all MRs that haven't received an
>> update for 2 weeks. The MRs are then dumped into an issue on the
>> triage project (ex at [2]). Anyone who is willing to help out with cat
>> herding can subscribe to that project and gets notified of these
>> auto-generated issues. We can then walk through the list of stales to
>> work out a solution for getting them moving (assign a helpful
>> reviewer, ping, review ourselves).
>>
>> Any further thoughts?
>>
>> [1] https://invent.kde.org/sitter/triage/-/pipelines
>> [2] https://gitlab.com/gitlab-org/gitlab-triage
>> [3] https://invent.kde.org/sitter/demo/-/issues/2 (feel free to deal
>> with items on this list already)
>>
>> HS
>
>
> Cheers,
> Ben

Re: stale MR triage

[Harald Sitter]

On Fri, Sep 18, 2020 at 12:36 AM Albert Astals Cid  wrote:
>
> El dijous, 17 de setembre de 2020, a les 16:57:32 CEST, Harald Sitter va 
> escriure:
> > Griaß eich!
> >
> > In the KF6 BOF we were chatting about merge requests not being nearly
> > as actively watched because people didn't necessarily subscribe to all
> > projects. While that is a solvable problem by asking people to kindly
> > subscribe, it got us thinking that we should have a way to deal with
> > stale MRs in general. For all projects.
> >
> > So here's the proposal:
> >
> > We'll setup a new triage project (prototype at [1]; going to move)
> > that project runs a pipeline once a week that runs the existing
> > gitlab-triage tool [1] to collect all MRs that haven't received an
> > update for 2 weeks. The MRs are then dumped into an issue on the
> > triage project (ex at [2]). Anyone who is willing to help out with cat
> > herding can subscribe to that project and gets notified of these
> > auto-generated issues. We can then walk through the list of stales to
> > work out a solution for getting them moving (assign a helpful
> > reviewer, ping, review ourselves).
> >
> > Any further thoughts?
>
> My default "sort MR by age" view in Okular is now unusable since there's 
> suddenly a bunch of MRs that are now 2 days old instead of 9 months old.

To clarify: that's "sort by last update", not age, right? The creation
date of an MR shouldn't change, the update date however does.

> That makes me very unhappy and more unproductive.

I'm sorry, I hadn't thought of the possibility that merely mentioning
an MR counts as an update to the MR.

> And i guess it basically breaks your code since next month those MRs are not 
> without updates for 2 weeks because you just linked to them last week?

It's not the biggest concern, ideally every week all stale MRs should
be dealt with somehow anyway, so there'd be some update which would
make them not stale. Should they become stale again the delay
increases of course, but they'll appear again after 4 weeks from the
original stale mention.

> How can we fix that?

I fear that'd need taking up with gitlab. The reason they become
updated is because linking anywhere on gitlab to any MR introduces a
backreference "so and so mentioned this MR in issue #3" which as we've
found out is considered an update to the MR thus messing with the
last-update sorting. It really is not the most reasonable behavior
IMHO.

Would sorting by creation instead work for you?

HS

stale MR triage

[Harald Sitter]

Griaß eich!

In the KF6 BOF we were chatting about merge requests not being nearly
as actively watched because people didn't necessarily subscribe to all
projects. While that is a solvable problem by asking people to kindly
subscribe, it got us thinking that we should have a way to deal with
stale MRs in general. For all projects.

So here's the proposal:

We'll setup a new triage project (prototype at [1]; going to move)
that project runs a pipeline once a week that runs the existing
gitlab-triage tool [1] to collect all MRs that haven't received an
update for 2 weeks. The MRs are then dumped into an issue on the
triage project (ex at [2]). Anyone who is willing to help out with cat
herding can subscribe to that project and gets notified of these
auto-generated issues. We can then walk through the list of stales to
work out a solution for getting them moving (assign a helpful
reviewer, ping, review ourselves).

Any further thoughts?

[1] https://invent.kde.org/sitter/triage/-/pipelines
[2] https://gitlab.com/gitlab-org/gitlab-triage
[3] https://invent.kde.org/sitter/demo/-/issues/2 (feel free to deal
with items on this list already)

HS

Re: plasma-disks in kdereview

[Harald Sitter]

It's been 3 weeks so I'm considering it in good enough shape for
moving to plasma.

Thanks! :)

On Tue, Aug 4, 2020 at 3:46 PM Harald Sitter  wrote:
>
> Hey
>
> It'd be awesome if I could get some eyes at the new
> https://invent.kde.org/system/plasma-disks which implements smart
> monitoring as requested in https://bugs.kde.org/show_bug.cgi?id=254313
> I've opted to not put it in kinfocenter directly because it is a bit
> much with its own kded module and kauth helper, plus I guess there's
> an opportunity to add further monitoring tech for RAIDs or something.
> It still targets plasma releases though.
>
> This runtime depends on smartctl (from smartmontools) to read the
> smart status via a kauth helper. Device states are managed inside a
> kded that maps the devices into dbus from which the kinfocenter KCM
> then loads the data for visualization.
>
> Ta
> HS

Re: plasma-disks in kdereview

[Harald Sitter]

On Wed, Aug 5, 2020 at 12:00 AM Albert Astals Cid  wrote:
>
> El dimarts, 4 d’agost de 2020, a les 15:46:20 CEST, Harald Sitter va escriure:
> > Hey
> >
> > It'd be awesome if I could get some eyes at the new
> > https://invent.kde.org/system/plasma-disks which implements smart
> > monitoring as requested in https://bugs.kde.org/show_bug.cgi?id=254313
> > I've opted to not put it in kinfocenter directly because it is a bit
> > much with its own kded module and kauth helper, plus I guess there's
> > an opportunity to add further monitoring tech for RAIDs or something.
> > It still targets plasma releases though.
> >
> > This runtime depends on smartctl (from smartmontools) to read the
> > smart status via a kauth helper. Device states are managed inside a
> > kded that maps the devices into dbus from which the kinfocenter KCM
> > then loads the data for visualization.
>
> I have a feeling that the qml i18n's won't work pick up the correct domain, 
> the -DTRANSLATION_DOMAIN only sets the domain for the C++ calls.

Good point. qml extraction was missing too.
As it turns out the qml kcms get their domain name from the kaboutdata
componentname, so I rejiggered the kcm accordingly and the x-test l10n
now works. I do wonder if it'd make more sense to make the kcm a
separate translation domain altogether, calling everything
plasma_disks feels a bit awkward. At the same time having 2
translation domains for 10 strings seems silly too :S

HS

plasma-disks in kdereview

[Harald Sitter]

Hey

It'd be awesome if I could get some eyes at the new
https://invent.kde.org/system/plasma-disks which implements smart
monitoring as requested in https://bugs.kde.org/show_bug.cgi?id=254313
I've opted to not put it in kinfocenter directly because it is a bit
much with its own kded module and kauth helper, plus I guess there's
an opportunity to add further monitoring tech for RAIDs or something.
It still targets plasma releases though.

This runtime depends on smartctl (from smartmontools) to read the
smart status via a kauth helper. Device states are managed inside a
kded that maps the devices into dbus from which the kinfocenter KCM
then loads the data for visualization.

Ta
HS

Re: Blacklisting of PIM from the CI system

[Harald Sitter]

It's a good idea. I thought about that briefly. Two problems
immediately came to mind that made me discard the idea:

a) for builds that are not from git we'd not know the commit count
(unless we glue that into the git release commit / tag - which I am
not sure we actually want, but is certainly a possibility)
b) more importantly: depending on the actual commit count it scales
poorly. for a repo like kdevelop it took 26 or so seconds to actually
count on the hard drives we use for CI when I tried a couple weeks
back.
personally I'd stay away from things that reduce the CI turn around :)

So, overall I think it may be a good solution, there are some hurdles
though. The benefit certainly would be that one could then actively
track dependencies down to the commit they were added to a framework.

On Thu, Dec 5, 2019 at 10:32 AM Luca Beltrame  wrote:
>
> Il giorno Thu, 05 Dec 2019 01:23:28 +0100
> Kevin Kofler  ha scritto:
>
> > The git SHA is not going to work for this, because it is not
> > monotonic. What you really want to know is whether you have something
> > >= 5.65.0.abcd123, and having a 5.65.0.commithash version is not
> > >going to tell you that.
>
> Perhaps (like we do in openSUSE for some git snapshots) using the
> number of commits since the last tag? It should increase monotonically.
>
> (might be a bad idea in this context, but I thought I'd throw that into
> the mix)
>
>

Re: Blacklisting of PIM from the CI system

[Harald Sitter]

The point is not to do a version compare against it, but to have the
actually available sha reported in the build logs.

e.g. cmake reports
```
-- Found KF5I18n:
/usr/lib/x86_64-linux-gnu/cmake/KF5I18n/KF5I18nConfig.cmake (found
version "5.64.0.abcd123")
```

On Thu, Dec 5, 2019 at 1:23 AM Kevin Kofler  wrote:
>
> Harald Sitter wrote:
> > Random thought to make dependency problems more obvious: glue the git
> > sha into the cmake package version for frameworks (when built from
> > git) so it finds "5.65.0.abcd123" given us a lead on what is actually
> > available.
>
> The git SHA is not going to work for this, because it is not monotonic. What
> you really want to know is whether you have something >= 5.65.0.abcd123, and
> having a 5.65.0.commithash version is not going to tell you that.
>
> Kevin Kofler
>

Re: Blacklisting of PIM from the CI system

[Harald Sitter]

On Tue, Dec 3, 2019 at 8:55 PM Volker Krause  wrote:
>
> On Sunday, 1 December 2019 04:00:19 CET Ben Cooksley wrote:
> > On Sun, Dec 1, 2019 at 10:17 AM Volker Krause  wrote:
> > > On Saturday, 30 November 2019 19:14:38 CET Ben Cooksley wrote:
> [...]
> > > > Fixing the current set of failures will not prevent this blacklisting
> > > > action from being carried out - as a recurring issue it needs a
> > > > permanent solution.
> > >
> > > What do you envision this permanent solution to look like?
> >
> > It's hard to say.
> >
> > Given the current problem, which is changes being actively made that
> > break the build, the ideal solution would be the developers who are
> > making these breakages to actively keep an eye on their jobs on the CI
> > system.
>
> That isn't an entirely fair statement IMHO, the changes triggering this were
> perfectly fine with either the latest Frameworks release or a recent enough
> master, just not the delayed master we had available on the CI at this point.
> Seeing this hit other master-tracking builds too (e.g. OpenSuse in #kontact
> this morning), I completely agree though this needs a better solution overall.
>
> Tracking the latest release would be one approach, but from direct discussion
> I understand that's currently not a viable solution due to Plasma's needs. Not
> allowing changes for master compatibility (with the corresponding version
> #ifdefs) is also counter-productive though, as it prevents early testing of
> Frameworks changes (even if just locally). So the best I can think of is
> making sure this situation is widely enough understood, and we have a way to
> find out which exact revisions of Frameworks are deployed. Then we can maybe
> get to the point where we can defer such commits until a recent enough
> revision is available, which IIUC would usually take 48-72h.

Random thought to make dependency problems more obvious: glue the git
sha into the cmake package version for frameworks (when built from
git) so it finds "5.65.0.abcd123" given us a lead on what is actually
available.

https://phabricator.kde.org/D24641 would give us the foundation for that.

Not sure how viable this is, or even if it adds much value. After all
everyone still needs to know to check the specific in the build logs.

Food for thought I guess.

HS

Re: rekonq to unmaintained

[Harald Sitter]

now unmaintained
https://commits.kde.org/sysadmin/repo-metadata/67d487fe1262c65cbef2fdc0c10e80ad14fc22fc

On Thu, Sep 12, 2019 at 3:45 PM Harald Sitter  wrote:
>
> I'd like to suggest moving rekonq to unmaintained
>
> Its master branch is still kdelibs4, the frameworks branch hasn't seen
> any progress in 21 months. Hasn't had a release in years. Very
> unmaintained all in all.
>
> Any objections?
>
> HS

rekonq to unmaintained

[Harald Sitter]

I'd like to suggest moving rekonq to unmaintained

Its master branch is still kdelibs4, the frameworks branch hasn't seen
any progress in 21 months. Hasn't had a release in years. Very
unmaintained all in all.

Any objections?

HS

Re: Possible to move some KF5 frameworks to invent?

[Harald Sitter]

On Tue, Aug 13, 2019 at 12:54 PM Ben Cooksley  wrote:
>
> On Mon, Aug 12, 2019 at 11:48 PM David Faure  wrote:
> >
> > On lundi 12 août 2019 13:04:29 CEST Ben Cooksley wrote:
> > > On Mon, Aug 12, 2019 at 10:54 PM Albert Vaca Cintora
> > >
> > >  wrote:
> > > > On Mon, Aug 12, 2019, 18:46 Ben Cooksley  wrote:
> > > >> On Mon, Aug 12, 2019 at 10:37 PM Albert Vaca Cintora
> > > >>
> > > >>  wrote:
> > > >> > Could we use sysadmin/repo-metadata to know which branch is stable 
> > > >> > and
> > > >> > therefore should be protected and trigger the hooks for closing bugs,
> > > >> > etc?>>
> > > >> Unfortunately that only tells us what the current stable branch is -
> > > >> it doesn't let us know what the other (either historical or up and
> > > >> coming) stable branches are called.
> > > >
> > > > Maybe that is enough? IMO, it makes sense to not consider a bug is 
> > > > closed
> > > > until the commit that fixes it has been merged to either master or the
> > > > current stable branch.
> > >
> > > Unfortunately not, as both future and past stable branches have been
> > > used in the past by distributions as a source of patches for those
> > > maintaining LTS releases.
> > >
> > > Additionally, these branches are authoritative as to what we
> > > previously released
> >
> > That's what tags are for, not branches.
> >
> > > and are needed in the event we need to make
> > > another release of these branches.
> >
> > Right. But this only happens with recent stable branches, not
> > really old stuff like "enterprise3".
> >
> > In any case, we should be able to make a list of stable branches,
> > especially if we can use wildcards like Applications/*.
>
> Unfortunately the problem isn't with Frameworks, Applications and
> Plasma - they're easy to handle and their naming can be scripted
> without too much trouble.
> The problem lies with Extragear, which has a large number of projects,
> all of which have different rules for what a stable branch is named.

As Albert said, the solution would be to establish a common scheme for
protected branches.

> For these, someone ends up with having to maintain and update that
> list as needed.
>
> Maintaining this list would also be complicated because our hooks have
> no idea whether Gitlab considers a branch protected or not - so either
> our hooks handle whether force pushes are allowed or not, or we end up
> duplicating the knowledge in two places.

These are very solvable problems, even with a random-name schemes. We
know which branches are/were used as trunk/stable and could have an
automated system keeping tracking. We can also determine/manage which
branches are marked protected on the gitlab side via the API.

HS

Re: Possible to move some KF5 frameworks to invent?

[Harald Sitter]

On Mon, Aug 12, 2019 at 12:23 AM David Faure  wrote:
>
> On dimanche 11 août 2019 22:14:19 CEST Albert Astals Cid wrote:
> > without having your own fork of a repository, that is annoying for various
> > reasons
>
> If creating a fork can be scripted, then that could be a fallback solution
> (to the problem of "committing everywhere" like some of us do).

https://docs.gitlab.com/ce/api/projects.html#fork-project

there are gitlab CLIs in various languages which I expect already
implement this in an easy to use fashion

HS

Re: binary compatibility and qwidget::event

[Harald Sitter]

That makes sense. Thanks all! :)

On Fri, Jul 12, 2019 at 1:09 PM Volker Krause  wrote:
>
> On Friday, 12 July 2019 11:24:35 CEST Harald Sitter wrote:
> > But why was that BIC to begin with? Which of the "don'ts" did it violate?
>
> IIUC re-implementing a virtual method from a base class (in absence of
> complications like multi-inheritance or co-variant return types) does not
> change the ABI (it changes vtable content, but not vtable layout).
>
> It does however create cases where old binaries still call the old methods
> (see https://community.kde.org/Policies/
> Binary_Compatibility_Issues_With_C%2B%2B#Adding_a_reimplemented_virtual_function).
> That might not always be a relevant problem though.
>
> In my understanding this is the same for the final -> !final case discussed in
> #plasma in this context. It does not impact the ABI at all (it does not even
> change vtable content), but some optimizations in old binaries might no longer
> reflect the new behavior.
>
> Regards,
> Volker
>
> > On Fri, Jul 12, 2019 at 11:18 AM Kai Uwe Broulik 
> wrote:
> > > To avoid situations like [1] and [2]
> > >
> > > [1]
> > > https://cgit.kde.org/kiconthemes.git/commit/?id=1e9af6c54470e890597739f8f2
> > > 189b0743a00b6f [2]
> > > https://cgit.kde.org/kiconthemes.git/commit/?id=083bb8a80fd5941e6fcbaf1ec1
> > > 2a08d8f8c881a5>
> > > Am 12.07.19 um 11:14 schrieb Harald Sitter:
> > > > Hey
> > > >
> > > > our binary compatibility page [1] states that one should
> > > >
> > > > "reimplement event in QObject-derived classes, even if the body for
> > > > the function is just calling the base class' implementation."
> > > >
> > > > Does anyone know the reason this helps maintain BC?
> > > >
> > > > [1]
> > > > https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2
> > > > B%2B
> > > >
> > > > HS
>

Re: binary compatibility and qwidget::event

[Harald Sitter]

But why was that BIC to begin with? Which of the "don'ts" did it violate?

On Fri, Jul 12, 2019 at 11:18 AM Kai Uwe Broulik  wrote:
>
> To avoid situations like [1] and [2]
>
> [1]
> https://cgit.kde.org/kiconthemes.git/commit/?id=1e9af6c54470e890597739f8f2189b0743a00b6f
> [2]
> https://cgit.kde.org/kiconthemes.git/commit/?id=083bb8a80fd5941e6fcbaf1ec12a08d8f8c881a5
>
> Am 12.07.19 um 11:14 schrieb Harald Sitter:
> > Hey
> >
> > our binary compatibility page [1] states that one should
> >
> > "reimplement event in QObject-derived classes, even if the body for
> > the function is just calling the base class' implementation."
> >
> > Does anyone know the reason this helps maintain BC?
> >
> > [1] 
> > https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B
> >
> > HS
> >
>
>
>

binary compatibility and qwidget::event

[Harald Sitter]

Hey

our binary compatibility page [1] states that one should

"reimplement event in QObject-derived classes, even if the body for
the function is just calling the base class' implementation."

Does anyone know the reason this helps maintain BC?

[1] https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B

HS

Re: CI system maintainability

[Harald Sitter]

On Thu, Mar 28, 2019 at 3:57 PM David Jarvie  wrote:
> I agree. Mandatory reviews might work if there is a team of active people 
> working on a project, but if there is only one person with real knowledge of 
> the code

We do have common ownership of code, so if there is only one person
with real knowledge of the code that is a problem in of itself... A
problem which really should get solved... For example by having
mandatory reviews so someone actually has to review code they know
just as little about as everyone else, so in turn they now know a bit
more and can more confidently do reviews ;)

Now to be sure, I am not certain mandatory reviews are in fact the
answer to the problem at hand, nor if they would in fact be possible
to implement reliable. From personal experience I'll say that reviews
almost always are worth it, even for the simple typo fixes. And I also
almost find someone to give at least a casual review even when they
know nothing of the code base. Sometimes perhaps even "I couldn't say
if this change is good, but the code looks correct at least +1" is
better than nobody having looked at the code at all. It ultimately
also becomes a matter of busfactor. If nobody ever has reason to look
at code with a single principal author the busfactor will never
improve.

HS

Re: KDE's release script not functional on stable openSUSE

[Harald Sitter]

2.1 reached EOL almost 2 years ago. I suggest you ruby-build or rvm a
newer version isolated from your system's.

On Thu, Jan 24, 2019 at 11:29 AM Jaroslaw Staniek  wrote:
>
> Hi Jonathan,
> The releaseme tools require ruby 2.3 while openSUSE 42.3 depends on 2.1. And 
> it lacks multiple ruby version support (no RVM).
> After forcibly switching to ruby 2.3 or 2.4 openSUSE stops being functional 
> because yast (core admin tool) depends on ruby 2.1.
> Releaseme worked *great* but I had to roll back to 2.1, making the OS again 
> not capable to develop KDE software releases I work on. Advice welcome.
> Maybe you know a way for isolated ruby installation or other workaround.
> Maybe it would be best if such requirements were better suited for typical 
> capabilities.
> Logs blow:
>
> releaseme/tarme.rb --version 3.1.91 --origin stable --from-config kdb
> ..releaseme/lib/releaseme/requirement_checker.rb:116:in `check': Not all 
> requirements met. (RuntimeError)
> from /home/jarek/dev/src/releaseme/lib/releaseme/requirements.rb:3:in 
> `'
> from /home/jarek/dev/src/releaseme/lib/releaseme/logable.rb:28:in 
> `require_relative'
> from /home/jarek/dev/src/releaseme/lib/releaseme/logable.rb:28:in ` (required)>'
> from /home/jarek/dev/src/releaseme/lib/releaseme/cmakeeditor.rb:24:in 
> `require_relative'
> from /home/jarek/dev/src/releaseme/lib/releaseme/cmakeeditor.rb:24:in 
> `'
> from /home/jarek/dev/src/releaseme/lib/releaseme.rb:22:in 
> `require_relative'
> from /home/jarek/dev/src/releaseme/lib/releaseme.rb:22:in ` (required)>'
> from /home/jarek/dev/src/releaseme/tarme.rb:25:in `require_relative'
> from /home/jarek/dev/src/releaseme/tarme.rb:25:in `'
> - Ruby 2.3.0 or 2.4.0 or 2.5.0 or 2.6.0 required.
>
> --
> regards, Jaroslaw Staniek
>
> KDE:
> : A world-wide network of software engineers, artists, writers, translators
> : and facilitators committed to Free Software development - kde.org
> KEXI:
> : A visual database apps builder - kexi-project.org calligra.org/kexi
>   twitter.com/kexi_project facebook.com/kexi.project t.me/kexi_project
> Qt Certified Specialist:
> : linkedin.com/in/jstaniek

Re: Kdiff3 release.

[Harald Sitter]

this should help https://community.kde.org/ReleasingSoftware
On Thu, Aug 2, 2018 at 9:25 AM Michael Reeves  wrote:
>
> That's what I'm looking at doing. The translation need some minor updating. 
> In particular the docs were changed to correct some obsolete information 
> regarding windows and preprocessor command handling.
>
> On Tue, Jul 31, 2018, 5:26 AM Luigi Toscano  wrote:
>>
>> Michael Reeves ha scritto:
>> > What would be next step in getting a release setup fir kdiff3? Do we 
>> > have
>> > documentation on the process?
>> >
>>
>> Right now kdiff3 is logically placed in a group called historically
>> "playground". The idea is: kick start the program, start few *unstable*
>> releases, ask for a community review (see "kdereview") and then allow the
>> program to have stable releases, either with its own cycle or as part of
>> another "product".
>>
>> The workflow is described here:
>> https://community.kde.org/Policies/Application_Lifecycle
>>
>> kdiff3 is a bit weird as it could have used the Incubator process and go
>> directly to kdereview, but that's not too relevant now.
>>
>> You may want to release a first unstable release, then ask for a community
>> review, or ask for a review first and then be moved to the relevant place and
>> have stable releases.
>>
>> Ciao
>> --
>> Luigi

Re: http://commitfilter.kde.org/ ?

[Harald Sitter]

See

http://markmail.org/thread/hekkp2rcdvir732q
http://markmail.org/thread/b6neyhjak2h4g2yd

On Sun, Oct 29, 2017 at 9:47 PM, Martin Koller  wrote:
> Should http://commitfilter.kde.org/ still exist ?
> Currently I get "unknown host"
>
> --
> Best regards/Schöne Grüße
>
> Martin
> A: Because it breaks the logical sequence of discussion
> Q: Why is top posting bad?
>
> ()  ascii ribbon campaign - against html e-mail
> /\- against proprietary attachments
>
> Geschenkideen, Accessoires, Seifen, Kulinarisches: www.lillehus.at
>
>

Re: Elisa is in kdereview

[Harald Sitter]

On Wed, Jun 21, 2017 at 11:15 PM, Matthieu Gallien
 wrote:
> Hello,
> On mercredi 21 juin 2017 23:01:23 CEST Albert Astals Cid wrote:
>> El divendres, 16 de juny de 2017, a les 22:44:03 CEST, Matthieu Gallien va
>>
>> escriure:
>> > Hello,
>> >
>> > Elisa is now in kdereview and aiming for extragear/multimedia.
>> >
>> > It is a music player from a design from Andrew Lake.
>> > It is using Qt multimedia for playback and a few KDE frameworks. Its UI is
>> > using Qml.
>> >
>> > Its aim is to be simple to use and hopefully in the future powerfull when
>> > needed.
>> >
>> > It should be usable without needing online services but will use them in
>> > the future.
>> >
>> > A few integration bits are missing with respect to Baloo before I can do a
>> > release. Currently music can only be read if in its database that can be
>> > filled by Baloo or a custom file indexer if Baloo is not there.
>>
>> That's kind of a weird design decision, basically i started elisa, it didn't
>> see any of my music, i didn't find a way to add it, so i removed elisa.
>
> It is not a design decision but the current state. I want to also support the
> "let's play this file now" use case. I just had not yet enough time to do it.
>
> I am also planning to add a "busy" indicator to show that Elisa is currently
> getting tracks from Baloo or default music directory (as per QStandardPaths).
> If no music is found, I also want to add a passive notification offering the
> possibility to configure the path to use to search music. I even have a task 
> in
> phabricator for that.
>
>> That'd be my workflow as a regular user.
>
> I am already convinced that first impression is important. At the same time, I
> did request to move to extragear to get covered by CI and to get feedback from
> kde-core-devel.

I think it's fine. Not perfect, but good enough for starters.
The error case handling could definitely be better (no baloo database,
indexing in progress, baloo disabled, baloo broken, no music in
database).

User experience quirks aside, Elisa seems to be in really good shape.
Builds. Plays music. I18n seems to be in order as well.

HS