D25551: Mark KXmlRpcClient as porting aid

[Daniel Vrátil]

dvratil accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R312 KXmlRpcClient

BRANCH
  portaid

REVISION DETAIL
  https://phabricator.kde.org/D25551

To: nicolasfella, #frameworks, dvratil
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

Re: kcoreaddons_add_plugin executable/plugin directory conflict

[Daniel Vrátil]

On Tuesday, 12 May 2020 01:20:02 CEST David Faure wrote:
> On lundi 11 mai 2020 14:07:28 CEST Friedrich W. H. Kossebau wrote:
> > Am Montag, 11. Mai 2020, 10:21:10 CEST schrieb Daniel Vrátil:
> > > Hi all,
> > > 
> > > I'm moving some plugins in kaddressbook and I ran into a problem with
> > 
> > > kcoreaddons_add_plugin:
> > Problem found because you require ECM >= 5.38, triggering the use of the
> > CMAKE_BUILD_DIR/bin as executable artifact placement dir by
> > KDECMakeSettings, compare
> > https://api.kde.org/ecm/kde-module/KDECMakeSettings.html#build-settings
> > 
> > > I have this in the plugin CMakeLists.txt:
> > > 
> > > kaddressbook_add_plugin(
> > > 
> > >   kaddressbook_importexportvcardplugin
> > >   JSON kaddressbook_importexportvcardplugin.json
> > >   SOURCES ${kaddressbook_importexport_vcard_SRCS}
> > >   INSTALL_NAMESPACE kaddressbook/importexportplugin
> > > 
> > > )
> > > 
> > > When I run make on the entire project, it fails with
> > > 
> > > [ 65%] Linking CXX shared module ../../../../bin/kaddressbook/
> > > importexportplugin/kaddressbook_importexportvcardplugin.so
> > > ld: error: cannot open output file ../../../../bin/kaddressbook/
> > > importexportplugin/kaddressbook_importexportvcardplugin.so: Not a
> > > directory
> > > 
> > > Alternatively, if a plugin gets created first, it fails on
> > > 
> > > [ 96%] Linking CXX executable ../bin/kaddressbook
> > > ld: error: cannot open output file ../bin/kaddressbook: Is a directory
> > > 
> > > I realized the problem is that CMake is unable to create the plugin in
> > > $BUILDDIR/bin/kaddressbook/importexportplugin/ directory, because
> > > there's
> > > already $BUILDDIR/bin/kaddressbook executable (or vice versa).
> > > 
> > > I would assume this is a very common problem - having plugins
> > > "namespaced"
> > > in the same directory as is the name of the program (and executable).
> 
> I didn't anticipate this because that name conflict with an executable
> doesn't happen in KDE Frameworks, where the install namespace starts with
> kf5 :(
> 
> > > Should the macro maybe put the plugins into
> > > "$BUILDDIR/bin/plugins/$INSTALL_NAMESPACE/"? Would that make the lookup
> > > any more complicated?
> 
> Yes, at least that was the initial reasoning, because "." is a default
> search path for plugins in Qt. But see below.
> > > Is there some known solution/workaround to this? I'd prefer not to have
> > > to
> > > rename the executable or the plugins namespaces, since having different
> > > name for the program and the plugin "namespace" somewhat defeats the
> > > purpose of a namespace...
> 
> Well, for kontact I used kontact5 as plugin namespace, to avoid ever risking
> a mixup between qt5/kf5-based plugins and qt6/kf6-based plugins. This
> sounds like a good solution to both problems.

Makes sense, I adopted this approach. Thanks, David.

/Dan

> 
> But otherwise:
> 
> I later realized that while '.' is in the search path, it has less priority
> than QT_PLUGIN_PATH, so it was picking up my installed plugins instead of
> the locally built ones. So now modules/ECMAddTests.cmake sets the
> QT_PLUGIN_PATH env var for when running tests
> (https://phabricator.kde.org/D8660). Therefore we could indeed move all
> plugins to a subdir.
> The attached patches do that.
> 
> We however lose the magic of finding plugins in $PWD, when running a
> test/program directly (rather than via ctest) (i.e. like an IDE would run
> them), if the plugins aren't otherwise available in the system.
> 
> So I'm thinking how about just adding a 5? :-)


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

kcoreaddons_add_plugin executable/plugin directory conflict

[Daniel Vrátil]

Hi all,

I'm moving some plugins in kaddressbook and I ran into a problem with 
kcoreaddons_add_plugin:

I have this in the plugin CMakeLists.txt:

kaddressbook_add_plugin(
kaddressbook_importexportvcardplugin
JSON kaddressbook_importexportvcardplugin.json
SOURCES ${kaddressbook_importexport_vcard_SRCS}
INSTALL_NAMESPACE kaddressbook/importexportplugin
)

When I run make on the entire project, it fails with

[ 65%] Linking CXX shared module ../../../../bin/kaddressbook/
importexportplugin/kaddressbook_importexportvcardplugin.so
ld: error: cannot open output file ../../../../bin/kaddressbook/
importexportplugin/kaddressbook_importexportvcardplugin.so: Not a directory

Alternatively, if a plugin gets created first, it fails on

[ 96%] Linking CXX executable ../bin/kaddressbook
ld: error: cannot open output file ../bin/kaddressbook: Is a directory

I realized the problem is that CMake is unable to create the plugin in 
$BUILDDIR/bin/kaddressbook/importexportplugin/ directory, because there's 
already $BUILDDIR/bin/kaddressbook executable (or vice versa).

I would assume this is a very common problem - having plugins "namespaced" in 
the same directory as is the name of the program (and executable). Should the 
macro maybe put the plugins into "$BUILDDIR/bin/plugins/$INSTALL_NAMESPACE/"? 
Would that make the lookup any more complicated?

Is there some known solution/workaround to this? I'd prefer not to have to 
rename the executable or the plugins namespaces, since having different name 
for the program and the plugin "namespace" somewhat defeats the purpose of a 
namespace...

/Dan

-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D29371: KMainWindow: remove doc paragraph about KGlobal::ref usage

[Daniel Vrátil]

dvratil added a reviewer: Frameworks.

REPOSITORY
  R263 KXmlGui

REVISION DETAIL
  https://phabricator.kde.org/D29371

To: dvratil, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29371: KMainWindow: remove doc paragraph about KGlobal::ref usage

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  There's no sign of KGlobal (obviously, it's been moved to kdelibs4support)
  or any other similar code in KMainWindow, so remove the outdated paragraph
  from the documentation.

REPOSITORY
  R263 KXmlGui

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D29371

AFFECTED FILES
  src/kmainwindow.h

To: dvratil, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D24443: Add a plugin system

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> calendarentry.cpp:36
> +
> +CalendarEntry::~CalendarEntry()
> +{

` = default` (instead of empty body)

> calendarentry.cpp:57
> +{
> +return ReadWrite;
> +}

Should that be `d->type`? How can implementations change it? Will 
implementation have to access `calendarentry_p.h`?

REPOSITORY
  R172 KCalendar Core

REVISION DETAIL
  https://phabricator.kde.org/D24443

To: nicolasfella, #frameworks, #plasma, #kde_pim
Cc: ognarb, kde-pim, dkardarakos, vkrause, dvratil, davidedmundson, dhaumann, 
fbampaloukas, dcaliste, dvasin, rodsevich, winterz, mlaurent, knauss

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil added a comment.


  Ping? :)

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D21902

To: dvratil, mart, #frameworks, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27671: Dialog: disconnect from QWindow signals in destructor

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:b6213cdd59cc: Dialog: disconnect from QWindow signals in 
destructor (authored by dvratil).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27671?vs=76441&id=76442

REVISION DETAIL
  https://phabricator.kde.org/D27671

AFFECTED FILES
  src/plasmaquick/dialog.cpp

To: dvratil, #plasma, nicolasfella, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D23339: Fix memory leak in ConfigView and Dialog

[Daniel Vrátil]

dvratil added a comment.


  Fix for the crash: https://phabricator.kde.org/D27671

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D23339

To: dvratil, #plasma, #frameworks, apol, davidedmundson
Cc: nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D23339: Fix memory leak in ConfigView and Dialog

[Daniel Vrátil]

dvratil added a dependent revision: D27671: Dialog: disconnect from QWindow 
signals in destructor.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D23339

To: dvratil, #plasma, #frameworks, apol, davidedmundson
Cc: nicolasfella, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27671: Dialog: disconnect from QWindow signals in destructor

[Daniel Vrátil]

dvratil added a dependency: D23339: Fix memory leak in ConfigView and Dialog.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D27671

To: dvratil, #plasma, nicolasfella
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27671: Dialog: disconnect from QWindow signals in destructor

[Daniel Vrátil]

dvratil created this revision.
dvratil added reviewers: Plasma, nicolasfella.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  When Dialog is being destroyed, its QWindow super-class may still emit
  some signals from its destructor. Dialog is connected so some of them,
  so this leads to Qt invoking slots on Dialog, whose destructor has
  already been called, leading to crashes.
  
  This patch disconnects all internal connections in Dialog's destructor.
  
  See https://phabricator.kde.org/D23339 for details.

TEST PLAN
  Plasma no longer crashes when a notification appears

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27671

AFFECTED FILES
  src/plasmaquick/dialog.cpp

To: dvratil, #plasma, nicolasfella
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil updated this revision to Diff 76365.
dvratil added a comment.


  - Rebase to master

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21902?vs=60891&id=76365

BRANCH
  arcpatch-D21902

REVISION DETAIL
  https://phabricator.kde.org/D21902

AFFECTED FILES
  src/declarativeimports/calendar/daysmodel.cpp
  src/declarativeimports/calendar/daysmodel.h
  src/declarativeimports/calendar/eventdatadecorator.cpp
  src/declarativeimports/calendar/eventdatadecorator.h
  src/declarativeimports/calendar/eventpluginsmanager.cpp
  src/declarativeimports/calendar/eventpluginsmanager.h

To: dvratil, mart, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil added a reviewer: Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D21902

To: dvratil, mart, #frameworks, #plasma
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27627: Fix initialization order

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:1e11099c1616: Fix initialization order (authored by 
dvratil).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27627?vs=76306&id=76361

REVISION DETAIL
  https://phabricator.kde.org/D27627

AFFECTED FILES
  src/plasmaquick/appletquickitem.cpp
  src/scriptengines/qml/plasmoid/dropmenu.cpp

To: dvratil, #plasma, #frameworks, apol
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D23339: Fix memory leak in ConfigView and Dialog

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:4b93b71a4924: Fix memory leak in ConfigView and Dialog 
(authored by dvratil).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23339?vs=76305&id=76360

REVISION DETAIL
  https://phabricator.kde.org/D23339

AFFECTED FILES
  src/plasmaquick/configview.cpp
  src/plasmaquick/configview.h
  src/plasmaquick/dialog.h

To: dvratil, #plasma, #frameworks, apol, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27627: Fix initialization order

[Daniel Vrátil]

dvratil created this revision.
dvratil added reviewers: Plasma, Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27627

AFFECTED FILES
  src/plasmaquick/appletquickitem.cpp
  src/scriptengines/qml/plasmoid/dropmenu.cpp

To: dvratil, #plasma, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D23339: Fix memory leak in ConfigView and Dialog

[Daniel Vrátil]

dvratil added reviewers: Plasma, Frameworks.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D23339

To: dvratil, #plasma, #frameworks
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D23339: Fix memory leak in ConfigView and Dialog

[Daniel Vrátil]

dvratil updated this revision to Diff 76305.
dvratil added a comment.


  - Rebase on current master

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23339?vs=64284&id=76305

BRANCH
  arcpatch-D23339

REVISION DETAIL
  https://phabricator.kde.org/D23339

AFFECTED FILES
  src/plasmaquick/configview.cpp
  src/plasmaquick/configview.h
  src/plasmaquick/dialog.h

To: dvratil
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27538: Registry: don't destroy the callback on globalsync

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R127:4ceb35672dfa: Registry: don't destroy the callback 
on globalsync (authored by dvratil).

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27538?vs=76084&id=76097

REVISION DETAIL
  https://phabricator.kde.org/D27538

AFFECTED FILES
  src/client/registry.cpp

To: dvratil, #kwin, zzag
Cc: zzag, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27538: Registry: don't destroy the callback on globalsync

[Daniel Vrátil]

dvratil edited the summary of this revision.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D27538

To: dvratil, #kwin
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27538: Registry: don't destroy the callback on globalsync

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: KWin.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  Instead just unref it, because the wl_display_dispatch_queue_pending
  will try to destroy the callback afterwards as well, leading to
  invalid read/write.
  
  Fixes Valgrind warnings when running KScreen tests:
  
  460922== Invalid read of size 4
  ---
  
  460922==at 0x5CE5B34: wl_proxy_unref (wayland-client.c:230)
  ---
  
  460922==by 0x5CE5C33: destroy_queued_closure (wayland-client.c:292)
  ---
  
  460922==by 0x5CE74AB: dispatch_queue (wayland-client.c:1591)
  
  
  460922==by 0x5CE74AB: wl_display_dispatch_queue_pending 
(wayland-client.c:1833)
  
---
  
  460922==by 0x4E0240D: KWayland::Client::EventQueue::dispatch() 
(src/frameworks/kwayland/src/client/event_queue.cpp:96)
  
--
  
  g==460922==  Address 0x17233aac is 44 bytes inside a block of size 80 free'd
  
  460922==at 0x483B9F5: free (vg_replace_malloc.c:540)
  
  
  460922==by 0x4E15B60: destroy 
(src/frameworks/kwayland/src/client/wayland_pointer_p.h:63)
  
-
  
  460922==by 0x4E15B60: 
KWayland::Client::Registry::Private::globalSync(void*, wl_callback*, unsigned 
int) (src/frameworks/kwayland/src/client/registry.cpp:548)
  
--
  
  ...
  
  460922==by 0x5CE74AB: dispatch_queue (wayland-client.c:1591)
  
  
  460922==by 0x5CE74AB: wl_display_dispatch_queue_pending 
(wayland-client.c:1833)
  
---
  
  460922==by 0x4E0240D: KWayland::Client::EventQueue::dispatch() 
(src/frameworks/kwayland/src/client/event_queue.cpp:96)
  
--

TEST PLAN
  Run testkwaylandbackend from libkscreen under Valgrind - no more invalid reads

REPOSITORY
  R127 KWayland

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27538

AFFECTED FILES
  src/client/registry.cpp

To: dvratil, #kwin
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D24597: [Konversation] Port from deprecated KTcpSocket to QSslSocket

[Daniel Vrátil]

dvratil accepted this revision.
dvratil added a comment.
This revision is now accepted and ready to land.


  Looks good to me, thanks!

REPOSITORY
  R7 Konversation

BRANCH
  ahmad/ktcpsocket-qsslsocket (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24597

To: ahmadsamir, #konversation, dvratil, vkrause
Cc: kde-frameworks-devel, #konversation

D24443: Add a plugin system

[Daniel Vrátil]

dvratil added a comment.


  - I would add `name` property to the `CalendarEntry`
  - I would add `color` property to the `CalendarEntry`, so that calendar that 
is e.g. green in KOrganizer would show up green in the Plasma applet, too.
  
  The Akonadi plugin for this will need to expose a list of multiple calendars, 
otherwise the user would only be able to toggle all-or-nothing, but not to 
toggle individual calendars. In Akonadi, we can have a tree of calendar 
folders, but realistically what you usually see is only the account folder with 
calendar subfolders, without any deeper nesting. I wonder if maybe we could 
have some system of "groups", so e.g. all different plugins that provide some 
holidays plugins would make the calendars members of "holidays" group, the 
Akonadi calendars could be groupped by the account name they belong to...this 
could be then shown nicely in the (QML) UI.

INLINE COMMENTS

> calendarentry.h:28
> +
> +class Q_DECL_EXPORT CalendarEntry : public QObject
> +{

I'm confused, does this represent a single calendar or  event? To me a calendar 
entry is an event. I realize `Calendar` cannot be used, but I think it should 
be a better name, or maybe be a nested class in `CalendarPlugin` (if you can 
nest QObjects, I actually don't know...).

> calendarentry.h:28
> +
> +class Q_DECL_EXPORT CalendarEntry : public QObject
> +{

Should this be declared in the `KCalendarCore` namespace?

> calendarentry.h:48
> +CalendarEntry(const QString &id, const QString &description, const 
> QString &icon, CalendarType type, KCalendarCore::Calendar::Ptr calendar);
> +~CalendarEntry();
> +

`override` (overrides QObject's dtor)

> calendarentry.h:57
> +public Q_SLOTS:
> +void sync();
> +

I would move `sync()` from here to be a virtual method on the plugin - 
`sync(const CalendarEntry::Ptr &)`. The implementations would reimplement it to 
handle sync, which feels cleaner than having to connect to `syncRequested()` 
signal on each calendar that the plugin owns, and it decouples data (calendar) 
from logic (plugin).

> calendarentry.h:63
> +private:
> +CalendarEntryPrivate *d;
> +};

Use `std::unique_ptr const d` for automatic memory 
management.

> calendarplugin.h:31
> +public:
> +CalendarPlugin(QObject *parent, const QVariantList args);
> +

`const QVariantList &args`

> calendarplugin.h:33
> +
> +virtual std::vector calendars() const;
> +

I'd go for QVector, here.

> calendarplugin.h:33
> +
> +virtual std::vector calendars() const;
> +

Should it be a pure virtual function? There's no point in implementing a 
`CalendarPlugin` if you don't reimplement this method...

> calendarplugin.h:39
> +private:
> +void *d;
> +};

Unused?

REVISION DETAIL
  https://phabricator.kde.org/D24443

To: nicolasfella, #frameworks, #plasma, #kde_pim
Cc: vkrause, dvratil, davidedmundson, dhaumann

D23385: More porting from QRegExp to QRegularExpression

[Daniel Vrátil]

dvratil added inline comments.

INLINE COMMENTS

> katedocument.cpp:4676
>  foreach (const QString &pattern, wildcards) {
> -QRegExp wildcard(pattern, Qt::CaseSensitive, QRegExp::Wildcard);
> +QRegularExpression wildcard(QLatin1Char('^') + 
> QRegularExpression::wildcardToRegularExpression(pattern) + QLatin1Char('$'));
>  

@dhaumann `QRegularExpression::wildcardToRegularExpression()` was introduced in 
Qt 5.12, but Frameworks depend on 5.11 :-( So this change breaks build...

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D23385

To: dhaumann, cullmann
Cc: dvratil, kwrite-devel, kde-frameworks-devel, LeGast00n, GB_2, domson, 
michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann

D23340: Fix memory leak in KConfigWatcher

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R237:b9cf875e2e7d: Fix memory leak in KConfigWatcher (authored 
by dvratil).

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D23340?vs=64285&id=64290

REVISION DETAIL
  https://phabricator.kde.org/D23340

AFFECTED FILES
  src/core/kconfigwatcher.cpp
  src/core/kconfigwatcher.h

To: dvratil, davidedmundson
Cc: davidedmundson, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D23340: Fix memory leak in KConfigWatcher

[Daniel Vrátil]

dvratil created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REPOSITORY
  R237 KConfig

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23340

AFFECTED FILES
  src/core/kconfigwatcher.cpp
  src/core/kconfigwatcher.h

To: dvratil
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23339: Fix memory leak in ConfigView and Dialog

[Daniel Vrátil]

dvratil created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D23339

AFFECTED FILES
  src/plasmaquick/configview.cpp
  src/plasmaquick/configview.h
  src/plasmaquick/dialog.h

To: dvratil
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

Re: KCalendarCore plugins/datasources?

[Daniel Vrátil]

On Sunday, 11 August 2019 12:12:10 CEST Bhushan Shah wrote:
> [I am not subscribed to kde-pim list, please keep me or k-f-d in CC]
> 
> Hello,

Hi Bhushan,

> 
> So yesterday I was discussing this with the Volker in #kde-devel, that
> currently kcalcore doesn't provide a "plugin interface" to create a
> various data sources like, file storage, online/cloud storage, or
> akonadi storage, and Akonadi have it's own custom code for this.
> Would it make sense to have something like this in kcalcore itself?
> Volker mentioned to me that it needs close look at Akonadi based
> implementation and trying to finalize API.

There's KCalendarCore::CalStorage which allows users to write their own 
calendar data sources, which could be used as a plugin interface. Extending 
the library with a plugin infrastructure would also be OK IMO, but if we want 
to have a bunch of plugins to integrate between KCalendarCore and various 
backend-specific libraries (e.g. KGAPI, KDAV, Kolab, ...), we should create 
KCalendarAddons (or something like that), assuming there's actually any demand 
for this.

For Akonadi the way it works is that we create an instance of 
KCalendarCore::Calendar and directly populate it with entities loaded from 
Akonadi. We don't use the CalStorage interface for that as we don't need that 
kind of abstraction to integrate between KCalendarCore and Akonadi.

Regards,
Dan

> 
> 
> Thanks


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R296:811e78ef0ee3: Calendar events: allow plugins to show 
event details (authored by dvratil).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D21901?vs=60890&id=61798#toc

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21901?vs=60890&id=61798

REVISION DETAIL
  https://phabricator.kde.org/D21901

AFFECTED FILES
  src/calendarevents/calendareventsplugin.cpp
  src/calendarevents/calendareventsplugin.h

To: dvratil, #frameworks, mart, apol
Cc: ngraham, broulik, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, 
bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil added a reviewer: Frameworks.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D21902

To: dvratil, mart, #frameworks
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil added a comment.


  Ping?

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D21902

To: dvratil, mart, #frameworks
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil added a comment.


  Ping?

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D21901

To: dvratil, #frameworks, mart
Cc: ngraham, broulik, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, 
bruns

D22414: Fix build due to missing QVector include

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R290:886f7f4004e5: Fix build due to missing QVector include 
(authored by dvratil).

REPOSITORY
  R290 KPackage

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22414?vs=61631&id=61634

REVISION DETAIL
  https://phabricator.kde.org/D22414

AFFECTED FILES
  src/kpackage/private/versionparser.cpp

To: dvratil, dfaure, davidedmundson
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22413: Fix build after 0b2fe3cf21

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R244:95e9d5f22ea3: Fix build after 0b2fe3cf21 (authored by 
dvratil).

REPOSITORY
  R244 KCoreAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22413?vs=61628&id=61633

REVISION DETAIL
  https://phabricator.kde.org/D22413

AFFECTED FILES
  src/lib/text/kstringhandler.cpp

To: dvratil, dfaure, broulik
Cc: broulik, kde-frameworks-devel, mlaurent, LeGast00n, michaelh, ngraham, bruns

D22414: Fix build due to missing QVector include

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  I get 'implicit instantiation of undefined template 'QVector'
  when building against Qt 5.12.4 without this change.

TEST PLAN
  Builds against Qt 5.12.4

REPOSITORY
  R290 KPackage

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D22414

AFFECTED FILES
  src/kpackage/private/versionparser.cpp

To: dvratil, dfaure
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D22413: Fix build after 0b2fe3cf21

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: dfaure.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  QVector is not unused in kstringhandler.cpp

TEST PLAN
  kcoreaddons master builds against Qt 5.12.4

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D22413

AFFECTED FILES
  src/lib/text/kstringhandler.cpp

To: dvratil, dfaure
Cc: kde-frameworks-devel, mlaurent, LeGast00n, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil updated this revision to Diff 60891.
dvratil added a comment.


  Adapt to API changes in D21901 

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21902?vs=60066&id=60891

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D21902

AFFECTED FILES
  src/declarativeimports/calendar/daysmodel.cpp
  src/declarativeimports/calendar/daysmodel.h
  src/declarativeimports/calendar/eventdatadecorator.cpp
  src/declarativeimports/calendar/eventdatadecorator.h
  src/declarativeimports/calendar/eventpluginsmanager.cpp
  src/declarativeimports/calendar/eventpluginsmanager.h

To: dvratil, mart
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil updated this revision to Diff 60890.
dvratil added a comment.


  Make the change truly API and ABI compatible

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21901?vs=60068&id=60890

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D21901

AFFECTED FILES
  src/calendarevents/calendareventsplugin.cpp
  src/calendarevents/calendareventsplugin.h

To: dvratil, #frameworks, mart
Cc: ngraham, broulik, kde-frameworks-devel, LeGast00n, michaelh, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil added a comment.


  So, we are doomed to `CalendarEventsPluginV2`?  *sadpanda*

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D21901

To: dvratil, #frameworks, mart
Cc: broulik, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil added a reviewer: mart.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D21901

To: dvratil, #frameworks, mart
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil added a dependent revision: D21906: Events plugin: implement the new 
showEvent() API.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D21901

To: dvratil, #frameworks
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil added a dependent revision: D21905: [Digital Clock] Open KOrganizer 
with event details on click.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D21902

To: dvratil, mart
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil updated this revision to Diff 60068.
dvratil added a comment.


  - Fix ABI compatibility

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21901?vs=60065&id=60068

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D21901

AFFECTED FILES
  src/calendarevents/calendareventsplugin.cpp
  src/calendarevents/calendareventsplugin.h

To: dvratil, #frameworks
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: mart.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  This extends the API to allow calendar UIs to request opening a detailed
  view of an event.
  
  Also contains a minor cleanup of unused m_eventPlugins member.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D21902

AFFECTED FILES
  src/declarativeimports/calendar/daysmodel.cpp
  src/declarativeimports/calendar/daysmodel.h
  src/declarativeimports/calendar/eventdatadecorator.cpp
  src/declarativeimports/calendar/eventdatadecorator.h
  src/declarativeimports/calendar/eventpluginsmanager.cpp
  src/declarativeimports/calendar/eventpluginsmanager.h

To: dvratil, mart
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21902: Calendar: add method to show event details

[Daniel Vrátil]

dvratil added a dependency: D21901: Calendar events: allow plugins to show 
event details.

REPOSITORY
  R242 Plasma Framework (Library)

REVISION DETAIL
  https://phabricator.kde.org/D21902

To: dvratil, mart
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil added a dependent revision: D21902: Calendar: add method to show event 
details.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D21901

To: dvratil, #frameworks
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil added a reviewer: Frameworks.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D21901

To: dvratil, #frameworks
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21901: Calendar events: allow plugins to show event details

[Daniel Vrátil]

dvratil created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  This adds new method to the plugin API that will allow plugins to show
  event details when user clicks on the event, for example by
  opening KOrganizer.

REPOSITORY
  R296 KDeclarative

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D21901

AFFECTED FILES
  src/calendarevents/calendareventsplugin.h

To: dvratil
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21314: Don't enable QT_STRICT_ITERATORS on Windows.

[Daniel Vrátil]

dvratil accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D21314

To: dfaure, vkrause, dvratil
Cc: apol, kde-frameworks-devel, kde-buildsystem, bencreasy, michaelh, ngraham, 
bruns

Re: New framework: KCalCore

[Daniel Vrátil]

On Sunday, 14 April 2019 20:17:54 CEST David Faure wrote:
> On dimanche 14 avril 2019 19:46:02 CEST David Jarvie wrote:
> > On 14 April 2019 12:31:41 BST, David Faure  wrote:
> > > On dimanche 7 avril 2019 14:45:09 CEST Volker Krause wrote:
> > > > Hi,
> > > > 
> > > > I'd like to propose KCalCore for review to move from KDE PIM to KF5.
> > > > 
> > > > KCalCore is an implementation of the iCalendar standard based on
> > > 
> > > libical,
> > > 
> > > I wonder about the name, which doesn't mean much outside the circle of
> > > PIM people. Shouldn't this be called KCalendar ?
> > > 
> > > If the "Core" simply means non-GUI, we certainly don't have that word
> > > in every non-GUI framework.
> > 
> > Renaming makes sense. KCalendar suggests it could be about calendar
> > systems,
> Indeed.
> 
> > so to avoid that confusion, perhaps call it KiCalendar?
> 
> Doesn't read very well
> I would want to say KCalendarEvents but I guess the more correct generic
> term would be KCalendarIncidences ... not convicing either.
> 
> Maybe KCal is enough? Reminds of iCal.

Wasn't KCal the original name of the library from pre-Akonadi times? KCalCore 
was a fork of KCal with the pre-Akonadi "Resources" system removed...

-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Re: CI system maintainability

[Daniel Vrátil]

On Thursday, March 28, 2019 3:39:54 PM CET Friedrich W. H. Kossebau wrote:
> Am Donnerstag, 28. März 2019, 11:27:44 CET schrieb Daniel Vrátil:
> > I'm completely fine with mandatory code review for everything and I'd be
> > happy to have this in PIM. I think the biggest problem in PIM to overcome
> > will be finding the reviewers - I dare say I'm currently the only one who
> > has at least a little idea about what's going on in Akonadi, so getting
> > for
> > instance Laurent to review my Akonadi patches might be hard - same for me
> > reviewing Laurent's KMail patches. This will require non-trivial amount of
> > effort for all members of the community to gain deeper understanding of
> > the
> > other components within the project and a willingness to step up and do a
> > code review even if they don't feel 100% comfortable with the code base.
> > But I believe that in the long run the benefits clearly out-weight the
> > cost.
> So why do you not do this already? Why would you only do this is there is a
> policy requiring you to do it?
> And why do you think this policy-driven behavioural change will work or is
> needed with everyone else?

I wrote this with my PIM hat on and targeted the PIM community, without 
realizing this is a cross-post, so I apologize for the confusion. I believe 
each project should choose whatever workflow and policy suits them.

That said, I do put up for review everything that is not Akonadi/PIM core 
(even changes to PIM components that are not "mine"). The reason I don't do 
this for Akonadi is that there's no-one really to review my code, because no-
one else works on it, or at least that has been my perception of the situation 
lately.

I've been thinking about bringing this topic up on the upcoming PIM sprint as 
I would like to have my code reviewed, I think it's a very good and healthy 
thing, but in case of KDE PIM, I think we need to agree that we'll actually 
review each others code, even if it's not "our own" code-base.

> Also, how do you ensure the review is actually of quality?

How do you ensure that the code people commit without a review is of quality?

> And not just socially driven "+1 for my buddie!", where buddy then also
> mentally shifts part of responsibility to other buddy, because, he, more
> eyes means I do not need to do the full work myself, glitches will be
> caught be them surely! Reviewer found a code formatting issue, done here,
> review happened.

This is a fair point, and I certainly am guilty of doing this occasionally in 
the past. But even reviewer can have a reviewer: if you keep accepting patches 
of "dubious quality" (break build, don't work, contain typos ...), someone 
else from the community will eventually notice and point out you should maybe 
be more careful with your reviews or pay attention to certain aspects (like 
"does it actually work?).

> The opposite also has been seen, reviewers feel they need to do "real"
> review and find things, so start to nitpick or to demand wrong changes,
> wasting everyone's time.
> 
> For myself I know I will happily have other people review my patches, but
> only if there are qualified people to be expected to do it with the needed
> quality in a reasonable time.
> 
> Then, trying to force by that policy other people to become proper
> specialist for certain other code projects to do qualified reviews,
> actually is a trade- off. They will have less time for their actual code
> project, becoming less a specialist there (or having time to review other
> people's contribution to that project).
> We need more contributors, not force existing contributors to distribute
> their abilities & resources over more projects (and thus having less for
> their actual one).

AFAIU the policy right now is keep putting up patches for review until you get 
commit access, then continue doing so until you feel like you don't have to 
anymore. 

But having experienced contributors putting up their code for review is a good 
thing as it allows newcomers (and not just them!) to better understand what is 
happening in the project and simple reviews can be a good starting point for 
them to get more involved in the community.

> I am also not aware of many contributors to KDE projects who are not capable
> to make a responsible decision between the few obvious simple fixes and
> those normal changes which better get feedback from others first. If one is
> unsure about one's post-beer late-night quick hack, one will upload it for
> review, no? At least if one's previous late-night commits turned out to be
> late-night mental state impacted.

This I think is somewhat related to the tooling as I said

Re: CI system maintainability

[Daniel Vrátil]

On Thursday, March 28, 2019 9:50:47 AM CET Kevin Ottens wrote:
> Hello,
> 
> On Thursday, 28 March 2019 09:41:29 CET Luca Beltrame wrote:
> > In data giovedì 28 marzo 2019 09:29:22 CET, Kevin Ottens ha scritto:
> > > at your screen or pair with you" in the past. Clearly this compromise
> > > gets
> > > somewhat exploited and that's especially bad in the case of a fragile
> > > and
> > > central component like KDE PIM.
> > 
> > I'm not sure I agree. I can't speak for seasoned developers, but I've
> > found
> > myself in a situation (more than once) where the fix is trivial (compile
> > error, missing ";", etc) and being forced to go through review would (IMO)
> > unnecessarily raise friction.

This is partially a problem of tooling IMO - review for even a trivial change 
will not cause any friction, if the tooling makes it super-easy and natural 
part of your workflow (and then you can always poke someone on IRC "hey, do 
you have 5 seconds for this super-simple review?").

Using arc with Phab is a bit annoying, so I can understand people fighting 
this. Gitlab with their "click this link to turn your commit into a merge 
request" is much better, plus it can be merged by the reviewer with a single 
click so you as a developer don't even need to care about the MR anymore.

> I don't fully disagree with this (although I'd have stories on nefarious
> typos even for what was supposed to be a "trivial fix"). But it becomes a
> question of trade-off, IOW "how hurtful to the project mandatory reviews?"
> vs "how hurtful to the project a central component being very regularly
> broken?".
> 
> I'd argue we're loosing more with the current state of PIM than we'd loose
> with mandatory reviews.

I'm completely fine with mandatory code review for everything and I'd be happy 
to have this in PIM. I think the biggest problem in PIM to overcome will be 
finding the reviewers - I dare say I'm currently the only one who has at least 
a little idea about what's going on in Akonadi, so getting for instance 
Laurent to review my Akonadi patches might be hard - same for me reviewing 
Laurent's KMail patches. This will require non-trivial amount of effort for 
all members of the community to gain deeper understanding of the other 
components within the project and a willingness to step up and do a code 
review even if they don't feel 100% comfortable with the code base. But I 
believe that in the long run the benefits clearly out-weight the cost.

Btw we practice this policy at work and I do truly appreciate it, not only as 
a huge learning experience but so many times just having a second pair of eyes 
to glance over my code has revealed issues that sometimes almost make me 
question my career choice as a programmer :-) I think this is especially 
important for projects like PIM, where most of us contribute at work in 
between waiting for CI and replying to 15 Slack threads or in the evening 
after a long day

Cao,
Dan

> Regards.


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D15851: Fix signals not being emitted when merging two persons

[Daniel Vrátil]

dvratil accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R307 KPeople

BRANCH
  forgotten-signal (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D15851

To: poboiko, #frameworks, dvratil
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:7615e3405c30: Fix deletion of files from DAV (authored by 
dvratil).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15697?vs=42175&id=42192

REVISION DETAIL
  https://phabricator.kde.org/D15697

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: dvratil, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

dvratil edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15697

To: dvratil, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

dvratil updated this revision to Diff 42175.
dvratil added a comment.


  Improve commit message
  
  I thought you were referring to rmdir in the kio slave, but now I
  realize rmdir() is KIO API, not slave API.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15697?vs=42151&id=42175

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15697

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: dvratil, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

dvratil created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dvratil requested review of this revision.

REVISION SUMMARY
  Always assume del() deletes a file, not a directory so don't try
  to append slash at the end of the path. Some servers don't like
  that.
  
  This partially restores change 1bdb286 
 
from Dawit.
  
  BUG: 355441
  FIXED-IN: 5.51

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D15697

AFFECTED FILES
  src/ioslaves/http/http.cpp

To: dvratil
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15697: Fix deletion of files from DAV

[Daniel Vrátil]

dvratil added a reviewer: dfaure.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D15697

To: dvratil, dfaure
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15497: Fix CMakeLists.txt issues

[Daniel Vrátil]

dvratil accepted this revision.
dvratil added a comment.
This revision is now accepted and ready to land.


  Thanks!

REPOSITORY
  R307 KPeople

BRANCH
  cmake-issues (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D15497

To: poboiko, #frameworks, dvratil, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15431: Make PersonPluginManager API public

[Daniel Vrátil]

dvratil accepted this revision.
dvratil added a comment.
This revision is now accepted and ready to land.


  Nice, thank you!

REPOSITORY
  R307 KPeople

REVISION DETAIL
  https://phabricator.kde.org/D15431

To: poboiko, #kde_pim, dvratil, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns, dvasin, rodsevich, winterz, 
vkrause, mlaurent, knauss, dvratil

D15497: Fix CMakeLists.txt issues

[Daniel Vrátil]

dvratil requested changes to this revision.
dvratil added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> personactionsmodel.cpp:21
>  #include 
> -#include 
> +#include 
>  #include 

This looks suspicious. When compiling, you should not need to refer to 
``, the includes should actually be

  #include "persondata.h"
  #include "widgets/actions.h"

otherwise, there's a risk that a wrong header gets included from installed 
KPeople instead of from the source tree.

I suspect this module lived elsewhere and got just moved into KPeople without 
adjusting the includes.

REPOSITORY
  R307 KPeople

REVISION DETAIL
  https://phabricator.kde.org/D15497

To: poboiko, #frameworks, dvratil, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D15431: Make PersonPluginManager API public

[Daniel Vrátil]

dvratil requested changes to this revision.
dvratil added a comment.
This revision now requires changes to proceed.


  Just some minor details and we are good to go :)

INLINE COMMENTS

> personpluginmanager.h:34
>  class KPEOPLE_EXPORT PersonPluginManager
>  {

Sorry, forgot to mention this in the first round. Since this class is now going 
to be part of public API, we should add at least a brief documentation and

  @since 5.51

(the version of Frameworks that this will land in)

> personpluginmanager.h:43
> + */
> +static void setAutoloadDataSourcePlugins(bool 
> loadSystemDataSourcePlugins);
>  static QList dataSourcePlugins();

Could you rename that argument as well, please?

> personpluginmanager.h:48
> + *
> + * Takes ownership of the "source"
> + */

Use `@p sourceId` and `@p source` instead of quotes, doxygen will then know it 
refers to the arguments

REPOSITORY
  R307 KPeople

REVISION DETAIL
  https://phabricator.kde.org/D15431

To: poboiko, #kde_pim, dvratil, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns, dvasin, rodsevich, winterz, 
vkrause, mlaurent, knauss, dvratil

D15431: Make PersonPluginManager API public

[Daniel Vrátil]

dvratil requested changes to this revision.
dvratil added a comment.
This revision now requires changes to proceed.


  Looks good, just some minor nitpicks.
  
  -
  
  > There some ancient compatibility code inside 
PersonPluginManagerPrivate::loadDataSourcePlugins(), which was added 3 years 
ago. Apparently, it's not needed anymore.
  
  Hard to tell :) We probably shouldn't break backwards compatibility?
  
  > Inside widgets/CMakeLists.txt, its headers are installed with PREFIX 
KPeople instead of KPeople/Widgets. Because of that #include 
 does not work, because it refers to 
non-existent header kpeople/persondetailsview.h.
  
  Well spotted. I think it's safe to just fix it. It shouldn't break any code, 
since no code using this would compile atm.
  
  > In the same file, there is redundant install(FILES [...]), which does not 
install any file (line 46)
  
  I see it on line 48, but indeed looks redundant :)
  
  Technically, all changes to Frameworks should go through review, but if you 
don't want to upload another Phab diff you can just refer to this review in the 
commit message.

INLINE COMMENTS

> personpluginmanager.cpp:103
> +{
> +s_instance->m_mutex.lock();
> +if (!s_instance->dataSourcePlugins.contains(sourceId)) {

Use `QMutexLocker` please

> personpluginmanager.cpp:107
> +} else {
> +qWarning() << "Attempting to load data source that is already 
> loaded!";
> +}

This may lead to memory leaks since we are transferring ownership into of the 
`source` into `PersonPluginManager`. I would suggest, in case of duplication, 
to `deleteLater` the original source and replace it with the new one.

> personpluginmanager.h:43
> + */
> +static void setLoadSystemDataSourcePlugins(bool 
> loadSystemDataSourcePlugins);
>  static QList dataSourcePlugins();

How about `setAutoloadDataSourcePlugins`? They don't necessarily have to be 
"system plugins" and we are not preventing them from loading, just from 
autoloading :)

> personpluginmanager.h:45
>  static QList dataSourcePlugins();
> +static void setDataSource(const QString &sourceId, BasePersonsDataSource 
> *source);
>  static BasePersonsDataSource *dataSource(const QString &sourceId);

I would call this `addDataSource`. Also please document that it takes ownership 
of the `source`.

REPOSITORY
  R307 KPeople

REVISION DETAIL
  https://phabricator.kde.org/D15431

To: poboiko, #kde_pim, dvratil, apol
Cc: kde-frameworks-devel, michaelh, ngraham, bruns, dvasin, rodsevich, winterz, 
vkrause, mlaurent, knauss, dvratil

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil added a comment.


  In D10040#286064 , @davidedmundson 
wrote:
  
  > @dvratil want me to finish this?
  
  
  @davidedmundson  Yes, I'd appreciate it. I won't have time to look into this 
any time soon, sorry :(

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D10040

To: dvratil, graesslin, sebas, #kwin
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil marked 2 inline comments as done.
dvratil added inline comments.

INLINE COMMENTS

> davidedmundson wrote in outputdevice_interface.cpp:450
> You're not actually sending the serial number or eisa anywhere..
> 
> The others do it in sendGeometry (including the manufacturer)
> 
> When you do add it, you need to check the client is bound with version 2, 
> otherwise you'll crash the client.
> 
> Something like
>  if(wl_proxy_get_version(resourceOfBoundClient) >= 
> ORG_KDE_KWIN_SETSERIAL_SINCE_VERSION) {
> 
>   org_kde_output_...

I am sending the stuff in `sendGeometry()` (see right above)

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D10040

To: dvratil, graesslin, sebas
Cc: davidedmundson, plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil updated this revision to Diff 25806.
dvratil marked an inline comment as done.
dvratil added a comment.


  Fix versioning

REPOSITORY
  R127 KWayland

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10040?vs=25802&id=25806

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10040

AFFECTED FILES
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: dvratil, graesslin, sebas
Cc: davidedmundson, plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil added a dependent revision: D10042: Parse EDID on the backend side.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D10040

To: dvratil, graesslin, sebas
Cc: davidedmundson, plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, 
alexeymin, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, 
sebas, apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil added a dependent revision: D10041: Improve EDID information on Wayland.

REPOSITORY
  R127 KWayland

REVISION DETAIL
  https://phabricator.kde.org/D10040

To: dvratil, graesslin, sebas
Cc: plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, alexeymin, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, mart, hein

D10040: Add serial number and EISA ID to OutputDevice interface

[Daniel Vrátil]

dvratil created this revision.
dvratil added reviewers: graesslin, sebas.
Restricted Application added subscribers: Frameworks, plasma-devel.
Restricted Application added projects: Plasma on Wayland, Frameworks.
dvratil requested review of this revision.

REPOSITORY
  R127 KWayland

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D10040

AFFECTED FILES
  src/client/outputdevice.cpp
  src/client/outputdevice.h
  src/client/protocols/outputdevice.xml
  src/server/outputdevice_interface.cpp
  src/server/outputdevice_interface.h

To: dvratil, graesslin, sebas
Cc: plasma-devel, #frameworks, schernikov, ZrenBot, ngraham, alexeymin, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, eliasp, sebas, 
apol, mart, hein

D9233: Fix build against older TagLib

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R286:d2d0ae83d2ca: Fix build against TagLib < 1.11 
(authored by dvratil).

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D9233?vs=23583&id=23588

REVISION DETAIL
  https://phabricator.kde.org/D9233

AFFECTED FILES
  src/extractors/taglibextractor.cpp

To: dvratil, mgallien, aacid
Cc: #frameworks

D9233: Fix build against older TagLib

[Daniel Vrátil]

dvratil created this revision.
dvratil added a reviewer: mgallien.
dvratil added a project: Frameworks.

REVISION SUMMARY
  Fix build of KFileMetadata against TagLib older than 1.11.
  
  Fixes build on Windows where Craft only has Taglib 1.9.2 by default.

REPOSITORY
  R286 KFileMetaData

REVISION DETAIL
  https://phabricator.kde.org/D9233

AFFECTED FILES
  src/extractors/taglibextractor.cpp

To: dvratil, mgallien
Cc: #frameworks

D6665: Make kssl compile against OpenSSL 1.1.0

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R239:9a990c69c606: Make kssl compile against OpenSSL 1.1.0 
(authored by dvratil).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D6665?vs=16613&id=20890#toc

REPOSITORY
  R239 KDELibs4Support

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D6665?vs=16613&id=20890

REVISION DETAIL
  https://phabricator.kde.org/D6665

AFFECTED FILES
  src/kssl/kopenssl.cpp
  src/kssl/kopenssl.h
  src/kssl/kssl.cpp
  src/kssl/ksslcallback.c
  src/kssl/ksslcertchain.cpp
  src/kssl/ksslcertificate.cpp

To: dvratil, #frameworks, dfaure
Cc: aacid, arojas, fvogt, ltoscano, rdieter, #frameworks

D8332: Added baloo urls into places model

[Daniel Vrátil]

dvratil requested changes to this revision.
dvratil added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfileplacesmodel.cpp:151
>  
> +// if baloo is enabled, add new ulrs even if the bookbark file is not 
> empty
> +if (d->fileIndexingEnabled &&

typo: `bookbark`

> kfileplacesmodel.cpp:153
> +if (d->fileIndexingEnabled &&
> +root.metaDataItem(QStringLiteral("withBaloon")) != 
> QStringLiteral("true")) {
> +

typo: `withBaloo`

> kfileplacesmodel.cpp:155
> +
> +root.setMetaDataItem("withBaloon", "true");
> +KFilePlacesItem::createSystemBookmark(d->bookmarkManager,

typo: `withBaloo`

> kfileplacesview.cpp:1321
> +{
> +QString date = QString::number(year) + '-';
> +if (month < 10) {

You can use the 'fill' feature of `QString::arg()` to achieve the same thing 
with cleaner code:

  QString("%1-%2-%3").arg(year).arg(month, 2, 10, 0).arg(day, 2, 10, 0);

> kfileplacesview.cpp:1344
> +
> +if (path.endsWith(QLatin1String("documents"))) {
> +searchUrl = searchUrlForType(QStringLiteral("Document"));

You should probably match by `/documents` so that you don't match invalid paths 
(`search:/foodocuments`)

> kfileplacesview.cpp:1360
> +{
> +static const QString jsonQuery("{\"dayFilter\": 0,\
> + \"monthFilter\": 0, \

use `QStringLiteral` for `jsonQuery`, then it won't have to be static

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8332

To: renatoo, #frameworks, #dolphin, #kde_applications, dvratil
Cc: dvratil, ngraham, #frameworks

Re: Build failures in libraries on CI

[Daniel Vrátil]

On Wednesday, 20 September 2017 13:20:22 CEST Ben Cooksley wrote:
> On Wed, Sep 20, 2017 at 11:15 PM, Daniel Vrátil  wrote:
> > On Wednesday, 20 September 2017 12:13:45 CEST Ben Cooksley wrote:
> >> Hi all,
> > 
> > 
> > 
> >> In the case of syndication, it is relying on Qt features it is not
> >> supposed to use as they were introduced in a version later than it's
> >> minimum requirements (and for which alternative API is available if
> >> the compiler message is any indication)
> >> 
> >> See
> >> https://build.kde.org/view/CI%20Management/job/Dependency%20Build%20Extra
> >> ge
> >> ar%20kf5-qt5%20FreeBSDQt5.7/19/consoleText
> > 
> > Syndication master has already been fixed on Monday, let me know if there
> > are any further failures.
> 
> Thanks, i've now triggered a Dependency Build for FreeBSD.
> 
> The only other outstanding PIM related breakages are:
> 1) in Calligra -
> https://build.kde.org/view/Extragear/job/Extragear%20calligra%20kf5-qt5%20SU

Calligra already has a patch on review: https://phabricator.kde.org/D7873

> SEQt5.9/ 2) in Zanshin -
> https://build.kde.org/view/Extragear/job/Extragear%20zanshin%20kf5-qt5%20SUS
> EQt5.9/

IIUC Kevin would like to keep compatibility with the currently released API of 
KCalCore for now, and in case of Zanshin that cannot be done with just a few 
#ifdefs, so for now please ignore Zanshin being red.

> > Dan
> 
> Cheers,
> Ben
> 
> >> Can we get these fixed please?
> >> 
> >> Cheers,
> >> Ben
> > 
> > --
> > Daniel Vrátil
> > www.dvratil.cz | dvra...@kde.org
> > IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)
> > 
> > GPG Key: 0x4D69557AECB13683
> > Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

Re: Build failures in libraries on CI

[Daniel Vrátil]

On Wednesday, 20 September 2017 12:13:45 CEST Ben Cooksley wrote:
> Hi all,
> 

> 
> In the case of syndication, it is relying on Qt features it is not
> supposed to use as they were introduced in a version later than it's
> minimum requirements (and for which alternative API is available if
> the compiler message is any indication)
> 
> See
> https://build.kde.org/view/CI%20Management/job/Dependency%20Build%20Extrage
> ar%20kf5-qt5%20FreeBSDQt5.7/19/consoleText

Syndication master has already been fixed on Monday, let me know if there are 
any further failures.

Dan

> 
> Can we get these fixed please?
> 
> Cheers,
> Ben


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

GPG Key: 0x4D69557AECB13683
Fingerprint: 0ABD FA55 A4E6 BEA9 9A83 EA97 4D69 557A ECB1 3683

signature.asc
Description: This is a digitally signed message part.

D7043: Fix assert in UrlUtil::firstChildUrl()

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:7e03e503b00c: Fix assert in UrlUtil::firstChildUrl() 
(authored by dvratil).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7043?vs=17511&id=17540

REVISION DETAIL
  https://phabricator.kde.org/D7043

AFFECTED FILES
  autotests/urlutiltest.cpp
  src/filewidgets/urlutil_p.h

To: dvratil, dfaure, gregormi
Cc: #frameworks

D7043: Fix assert in UrlUtil::firstChildUrl()

[Daniel Vrátil]

dvratil created this revision.
dvratil added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  When lastUrl = "file:///folder/" and currentUrl = "file:///folder" then 
currentUrl.isParentOf(lastUrl)
  returns true, which causes the code to hit an assert couple lines below the 
check and also to not
  adhere to its own documentation.
  
  This change fixes the behavior by first normalizing both URLs and then 
comparing them.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D7043

AFFECTED FILES
  autotests/urlutiltest.cpp
  src/filewidgets/urlutil_p.h

To: dvratil, dfaure, gregormi
Cc: #frameworks

D6960: Properly handle org.fd.Notifications service failing to start

[Daniel Vrátil]

dvratil created this revision.
dvratil added a project: Frameworks.

REVISION SUMMARY
  Handle the situation when the startServiceByName invoked from the 
NotifyByPopup plugin fails by setting dbusServiceExists back to false. This 
forces the plugin to fallback to KPassivePopup instead of putting all 
notifications into a queue that will likely never be processed.

REPOSITORY
  R289 KNotifications

REVISION DETAIL
  https://phabricator.kde.org/D6960

AFFECTED FILES
  src/notifybypopup.cpp

To: dvratil, mck182, #frameworks

D6665: Make kssl compile against OpenSSL 1.1.0

[Daniel Vrátil]

dvratil added a comment.


  @dfaure neither do I :-) The patch does not require understanding of the API, 
I just want a formal review. Feel free to add someone who knows openssl if you 
want - no-one really touched the code for 10 years, so I don't know who to add 
for review.
  
  @ltoscano Not sure. If both Qt and KSSL use dlsym() to resolve symbols from 
the library instead of linking it directly, then there should be no problem? 
But it's probably safer to use the same versions in both.

REPOSITORY
  R239 KDELibs4Support

REVISION DETAIL
  https://phabricator.kde.org/D6665

To: dvratil, #frameworks, dfaure
Cc: ltoscano, rdieter, #frameworks

D6665: Make kssl compile against OpenSSL 1.1.0

[Daniel Vrátil]

dvratil edited the summary of this revision.

REPOSITORY
  R239 KDELibs4Support

REVISION DETAIL
  https://phabricator.kde.org/D6665

To: dvratil, #frameworks, dfaure
Cc: rdieter, #frameworks

D6665: Make kssl compile against OpenSSL 1.1.0

[Daniel Vrátil]

dvratil created this revision.
dvratil added a project: Frameworks.

REVISION SUMMARY
  OpenSSL 1.1.0 contains some source-incompatible changes, most notably making 
most of the
  structures opaque and introducing new getter/setter functions to modify the 
structures. This
  patch adds some of the newly introduced functions to the KOpenSSL class and 
modifies the code to call them. The implementation of those newly introduced 
methods contains both OpenSSL < 1.1 compatible code (direct structure member 
access) and calls to real functions resolved from OpenSSL>= 1.1 library. Which 
implementation is used is decided at compile time. Some of the existing methods 
were renamed to match the OpenSSL 1.1 naming and to avoid conflicts with 
backward-compatibility names provided by OpenSSL 1.1.
  
  KSSLCertificate::toNetscape() returns empty result when built against OpenSSL 
1.1 since I wasn't able to find a proper equivalent in OpenSSL 1.1 API (and 
there does not seem to be any).

TEST PLAN
  The code compiles under both OpenSSL 1.1 and OpenSSL 1.0.x. I did not test 
the actual functionality.

REPOSITORY
  R239 KDELibs4Support

REVISION DETAIL
  https://phabricator.kde.org/D6665

AFFECTED FILES
  src/kssl/kopenssl.cpp
  src/kssl/kopenssl.h
  src/kssl/kssl.cpp
  src/kssl/ksslcallback.c
  src/kssl/ksslcertchain.cpp
  src/kssl/ksslcertificate.cpp

To: dvratil, #frameworks, dfaure
Cc: #frameworks

D5344: Relax constraints for processing QGroupBoxes

[Daniel Vrátil]

dvratil accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R265 KConfigWidgets

REVISION DETAIL
  https://phabricator.kde.org/D5344

To: vkrause, dvratil, dfaure
Cc: #frameworks

[Differential] [Closed] D3811: Use KPlugin for calendar applet plugins

[Daniel Vrátil]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:3aec1bf2d379: Use KPlugin to load Calendar plugins 
(authored by dvratil).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3811?vs=10022&id=10101

REVISION DETAIL
  https://phabricator.kde.org/D3811

AFFECTED FILES
  src/declarativeimports/calendar/CMakeLists.txt
  src/declarativeimports/calendar/eventpluginsmanager.cpp
  src/declarativeimports/calendar/eventpluginsmanager.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dvratil, mart, davidedmundson, aacid
Cc: #frameworks, #plasma

[Differential] [Updated, 55 lines] D3811: Use KPlugin for calendar applet plugins

[Daniel Vrátil]

dvratil updated this revision to Diff 10022.
dvratil added a comment.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.


  Fix loading of legacy plugins.

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3811?vs=9365&id=10022

REVISION DETAIL
  https://phabricator.kde.org/D3811

AFFECTED FILES
  src/declarativeimports/calendar/CMakeLists.txt
  src/declarativeimports/calendar/eventpluginsmanager.cpp
  src/declarativeimports/calendar/eventpluginsmanager.h

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dvratil, mart, aacid
Cc: #frameworks, #plasma

Re: Review Request 128650: [KBookmarks] Port from QRegExp to QRegularExpression

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128650/
---

(Updated Sept. 8, 2016, 10:11 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 68db76e9d8afce989d246768ebed7a12f9414b34 by Daniel Vrátil 
to branch master.


Repository: kbookmarks


Description
---

Kleopatra was crashing on shutdown during cleanup of global statics with 
QRegExps. Based on [0] I was able to trace the pointer back to .rodata in 
libKF5KBookmarks.so. This ports to QRegularExpression, which is not affected by 
the issue explained in [0].


[0] https://blogs.kde.org/2015/11/05/qregexp-qstringliteral-crash-exit


Diffs
-

  src/kbookmarkimporter_ie.cpp 7dcbfb3 
  src/kbookmarkmanager.cpp dd7a98b 

Diff: https://git.reviewboard.kde.org/r/128650/diff/


Testing
---


Thanks,

Daniel Vrátil

Re: Review Request 128650: [KBookmarks] Port from QRegExp to QRegularExpression

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128650/
---

(Updated Sept. 6, 2016, 10:33 p.m.)


Review request for KDE Frameworks.


Changes
---

Port to QRegularExpression


Summary (updated)
-

[KBookmarks] Port from QRegExp to QRegularExpression


Repository: kbookmarks


Description (updated)
---

Kleopatra was crashing on shutdown during cleanup of global statics with 
QRegExps. Based on [0] I was able to trace the pointer back to .rodata in 
libKF5KBookmarks.so. This ports to QRegularExpression, which is not affected by 
the issue explained in [0].


[0] https://blogs.kde.org/2015/11/05/qregexp-qstringliteral-crash-exit


Diffs (updated)
-

  src/kbookmarkimporter_ie.cpp 7dcbfb3 
  src/kbookmarkmanager.cpp dd7a98b 

Diff: https://git.reviewboard.kde.org/r/128650/diff/


Testing
---


Thanks,

Daniel Vrátil

Review Request 128650: [KBookmarks] Fix crash on app shutdown due to QRegExp(QStringLiteral())

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128650/
---

Review request for KDE Frameworks.


Repository: kbookmarks


Description
---

Kleopatra was crashing on shutdown during cleanup of global statics with 
QRegExps. Based on [0] I was able to trace the pointer back to .rodata in 
libKF5KBookmarks.so. The crash is gone when QRegExp(QLatin1String()) is used.


[0] https://blogs.kde.org/2015/11/05/qregexp-qstringliteral-crash-exit


Diffs
-

  src/kbookmarkimporter_ie.cpp 7dcbfb3 
  src/kbookmarkmanager.cpp dd7a98b 

Diff: https://git.reviewboard.kde.org/r/128650/diff/


Testing
---


Thanks,

Daniel Vrátil

Re: Review Request 128606: [kapidox] Read library fancyname from metainfo.yaml before parsing it from CMakeLists.txt

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128606/
---

(Updated Aug. 5, 2016, 2:11 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Olivier Churlaud.


Changes
---

Submitted with commit c10f62202065b4b760764882689fee00b4f35d75 by Daniel Vrátil 
to branch master.


Repository: kapidox


Description
---

We have some a few repositories called "akonadi-something" (for instance 
"akonadi-calendar"). The current code in kapidox will only parse "fancyname" of 
the library from `project()` macro in CMakeLists.txt. We'd like to be able to 
customize the name though, because in our CMakeLists.txt we have 
`project(Akonadi-Calendar)` (because CMake does not allow spaces in project 
names), which would make the library to be listed as "Akonadi-Calendar" in the 
generated docs, but we'd like it to be listed as "Akonadi Calendar". This patch 
makes kapidox to first look for `fancyname` entry in metainfo.yaml and fall 
back to parsing CMakeLists.txt otherwise.

This also fixes the regex for parsing CMakeLists.txt to support project names 
with dashes and underscores. The current version of the regex returns only 
"Akonadi" for `project(Akonadi-Calendar)`.


Diffs
-

  src/kapidox/preprocessing.py 606d7db 
  src/kapidox/utils.py 5551bf4 

Diff: https://git.reviewboard.kde.org/r/128606/diff/


Testing
---

1) Added `fancyname: Akonadi Calendar` to akonadi-calendar.git/metainfo.yaml 
and ran kapidox_generate - the library is listed as "Akonadi Calendar" in the 
Library list table
2) Ran kapidox_generate on akonadi-contacts.git without adding `fancyname` to 
metainfo.yaml, the library was correctly listed as "Akonadi-Contacts".


Thanks,

Daniel Vrátil

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 128606: [kapidox] Read library fancyname from metainfo.yaml before parsing it from CMakeLists.txt

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128606/
---

Review request for KDE Frameworks and Olivier Churlaud.


Repository: kapidox


Description
---

We have some a few repositories called "akonadi-something" (for instance 
"akonadi-calendar"). The current code in kapidox will only parse "fancyname" of 
the library from `project()` macro in CMakeLists.txt. We'd like to be able to 
customize the name though, because in our CMakeLists.txt we have 
`project(Akonadi-Calendar)` (because CMake does not allow spaces in project 
names), which would make the library to be listed as "Akonadi-Calendar" in the 
generated docs, but we'd like it to be listed as "Akonadi Calendar". This patch 
makes kapidox to first look for `fancyname` entry in metainfo.yaml and fall 
back to parsing CMakeLists.txt otherwise.

This also fixes the regex for parsing CMakeLists.txt to support project names 
with dashes and underscores. The current version of the regex returns only 
"Akonadi" for `project(Akonadi-Calendar)`.


Diffs
-

  src/kapidox/preprocessing.py 606d7db 
  src/kapidox/utils.py 5551bf4 

Diff: https://git.reviewboard.kde.org/r/128606/diff/


Testing
---

1) Added `fancyname: Akonadi Calendar` to akonadi-calendar.git/metainfo.yaml 
and ran kapidox_generate - the library is listed as "Akonadi Calendar" in the 
Library list table
2) Ran kapidox_generate on akonadi-contacts.git without adding `fancyname` to 
metainfo.yaml, the library was correctly listed as "Akonadi-Contacts".


Thanks,

Daniel Vrátil

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128575: KRearrangeColumnsProxyModel: fix assert when running against debug build of Qt

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128575/
---

(Updated Aug. 2, 2016, 6:15 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and David Faure.


Repository: kitemmodels


Description
---

When Qt is built in debug mode, QAbstractItemModel::setModel() will invoke a 
Q_ASSERT that unconditionally queries index of the first row of the model:

Q_ASSERT_X(d->model->index(0,0) == d->model->index(0,0),
   "QAbstractItemView::setModel",
   "A model should return the exact same index "
   "(including its internal id/pointer) when asked for it twice in 
a row.");

This breaks the assumption in the KRearrangeColumnsProxyModel::index() that 
when queried for a valid index, the source model must be able to provide a 
valid index as well.

This patch simply removes the Q_ASSERT from 
KRearrangeColumnsProxyModel::index() and instead returns an invalid 
QModelIndex, as expected.


Diffs
-

  src/krearrangecolumnsproxymodel.cpp 72e4f14 

Diff: https://git.reviewboard.kde.org/r/128575/diff/


Testing
---

Triggered by Kleopatra on start, no more assert with this patch.


Thanks,

Daniel Vrátil

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128578: KRearrangeColumnsProxyModel: fix assert in index(0, 0) on empty model

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128578/#review97998
---


Ship it!




(assuming you approve of my change)

- Daniel Vrátil


On Aug. 2, 2016, 2:39 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128578/
> ---
> 
> (Updated Aug. 2, 2016, 2:39 p.m.)
> 
> 
> Review request for KDE Frameworks and Daniel Vrátil.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> KRearrangeColumnsProxyModel: fix assert in index(0, 0) on empty model
> 
> 
> Diffs
> -
> 
>   autotests/krearrangecolumnsproxymodeltest.cpp 
> 056b2f024ff455d7a85b39f731c920d03f6fd00b 
>   src/krearrangecolumnsproxymodel.cpp 
> 72e4f140d2f1098e63c49ffcab3a33ed9d409ccb 
> 
> Diff: https://git.reviewboard.kde.org/r/128578/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Faure
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128575: KRearrangeColumnsProxyModel: fix assert when running against debug build of Qt

[Daniel Vrátil]

> On Aug. 2, 2016, 2:24 p.m., David Faure wrote:
> > I'll write a corresponding unittest. Why was the source index invalid? 
> > rowCount==0 in the source model?

Yes, the source model was empty.


- Daniel


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128575/#review97996
---


On Aug. 2, 2016, 2:14 p.m., Daniel Vrátil wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128575/
> ---
> 
> (Updated Aug. 2, 2016, 2:14 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> ---
> 
> When Qt is built in debug mode, QAbstractItemModel::setModel() will invoke a 
> Q_ASSERT that unconditionally queries index of the first row of the model:
> 
> Q_ASSERT_X(d->model->index(0,0) == d->model->index(0,0),
>"QAbstractItemView::setModel",
>"A model should return the exact same index "
>"(including its internal id/pointer) when asked for it twice 
> in a row.");
> 
> This breaks the assumption in the KRearrangeColumnsProxyModel::index() that 
> when queried for a valid index, the source model must be able to provide a 
> valid index as well.
> 
> This patch simply removes the Q_ASSERT from 
> KRearrangeColumnsProxyModel::index() and instead returns an invalid 
> QModelIndex, as expected.
> 
> 
> Diffs
> -
> 
>   src/krearrangecolumnsproxymodel.cpp 72e4f14 
> 
> Diff: https://git.reviewboard.kde.org/r/128575/diff/
> 
> 
> Testing
> ---
> 
> Triggered by Kleopatra on start, no more assert with this patch.
> 
> 
> Thanks,
> 
> Daniel Vrátil
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Review Request 128575: KRearrangeColumnsProxyModel: fix assert when running against debug build of Qt

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128575/
---

Review request for KDE Frameworks and David Faure.


Repository: kitemmodels


Description
---

When Qt is built in debug mode, QAbstractItemModel::setModel() will invoke a 
Q_ASSERT that unconditionally queries index of the first row of the model:

Q_ASSERT_X(d->model->index(0,0) == d->model->index(0,0),
   "QAbstractItemView::setModel",
   "A model should return the exact same index "
   "(including its internal id/pointer) when asked for it twice in 
a row.");

This breaks the assumption in the KRearrangeColumnsProxyModel::index() that 
when queried for a valid index, the source model must be able to provide a 
valid index as well.

This patch simply removes the Q_ASSERT from 
KRearrangeColumnsProxyModel::index() and instead returns an invalid 
QModelIndex, as expected.


Diffs
-

  src/krearrangecolumnsproxymodel.cpp 72e4f14 

Diff: https://git.reviewboard.kde.org/r/128575/diff/


Testing
---

Triggered by Kleopatra on start, no more assert with this patch.


Thanks,

Daniel Vrátil

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Jenkins-kde-ci: kdbusaddons master kf5-qt5 » Linux, gcc - Build # 11 - Still Unstable!

[Daniel Vrátil]

On Saturday, April 23, 2016 1:44:16 AM CEST Ben Cooksley wrote:
> On Fri, Apr 22, 2016 at 11:11 PM, David Faure  wrote:
> > On Thursday 21 April 2016 14:57:52 no-re...@kde.org wrote:
> >> GENERAL INFO
> >> 
> >> BUILD UNSTABLE
> >> Build URL:
> >> https://build.kde.org/job/kdbusaddons%20master%20kf5-qt5/PLATFORM=Linux,
> >> compiler=gcc/11/> 
> > Wow, ASAN detected a real bug in Qt here.
> > 
> > Fixed in https://codereview.qt-project.org/156728
> 
> As this issue essentially breaks any test on the CI system that
> depends on Qt I suggest all developers affected by this indicate this
> on the relevant reviews.

Hey guys,

any chance to get this patch into our Qt build on the CI since it has been 
integrated upstream now? As it affects all tests that interact with DBus it 
would be nice to resolve this on the CI.

Thanks
Dan

> 
> Cheers,
> Ben
> 
> > --
> > David Faure, fa...@kde.org, http://www.davidfaure.fr
> > Working on KDE Frameworks 5
> > 
> > ___
> > Kde-frameworks-devel mailing list
> > Kde-frameworks-devel@kde.org
> > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
> 
> _______
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

KService/sycoca assert in tests on CI

[Daniel Vrátil]

Hi guys,

some KDE PIM unit tests are failing on the CI due to an assert in ksycoca, for 
example here: [0]

QWARN  : TestBlogger1::testNetwork() kf5.kservice.sycoca: ERROR: KSycoca 
database corruption!
QWARN  : TestBlogger1::testNetwork() kf5.kservice.sycoca: ERROR: KSycoca 
database corruption!
QWARN  : TestBlogger1::testNetwork() kf5.kservice.sycoca: ERROR: KSycoca 
database corruption!
QWARN  : TestBlogger1::testNetwork() kf5.kservice.sycoca: ERROR: KSycoca 
database corruption!
QDEBUG : TestBlogger1::testNetwork() Recreating ksycoca file ("/home/
jenkins/.qttest/cache/ksycoca5_C_446ImL1tUTkflcd3IPkQK3mGYhc=", version 303)
QDEBUG : TestBlogger1::testNetwork() Menu "applications-kmenuedit.menu" not 
found.
QDEBUG : TestBlogger1::testNetwork() Saving
QWARN  : TestBlogger1::testNetwork() kf5.kservice.sycoca: ERROR: KSycoca 
database corruption!
QFATAL : TestBlogger1::testNetwork() ASSERT: "!dirs.isEmpty()" in file /home/
jenkins/sources/kservice/stable-kf5-qt5/src/sycoca/ksycoca.cpp, line 626

It works locally, so it's possible it's something on the CI, altought we've 
been seeing this problem for quite a while (even before switch to Docker). Any 
ideas what's going on there? Is it a problem on the CI side, our side or maybe 
something in sycoca?

Thanks
Dan


[0] https://build.kde.org/view/KDE-PIM/job/kblog
%20Applications-16.04%20stable-kf5-qt5/7/PLATFORM=Linux,compiler=gcc/
testReport/junit/%28root%29/TestSuite/kblog_testblogger1/


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: MimeType issues on CI

[Daniel Vrátil]

On 2016-04-03 15:28, Scarlett Clark wrote:

On Sun, Apr 3, 2016 at 5:16 AM, Daniel Vrátil 
wrote:


On Sunday, April 3, 2016 12:58:24 PM CEST David Faure wrote:

On Sunday 03 April 2016 12:43:27 Daniel Vrátil wrote:

Turns out when you include /usr/share in XDG_DATA_DIRS, all mime

types

that
might have been provided by other dependencies in
/srv/jenkins/.../foo/inst/ usr/share/mime are ignored, possibly

due to

update-mime-database not being ran for those directories.


I'm not sure I fully understand this sentence. Do you mean the

value of

XDG_DATA_DIRS at update-mime-database time or during the running

of unit

tests?


XDG_DATA_DIRS value when running the unit tests.


update-mime-database works on one specific install dir and creates

files

there. The value of XDG_DATA_DIRS during the run of

update-mime-database

shouldn't matter.

If the stuff /srv/jenkins/.../foo/inst/usr/share/mime is ignored,

it can be

either



1) because update-mime-database wasn't run on that dir
(which is my current hypothesis for the KIO kfileitemtest

failure which

needs a mimetype from kcoreaddons)


The thing is that the content of
/srv/jenkins/.../foo/inst/usr/share/mime is
NOT ignored, when /usr/share is NOT in XDG_DATA_DIRS.

Once you include /usr/share in XDG_DATA_DIRS, the content of the
/srv/.../mime
folders IS ignored.

Akonadi installs a mime file to share/mime/packages/akonadi-mime.xml
which
introduces some new mimetypes and are used to find the right
serialize plugins
for those types. With /usr/share in XDG_DATA_DIRS Akonadi is not
able to find
serializer plugins for application/x-vnd.akonadi.calendar.todo for
instance,
because it (using QMimeDatabase) does not know about this mime type.
Removing
/usr/share from XDG_DATA_DIRS makes Akonadi (QMimeDatabase) find
application/
x-vnd.akonadi.calendar.todo just find and thus find a respective
serializer
plugin for the type.


2) because that dir isn't in
XDG_DATA_DIRS (this is not the problem) later on.


I verified that it is there.


3) because
update-mime-database ran but didn't create the stuff it's supposed

to

create -- I understand your email as saying this might be the

problem, but

I can't confirm it:


If I compare output of "make install" from CI and running it
locally, it
indeed appears that update-mime-database is not run on the CI at
all:

CI:
16:22:17 Install the project...
16:22:17 -- Install configuration: "Debug"
16:22:17 -- Installing:
/home/jenkins/sources/akonadi/kf5-qt5/local-inst/srv/


jenkins/install/ubuntu/x86_64/g++/kf5-qt5/kde/pim/akonadi/inst/usr/share/mime/

packages/akonadi-mime.xml
16:22:17 -- Installing:
/home/jenkins/sources/akonadi/kf5-qt5/local-inst/srv/


jenkins/install/ubuntu/x86_64/g++/kf5-qt5/kde/pim/akonadi/inst/usr/lib/x86_64-

linux-gnu/cmake/KF5Akonadi/KF5AkonadiConfig.cmake

Locally:
-- Install configuration: "Debug"
-- Up-to-date: /opt/kde-devel/share/mime/packages/akonadi-mime.xml
-- Updating MIME database at /opt/kde-devel/share/mime
-- Up-to-date:
/opt/kde-devel/lib64/cmake/KF5Akonadi/KF5AkonadiConfig.cmake



on my own system, I have /usr/share/mime/packages/kde.xml defining
application/x-smb-workgroup and /usr/share in XDG_DATA_DIRS, and

yet

running update-mime-database on

/d/kde/inst/kde_frameworks/share/mime

creates




/d/kde/inst/kde_frameworks/share/mime/application/x-smb-workgroup.xml

as

expected.

I manually ran update-mime-database on the akonadi and

kdepim-runtime

install dirs on Jenkins and the Zanshin tests magically started

passing

again.

Yes, which only proves that the problem was that it didn't run,

not that the

value of XDG_DATA_DIRS is the problem.


Our idea for a quick dirty fix was to simply run

update-mime-database once

CI sets up the environment variables but before the test is

executed.


David, do you have any better suggestions?


Yes, I'd like to know why "make install" in e.g. kcoreaddons

doesn't seem to

run update-mime-database on CI while it does here (and given that
update-mime-database *is* found in the CI).
With the new CI I'm having trouble debugging this, there are no

sources and

build dirs from previous runs anymore :-)


update-mime-database  seems to be run by ECM, the documentation says

# The follow macro is available::
#
#   update_xdg_mimetypes()
#
# Updates the XDG mime database at install time (unless the
``$DESTDIR``
# environment variable is set, in which case it is up to
package managers
to
# perform this task).

I suspect that CI uses "make DESTDIR=/srv/jenkins/./foo/inst
install" to
install stuff, thus not triggering the update, as documented and
it's up to
the CI script to run it manually



Dan


Sorry I have not been more responsive. I am visiting family and have
been traveling.

I think I found the issue. The function that checks whether our
scripts should run it was not looking in /usr/share/mime

Just commited

Re: MimeType issues on CI (Was: Re: Delaying KF 5.21)

[Daniel Vrátil]

On Sunday, April 3, 2016 12:58:24 PM CEST David Faure wrote:
> On Sunday 03 April 2016 12:43:27 Daniel Vrátil wrote:
> > Turns out when you include /usr/share in XDG_DATA_DIRS, all mime types
> > that
> > might have been provided by other dependencies in
> > /srv/jenkins/.../foo/inst/ usr/share/mime are ignored, possibly due to
> > update-mime-database not being ran for those directories.
> 
> I'm not sure I fully understand this sentence. Do you mean the value of
> XDG_DATA_DIRS at update-mime-database time or during the running of unit
> tests?

XDG_DATA_DIRS value when running the unit tests.

> update-mime-database works on one specific install dir and creates files
> there. The value of XDG_DATA_DIRS during the run of update-mime-database
> shouldn't matter.
> 
> If the stuff /srv/jenkins/.../foo/inst/usr/share/mime is ignored, it can be
> either 

> 1) because update-mime-database wasn't run on that dir
>(which is my current hypothesis for the KIO kfileitemtest failure which
> needs a mimetype from kcoreaddons) 

The thing is that the content of /srv/jenkins/.../foo/inst/usr/share/mime is 
NOT ignored, when /usr/share is NOT in XDG_DATA_DIRS.

Once you include /usr/share in XDG_DATA_DIRS, the content of the /srv/.../mime 
folders IS ignored.

Akonadi installs a mime file to share/mime/packages/akonadi-mime.xml which 
introduces some new mimetypes and are used to find the right serialize plugins 
for those types. With /usr/share in XDG_DATA_DIRS Akonadi is not able to find 
serializer plugins for application/x-vnd.akonadi.calendar.todo for instance, 
because it (using QMimeDatabase) does not know about this mime type. Removing 
/usr/share from XDG_DATA_DIRS makes Akonadi (QMimeDatabase) find application/
x-vnd.akonadi.calendar.todo just find and thus find a respective serializer 
plugin for the type.


> 2) because that dir isn't in
> XDG_DATA_DIRS (this is not the problem) later on.

I verified that it is there.

> 3) because
> update-mime-database ran but didn't create the stuff it's supposed to
> create -- I understand your email as saying this might be the problem, but
> I can't confirm it:

If I compare output of "make install" from CI and running it locally, it 
indeed appears that update-mime-database is not run on the CI at all:

CI:
16:22:17 Install the project...
16:22:17 -- Install configuration: "Debug"
16:22:17 -- Installing: /home/jenkins/sources/akonadi/kf5-qt5/local-inst/srv/
jenkins/install/ubuntu/x86_64/g++/kf5-qt5/kde/pim/akonadi/inst/usr/share/mime/
packages/akonadi-mime.xml
16:22:17 -- Installing: /home/jenkins/sources/akonadi/kf5-qt5/local-inst/srv/
jenkins/install/ubuntu/x86_64/g++/kf5-qt5/kde/pim/akonadi/inst/usr/lib/x86_64-
linux-gnu/cmake/KF5Akonadi/KF5AkonadiConfig.cmake


Locally:
-- Install configuration: "Debug"
-- Up-to-date: /opt/kde-devel/share/mime/packages/akonadi-mime.xml
-- Updating MIME database at /opt/kde-devel/share/mime
-- Up-to-date: /opt/kde-devel/lib64/cmake/KF5Akonadi/KF5AkonadiConfig.cmake

> 
> on my own system, I have /usr/share/mime/packages/kde.xml defining
> application/x-smb-workgroup and /usr/share in XDG_DATA_DIRS, and yet
> running update-mime-database on /d/kde/inst/kde_frameworks/share/mime
> creates
> /d/kde/inst/kde_frameworks/share/mime/application/x-smb-workgroup.xml as
> expected.
> > I manually ran update-mime-database on the akonadi and kdepim-runtime
> > install dirs on Jenkins and the Zanshin tests magically started passing
> > again.
> Yes, which only proves that the problem was that it didn't run, not that the
> value of XDG_DATA_DIRS is the problem.
> 
> > Our idea for a quick dirty fix was to simply run update-mime-database once
> > CI sets up the environment variables but before the test is executed.
> > 
> > David, do you have any better suggestions?
> 
> Yes, I'd like to know why "make install" in e.g. kcoreaddons doesn't seem to
> run update-mime-database on CI while it does here (and given that
> update-mime-database *is* found in the CI).
> With the new CI I'm having trouble debugging this, there are no sources and
> build dirs from previous runs anymore :-)

update-mime-database  seems to be run by ECM, the documentation says

# The follow macro is available::
#
#   update_xdg_mimetypes()
#
# Updates the XDG mime database at install time (unless the ``$DESTDIR``
# environment variable is set, in which case it is up to package 
managers 
to
# perform this task).

I suspect that CI uses "make DESTDIR=/srv/jenkins/./foo/inst install" to 
install stuff, thus not triggering the update, as documented and it's up to 
the CI script to run it manually

Dan

-- 
Daniel 

MimeType issues on CI (Was: Re: Delaying KF 5.21)

[Daniel Vrátil]

Hi David,

I've been debugging similar failures in Kevin's Zanshin tests with Ben and 
eventually tracked it down the same mime database problem.

On Sunday, April 3, 2016 10:03:46 PM CEST Ben Cooksley wrote:
> On Sun, Apr 3, 2016 at 9:09 PM, David Faure  wrote:
> > I don't feel good releasing KF 5.21 with 14 failing builds in the CI.
> > Let's fix CI first, then we can release.
> 
> Virtually all of those were networking faults on the part of the CI
> system - sorry about those.
> We'll need to come up with some way of determining the build host at
> fault, and more importantly debugging why those happen.
> 
> > I'm debugging the kservice unit test failure, which is an actual bug (not
> > a CI problem).
> > 
> > I'll also try to debug the kcoreaddons mimetype issue, that one being a CI
> > problem.
> If you're running those manually on the CI system, try dropping /usr
> and /usr/share from XDG_DATA_DIRS.
> Sounds illogical, but the presence of those actually breaks Akonadi
> related unit tests, even though nothing Qt or Akonadi related is
> installed in /usr (go figure?)

Turns out when you include /usr/share in XDG_DATA_DIRS, all mime types that 
might have been provided by other dependencies in /srv/jenkins/.../foo/inst/
usr/share/mime are ignored, possibly due to update-mime-database not being ran 
for those directories.

I manually ran update-mime-database on the akonadi and kdepim-runtime install 
dirs on Jenkins and the Zanshin tests magically started passing again.

Our idea for a quick dirty fix was to simply run update-mime-database once CI 
sets up the environment variables but before the test is executed.

David, do you have any better suggestions?

Dan

> 
> > KTextEditor people, please take a look at
> > https://build.kde.org/view/Frameworks%20kf5-qt5/job/ktexteditor%20master%2
> > 0kf5-qt5/13/PLATFORM=Linux,compiler=gcc/console and please don't wait for
> > me to play unittest police, you can keep an eye on the CI without me...
> > 
> > Plasma people  err ... why isn't plasma-framework listed on
> > https://build.kde.org/view/Frameworks%20kf5-qt5/ anymore?
> > I see that plasma-framework is green again though, well done.
> > I'm just very worried if that view doesn't have all of the frameworks
> > anymore. Would sometime have the time to go through the list and check
> > what else is missing?
> Cheers,
> Ben
> 
> > --
> > David Faure, fa...@kde.org, http://www.davidfaure.fr
> > Working on KDE Frameworks 5
> 
> ___
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


-- 
Daniel Vrátil
www.dvratil.cz | dvra...@kde.org
IRC: dvratil on Freenode (#kde, #kontact, #akonadi, #fedora-kde)

signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127091: Fix IconItem regression in 5.19

[Daniel Vrátil]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127091/
---

(Updated Feb. 16, 2016, 3:16 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Plasma and David Edmundson.


Changes
---

Submitted with commit 525bf2d377c21b41971a72d694a507bf2af0ada6 by Dan Vrátil to 
branch master.


Bugs: 359388
http://bugs.kde.org/show_bug.cgi?id=359388


Repository: plasma-framework


Description
---

Fix sni-qt icons (or generally any non-theme icons) not showing in systray 
after 5184ac94. The fallback code was assuming that the name always comes from 
QIcon::iconTheme icon.


Diffs
-

  src/declarativeimports/core/iconitem.cpp 1d7921a 

Diff: https://git.reviewboard.kde.org/r/127091/diff/


Testing
---

I can see sni-qt apps icons in Plasma systray again.


Thanks,

Daniel Vrátil

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel