Re: Review Request: Focus goes to location bar when opening link in new tab in foreground
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105984/#review18070 --- This review has been submitted with commit a2ae8003352442efdb5731b3fd3de3107b0c1e99 by Dawit Alemayehu to branch master. - Commit Hook On Aug. 25, 2012, 7:41 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105984/ --- (Updated Aug. 25, 2012, 7:41 p.m.) Review request for KDE Base Apps and David Faure. Description --- The attached patch address the bug reported in #304933. Right now if Konqueror is configured to open new tabs in the foreground, i.e. the Open tabs in the background option is unchecked, then the keyboard focus is put on the location bar instead of the view. This addresses bugs 304865 and 304933. http://bugs.kde.org/show_bug.cgi?id=304865 http://bugs.kde.org/show_bug.cgi?id=304933 Diffs - konqueror/src/konqframe.cpp 10ed7cd konqueror/src/konqview.cpp db9ffd4 konqueror/src/konqviewmanager.cpp 5352eeb Diff: http://git.reviewboard.kde.org/r/105984/diff/ Testing --- Thanks, Dawit Alemayehu
Review Request: Move checks for locking primitives in KSharedDataCache to CMake
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106174/ --- Review request for kdelibs and Michael Pyne. Description --- This patch does move some run-time checks (mainly for different locking mechanisms) in KSharedDataCache to build time (CMake). This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Diffs - kdecore/util/config-util.h.cmake 83ccdf7 kdecore/util/kshareddatacache.cpp 393902e kdecore/util/kshareddatacache_p.h ec5a7a0 kdecore/util/ConfigureChecks.cmake fe9f47e Diff: http://git.reviewboard.kde.org/r/106174/diff/ Testing --- On OpenBSD-CURRENT. Thanks, Vadim Zhukov
Review Request: Mutex cleanup in KSharedDataCache
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106221/ --- Review request for kdelibs and Michael Pyne. Description --- - Remove support for thread-shared-only locks: they do not help at all but makes app crash instead of slowly work; lack of decent locks to be resolved later. - Add error checking with console printouts. - Consistently use :: before global functions. Probably failed mutexes should be destroyed in case of some errors, any thoughts? This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Diffs - kdecore/util/kshareddatacache_p.h ec5a7a0 kdecore/util/kshareddatacache.cpp 393902e Diff: http://git.reviewboard.kde.org/r/106221/diff/ Testing --- On OpenBSD-CURRENT. Thanks, Vadim Zhukov
Re: Review Request: Mutex cleanup in KSharedDataCache
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106221/ --- (Updated Aug. 26, 2012, 7:02 p.m.) Review request for kdelibs and Michael Pyne. Changes --- Fixed lie in comment. Description --- - Remove support for thread-shared-only locks: they do not help at all but makes app crash instead of slowly work; lack of decent locks to be resolved later. - Add error checking with console printouts. - Consistently use :: before global functions. Probably failed mutexes should be destroyed in case of some errors, any thoughts? This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Diffs (updated) - kdecore/util/kshareddatacache.cpp 393902e kdecore/util/kshareddatacache_p.h ec5a7a0 Diff: http://git.reviewboard.kde.org/r/106221/diff/ Testing --- On OpenBSD-CURRENT. Thanks, Vadim Zhukov
Review Request: Add spinlocks lock type, based on GCC intrisincs
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106224/ --- Review request for kdelibs and Michael Pyne. Description --- Add simple spin locking mechanism. Written by Michael Pyne as from https://bugs.kde.org/attachment.cgi?id=73282 , with some tweaking by me. This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Diffs - kdecore/util/ConfigureChecks.cmake fe9f47e kdecore/util/config-util.h.cmake 83ccdf7 kdecore/util/kshareddatacache_p.h ec5a7a0 Diff: http://git.reviewboard.kde.org/r/106224/diff/ Testing --- On OpenBSD-CURRENT, i386 Thanks, Vadim Zhukov
Re: Review Request: Config cleanup on removeActivity
On Aug. 27, 2012, 12:42 a.m., Lamarque Vieira Souza wrote: Ship It! Which branch is this, since it is not master as specified? - Ivan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106097/#review18065 --- On Aug. 20, 2012, 11:21 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106097/ --- (Updated Aug. 20, 2012, 11:21 a.m.) Review request for KDE Runtime and Plasma. Description --- Remove the related configuration entry (which holds the current desktop) when an activity is removed. Diffs - src/service/Activities.cpp e7c1a7bfd5415127b804a2af4bb0b920fb2118ac Diff: http://git.reviewboard.kde.org/r/106097/diff/ Testing --- Yes Thanks, makis marimpis
Re: Review Request: Config cleanup on removeActivity
On Aug. 27, 2012, 12:42 a.m., Lamarque Vieira Souza wrote: Ship It! Ivan Čukić wrote: Which branch is this, since it is not master as specified? (in master, VD switcher is moved into a separate plugin) - Ivan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106097/#review18065 --- On Aug. 20, 2012, 11:21 a.m., makis marimpis wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106097/ --- (Updated Aug. 20, 2012, 11:21 a.m.) Review request for KDE Runtime and Plasma. Description --- Remove the related configuration entry (which holds the current desktop) when an activity is removed. Diffs - src/service/Activities.cpp e7c1a7bfd5415127b804a2af4bb0b920fb2118ac Diff: http://git.reviewboard.kde.org/r/106097/diff/ Testing --- Yes Thanks, makis marimpis
Re: Review Request: Add spinlocks lock type, based on GCC intrisincs
On domingo, 26 de agosto de 2012 19.09.15, Vadim Zhukov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106224/ --- Review request for kdelibs and Michael Pyne. Description --- Add simple spin locking mechanism. Written by Michael Pyne as from https://bugs.kde.org/attachment.cgi?id=73282 , with some tweaking by me. This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Please use the Qt atomic types. Until GCC 4.7, they generate better code. -- 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 signature.asc Description: This is a digitally signed message part.
Re: Review Request: Use a qml based screen locker in place of the screensaver
On Aug. 25, 2012, 9:02 a.m., Oswald Buddenhagen wrote: ksmserver/screenlocker/greeter/greeterapp.cpp, line 199 http://git.reviewboard.kde.org/r/106124/diff/2/?file=80792#file80792line199 looks anything between pointless and harmful. care to explain? was a misunderstanding of the architecture. if fact is not necessary - Marco --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106124/#review17977 --- On Aug. 24, 2012, 11:30 a.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106124/ --- (Updated Aug. 24, 2012, 11:30 a.m.) Review request for KDE Runtime. Description --- this is the finalization of the old screenlocker branch in workspace: the screen saver goes away (discussed at the time, about one year ago) and the screen locker gets managed by ksmserver, with a greeter that has the ui dine in qml. The same qml ui gets loaded by the plasma based greeter when the allow widgets on screen locker is enabled. the screensaver kcm is now called Screen locker and is way simpler, the screen saver chooser is gone from it. Diffs - kcontrol/screensaver/CMakeLists.txt e4dcc3a kcontrol/screensaver/screensaver.ui 0ad5cd8 kcontrol/screensaver/scrnsave.h 7c8deba kcontrol/screensaver/scrnsave.cpp c0507d4 krunner/CMakeLists.txt 21eac6f krunner/dbus/org.freedesktop.ScreenSaver.xml 5efd943 krunner/dbus/org.kde.screensaver.xml e700b88 krunner/kcfg/kscreensaversettings.kcfg c8f76f3 krunner/kcfg/kscreensaversettings.kcfgc af9133d krunner/krunnerapp.h 040198d krunner/krunnerapp.cpp eea6220 krunner/lock/CMakeLists.txt cf9a67e krunner/lock/autologout.h 0c444050 krunner/lock/autologout.cc c86e29a krunner/lock/config-krunner-lock.h.cmake 7bfdfd6 krunner/lock/kscreenlocker.notifyrc 14e37ec krunner/lock/lockdlg.h f25e55f krunner/lock/lockdlg.cc 14a9b34 krunner/lock/lockprocess.h 8b6d9a8 krunner/lock/lockprocess.cc 65c7f1d krunner/lock/main.h 8a60353 krunner/lock/main.cc 7b41024 krunner/main.cpp 84a547b krunner/screensaver/saverengine.h 3384d4a krunner/screensaver/saverengine.cpp 4d90faa krunner/screensaver/xautolock.h 3db3233 krunner/screensaver/xautolock.cpp 7124215 krunner/screensaver/xautolock_c.h 3b82f5c krunner/screensaver/xautolock_diy.c b9df2f8 krunner/screensaver/xautolock_engine.c d6d0cf5 ksmserver/CMakeLists.txt 5f0fd34 ksmserver/config-ksmserver.h.cmake 933da35 ksmserver/main.cpp 430a61a ksmserver/screenlocker/CMakeLists.txt PRE-CREATION ksmserver/screenlocker/Messages.sh PRE-CREATION ksmserver/screenlocker/autologout.h PRE-CREATION ksmserver/screenlocker/autologout.cpp PRE-CREATION ksmserver/screenlocker/data/CMakeLists.txt PRE-CREATION ksmserver/screenlocker/data/force_krunner_lock_shortcut_unreg.cpp PRE-CREATION ksmserver/screenlocker/data/kscreenlocker_locksession-shortcut.upd PRE-CREATION ksmserver/screenlocker/dbus/org.freedesktop.ScreenSaver.xml PRE-CREATION ksmserver/screenlocker/dbus/org.kde.screensaver.xml PRE-CREATION ksmserver/screenlocker/greeter/CMakeLists.txt PRE-CREATION ksmserver/screenlocker/greeter/Messages.sh PRE-CREATION ksmserver/screenlocker/greeter/greeter.h PRE-CREATION ksmserver/screenlocker/greeter/greeter.cpp PRE-CREATION ksmserver/screenlocker/greeter/greeterapp.h PRE-CREATION ksmserver/screenlocker/greeter/greeterapp.cpp PRE-CREATION ksmserver/screenlocker/greeter/main.cpp PRE-CREATION ksmserver/screenlocker/greeter/screensaverwindow.h PRE-CREATION ksmserver/screenlocker/greeter/screensaverwindow.cpp PRE-CREATION ksmserver/screenlocker/greeter/sessions.h PRE-CREATION ksmserver/screenlocker/greeter/sessions.cpp PRE-CREATION ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/Greeter.qml PRE-CREATION ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/SessionSwitching.qml PRE-CREATION ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/contents/ui/main.qml PRE-CREATION ksmserver/screenlocker/greeter/themes/org.kde.passworddialog/metadata.desktop PRE-CREATION ksmserver/screenlocker/interface.h PRE-CREATION ksmserver/screenlocker/interface.cpp PRE-CREATION ksmserver/screenlocker/kcfg/kscreensaversettings.kcfg PRE-CREATION ksmserver/screenlocker/kcfg/kscreensaversettings.kcfgc PRE-CREATION ksmserver/screenlocker/kscreenlocker.notifyrc PRE-CREATION ksmserver/screenlocker/ksldapp.h PRE-CREATION ksmserver/screenlocker/ksldapp.cpp PRE-CREATION
Re: Review Request: Move checks for locking primitives in KSharedDataCache to CMake
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106174/#review18113 --- Why do you want to do this ? Beside that, you are using CMakePushCheckState.cmake, which is part of cmake only since version 2.8.6. trunk still requires cmake 2.6.4, so this is not allowed, use MacroPushrequiredVars.cmake, which we have in kdelibs, instead. - Alexander Neundorf On Aug. 26, 2012, 5:15 p.m., Vadim Zhukov wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106174/ --- (Updated Aug. 26, 2012, 5:15 p.m.) Review request for kdelibs and Michael Pyne. Description --- This patch does move some run-time checks (mainly for different locking mechanisms) in KSharedDataCache to build time (CMake). This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Diffs - kdecore/util/config-util.h.cmake 83ccdf7 kdecore/util/kshareddatacache.cpp 393902e kdecore/util/kshareddatacache_p.h ec5a7a0 kdecore/util/ConfigureChecks.cmake fe9f47e Diff: http://git.reviewboard.kde.org/r/106174/diff/ Testing --- On OpenBSD-CURRENT. Thanks, Vadim Zhukov
Re: Review Request: Only show the screen config changed message once.
On Aug. 26, 2012, 11:42 a.m., Kai Uwe Broulik wrote: That warning is there to remind the maintainer to re-factor that code and get rid of that message ;-) I’d vote for using a KNotify notification: 1. the user can configure it in a way he wants to (ie. show message? play sound? run application when event occurs?) and 2. it is a passive notification that emerges from the notification area rather than a dialog popping out of nowhere with no context. FTR: I already made a patch half a year ago or so but was told there was going to be a major refactor in this area and so I left it. Refactor is still going on, you can checkout libkscreen repo in my scratch. About this patch, it looks ok so feel free to ship it. - Alex --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106212/#review18029 --- On Aug. 26, 2012, 8:40 a.m., Frederik Gladhorn wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106212/ --- (Updated Aug. 26, 2012, 8:40 a.m.) Review request for KDE Base Apps and Alex Fiestas. Description --- The dialog from the kded would show up once for each event (eg unpluggin and pluggin in again) related to monitor changes. I wonder what the #warning Modal dialog, stupid, fix. was supposed to fix. The dialog was redone since it's actually not very easy to re-use KMessageBox in a non-modal way. Diffs - kcontrol/randr/module/randrmonitor.h 057c991 kcontrol/randr/module/randrmonitor.cpp 32d0d0a Diff: http://git.reviewboard.kde.org/r/106212/diff/ Testing --- Thanks, Frederik Gladhorn
Re: Review Request: Mutex cleanup in KSharedDataCache
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106221/ --- (Updated Aug. 27, 2012, 8:50 a.m.) Review request for kdelibs and Michael Pyne. Changes --- Added error text printing, as requested by mpyne@. Please note: strerror() is not thread-safe, so we have to use strerror_r(), which doesn't exists everywhere. So I've added another CMake check, and if it fails, still print error code directly. Description --- - Remove support for thread-shared-only locks: they do not help at all but makes app crash instead of slowly work; lack of decent locks to be resolved later. - Add error checking with console printouts. - Consistently use :: before global functions. Probably failed mutexes should be destroyed in case of some errors, any thoughts? This addresses bug 305023. http://bugs.kde.org/show_bug.cgi?id=305023 Diffs (updated) - kdecore/util/ConfigureChecks.cmake fe9f47e kdecore/util/config-util.h.cmake 83ccdf7 kdecore/util/kshareddatacache.cpp 393902e kdecore/util/kshareddatacache_p.h ec5a7a0 Diff: http://git.reviewboard.kde.org/r/106221/diff/ Testing --- On OpenBSD-CURRENT. Thanks, Vadim Zhukov
Re: Review Request: Move checks for locking primitives in KSharedDataCache to CMake
On Aug. 26, 2012, 9:35 p.m., Michael Pyne wrote:
I've looked over everything and didn't see anything I didn't like. I'd like
to apply and and compile and run what little test cases do exist, but
git-apply won't run as kdecore/util/{ConfigureChecks,config-util.h}.cmake
don't exist yet. Are those from stray commits on your end or is there an
earlier review request that includes them?
I'm trying to apply to KDE/4.10 as this is not strictly a bugfix, though I
didn't see either of those in KDE/4.9 or frameworks branches either.
Hm-m-m, I've checked out trunk of kdelibs... See the log of my repo:
commit 2975583acb23334efc2f13860d0f9c36462c4524
Author: Vadim Zhukov persg...@gmail.com
Date: Mon Aug 27 12:01:31 2012 +0400
Improve error printing, and mention another (related) reason to avoid
semaphores.
commit 487edb5e59b16edb63c70d3100dc1c9cc608eea8
Author: Vadim Zhukov persg...@gmail.com
Date: Sun Aug 26 22:30:12 2012 +0400
- Remove support for thread-shared-only locks: they do not help at all
but makes app crash instead of slowly work; lack of decent locks to be resolved
later.
- Add error checking with console printouts.
- Consistently use :: before global functions.
commit 04adbc24f5bc14828fcd8da84a322993df4a8f35
Author: Vadim Zhukov persg...@gmail.com
Date: Sun Aug 26 21:17:12 2012 +0400
Move checks for functionality (especially locking mechanisms) to CMake.
commit 8af088493827cb1eaf6bb4aa4df89f2147986a1a
Merge: 58fd025 fed5ee7
Author: David Faure fa...@kde.org
Date: Thu Aug 16 11:04:58 2012 +0200
Merge branch 'KDE/4.10'
commit 0e8325376aefb912ab318c2795f276fc23a9837c
Author: Sergio Martins iamser...@gmail.com
Date: Tue Aug 14 23:16:51 2012 +0100
s/newer then/newer than
(cherry picked from commit fef8d2c8ce38c735cbf2fa196006ead991a17171)
commit 75328c0cc379d80f179ce77fc66d19706290a674
Merge: 557c126 c67bfaf
Author: David Faure fa...@kde.org
Date: Fri Jul 20 13:23:52 2012 +0200
Merge branch 'KDE/4.9'
commit d178c64d53fcc6ab50d35c4f178b21322b7fa357
Author: Michael Pyne mp...@kde.org
Date: Sat Jun 16 23:32:09 2012 -0400
kshareddatacache: Correct the findEmptyPages loop boundary.
I did review the last patch to remove 'pagesNeeded' from the loop test,
but I missed that it has '+ 1' instead of '- 1' (which is still too
conservative). The correct thing is to not have any addition at all
since we use operator() for the test.
Graciously pointed out by kde_pepo, thanks!
- Vadim
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106174/#review18055
---
On Aug. 26, 2012, 5:15 p.m., Vadim Zhukov wrote:
---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106174/
---
(Updated Aug. 26, 2012, 5:15 p.m.)
Review request for kdelibs and Michael Pyne.
Description
---
This patch does move some run-time checks (mainly for different locking
mechanisms) in KSharedDataCache to build time (CMake).
This addresses bug 305023.
http://bugs.kde.org/show_bug.cgi?id=305023
Diffs
-
kdecore/util/config-util.h.cmake 83ccdf7
kdecore/util/kshareddatacache.cpp 393902e
kdecore/util/kshareddatacache_p.h ec5a7a0
kdecore/util/ConfigureChecks.cmake fe9f47e
Diff: http://git.reviewboard.kde.org/r/106174/diff/
Testing
---
On OpenBSD-CURRENT.
Thanks,
Vadim Zhukov
Re: Review Request: print-manager on kdereview
El Divendres, 24 d'agost de 2012, a les 23:29:19, Daniel Nicoletti va escriure: Sorry forgot to reply to all.. 2012/8/23 Christoph Feck christ...@maxiom.de: On Wednesday 22 August 2012 21:39:11 Daniel Nicoletti wrote: two years ago I started print-manager, at that time I was using Debian which is affected by this bug: https://bugs.kde.org/show_bug.cgi?id=271957 because of this (and the fact I'm not a python fan to fix the issue) I started print-manager a C++ implementation that could replace the current solution and fix the bug I had. Given the fact that the above mentioned bug is about authorization, what does printer-manager do to avoid bug 242648, and how could its methods be transfered to other modules needing KAuth, such as the kdm or clock modules? Well CUPS has it's own API for authorization, which is why I avoided the polkit solution s-c-p-gnome now uses. It wasn't easy to make it work right since CUPS API blocks, but it works reliable now that I wrapped it on a thread. * a KDED module to notify the user about new printers plugged into USB being configured Thanks for caring about blocking D-Bus calls in kded. Hopefully QtDBus is thread-safe :) Will do a more deep review in the coming days. Well I've been moving all my kded modules to threads to avoid those blocks/deadlocks that happened in the past, and till now I had no issue with that. and please review the code :D which is right now at https://projects.kde.org/projects/playground/base/print-manager but a sysadmin request to move has already been filled Sorry, cannot read sysadmin bugs. Where do you want it to move? Since the idea is to replace s-c-p-kde it would reside where it is now, so these repos would maybe go to unmaintained: git.kde.org:printer-applet http://websvn.kde.org/trunk/KDE/kdeadmin/system-config-printer-kde/?view=log You do not seem to be the maintainer of any of these two software. I'm interested in knowing the opinion of the maintainer of them regarding your software kicking them out to unmaintained. Cheers, Albert As the print-manager repo provides both things * a plasmoid and a kded module to replace printer-applet repo * a KCM to replace s-c-p-kde KCM I don't really know the best destination and I would rather not split this repo since it uses an internal lib, maybe kde workspace would be a good place to stay.
Re: Review Request: print-manager on kdereview
On Monday 27 August 2012 19:37:52 Albert Astals Cid wrote: El Divendres, 24 d'agost de 2012, a les 23:29:19, Daniel Nicoletti Since the idea is to replace s-c-p-kde it would reside where it is now, so these repos would maybe go to unmaintained: git.kde.org:printer-applet http://websvn.kde.org/trunk/KDE/kdeadmin/system-config-printer-kd e/?view=log You do not seem to be the maintainer of any of these two software. I'm interested in knowing the opinion of the maintainer of them regarding your software kicking them out to unmaintained. Replacing a Python-based code with (debuggable) C++ code is worth the change alone. You do not want to see the Python backtraces we get... (and yes, Daniel already asked Jonathan :) Christoph Feck (kdepepo)
Re: Review Request: print-manager on kdereview
On Sunday 26 August 2012 06:34:30 Daniel Nicoletti wrote: 2012/8/25 Christoph Feck christ...@maxiom.de: On Saturday 25 August 2012 04:29:19 Daniel Nicoletti wrote: 2012/8/23 Christoph Feck christ...@maxiom.de: On Wednesday 22 August 2012 21:39:11 Daniel Nicoletti wrote: Well CUPS has it's own API for authorization, which is why I avoided the polkit solution s-c-p-gnome now uses. It wasn't easy to make it work right since CUPS API blocks, but it works reliable now that I wrapped it on a thread. Okey, so what you do in printer-manager is completely unrelated to kauth. What I am interested in is, how we can use your knowledge to avoid the mistake in the kauth design for frameworks. Hmm what kind of mistake are you talking about? I like polkit, but I don't think it fits CUPS design, for printd (a possible CUPS replacement) it will make all sense. I am talking about whatever design mistake is responsible for bug 242648, which isn't about CUPS at all. Sorry for side-tracking this thread. I was just thinking that with your expertise on the authorization-related stuff, we could plan better for the future, so that we don't carry this bug over to frameworks. If I understand mentioned bug correctly, it is not possible to fix the crash with the current design of kauth/polkit interaction, because the polkit frameworks expects the password dialog to run in its own processes. It's not exactly a crash, the way s-c-p-kde is working today requires it to run printer-applet as root (which is what some distros do), or have CUPS not to ask password for local admin tasks (Ubuntu case), and in Debian at the time I started print-manager it simply doesn't work. The reason being that it couldn't show a CUPS password dialog and all cups communication was in the GUI thread so it would block.. In other words it wouldn't have an easy fix. If GNOME still uses polkit, how did they solve the out-of-process password dialogs? s-c-p-gnome now uses polkit but I don't know exactly how does that work and since CUPS has a method to handle password I think this makes the problem much more complex. kde workspace would be a good place to stay. Hm, kdeadmin is not moved to git yet, but I guess it still would be possible to create a KDE Admin project group. Right, I'm really not sure about the location, since it's a hardware component I'd say kde workspace under kcontrol dir seems a good place but whatever helps packagers or anything is fine to me :) Best,
Re: Review Request: print-manager on kdereview
2012/8/27 Christoph Feck christ...@maxiom.de: On Sunday 26 August 2012 06:34:30 Daniel Nicoletti wrote: 2012/8/25 Christoph Feck christ...@maxiom.de: On Saturday 25 August 2012 04:29:19 Daniel Nicoletti wrote: 2012/8/23 Christoph Feck christ...@maxiom.de: On Wednesday 22 August 2012 21:39:11 Daniel Nicoletti wrote: Well CUPS has it's own API for authorization, which is why I avoided the polkit solution s-c-p-gnome now uses. It wasn't easy to make it work right since CUPS API blocks, but it works reliable now that I wrapped it on a thread. Okey, so what you do in printer-manager is completely unrelated to kauth. What I am interested in is, how we can use your knowledge to avoid the mistake in the kauth design for frameworks. Hmm what kind of mistake are you talking about? I like polkit, but I don't think it fits CUPS design, for printd (a possible CUPS replacement) it will make all sense. I am talking about whatever design mistake is responsible for bug 242648, which isn't about CUPS at all. Sorry for side-tracking this thread. I was just thinking that with your expertise on the authorization-related stuff, we could plan better for the future, so that we don't carry this bug over to frameworks. Ah ok, it's a completely different issue. Now from the backtrace you already know the issue is due to event loops, I didn't look further but the bugs I had with Apper is because SystemSettings call accept on the open module, then the accept opens an event loop (otherwise returning will close SystemSettings). So to fix this I maybe on KAuth or in SytemSettings a QPoiter needs to check if the event loop is valid. Maybe the KCM could pass an event that can be event-accept() to avoid blocking the call with an event loop. This is going a bit out of the scope of the thread but if you want I can take a closer look to the issue. Best, Daniel
Re: [RFC] Merging LightDM into KDE Workspaces (forwarded from plasma-devel)
Am 28.08.2012, 00:25 Uhr, schrieb Martin Sandsmark martin.sandsm...@kde.org: On Wed, Aug 22, 2012 at 01:58:57PM +0100, David Edmundson wrote: Features LightDM-KDE has that KDM does not: - Guest support login What does the DM have to do with this? David Edmundson schrieb On Thu, Aug 23, 2012 at 2:04 PM, Thomas Lübking wrote: Sorry, this is certainly too much in details, but what is that Features LightDM-KDE has that KDM does not: - Guest support login supposed to be? I mean, either there's a guest account on the machine or there's not. If you intend to sell a DM which runtime injects accounts or so, you just lost a customer ;-P It injects accounts. The part that does this is mostly in the downstream packaging. Also you can uncheck the box allow guest in the GUII don't /think/ it's on by default in our code, but Ubuntu packaging turns it on. To me it sounds as if the package contains a postinstall script to useradd guest for a pwdless login. I cannot say to really appreciate such feature :-\ - Ability to make customisations such as changing the background without having to edit XML Isn't the kdm config file just a simple .ini file? He's talking about the xml greeter. Cheers, Thomas
Re: KCM Authorization (was: Re: Review Request: print-manager on kdereview)
Am 27.08.2012, 22:53 Uhr, schrieb Daniel Nicoletti dantt...@gmail.com: So to avoid the application quiting modules that need to perform async tasks like talking to polkit or installing packages (Apper), they need to create an event loop so that current module accept() doesn't return (if it does quit() will be called). So the user clicks Apply, while setting the time or doing anything the user clicks to close the window, the event loop will return and some stuff will be already deleted which causes the crashes... As far as i understood the bottomline is the task to make an external process fully modal to the window, yesno? Aside that the NETWM spec probably SHOULD say that modal windows must not receive input focus and mouse events and also not be closed, weird ideas: a) add an eventfilter on the application to suck all input events (for that window) so clicking apply etc. isn't possible and the window also will not receive close events b) intercept sigterm and reject it as long as you're waiting for the async return Cheers, Thomas
Re: KCM Authorization (was: Re: Review Request: print-manager on kdereview)
2012/8/27 Thomas Lübking thomas.luebk...@gmail.com: Am 27.08.2012, 22:53 Uhr, schrieb Daniel Nicoletti dantt...@gmail.com: So to avoid the application quiting modules that need to perform async tasks like talking to polkit or installing packages (Apper), they need to create an event loop so that current module accept() doesn't return (if it does quit() will be called). So the user clicks Apply, while setting the time or doing anything the user clicks to close the window, the event loop will return and some stuff will be already deleted which causes the crashes... As far as i understood the bottomline is the task to make an external process fully modal to the window, yesno? Aside that the NETWM spec probably SHOULD say that modal windows must not receive input focus and mouse events and also not be closed, weird ideas: a) add an eventfilter on the application to suck all input events (for that window) so clicking apply etc. isn't possible and the window also will not receive close events b) intercept sigterm and reject it as long as you're waiting for the async return The main problem won't be fixed with this, a kquitapp will still crash the application. (tho maybe b could work but to me seems quite complex...) Nested event loops brings all kinds of trouble... surely there are places where you don't have an option, which is were you must check if the application is quitting.. and in the KCM case only with QPointer I could do this kind of crash go away QAppliction::isClosing() or something like that didn't do the trick (don't recall why)...
Re: Review Request: Add spinlocks lock type, based on GCC intrisincs
On Tuesday, August 28, 2012 00:41:16 Thiago Macieira wrote: On segunda-feira, 27 de agosto de 2012 18.20.15, Michael Pyne wrote: Please use the Qt atomic types. Until GCC 4.7, they generate better code. I agree, the reason it wasn't that way initially is mentioned in the discussion on the bug (but basically because you can't simply put QBasicAtomicInt in the union used to store the different lock types that are possible). Why not? QBasicAtomicInt are permitted in unions. Besides, why do you want it in a union in the first place? You should not access the data that it holds *except* via the QBasicAtomicInt functions. That would be the idea, yes (to use the public QBAI functions). The problem with having it in a union was that it's a non-POD type according to C++ 03 rules (or at least, that seemed to be the issue when I had tried that initially). Regards, - Michael Pyne signature.asc Description: This is a digitally signed message part.
Re: [RFC] Merging LightDM into KDE Workspaces (forwarded from plasma-devel)
On Tue, Aug 28, 2012 at 01:17:03AM +0200, Thomas Lübking wrote: To me it sounds as if the package contains a postinstall script to useradd guest for a pwdless login. I cannot say to really appreciate such feature :-\ Uh, that sounds kind of bad for a DM, yes. He's talking about the xml greeter. Now I'm really confused, isn't the wallpaper just set in /etc/kde4/kdm/kdmrc? -- Martin Sandsmark
Review Request: Make Sonnet::Speller::setLanguage threadsafe, fixing segfaults in krunner
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106242/ --- Review request for kdelibs and Plasma. Description --- Krunner's spellcheck plugin has been pretty broken since bd291d21f096a714a171e7af3a534ba345ca5659 (about two years ago) because it called Sonnet::Speller::setLanguage every time the spellchecker was invoked, which was (very much) not thread-safe. This patch makes Sonnet::Speller::setLanguage threadsafe by protecting all access to the internal dict pointer using QReadWriteLock. A related review request is 106244, which adds more fixes to the spellcheck feature. This addresses bugs 264779 and 303831. http://bugs.kde.org/show_bug.cgi?id=264779 http://bugs.kde.org/show_bug.cgi?id=303831 Diffs - kdecore/sonnet/speller.cpp b19e74d Diff: http://git.reviewboard.kde.org/r/106242/diff/ Testing --- Compiled, installed, used for a week or so, spellchecked a bunch of things. Thanks, Simeon Bird
Re: [RFC] Merging LightDM into KDE Workspaces (forwarded from plasma-devel)
On Mon, Aug 27, 2012 at 10:02 PM, Martin Sandsmark martin.sandsm...@kde.org wrote: Now I'm really confused, isn't the wallpaper just set in /etc/kde4/kdm/kdmrc? not if you're in themed mode. in which case the theme specifies it, afik -- Shaun Reich, KDE Software Developer (kde.org)
