Re: Review Request 119008: Adding QStringList KWindowInfo::activities() method
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/#review61256 --- src/kwindowinfo_x11.cpp https://git.reviewboard.kde.org/r/119008/#comment42657 are you sure it's Latin1? Normally string properties are utf8. - Martin Gräßlin On June 30, 2014, 10:22 a.m., Ivan Čukić wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/ --- (Updated June 30, 2014, 10:22 a.m.) Review request for KDE Frameworks, kwin and Martin Gräßlin. Repository: kwindowsystem Description --- NETWindowInfo provides the information about activities a window belong to. libtaskmanager uses it through kde4support. Since there is a new client that needs this information (activity switcher), I think it should go into the main API. Diffs - src/kwindowinfo.h e9b7a0a src/kwindowinfo.cpp 3095932 src/kwindowinfo_p.h 4c26c62 src/kwindowinfo_p_x11.h 2acb3f8 src/kwindowinfo_x11.cpp 87a887c Diff: https://git.reviewboard.kde.org/r/119008/diff/ Testing --- Yes Thanks, Ivan Čukić ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/#review61258 --- ping? It's just a test. - Vishesh Handa On June 26, 2014, 2:48 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/ --- (Updated June 26, 2014, 2:48 p.m.) Review request for KDE Frameworks. Repository: kinit Description --- Added it in order to debug something. Seemed like a useful things have. Diffs - CMakeLists.txt 631b9d0 tests/CMakeLists.txt PRE-CREATION tests/autostarttest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118960/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119008: Adding QStringList KWindowInfo::activities() method
On June 30, 2014, 9:01 a.m., Martin Gräßlin wrote: src/kwindowinfo_x11.cpp, line 301 https://git.reviewboard.kde.org/r/119008/diff/2/?file=285388#file285388line301 are you sure it's Latin1? Normally string properties are utf8. Well, since those are UUIDs, it does not make a difference. I put latin1 since it is (seems) a slightly faster conversion. I have nothing against changing it to uft8 if you want. - Ivan --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/#review61256 --- On June 30, 2014, 8:22 a.m., Ivan Čukić wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/ --- (Updated June 30, 2014, 8:22 a.m.) Review request for KDE Frameworks, kwin and Martin Gräßlin. Repository: kwindowsystem Description --- NETWindowInfo provides the information about activities a window belong to. libtaskmanager uses it through kde4support. Since there is a new client that needs this information (activity switcher), I think it should go into the main API. Diffs - src/kwindowinfo.h e9b7a0a src/kwindowinfo.cpp 3095932 src/kwindowinfo_p.h 4c26c62 src/kwindowinfo_p_x11.h 2acb3f8 src/kwindowinfo_x11.cpp 87a887c Diff: https://git.reviewboard.kde.org/r/119008/diff/ Testing --- Yes Thanks, Ivan Čukić ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119008: Adding QStringList KWindowInfo::activities() method
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/#review61260 --- Ship it! Ship It! - Martin Gräßlin On June 30, 2014, 11:12 a.m., Ivan Čukić wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/ --- (Updated June 30, 2014, 11:12 a.m.) Review request for KDE Frameworks, kwin and Martin Gräßlin. Repository: kwindowsystem Description --- NETWindowInfo provides the information about activities a window belong to. libtaskmanager uses it through kde4support. Since there is a new client that needs this information (activity switcher), I think it should go into the main API. Diffs - src/kwindowinfo.h e9b7a0a src/kwindowinfo.cpp 3095932 src/kwindowinfo_p.h 4c26c62 src/kwindowinfo_p_x11.h 2acb3f8 src/kwindowinfo_x11.cpp 87a887c Diff: https://git.reviewboard.kde.org/r/119008/diff/ Testing --- Yes Thanks, Ivan Čukić ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
KIconLoader problem with KFontInst
Hi all, there is a weird problem with the kf5 version of the KFontInst kcm module. if you load it from SystemSettings, then going back all icons of systemsettings categories disappear. since kfontinst has some own icons, I see it's doing this in KCmFontInst.cpp: KIconLoader::global()-addAppDir(KFI_NAME); KIconLoader::global()-reconfigure(KFI_NAME); if i put this patch in KIconThmemes, it appears to solve the issue: http://paste.opensuse.org/52060340 but still, it doesn't feel quite right.. any idea on what the issue actually is? -- Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KIconLoader problem with KFontInst
On Monday 30 June 2014, Martin Klapetek wrote: Maybe the KFontInst shouldn't use KIconLoader::global() and have its own instance? I think KFontInst should just get away with those personalized icons.. but still seems something is wrong in kiconloader anyways.. -- Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119008: Adding QStringList KWindowInfo::activities() method
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119008/ --- (Updated June 30, 2014, 9:57 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, kwin and Martin Gräßlin. Repository: kwindowsystem Description --- NETWindowInfo provides the information about activities a window belong to. libtaskmanager uses it through kde4support. Since there is a new client that needs this information (activity switcher), I think it should go into the main API. Diffs - src/kwindowinfo.h e9b7a0a src/kwindowinfo.cpp 3095932 src/kwindowinfo_p.h 4c26c62 src/kwindowinfo_p_x11.h 2acb3f8 src/kwindowinfo_x11.cpp 87a887c Diff: https://git.reviewboard.kde.org/r/119008/diff/ Testing --- Yes Thanks, Ivan Čukić ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KIconLoader problem with KFontInst
...or have the icons installed properly (hicolor?) and remove the custom addAppDir call altogether? -- Sent from my Nexus 7 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119008: Adding QStringList KWindowInfo::activities() method
On June 30, 2014, 9:01 a.m., Martin Gräßlin wrote:
src/kwindowinfo_x11.cpp, line 301
https://git.reviewboard.kde.org/r/119008/diff/2/?file=285388#file285388line301
are you sure it's Latin1? Normally string properties are utf8.
Ivan Čukić wrote:
Well, since those are UUIDs, it does not make a difference. I put latin1
since it is (seems) a slightly faster conversion.
I have nothing against changing it to uft8 if you want.
QLatin1Char(',')
About the property type: it really (and only) matters how it's set.
utf-8, ascii or latin1 are all equal on the scope, but you'd run into trouble
if you'd use eg. local8Bit or UTF-16 on the other side.
- Thomas
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119008/#review61256
---
On June 30, 2014, 9:57 a.m., Ivan Čukić wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119008/
---
(Updated June 30, 2014, 9:57 a.m.)
Review request for KDE Frameworks, kwin and Martin Gräßlin.
Repository: kwindowsystem
Description
---
NETWindowInfo provides the information about activities a window belong to.
libtaskmanager uses it through kde4support.
Since there is a new client that needs this information (activity switcher),
I think it should go into the main API.
Diffs
-
src/kwindowinfo.h e9b7a0a
src/kwindowinfo.cpp 3095932
src/kwindowinfo_p.h 4c26c62
src/kwindowinfo_p_x11.h 2acb3f8
src/kwindowinfo_x11.cpp 87a887c
Diff: https://git.reviewboard.kde.org/r/119008/diff/
Testing
---
Yes
Thanks,
Ivan Čukić
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: qt5 polkit-qt-1 and kdesrc-build
On Sat, Jun 28, 2014 at 5:09 PM, Sune Vuorela nos...@vuorela.dk wrote: On 2014-06-27, David Faure fa...@kde.org wrote: This question is still open. We're releasing kauth as part of KF5 but polkit-qt-1 isn't getting released. Is there any maintainer for polkit-qt-1? For that matter, who maintains KAuth, which optionally depends on polkit-qt-1? Isn't KAuth fully useless on linux without polkit-qt ? Yes. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KIconLoader problem with KFontInst
On Mon, Jun 30, 2014 at 12:02 PM, Martin Klapetek
martin.klape...@gmail.com wrote:
...or have the icons installed properly (hicolor?) and remove the custom
addAppDir call altogether?
Not that I grasp the issue, but if it is a general problem that could
have impact on every application using custom icon overloads/additions
in appdir, I am not sure that installing to system-wide hicolor would
be a very practical solution given the amount of apps apparently doing
this (and that's only the ones I have installed...):
locate /usr/share/kde4/apps/*/icons/{oxygen,hicolor} | cut -d/ -f6 |uniq
amarok
k3b
kate
kcachegrind
kdevelop
kfontinst
kmag
kmenuedit
kolourpaint
korgac
kuser
kwalletmanager
okular
quassel
HS
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KAuth and KF5
On Monday 30 June 2014 00:05:10 šumski wrote: On Thursday 26 of June 2014 12:14:49 Milian Wolff wrote: Hey, did you run it through valgrind? Here's what valgrind says: Sounds like a bug in Qt to me, I have to say. Looking at the code, QDBusConnectionPrivate::objectDestroyed looks pretty fragile, I mean it does this at the end: obj-disconnect(this); But from the code in QDBusConnectionPrivate::disconnectSignal nothing jumps out as dangerous directly. The fact, that valgrind is getting confused in the stack trace is not helping either ;-) Could you maybe try to compile qtbase in debug mode and reproduce the issue, such that we get a full stacktrace without optimizations etc.? Anyways, maybe Thiago (CC'ed) can give us some insight on whats going on here. Bye ==30114== Invalid read of size 8 ==30114==at 0xDF080C1: QDBusConnectionPrivate::disconnectSignal(QHashQString, QDBusConnectionPrivate::SignalHook::iterator) (qstring.h:814) ==30114==by 0xDF08430: QDBusConnectionPrivate::objectDestroyed(QObject*) (qdbusintegrator.cpp:1227) ==30114==by 0xDF4F37B: QDBusConnectionPrivate::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_qdbusconnection_p.cpp:131) ==30114==by 0x52F601D: QMetaObject::activate(QObject*, int, int, void**) (qobject.cpp:3680) ==30114==by 0x52F656E: QObject::destroyed(QObject*) (moc_qobject.cpp:205) ==30114==by 0x52FDA07: QObject::~QObject() (qobject.cpp:901) ==30114==by 0xDCD5228: PolkitQt1::Authority::~Authority() (polkitqt1- authority.cpp:164) ==30114==by 0xDCD4DE1: PolkitQt1::(anonymous namespace)::Q_QGS_s_globalAuthority::innerFunction()::Holder::~Holder() (polkitqt1-authority.cpp:40) ==30114==by 0x5C62F78: __run_exit_handlers (in /lib64/libc-2.19.so) ==30114==by 0x5C62FC4: exit (in /lib64/libc-2.19.so) ==30114==by 0x5C4CB0B: (below main) (in /lib64/libc-2.19.so) ==30114== Address 0x0 is not stack'd, malloc'd or (recently) free'd ==30114== ==30114== ==30114== Process terminating with default action of signal 11 (SIGSEGV) ==30114== Access not within mapped region at address 0x0 ==30114==at 0xDF080C1: QDBusConnectionPrivate::disconnectSignal(QHashQString, QDBusConnectionPrivate::SignalHook::iterator) (qstring.h:814) ==30114==by 0xDF08430: QDBusConnectionPrivate::objectDestroyed(QObject*) (qdbusintegrator.cpp:1227) ==30114==by 0xDF4F37B: QDBusConnectionPrivate::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_qdbusconnection_p.cpp:131) ==30114==by 0x52F601D: QMetaObject::activate(QObject*, int, int, void**) (qobject.cpp:3680) ==30114==by 0x52F656E: QObject::destroyed(QObject*) (moc_qobject.cpp:205) ==30114==by 0x52FDA07: QObject::~QObject() (qobject.cpp:901) ==30114==by 0xDCD5228: PolkitQt1::Authority::~Authority() (polkitqt1- authority.cpp:164) ==30114==by 0xDCD4DE1: PolkitQt1::(anonymous namespace)::Q_QGS_s_globalAuthority::innerFunction()::Holder::~Holder() (polkitqt1-authority.cpp:40) ==30114==by 0x5C62F78: __run_exit_handlers (in /lib64/libc-2.19.so) ==30114==by 0x5C62FC4: exit (in /lib64/libc-2.19.so) ==30114==by 0x5C4CB0B: (below main) (in /lib64/libc-2.19.so) On Wednesday 25 June 2014 23:17:29 Luca Beltrame wrote: šumski wrote: http://paste.opensuse.org/view/raw/45956382 In case the pastebin link expires: #0 QString (other=..., this=0x7fff18fa23b0) at ../../src/corelib/tools/qstring.h:814 #1 dbusInterfaceString () at qdbusintegrator.cpp:87 #2 QDBusConnectionPrivate::disconnectSignal (this=this@entry=0xa93640, it=...) at qdbusintegrator.cpp:2270 #3 0x7fde826414b1 in QDBusConnectionPrivate::objectDestroyed (this=0xa93640, obj=0xa6f720) at qdbusintegrator.cpp:1228 #4 0x7fde826883bc in QDBusConnectionPrivate::qt_static_metacall (_o=optimized out, _c=optimized out, _id=optimized out, _a=optimized out) at .moc/moc_qdbusconnection_p.cpp:131 #5 0x7fde83ebcd7e in QMetaObject::activate (sender=sender@entry=0xa6f720, signalOffset=optimized out, local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fff18fa2610) at kernel/qobject.cpp:3680 #6 0x7fde83ebd237 in QMetaObject::activate (sender=sender@entry=0xa6f720, m=m@entry=0x7fde842caf00 QObject::staticMetaObject, local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fff18fa2610) at kernel/qobject.cpp:3546 #7 0x7fde83ebd2cf in QObject::destroyed (this=this@entry=0xa6f720, _t1=_t1@entry=0xa6f720) at .moc/moc_qobject.cpp:205 #8 0x7fde83ec4768 in QObject::~QObject (this=0xa6f720, __in_chrg=optimized out) at kernel/qobject.cpp:901 #9 0x7fde7b36ae39 in PolkitQt1::Authority::~Authority (this=0xa6f720, __in_chrg=optimized out) at /usr/src/debug/polkit-qt-1-0.103.0/core/polkitqt1-authority.cpp:164 #10 0x7fde7b36aa62 in PolkitQt1::(anonymous
Re: Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/#review61278 --- Ship it! Ship It! - Aleix Pol Gonzalez On June 26, 2014, 2:48 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/ --- (Updated June 26, 2014, 2:48 p.m.) Review request for KDE Frameworks. Repository: kinit Description --- Added it in order to debug something. Seemed like a useful things have. Diffs - CMakeLists.txt 631b9d0 tests/CMakeLists.txt PRE-CREATION tests/autostarttest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118960/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/#review61279 --- This review has been submitted with commit b82102fec5a0f54669b54bc5847d3fd76309ae67 by Vishesh Handa to branch master. - Commit Hook On June 26, 2014, 2:48 p.m., Vishesh Handa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/ --- (Updated June 26, 2014, 2:48 p.m.) Review request for KDE Frameworks. Repository: kinit Description --- Added it in order to debug something. Seemed like a useful things have. Diffs - CMakeLists.txt 631b9d0 tests/CMakeLists.txt PRE-CREATION tests/autostarttest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118960/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118960: Add a test to print out KLauncher's autostart files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118960/ --- (Updated June 30, 2014, 12:45 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kinit Description --- Added it in order to debug something. Seemed like a useful things have. Diffs - CMakeLists.txt 631b9d0 tests/CMakeLists.txt PRE-CREATION tests/autostarttest.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/118960/diff/ Testing --- Thanks, Vishesh Handa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118985: KSharedConfig: move mainConfig and wasTestEnabled to the thread storage.
On June 29, 2014, 3:31 a.m., Matthew Dawson wrote: I'm not sure about moving the warning about immutable files to only happen on the main thread, as it is possible that an application may use it KSharedConfig on an alternate thread only. As this is mainly an optimization, I think the best thing is to just move to using the load() function on the atomic integer, as I believe that doesn't require processors to synchronize, and won't limit optimizations. Otherwise, I'm fine with the immutable files as that is simpler code wise. Thoughts? David Faure wrote: Well, I'm not sure we'd expect secondary threads to pop up a dialog though (as KConfig::isConfigWritable(true /*warn user*/) does, via QProcess(kdialog)). This bit (the warnUser code) isn't really about an optimization, but about doing that stuff only in the main thread, where GUI apps initialize the main config object and where user-interaction is expected. Matthew Dawson wrote: I wouldn't either, and I'd except most code access its main config on a primary thread first. I'm just thinking for those 1% of cases, that cause all the trouble. When I mentioned about an optimization, I meant is that the reason for this RR, not the point of the user warning. With the relaxed loading of the atomic int, I don't think there is much, if any of a performance hit. I don't think the code complexity is too bad to handle that 1% either. Is there any other reason for this change? Otherwise, I'd prefer to be safe. Otherwise, this patch looks good with the latest changes. This patch wasn't initially about optimization, but about fixing the behavior for the main thread vs other threads. I think the final version of the patch makes sense: * caching, including mainConfig - for all threads * clearing cache when qstandardpaths test mode gets enabled - for all threads because it's related to the above, even if unlikely to happen in other threads * warning the user with a kdialog - main thread, because this is a GUI operation, which is only expected in the main thread (even if it's technically possible to have it in other threads, since it's delegated to a separate process). This patch is definitely not about improving performance, but about correctness :) - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118985/#review61157 --- On June 29, 2014, 1:55 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118985/ --- (Updated June 29, 2014, 1:55 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- KSharedConfig: move mainConfig and wasTestEnabled to the thread storage. This enables the mainConfig optimization in all threads, and ensures the user warning only happens in the main thread. The test-mode-enabled logic is only really useful in the main thread, but it's simpler to just do it in all threads. REVIEW: 118985 Diffs - autotests/kconfigtest.cpp a8482b7099df5921909830082d758dc3095e3241 src/core/ksharedconfig.cpp b7d155d5893502921d35d7dd971188b6a93a0620 Diff: https://git.reviewboard.kde.org/r/118985/diff/ Testing --- no regression in make test in kconfig; still debugging races in helgrind threadtest (in kio). 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 118264: Switch to PolkitQt5-1
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/#review61286 --- Hi, I tried the patch, it's not building. [10/64] Building CXX object src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o FAILED: /usr/bin/c++ -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_SIGNALS_SLOTS_KEYWORDS -DQT_NO_URL_CAST_FROM_STRING -DQT_STRICT_ITERATORS -DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_QSTRINGBUILDER -DQT_WIDGETS_LIB -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=500 -Dkauth_backend_plugin_EXPORTS -std=c++0x -fno-exceptions -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type -g -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Isrc -I/home/kde-devel/frameworks/kauth/src -I/home/kde-devel/frameworks/kauth/src/include/polkit-qt5-1 -isystem /home/kde-devel/kde5/include -isystem /home/kde-devel/kde5/include/QtDBus -isystem /home/kde-devel/kde5/include/QtCore -isystem /home/kde-devel/kde5/mkspecs/linux-g++ -isystem /home/kde-devel/kde5/include/QtWidgets -isystem /home/kde-devel/kde5/include/Q tGui -MMD -MT src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o -MF src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o.d -o src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o -c /home/kde-devel/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.cpp In file included from /home/kde-devel/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.cpp:22:0: /home/kde-devel/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.h:31:31: fatal error: PolkitQt1/Authority: No such file or directory #include PolkitQt1/Authority ^ compilation terminated. [10/64] Automatic moc for target kauth_tests_static - Aleix Pol Gonzalez On May 28, 2014, 2:01 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/ --- (Updated May 28, 2014, 2:01 p.m.) Review request for KDE Frameworks and Kevin Ottens. Repository: kauth Description --- since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant, KAuth should switch to that, instead of building either w/ no backend, or with the branch... Diffs - cmake/KF5AuthMacros.cmake 4cdf3ab src/ConfigureChecks.cmake aff05ed Diff: https://git.reviewboard.kde.org/r/118264/diff/ Testing --- Got CMake message: Building PolkitQt5-1 KAuth backend and kauth-policy-gen also built KAuth macros generate correct polkit-1 rules (e.g. date kcm in plasma-desktop, etc) Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
tagging date
On Tuesday 24 June 2014 14:21:53 Jonathan Riddell wrote: Sorry for late cancellation but seems I can't chair and I can't find a replacement. Please prepare for final tagging this week, any last binary incompatibility must be in by the weekend. dfaure will tag for final release on Monday. No, on the first of July, I said :) That's tomorrow. -- 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
Re: Review Request 119005: Make desktoptojson include all values, not just the translated ones
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119005/#review61287 --- This seems wrong to me. QSettings is not a desktop file parser. It will get escaping wrong, compared to what the desktop entry spec says. Isn't the long term solution to write .json files directly anyway? - David Faure On June 29, 2014, 5:59 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119005/ --- (Updated June 29, 2014, 5:59 p.m.) Review request for KDE Frameworks and Sebastian Kügler. Repository: kservice Description --- It now uses QSettings to read the desktop file instead of kconfig which means that this tool could move to kcoreaddons. Diffs - src/desktoptojson/CMakeLists.txt 134dabe62ca051eea26631aedc72b108e1b60444 src/desktoptojson/kconfigtojson.cpp 5c0d751f05503e471ed9c49efc51bfdd22774be9 Diff: https://git.reviewboard.kde.org/r/119005/diff/ Testing --- now includes all fields (Comment + Comment[de] + Comment[...], etc) I guess desktoptojson could be move to kcoreaddons for 5.1 and have kservice_desktop_to_json forward to a new kcoreaddons_desktop_to_json for cmake source compatibility Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118264: Switch to PolkitQt5-1
On June 30, 2014, 4:23 p.m., Aleix Pol Gonzalez wrote: Hi, I tried the patch, it's not building. [10/64] Building CXX object src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o FAILED: /usr/bin/c++ -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_SIGNALS_SLOTS_KEYWORDS -DQT_NO_URL_CAST_FROM_STRING -DQT_STRICT_ITERATORS -DQT_USE_FAST_OPERATOR_PLUS -DQT_USE_QSTRINGBUILDER -DQT_WIDGETS_LIB -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_XOPEN_SOURCE=500 -Dkauth_backend_plugin_EXPORTS -std=c++0x -fno-exceptions -Wall -Wextra -Wcast-align -Wchar-subscripts -Wformat-security -Wno-long-long -Wpointer-arith -Wundef -Wnon-virtual-dtor -Woverloaded-virtual -Werror=return-type -g -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Isrc -I/home/kde-devel/frameworks/kauth/src -I/home/kde-devel/frameworks/kauth/src/include/polkit-qt5-1 -isystem /home/kde-devel/kde5/include -isystem /home/kde-devel/kde5/include/QtDBus -isystem /home/kde-devel/kde5/include/QtCore -isystem /home/kde-devel/kde5/mkspecs/linux-g++ -isystem /home/kde-devel/kde5/include/QtWidgets -isystem /home/kde-devel/kde5/inclu de/QtGui -MMD -MT src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o -MF src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o.d -o src/CMakeFiles/kauth_backend_plugin.dir/backends/polkit-1/Polkit1Backend.cpp.o -c /home/kde-devel/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.cpp In file included from /home/kde-devel/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.cpp:22:0: /home/kde-devel/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.h:31:31: fatal error: PolkitQt1/Authority: No such file or directory #include PolkitQt1/Authority ^ compilation terminated. [10/64] Automatic moc for target kauth_tests_static yes, doesn't build against master, see discussion before in the review. polkit-qt-1 needs fixing (absolute vs. relative vars) - Hrvoje --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/#review61286 --- On May 28, 2014, 4:01 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/ --- (Updated May 28, 2014, 4:01 p.m.) Review request for KDE Frameworks and Kevin Ottens. Repository: kauth Description --- since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant, KAuth should switch to that, instead of building either w/ no backend, or with the branch... Diffs - cmake/KF5AuthMacros.cmake 4cdf3ab src/ConfigureChecks.cmake aff05ed Diff: https://git.reviewboard.kde.org/r/118264/diff/ Testing --- Got CMake message: Building PolkitQt5-1 KAuth backend and kauth-policy-gen also built KAuth macros generate correct polkit-1 rules (e.g. date kcm in plasma-desktop, etc) Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119037: Fix KAuth backend loading
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119037/ --- Review request for KDE Frameworks and Vishesh Handa. Repository: kauth Description --- So far we were looking at relative directories to the install prefix, and it was never finding anything. The reason it didn't find anything is that nobody had tested kauth before with a plugin. This patch changes this so we rebase those relative paths on top of the QT_PLUGIN_PATH, so that we have a list of absolute paths to look for. Diffs - src/BackendsManager.cpp d63fb25 src/CMakeLists.txt 9fed6f2 src/ConfigureChecks.cmake e57873e Diff: https://git.reviewboard.kde.org/r/119037/diff/ Testing --- When I launch kcmshell5 clock, I get a crash because the backend is loaded and it crashes. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119005: Make desktoptojson include all values, not just the translated ones
On June 30, 2014, 4:31 p.m., David Faure wrote: This seems wrong to me. QSettings is not a desktop file parser. It will get escaping wrong, compared to what the desktop entry spec says. Isn't the long term solution to write .json files directly anyway? Okay, after reading the desktop file spec it seems that this doesn't make too much sense since there are some differences to INI files. All the plugin .desktop files I looked at don't use comments (# vs ;) so nothing broke here. However, I still think this tool should be in kcoreaddons since e.g. KPluginLoader is there. Would it make sense to write a QtCore only replacement that doesn't use kconfig or is this a waste of time? - Alexander --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119005/#review61287 --- On June 29, 2014, 7:59 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119005/ --- (Updated June 29, 2014, 7:59 p.m.) Review request for KDE Frameworks and Sebastian Kügler. Repository: kservice Description --- It now uses QSettings to read the desktop file instead of kconfig which means that this tool could move to kcoreaddons. Diffs - src/desktoptojson/CMakeLists.txt 134dabe62ca051eea26631aedc72b108e1b60444 src/desktoptojson/kconfigtojson.cpp 5c0d751f05503e471ed9c49efc51bfdd22774be9 Diff: https://git.reviewboard.kde.org/r/119005/diff/ Testing --- now includes all fields (Comment + Comment[de] + Comment[...], etc) I guess desktoptojson could be move to kcoreaddons for 5.1 and have kservice_desktop_to_json forward to a new kcoreaddons_desktop_to_json for cmake source compatibility Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119038: Allow loading KCMs from QT_PLUGIN_PATH subdirectories
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119038/ --- Review request for KDE Frameworks. Repository: kcmutils Description --- Previously we would crash with an assertion inside libdbus-1 because of additional / characters in the dbus path if X-KDE-Library in the .desktop file pointed to a plugin inside a subdirectory of the plugin path. We now just use the name of the library and strip all leading directories from the name that is registered in DBus This would allow moving all kcms into e.g. plugins/kcm5 which would allow cleaning up the plugins/ directory Diffs - src/kcmoduleproxy.cpp 7596f36e1e26ce080634f3cc0f89e27280423f25 Diff: https://git.reviewboard.kde.org/r/119038/diff/ Testing --- - moved $KF5/lib64/plugins/kcm_clock.so to $KF5/lib64/plugins/kcm5/kcm_clock.so - changed X-KDE-Library to kcm5/kcm_clock in $KF5/share/kservice5/clock.desktop - ran kbuildsycoca5 kcmshell5 clock before this patch - crash kcmshell5 clock after this patch - works as expected Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118264: Switch to PolkitQt5-1
On May 28, 2014, 12:56 p.m., Aurélien Gâteau wrote:
Building with polkit-qt-1 master (bac771e69887c9253f2b0973f6310810db0061f8)
fails with this error:
[ 55%] Building CXX object
autotests/CMakeFiles/KAuthHelperTest.dir/HelperTest.cpp.o
In file included from
/home/aurelien/build/kf5/frameworks/kauth/src/moc_Polkit1Backend.cpp:9:0,
from
/home/aurelien/build/kf5/frameworks/kauth/src/kauth_backend_plugin_automoc.cpp:3:
/home/aurelien/build/kf5/frameworks/kauth/src/../../../../../src/kf5/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.h:31:31:
fatal error: PolkitQt1/Authority: No such file or directory
#include PolkitQt1/Authority
^
compilation terminated.
In file included from
/home/aurelien/src/kf5/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.cpp:22:0:
/home/aurelien/src/kf5/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.h:31:31:
fatal error: PolkitQt1/Authority: No such file or directory
#include PolkitQt1/Authority
^
compilation terminated.
Hrvoje Senjan wrote:
Building with polkit-qt-1 master
(bac771e69887c9253f2b0973f6310810db0061f8) fails with this error:
you're right. bac771e in polkit-qt-1 broke it. i've tested with official
tarball + commit ddca404 ('Add Qt 5 support').
with polkit-qt-1 buildsystem that commit seems wrong. Kevin, can you tell
what was reasoning for the change?
Kevin Ottens wrote:
When I did that commit, @INCLUDE_INSTALL_DIR@ and @LIB_INSTALL_DIR@ were
absolute path. Might have changed in the meantime of course.
Hrvoje Senjan wrote:
with that commit, and without passing neither INCLUDE_INSTALL_DIR or
LIB_INSTALL_DIR, one gets:
set(POLKITQT-1_INCLUDE_DIR include/polkit-qt5-1)
...
set(POLKITQT-1_LIB_DIR lib64)
so, the commit fixes the vars when they are absolute, but breaks when
relative paths are used, and when no special path is used.
just to note - it was like this before, the 'Qt5 port patch' temporary
'fixed' it for the above two cases.
i guess it needs more bulletproof solution.
Well, I would say PolKit-Qt needs to be ported to exporting the targets and the
include directories with them.
Hrvoje, do you want me to take a look at it? Or you're planning to?
Either way, I don't think we should push that patch if it's not going to
compile anyway, given that today is the tagging.
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118264/#review58664
---
On May 28, 2014, 2:01 p.m., Hrvoje Senjan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118264/
---
(Updated May 28, 2014, 2:01 p.m.)
Review request for KDE Frameworks and Kevin Ottens.
Repository: kauth
Description
---
since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant,
KAuth should switch to that, instead of building either w/ no backend, or
with the branch...
Diffs
-
cmake/KF5AuthMacros.cmake 4cdf3ab
src/ConfigureChecks.cmake aff05ed
Diff: https://git.reviewboard.kde.org/r/118264/diff/
Testing
---
Got CMake message:
Building PolkitQt5-1 KAuth backend
and kauth-policy-gen also built
KAuth macros generate correct polkit-1 rules (e.g. date kcm in
plasma-desktop, etc)
Thanks,
Hrvoje Senjan
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KAuth and KF5
Em seg 30 jun 2014, às 14:14:10, Milian Wolff escreveu:
On Monday 30 June 2014 00:05:10 šumski wrote:
On Thursday 26 of June 2014 12:14:49 Milian Wolff wrote:
Hey,
did you run it through valgrind?
Here's what valgrind says:
Sounds like a bug in Qt to me, I have to say. Looking at the code,
QDBusConnectionPrivate::objectDestroyed looks pretty fragile, I mean it does
this at the end:
obj-disconnect(this);
But from the code in QDBusConnectionPrivate::disconnectSignal nothing jumps
out as dangerous directly. The fact, that valgrind is getting confused in
the stack trace is not helping either ;-) Could you maybe try to compile
qtbase in debug mode and reproduce the issue, such that we get a full
stacktrace without optimizations etc.?
Anyways, maybe Thiago (CC'ed) can give us some insight on whats going on
here.
This is happening in a global destructor during the use of a global static. My
guess would be that the global static has already been destroyed, hence the
issue.
Try this patch, which removes it. We have QStringLiteral nowadays.
--
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
Software Architect - Intel Open Source Technology Center
PGP/GPG: 0x6EF45358; fingerprint:
E067 918B B660 DBD1 105C 966C 33F5 F005 6EF4 5358
From eda5c8ede9fd35117146d13f1b55775c9007b672 Mon Sep 17 00:00:00 2001
From: Thiago Macieira thiago.macie...@intel.com
Date: Mon, 30 Jun 2014 08:31:22 -0700
Subject: [PATCH 1/1] Replace the const QString global static with a
QStringLiteral
It was originally created to avoid allocating memory for the QString at
every turn, but we have QStringLiteral for that today. It has also
served a very good run by catching qatomic.h implementations that had
bad cv qualifications.
Change-Id: Id6d952b8cce363015ec2611d346b4cccedecf137
---
src/dbus/qdbusintegrator.cpp | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp
index d4079e8..bc28d50 100644
--- a/src/dbus/qdbusintegrator.cpp
+++ b/src/dbus/qdbusintegrator.cpp
@@ -76,15 +76,21 @@ QT_BEGIN_NAMESPACE
static bool isDebugging;
#define qDBusDebug if (!::isDebugging); else qDebug
-Q_GLOBAL_STATIC_WITH_ARGS(const QString, orgFreedesktopDBusString, (QLatin1String(DBUS_SERVICE_DBUS)))
+QString orgFreedesktopDBusString()
+{
+return QStringLiteral(DBUS_SERVICE_DBUS);
+}
static inline QString dbusServiceString()
-{ return *orgFreedesktopDBusString(); }
+{
+return orgFreedesktopDBusString();
+}
+
static inline QString dbusInterfaceString()
{
// it's the same string, but just be sure
-Q_ASSERT(*orgFreedesktopDBusString() == QLatin1String(DBUS_INTERFACE_DBUS));
-return *orgFreedesktopDBusString();
+Q_ASSERT(orgFreedesktopDBusString() == QLatin1String(DBUS_INTERFACE_DBUS));
+return orgFreedesktopDBusString();
}
static inline QDebug operator(QDebug dbg, const QThread *th)
--
1.8.4.5
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119005: Make desktoptojson include all values, not just the translated ones
On June 30, 2014, 4:31 p.m., David Faure wrote: This seems wrong to me. QSettings is not a desktop file parser. It will get escaping wrong, compared to what the desktop entry spec says. Isn't the long term solution to write .json files directly anyway? Alexander Richardson wrote: Okay, after reading the desktop file spec it seems that this doesn't make too much sense since there are some differences to INI files. All the plugin .desktop files I looked at don't use comments (# vs ;) so nothing broke here. However, I still think this tool should be in kcoreaddons since e.g. KPluginLoader is there. Would it make sense to write a QtCore only replacement that doesn't use kconfig or is this a waste of time? After creating a test case I noticed that KDesktopFile/KConfig also doesn't quite adhere to the desktop file spec. Lists should be separated with a semicolon and use \; to include semicolons. However, it treats e.g. X-KDE-PluginInfo-Depends=foo;bar;escaped\;semicolon as a single string. KConfig uses a comma as a separator instead. Test case: http://paste.kde.org/pzvguaf91 - Alexander --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119005/#review61287 --- On June 29, 2014, 7:59 p.m., Alexander Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119005/ --- (Updated June 29, 2014, 7:59 p.m.) Review request for KDE Frameworks and Sebastian Kügler. Repository: kservice Description --- It now uses QSettings to read the desktop file instead of kconfig which means that this tool could move to kcoreaddons. Diffs - src/desktoptojson/CMakeLists.txt 134dabe62ca051eea26631aedc72b108e1b60444 src/desktoptojson/kconfigtojson.cpp 5c0d751f05503e471ed9c49efc51bfdd22774be9 Diff: https://git.reviewboard.kde.org/r/119005/diff/ Testing --- now includes all fields (Comment + Comment[de] + Comment[...], etc) I guess desktoptojson could be move to kcoreaddons for 5.1 and have kservice_desktop_to_json forward to a new kcoreaddons_desktop_to_json for cmake source compatibility Thanks, Alexander Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118264: Switch to PolkitQt5-1
On May 28, 2014, 2:56 p.m., Aurélien Gâteau wrote:
Building with polkit-qt-1 master (bac771e69887c9253f2b0973f6310810db0061f8)
fails with this error:
[ 55%] Building CXX object
autotests/CMakeFiles/KAuthHelperTest.dir/HelperTest.cpp.o
In file included from
/home/aurelien/build/kf5/frameworks/kauth/src/moc_Polkit1Backend.cpp:9:0,
from
/home/aurelien/build/kf5/frameworks/kauth/src/kauth_backend_plugin_automoc.cpp:3:
/home/aurelien/build/kf5/frameworks/kauth/src/../../../../../src/kf5/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.h:31:31:
fatal error: PolkitQt1/Authority: No such file or directory
#include PolkitQt1/Authority
^
compilation terminated.
In file included from
/home/aurelien/src/kf5/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.cpp:22:0:
/home/aurelien/src/kf5/frameworks/kauth/src/backends/polkit-1/Polkit1Backend.h:31:31:
fatal error: PolkitQt1/Authority: No such file or directory
#include PolkitQt1/Authority
^
compilation terminated.
Hrvoje Senjan wrote:
Building with polkit-qt-1 master
(bac771e69887c9253f2b0973f6310810db0061f8) fails with this error:
you're right. bac771e in polkit-qt-1 broke it. i've tested with official
tarball + commit ddca404 ('Add Qt 5 support').
with polkit-qt-1 buildsystem that commit seems wrong. Kevin, can you tell
what was reasoning for the change?
Kevin Ottens wrote:
When I did that commit, @INCLUDE_INSTALL_DIR@ and @LIB_INSTALL_DIR@ were
absolute path. Might have changed in the meantime of course.
Hrvoje Senjan wrote:
with that commit, and without passing neither INCLUDE_INSTALL_DIR or
LIB_INSTALL_DIR, one gets:
set(POLKITQT-1_INCLUDE_DIR include/polkit-qt5-1)
...
set(POLKITQT-1_LIB_DIR lib64)
so, the commit fixes the vars when they are absolute, but breaks when
relative paths are used, and when no special path is used.
just to note - it was like this before, the 'Qt5 port patch' temporary
'fixed' it for the above two cases.
i guess it needs more bulletproof solution.
Aleix Pol Gonzalez wrote:
Well, I would say PolKit-Qt needs to be ported to exporting the targets
and the include directories with them.
Hrvoje, do you want me to take a look at it? Or you're planning to?
Either way, I don't think we should push that patch if it's not going to
compile anyway, given that today is the tagging.
i'll take a look at it.
off course, review is open until polkit-qt-1 gets fixed (tm)
- Hrvoje
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118264/#review58664
---
On May 28, 2014, 4:01 p.m., Hrvoje Senjan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118264/
---
(Updated May 28, 2014, 4:01 p.m.)
Review request for KDE Frameworks and Kevin Ottens.
Repository: kauth
Description
---
since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant,
KAuth should switch to that, instead of building either w/ no backend, or
with the branch...
Diffs
-
cmake/KF5AuthMacros.cmake 4cdf3ab
src/ConfigureChecks.cmake aff05ed
Diff: https://git.reviewboard.kde.org/r/118264/diff/
Testing
---
Got CMake message:
Building PolkitQt5-1 KAuth backend
and kauth-policy-gen also built
KAuth macros generate correct polkit-1 rules (e.g. date kcm in
plasma-desktop, etc)
Thanks,
Hrvoje Senjan
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118985: KSharedConfig: move mainConfig and wasTestEnabled to the thread storage.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118985/#review61307 --- Ship it! Alright, LGTM. Just one final comment on the final patch, feel free to commit it once fixed. src/core/ksharedconfig.cpp https://git.reviewboard.kde.org/r/118985/#comment42692 Whitespace. - Matthew Dawson On June 29, 2014, 9:55 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118985/ --- (Updated June 29, 2014, 9:55 a.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- KSharedConfig: move mainConfig and wasTestEnabled to the thread storage. This enables the mainConfig optimization in all threads, and ensures the user warning only happens in the main thread. The test-mode-enabled logic is only really useful in the main thread, but it's simpler to just do it in all threads. REVIEW: 118985 Diffs - autotests/kconfigtest.cpp a8482b7099df5921909830082d758dc3095e3241 src/core/ksharedconfig.cpp b7d155d5893502921d35d7dd971188b6a93a0620 Diff: https://git.reviewboard.kde.org/r/118985/diff/ Testing --- no regression in make test in kconfig; still debugging races in helgrind threadtest (in kio). Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 119043: pollkit-qt-1 buildsystem adjustements
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118264: Switch to PolkitQt5-1
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/ --- (Updated June 30, 2014, 8:20 p.m.) Review request for KDE Frameworks and Kevin Ottens. Changes --- new version, adjusted to polkit-qt-1 review Repository: kauth Description --- since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant, KAuth should switch to that, instead of building either w/ no backend, or with the branch... Diffs (updated) - cmake/KF5AuthMacros.cmake 66ba949 src/ConfigureChecks.cmake e57873e src/backends/dbus/DBusHelperProxy.cpp 3719e4b src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/118264/diff/ Testing --- Got CMake message: Building PolkitQt5-1 KAuth backend and kauth-policy-gen also built KAuth macros generate correct polkit-1 rules (e.g. date kcm in plasma-desktop, etc) Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118264: Switch to PolkitQt5-1
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/ --- (Updated June 30, 2014, 8:20 p.m.) Review request for KDE Frameworks and Kevin Ottens. Repository: kauth Description --- since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant, KAuth should switch to that, instead of building either w/ no backend, or with the branch... Diffs - cmake/KF5AuthMacros.cmake 66ba949 src/ConfigureChecks.cmake e57873e src/backends/dbus/DBusHelperProxy.cpp 3719e4b src/backends/polkit-1/Polkit1Backend.cpp 165f7bb Diff: https://git.reviewboard.kde.org/r/118264/diff/ Testing --- Got CMake message: Building PolkitQt5-1 KAuth backend and kauth-policy-gen also built KAuth macros generate correct polkit-1 rules (e.g. date kcm in plasma-desktop, etc) Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118264: Switch to PolkitQt5-1
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118264/ --- (Updated June 30, 2014, 8:31 p.m.) Review request for KDE Frameworks and Kevin Ottens. Changes --- drop unrelated changes Repository: kauth Description --- since now master of polkit-qt-1 is able to exist w/o issues with Qt4 variant, KAuth should switch to that, instead of building either w/ no backend, or with the branch... Diffs (updated) - cmake/KF5AuthMacros.cmake 66ba949 src/ConfigureChecks.cmake e57873e Diff: https://git.reviewboard.kde.org/r/118264/diff/ Testing --- Got CMake message: Building PolkitQt5-1 KAuth backend and kauth-policy-gen also built KAuth macros generate correct polkit-1 rules (e.g. date kcm in plasma-desktop, etc) Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/
---
(Updated June 30, 2014, 10:28 p.m.)
Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and
Christophe Giboudeaux.
Changes
---
adjusted pc files paths.
added ${GOBJECT_LIBRARIES} and ${GIO_LIBRARIES} to agent's link libraries: the
build fails otherwise with -Wl, --no-undefined
requires would be great to fix, i agree, just need to think of the easiest way
to cover both Qt4 and Qt5 cases. maybe separate pc files...
Repository: polkit-qt-1
Description
---
added exported targets, simplify main CMakeLists...
Diffs (updated)
-
CMakeLists.txt 021bf88
PolkitQt-1Config.cmake.in 8b722a6
agent/CMakeLists.txt f1ba438
core/CMakeLists.txt e9b3ebb
gui/CMakeLists.txt 10b06ae
polkit-qt-1.pc.cmake 2f33204
polkit-qt-agent-1.pc.cmake 6ccc6dd
polkit-qt-core-1.pc.cmake a9e0750
polkit-qt-gui-1.pc.cmake 6b9c2cf
Diff: https://git.reviewboard.kde.org/r/119043/diff/
Testing
---
builds, cmake files visually inspected, seem fine
Thanks,
Hrvoje Senjan
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/#review61326 --- agent/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42703 LINK_PUBLIC agent/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42704 LINK_PRIVATE core/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42699 PUBLIC was added in CMake 2.8.12, use LINK_PUBLIC core/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42700 and LINK_PRIVATE core/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42698 Not needed anymore gui/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42701 LINK_PUBLIC gui/CMakeLists.txt https://git.reviewboard.kde.org/r/119043/#comment42702 LINK_PRIVATE - Christophe Giboudeaux On June 30, 2014, 8:28 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- (Updated June 30, 2014, 8:28 p.m.) Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae polkit-qt-1.pc.cmake 2f33204 polkit-qt-agent-1.pc.cmake 6ccc6dd polkit-qt-core-1.pc.cmake a9e0750 polkit-qt-gui-1.pc.cmake 6b9c2cf Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KAuth and KF5
On Monday 30 of June 2014 17:31:39 Thiago Macieira wrote: Em seg 30 jun 2014, às 14:14:10, Milian Wolff escreveu: On Monday 30 June 2014 00:05:10 šumski wrote: On Thursday 26 of June 2014 12:14:49 Milian Wolff wrote: Hey, did you run it through valgrind? Here's what valgrind says: Sounds like a bug in Qt to me, I have to say. Looking at the code, QDBusConnectionPrivate::objectDestroyed looks pretty fragile, I mean it does this at the end: obj-disconnect(this); But from the code in QDBusConnectionPrivate::disconnectSignal nothing jumps out as dangerous directly. The fact, that valgrind is getting confused in the stack trace is not helping either ;-) Could you maybe try to compile qtbase in debug mode and reproduce the issue, such that we get a full stacktrace without optimizations etc.? Anyways, maybe Thiago (CC'ed) can give us some insight on whats going on here. This is happening in a global destructor during the use of a global static. My guess would be that the global static has already been destroyed, hence the issue. Try this patch, which removes it. We have QStringLiteral nowadays. Confirming that segfault no longer happens with the patch! Cheers, Hrvoje 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 119043: pollkit-qt-1 buildsystem adjustements
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- (Updated June 30, 2014, 10:51 p.m.) Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Changes --- resolved issues Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs (updated) - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae polkit-qt-1.pc.cmake 2f33204 polkit-qt-agent-1.pc.cmake 6ccc6dd polkit-qt-core-1.pc.cmake a9e0750 polkit-qt-gui-1.pc.cmake 6b9c2cf Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Translations are broken in KF5 apps
Hi KDE Frameworks devs, I tried to run a couple of KF5-based KDE applications, namely Kate and Konsole (Russian and Ukrainian) and localization there looks very poor, even though from the translators' side the localization packages are complete or almost complete. One of the bugs that has been more or less tracked down is related to adding accelerators () at run-time. It has been discovered by Burkhard Lueck a few weeks ago (May 14-15). What a non-English user can see is, many menu items still appear in English while they have translations in *.mo files installed in /usr/share/locate/**/LC_MESSAGES. We could guess that translation of a menu item breaks when an accelerator is added at run-time (Burkhard said drclash adds/rearranges the at runtime). Because in a .mo there are only translations for strings without accelerators, a i18n(...) call applied to a -enabled string will fail to find its translation in the .mo. Codewise accelerator rearrangements seem to be done in kxmlgui/src/kcheckaccelerators.cpp and kwidgetsaddons/src/kacceleratormanager.cpp. I guess these are good places to start further investigation. Names and descriptions from .desktop files are not translated in many places. For example in System Settings for module names and descriptions, application names in Kickoff and plasmoid names in both widgets explorer and their settings dialogs. Wallpaper types in Desktop settings are translated even though they seem to come from .desktop files. Qt 5 translations are not being loaded. This is the most likely reason why we don't see translated buttons OK / Cancel / Apply in various dialogs. There is some evidence that .qm translation files from Tier 1 frameworks are still commonly not loaded properly: if you open Dolphin and look under the Devices label in the Places dock, you will see untranslated strings like 200,0 GiB Hard Drive which I expect to come from the Solid framework and to be translated in solid5_qt.qm. May be the file naming is wrong? A fresh build of translation files for Russian and Ukrainian can be found here: https://github.com/aspotashev/l10n-kf5-built Thanks to Lasse Liehu for parts of this e-mail completely written by him and for assistance with the other text! -- Alexander Potashev ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Translations are broken in KF5 apps
[: Alexander Potashev :] What a non-English user can see is, many menu items still appear in English while they have translations in *.mo files installed [...] One thing I know is still not being translated, because I didn't implement it yet, is menu names coming from *.rc files. This also holds for menu items which are submenu names. This should cover some of the cases you have observed. (Another such thing is stuff coming from *.kcfg files, though this is certainly not related to menus.) I'll see to get this done, ehm, soon. To note additionally, packaging of translations for Frameworks is still not resolved (to my knowledge?), so translations are actually not released right now. -- Chusslove Illich (Часлав Илић) 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: Translations are broken in KF5 apps
El Dilluns, 30 de juny de 2014, a les 23:18:18, Chusslove Illich va escriure: [: Alexander Potashev :] What a non-English user can see is, many menu items still appear in English while they have translations in *.mo files installed [...] One thing I know is still not being translated, because I didn't implement it yet, is menu names coming from *.rc files. This also holds for menu items which are submenu names. This should cover some of the cases you have observed. (Another such thing is stuff coming from *.kcfg files, though this is certainly not related to menus.) I'll see to get this done, ehm, soon. To note additionally, packaging of translations for Frameworks is still not resolved (to my knowledge?), so translations are actually not released right now. I see the translations in the tarballs. Why do you say they are not released? Cheers, Albert ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Translations are broken in KF5 apps
[: Chusslove Illich :] To note additionally, packaging of translations for Frameworks is still not resolved (to my knowledge?), so translations are actually not released right now. [: Albert Astals Cid :] I see the translations in the tarballs. Why do you say they are not released? My knowledge was lacking. -- Chusslove Illich (Часлав Илић) 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 119043: pollkit-qt-1 buildsystem adjustements
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- (Updated June 30, 2014, 11:53 p.m.) Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Changes --- added qt5 pc files Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs (updated) - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae polkit-qt-1.pc.cmake 2f33204 polkit-qt-agent-1.pc.cmake 6ccc6dd polkit-qt-core-1.pc.cmake a9e0750 polkit-qt-gui-1.pc.cmake 6b9c2cf polkit-qt5-1.pc.cmake PRE-CREATION polkit-qt5-agent-1.pc.cmake PRE-CREATION polkit-qt5-core-1.pc.cmake PRE-CREATION polkit-qt5-gui-1.pc.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/#review61334
---
CMakeLists.txt
https://git.reviewboard.kde.org/r/119043/#comment42707
Isn't it acceptable to depend on ECM here?
PolkitQt-1Config.cmake.in
https://git.reviewboard.kde.org/r/119043/#comment42708
why's that change?
PolkitQt-1Config.cmake.in
https://git.reviewboard.kde.org/r/119043/#comment42712
Passing the include dir is not needed anymore, given that those will be
pulled by the targets.
I suggest leaving them empty (or even not defining them). It won't stop
applications from building, it will just resolve to nothing.
PolkitQt-1Config.cmake.in
https://git.reviewboard.kde.org/r/119043/#comment42711
why don't you just do something like:
set(POLKITQT-1_CORE_LIBRARY ${POLKITQT-1_CORE_PCNAME})
This should work on all platforms.
core/CMakeLists.txt
https://git.reviewboard.kde.org/r/119043/#comment42709
Just use PUBLIC and PRIVATE, like the rest of frameworks
core/CMakeLists.txt
https://git.reviewboard.kde.org/r/119043/#comment42713
Maybe we want to set an EXPORT_NAME? This way the user won't need a
variable to link to the library.
It's better linking against libraries rather than variables because you
don't risk adding a typo and resolving to an empty string.
- Aleix Pol Gonzalez
On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119043/
---
(Updated June 30, 2014, 9:53 p.m.)
Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and
Christophe Giboudeaux.
Repository: polkit-qt-1
Description
---
added exported targets, simplify main CMakeLists...
Diffs
-
CMakeLists.txt 021bf88
PolkitQt-1Config.cmake.in 8b722a6
agent/CMakeLists.txt f1ba438
core/CMakeLists.txt e9b3ebb
gui/CMakeLists.txt 10b06ae
polkit-qt-1.pc.cmake 2f33204
polkit-qt-agent-1.pc.cmake 6ccc6dd
polkit-qt-core-1.pc.cmake a9e0750
polkit-qt-gui-1.pc.cmake 6b9c2cf
polkit-qt5-1.pc.cmake PRE-CREATION
polkit-qt5-agent-1.pc.cmake PRE-CREATION
polkit-qt5-core-1.pc.cmake PRE-CREATION
polkit-qt5-gui-1.pc.cmake PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/119043/diff/
Testing
---
builds, cmake files visually inspected, seem fine
Thanks,
Hrvoje Senjan
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements
On June 30, 2014, 8:37 p.m., Christophe Giboudeaux wrote: agent/CMakeLists.txt, line 10 https://git.reviewboard.kde.org/r/119043/diff/2/?file=285757#file285757line10 LINK_PUBLIC On June 30, 2014, 8:37 p.m., Christophe Giboudeaux wrote: core/CMakeLists.txt, line 13 https://git.reviewboard.kde.org/r/119043/diff/2/?file=285758#file285758line13 PUBLIC was added in CMake 2.8.12, use LINK_PUBLIC So? all frameworks is depending on cmake 2.8.12, and LINK_PUBLIC is considered deprecated there. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/#review61326 --- On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- (Updated June 30, 2014, 9:53 p.m.) Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae polkit-qt-1.pc.cmake 2f33204 polkit-qt-agent-1.pc.cmake 6ccc6dd polkit-qt-core-1.pc.cmake a9e0750 polkit-qt-gui-1.pc.cmake 6b9c2cf polkit-qt5-1.pc.cmake PRE-CREATION polkit-qt5-agent-1.pc.cmake PRE-CREATION polkit-qt5-core-1.pc.cmake PRE-CREATION polkit-qt5-gui-1.pc.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements
On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote: CMakeLists.txt, line 6 https://git.reviewboard.kde.org/r/119043/diff/4/?file=285775#file285775line6 Isn't it acceptable to depend on ECM here? not a wise choice. the master branch allows building both the Qt4 Qt5 variant (and depends on CMake 2.8.11) On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote: core/CMakeLists.txt, line 13 https://git.reviewboard.kde.org/r/119043/diff/4/?file=285778#file285778line13 Just use PUBLIC and PRIVATE, like the rest of frameworks PUBLIC and PRIVATE do not exist in cmake 2.8.11. On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote: PolkitQt-1Config.cmake.in, line 24 https://git.reviewboard.kde.org/r/119043/diff/4/?file=285776#file285776line24 Passing the include dir is not needed anymore, given that those will be pulled by the targets. I suggest leaving them empty (or even not defining them). It won't stop applications from building, it will just resolve to nothing. ...but would not be SC. Anything using POLKITQT-1_INCLUDE_DIR would fail to build. - Christophe --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/#review61334 --- On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- (Updated June 30, 2014, 9:53 p.m.) Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae polkit-qt-1.pc.cmake 2f33204 polkit-qt-agent-1.pc.cmake 6ccc6dd polkit-qt-core-1.pc.cmake a9e0750 polkit-qt-gui-1.pc.cmake 6b9c2cf polkit-qt5-1.pc.cmake PRE-CREATION polkit-qt5-agent-1.pc.cmake PRE-CREATION polkit-qt5-core-1.pc.cmake PRE-CREATION polkit-qt5-gui-1.pc.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119043: pollkit-qt-1 buildsystem adjustements
On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote: PolkitQt-1Config.cmake.in, line 24 https://git.reviewboard.kde.org/r/119043/diff/4/?file=285776#file285776line24 Passing the include dir is not needed anymore, given that those will be pulled by the targets. I suggest leaving them empty (or even not defining them). It won't stop applications from building, it will just resolve to nothing. Christophe Giboudeaux wrote: ...but would not be SC. Anything using POLKITQT-1_INCLUDE_DIR would fail to build. Only if it doesn't link against polkitqt-1. On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote: CMakeLists.txt, line 6 https://git.reviewboard.kde.org/r/119043/diff/4/?file=285775#file285775line6 Isn't it acceptable to depend on ECM here? Christophe Giboudeaux wrote: not a wise choice. the master branch allows building both the Qt4 Qt5 variant (and depends on CMake 2.8.11) I disagree, but accept your premise. On June 30, 2014, 11:43 p.m., Aleix Pol Gonzalez wrote: core/CMakeLists.txt, line 13 https://git.reviewboard.kde.org/r/119043/diff/4/?file=285778#file285778line13 Just use PUBLIC and PRIVATE, like the rest of frameworks Christophe Giboudeaux wrote: PUBLIC and PRIVATE do not exist in cmake 2.8.11. Ok, withdraw this complaint. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/#review61334 --- On June 30, 2014, 9:53 p.m., Hrvoje Senjan wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119043/ --- (Updated June 30, 2014, 9:53 p.m.) Review request for KDE Frameworks, Polkit Qt, Aleix Pol Gonzalez, and Christophe Giboudeaux. Repository: polkit-qt-1 Description --- added exported targets, simplify main CMakeLists... Diffs - CMakeLists.txt 021bf88 PolkitQt-1Config.cmake.in 8b722a6 agent/CMakeLists.txt f1ba438 core/CMakeLists.txt e9b3ebb gui/CMakeLists.txt 10b06ae polkit-qt-1.pc.cmake 2f33204 polkit-qt-agent-1.pc.cmake 6ccc6dd polkit-qt-core-1.pc.cmake a9e0750 polkit-qt-gui-1.pc.cmake 6b9c2cf polkit-qt5-1.pc.cmake PRE-CREATION polkit-qt5-agent-1.pc.cmake PRE-CREATION polkit-qt5-core-1.pc.cmake PRE-CREATION polkit-qt5-gui-1.pc.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/119043/diff/ Testing --- builds, cmake files visually inspected, seem fine Thanks, Hrvoje Senjan ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
