Re: Review Request 114298: prevent ksplashqml crash when a theme does not exist or cannot be loaded
On Dec. 5, 2013, 12:23 a.m., Pino Toscano wrote: ksplash/ksplashqml/SplashApp.cpp, lines 148-150 http://git.reviewboard.kde.org/r/114298/diff/1/?file=222537#file222537line148 What about just calling ::exit(2) to reach the stdlib function, avoiding the exitNow wrapper? So that's how one does that xD Thanks for the tip! - Harald --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114298/#review45154 --- On Dec. 4, 2013, 1:54 p.m., Harald Sitter wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114298/ --- (Updated Dec. 4, 2013, 1:54 p.m.) Review request for kde-workspace and Ivan Čukić. Repository: kde-workspace Description --- Do not crash when failing to load a theme, but instead exit. When failing to load the theme's qml file QDeclarativeView goes into error state, at which point we want to exit to prevent nullptr access problems. Diffs - ksplash/ksplashqml/SplashApp.cpp 7c528d056ee680f69a6d3d60d1dbeeae9d548846 Diff: http://git.reviewboard.kde.org/r/114298/diff/ Testing --- ksplashqml Foo --test - sigsev without patch - exits with patch Thanks, Harald Sitter
Re: Review Request 114298: prevent ksplashqml crash when a theme does not exist or cannot be loaded
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114298/#review45167 --- This review has been submitted with commit 3ab6ca57de19ed1559accd632b46e27b1a4fd0eb by Harald Sitter to branch KDE/4.11. - Commit Hook On Dec. 4, 2013, 1:54 p.m., Harald Sitter wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114298/ --- (Updated Dec. 4, 2013, 1:54 p.m.) Review request for kde-workspace and Ivan Čukić. Repository: kde-workspace Description --- Do not crash when failing to load a theme, but instead exit. When failing to load the theme's qml file QDeclarativeView goes into error state, at which point we want to exit to prevent nullptr access problems. Diffs - ksplash/ksplashqml/SplashApp.cpp 7c528d056ee680f69a6d3d60d1dbeeae9d548846 Diff: http://git.reviewboard.kde.org/r/114298/diff/ Testing --- ksplashqml Foo --test - sigsev without patch - exits with patch Thanks, Harald Sitter
Re: Review Request 114298: prevent ksplashqml crash when a theme does not exist or cannot be loaded
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114298/ --- (Updated Dec. 5, 2013, 10:14 a.m.) Status -- This change has been marked as submitted. Review request for kde-workspace and Ivan Čukić. Repository: kde-workspace Description --- Do not crash when failing to load a theme, but instead exit. When failing to load the theme's qml file QDeclarativeView goes into error state, at which point we want to exit to prevent nullptr access problems. Diffs - ksplash/ksplashqml/SplashApp.cpp 7c528d056ee680f69a6d3d60d1dbeeae9d548846 Diff: http://git.reviewboard.kde.org/r/114298/diff/ Testing --- ksplashqml Foo --test - sigsev without patch - exits with patch Thanks, Harald Sitter
Review Request 114314: Fix traceback in Python runner plugins
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114314/ --- Review request for kde-workspace and KDE Bindings. Bugs: https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1258088 http://bugs.kde.org/show_bug.cgi?id=https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1258088 Repository: kde-workspace Description --- Plamascript.Runner is the base of python krunner plugins. These plugins implement the C++ signals prepare, teardown, createRunOptions and reloadConfiguration in actual methods (the signal wiring happens in pyrunner.py which is the loading component). As a result of this calls to any of these methods will fall through to plasmascript.Runner whenever the actual runner does not implement them. However plasmascript.Runner is missing the implicit 'self' argument such that one gets silly python backtraces like File /usr/share/kde4/apps/plasma_scriptengine_python/pyrunner.py, line 90, in reloadConfiguration self.pyrunner.reloadConfiguration() To prevent this from happening the functions now have the implicit self argument. Also see: https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1258088 CCMAIL: 1258...@bugs.launchpad.net Diffs - plasma/generic/scriptengines/python/plasmascript.py 0ec38eb826cd8b7a052ed47081d05a3b644b03d1 Diff: http://git.reviewboard.kde.org/r/114314/diff/ Testing --- Simple runner plugin only implementing match, run and reloadConfiguration, each is called and no exceptions are thrown WRT attribute errors. from PyKDE4 import plasmascript from PyKDE4.plasma import Plasma from PyKDE4.kdeui import KIcon class KittehRunner(plasmascript.Runner): def match(self, context): print match def run(self, context, match): print run def reloadConfiguration(self): print reloadConfig def CreateRunner(parent): return KittehRunner(parent) Thanks, Harald Sitter
Re: Review Request 114314: Fix traceback in Python runner plugins
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114314/#review45228 --- Ship it! Ship It! - Luca Beltrame On Dec. 5, 2013, 11:11 a.m., Harald Sitter wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114314/ --- (Updated Dec. 5, 2013, 11:11 a.m.) Review request for kde-workspace and KDE Bindings. Bugs: https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1258088 http://bugs.kde.org/show_bug.cgi?id=https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1258088 Repository: kde-workspace Description --- Plamascript.Runner is the base of python krunner plugins. These plugins implement the C++ signals prepare, teardown, createRunOptions and reloadConfiguration in actual methods (the signal wiring happens in pyrunner.py which is the loading component). As a result of this calls to any of these methods will fall through to plasmascript.Runner whenever the actual runner does not implement them. However plasmascript.Runner is missing the implicit 'self' argument such that one gets silly python backtraces like File /usr/share/kde4/apps/plasma_scriptengine_python/pyrunner.py, line 90, in reloadConfiguration self.pyrunner.reloadConfiguration() To prevent this from happening the functions now have the implicit self argument. Also see: https://bugs.launchpad.net/ubuntu/+source/kde-workspace/+bug/1258088 CCMAIL: 1258...@bugs.launchpad.net Diffs - plasma/generic/scriptengines/python/plasmascript.py 0ec38eb826cd8b7a052ed47081d05a3b644b03d1 Diff: http://git.reviewboard.kde.org/r/114314/diff/ Testing --- Simple runner plugin only implementing match, run and reloadConfiguration, each is called and no exceptions are thrown WRT attribute errors. from PyKDE4 import plasmascript from PyKDE4.plasma import Plasma from PyKDE4.kdeui import KIcon class KittehRunner(plasmascript.Runner): def match(self, context): print match def run(self, context, match): print run def reloadConfiguration(self): print reloadConfig def CreateRunner(parent): return KittehRunner(parent) Thanks, Harald Sitter
Review Request 114321: Fix timezone saving in System Settings - Date Time
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/ --- Review request for kde-workspace. Bugs: 159171 and 323511 http://bugs.kde.org/show_bug.cgi?id=159171 http://bugs.kde.org/show_bug.cgi?id=323511 Repository: kde-workspace Description --- - fix saving/loading of timezones in kcmclock - do not mark the module as changed right after the new timezone gets loaded back Besides the above mentioned bugs, it also fixes https://bugzilla.redhat.com/show_bug.cgi?id=990146 Diffs - kcontrol/dateandtime/dtime.cpp 518afe5 kcontrol/dateandtime/helper.cpp 9168db3 kcontrol/dateandtime/main.cpp 2fa0f3e Diff: http://git.reviewboard.kde.org/r/114321/diff/ Testing --- In the past, this would for some reason only work sporadically and make ktimezoned utterly confused; now it correctly sets a symlink (instead of copying the file over) from /etc/localtime to the respective file under /usr/share/zoneinfo (as described in man tzset). The symlink points to the right location after each save. Launching the module again, it shows the correct timezone, as previously saved. Thanks, Lukáš Tinkl
Re: Review Request 112463: Port SMB kioslave to KF5/Qt5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112463/ --- (Updated Dec. 5, 2013, 11:44 p.m.) Review request for KDE Runtime and KDE Frameworks. Changes --- Updated with suggestions from Kevin. Replaced KDE::utime (deprecated warning) by just utime(QFile::enc..., ...) Repository: kde-runtime Description --- This is the initial port! I added two TODO lines in the diff for parts where i'm not sure if I've ported them correctly. Also, i needed a change in FindSamba.cmake to even get the samba detection working. That reviewrequest is waiting here: https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use samba 3.x Once i know that this is actually working then i will comment some qDebug lines. Diffs (updated) - kioslave/CMakeLists.txt fc594e4 kioslave/smb/CMakeLists.txt a3a2265 kioslave/smb/kio_smb.h c2229ab kioslave/smb/kio_smb.cpp 2c2523a kioslave/smb/kio_smb_auth.cpp 4d236b4 kioslave/smb/kio_smb_browse.cpp 5253be9 kioslave/smb/kio_smb_dir.cpp ba80c86 kioslave/smb/kio_smb_file.cpp 206526a kioslave/smb/kio_smb_internal.h 4b946c1 kioslave/smb/kio_smb_internal.cpp e943844 kioslave/smb/kio_smb_mount.cpp a5a7e8e kioslave/smb/kio_smb_win.h f1dcb6f kioslave/smb/kio_smb_win.cpp 14dd797 Diff: http://git.reviewboard.kde.org/r/112463/diff/ Testing --- It compiles and gets loaded just fine. I tried testing this on an actual samba share, but i kept getting a 111 error (connection refused) from kio_smb so i'm hoping that is a local issue here. If someone else could try this out and verify that it's either working or broken. Thanks, Mark Gaiser
Re: Review Request 114321: Fix timezone saving in System Settings - Date Time
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/#review45240 --- Note that Debian-based systems actually do copy the file rather than symlink - main reason being that if you use a symlink and your /usr is mounted on a separate partition, anything that starts before /usr gets mounted will not have the correct timezone. - Martin Klapetek On Dec. 5, 2013, 11:24 p.m., Lukáš Tinkl wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/ --- (Updated Dec. 5, 2013, 11:24 p.m.) Review request for kde-workspace. Bugs: 159171 and 323511 http://bugs.kde.org/show_bug.cgi?id=159171 http://bugs.kde.org/show_bug.cgi?id=323511 Repository: kde-workspace Description --- - fix saving/loading of timezones in kcmclock - do not mark the module as changed right after the new timezone gets loaded back Besides the above mentioned bugs, it also fixes https://bugzilla.redhat.com/show_bug.cgi?id=990146 Diffs - kcontrol/dateandtime/dtime.cpp 518afe5 kcontrol/dateandtime/helper.cpp 9168db3 kcontrol/dateandtime/main.cpp 2fa0f3e Diff: http://git.reviewboard.kde.org/r/114321/diff/ Testing --- In the past, this would for some reason only work sporadically and make ktimezoned utterly confused; now it correctly sets a symlink (instead of copying the file over) from /etc/localtime to the respective file under /usr/share/zoneinfo (as described in man tzset). The symlink points to the right location after each save. Launching the module again, it shows the correct timezone, as previously saved. Thanks, Lukáš Tinkl
Re: Review Request 114321: Fix timezone saving in System Settings - Date Time
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/#review45241 --- kcontrol/dateandtime/helper.cpp http://git.reviewboard.kde.org/r/114321/#comment32322 What's the problem about using zic? If zic is broken (distro related?) zic should be fixed, yesno? kcontrol/dateandtime/helper.cpp http://git.reviewboard.kde.org/r/114321/#comment32323 assuming zic is not available and the fallback is required, this should maybe inspect the type of the present localtime (if) and link/copy the new timezone respectively? kcontrol/dateandtime/helper.cpp http://git.reviewboard.kde.org/r/114321/#comment32321 this looks like it'll break compilation on solaris? kcontrol/dateandtime/helper.cpp http://git.reviewboard.kde.org/r/114321/#comment32326 iff ASCII is wrong, this should be rather ::toLocal8Bit(), yesno? - Thomas Lübking On Dec. 5, 2013, 11:24 p.m., Lukáš Tinkl wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/ --- (Updated Dec. 5, 2013, 11:24 p.m.) Review request for kde-workspace. Bugs: 159171 and 323511 http://bugs.kde.org/show_bug.cgi?id=159171 http://bugs.kde.org/show_bug.cgi?id=323511 Repository: kde-workspace Description --- - fix saving/loading of timezones in kcmclock - do not mark the module as changed right after the new timezone gets loaded back Besides the above mentioned bugs, it also fixes https://bugzilla.redhat.com/show_bug.cgi?id=990146 Diffs - kcontrol/dateandtime/dtime.cpp 518afe5 kcontrol/dateandtime/helper.cpp 9168db3 kcontrol/dateandtime/main.cpp 2fa0f3e Diff: http://git.reviewboard.kde.org/r/114321/diff/ Testing --- In the past, this would for some reason only work sporadically and make ktimezoned utterly confused; now it correctly sets a symlink (instead of copying the file over) from /etc/localtime to the respective file under /usr/share/zoneinfo (as described in man tzset). The symlink points to the right location after each save. Launching the module again, it shows the correct timezone, as previously saved. Thanks, Lukáš Tinkl
Re: Review Request 114321: Fix timezone saving in System Settings - Date Time
On Dec. 5, 2013, 11:52 p.m., Martin Klapetek wrote: Note that Debian-based systems actually do copy the file rather than symlink - main reason being that if you use a symlink and your /usr is mounted on a separate partition, anything that starts before /usr gets mounted will not have the correct timezone. And it's not just that... Old versions of Red Hat also copied the file, and Debian stores the current tz name in /etc/timezone, and Red Hat and openSUSE store it in /etc/sysconfig/clock. Some older distros have the tz files in /usr/lib/zoneinfo instead of /usr/share/zoneinfo. And who knows what else other distros do! This code scares me (the old code especially so, why is it calling zic?), we're making all sorts of assumptions about the underlying system that we can't be sure of and changing things that could break the users system. Personally, I think the distros need to take care of this side of things by providing a script for us to call. I think most distros do have different command line tools to do it, but finding and maintaining them all would be a nightmare. Perhaps we need to install and call a script kde-set-timezone which the distros can then modify to call their own tools? H, I almost find myself wishing systemd would take care of all this for us :-) - John --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/#review45240 --- On Dec. 5, 2013, 11:24 p.m., Lukáš Tinkl wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/ --- (Updated Dec. 5, 2013, 11:24 p.m.) Review request for kde-workspace. Bugs: 159171 and 323511 http://bugs.kde.org/show_bug.cgi?id=159171 http://bugs.kde.org/show_bug.cgi?id=323511 Repository: kde-workspace Description --- - fix saving/loading of timezones in kcmclock - do not mark the module as changed right after the new timezone gets loaded back Besides the above mentioned bugs, it also fixes https://bugzilla.redhat.com/show_bug.cgi?id=990146 Diffs - kcontrol/dateandtime/dtime.cpp 518afe5 kcontrol/dateandtime/helper.cpp 9168db3 kcontrol/dateandtime/main.cpp 2fa0f3e Diff: http://git.reviewboard.kde.org/r/114321/diff/ Testing --- In the past, this would for some reason only work sporadically and make ktimezoned utterly confused; now it correctly sets a symlink (instead of copying the file over) from /etc/localtime to the respective file under /usr/share/zoneinfo (as described in man tzset). The symlink points to the right location after each save. Launching the module again, it shows the correct timezone, as previously saved. Thanks, Lukáš Tinkl
Re: Review Request 114321: Fix timezone saving in System Settings - Date Time
On Dec. 5, 2013, 11:52 p.m., Martin Klapetek wrote: Note that Debian-based systems actually do copy the file rather than symlink - main reason being that if you use a symlink and your /usr is mounted on a separate partition, anything that starts before /usr gets mounted will not have the correct timezone. John Layt wrote: And it's not just that... Old versions of Red Hat also copied the file, and Debian stores the current tz name in /etc/timezone, and Red Hat and openSUSE store it in /etc/sysconfig/clock. Some older distros have the tz files in /usr/lib/zoneinfo instead of /usr/share/zoneinfo. And who knows what else other distros do! This code scares me (the old code especially so, why is it calling zic?), we're making all sorts of assumptions about the underlying system that we can't be sure of and changing things that could break the users system. Personally, I think the distros need to take care of this side of things by providing a script for us to call. I think most distros do have different command line tools to do it, but finding and maintaining them all would be a nightmare. Perhaps we need to install and call a script kde-set-timezone which the distros can then modify to call their own tools? H, I almost find myself wishing systemd would take care of all this for us :-) You mean http://www.freedesktop.org/wiki/Software/systemd/timedated/ , right? - Jan --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/#review45240 --- On Dec. 5, 2013, 11:24 p.m., Lukáš Tinkl wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/ --- (Updated Dec. 5, 2013, 11:24 p.m.) Review request for kde-workspace. Bugs: 159171 and 323511 http://bugs.kde.org/show_bug.cgi?id=159171 http://bugs.kde.org/show_bug.cgi?id=323511 Repository: kde-workspace Description --- - fix saving/loading of timezones in kcmclock - do not mark the module as changed right after the new timezone gets loaded back Besides the above mentioned bugs, it also fixes https://bugzilla.redhat.com/show_bug.cgi?id=990146 Diffs - kcontrol/dateandtime/dtime.cpp 518afe5 kcontrol/dateandtime/helper.cpp 9168db3 kcontrol/dateandtime/main.cpp 2fa0f3e Diff: http://git.reviewboard.kde.org/r/114321/diff/ Testing --- In the past, this would for some reason only work sporadically and make ktimezoned utterly confused; now it correctly sets a symlink (instead of copying the file over) from /etc/localtime to the respective file under /usr/share/zoneinfo (as described in man tzset). The symlink points to the right location after each save. Launching the module again, it shows the correct timezone, as previously saved. Thanks, Lukáš Tinkl
Re: Review Request 114321: Fix timezone saving in System Settings - Date Time
On Dec. 5, 2013, 11:52 p.m., Martin Klapetek wrote: Note that Debian-based systems actually do copy the file rather than symlink - main reason being that if you use a symlink and your /usr is mounted on a separate partition, anything that starts before /usr gets mounted will not have the correct timezone. John Layt wrote: And it's not just that... Old versions of Red Hat also copied the file, and Debian stores the current tz name in /etc/timezone, and Red Hat and openSUSE store it in /etc/sysconfig/clock. Some older distros have the tz files in /usr/lib/zoneinfo instead of /usr/share/zoneinfo. And who knows what else other distros do! This code scares me (the old code especially so, why is it calling zic?), we're making all sorts of assumptions about the underlying system that we can't be sure of and changing things that could break the users system. Personally, I think the distros need to take care of this side of things by providing a script for us to call. I think most distros do have different command line tools to do it, but finding and maintaining them all would be a nightmare. Perhaps we need to install and call a script kde-set-timezone which the distros can then modify to call their own tools? H, I almost find myself wishing systemd would take care of all this for us :-) Jan Kundrát wrote: You mean http://www.freedesktop.org/wiki/Software/systemd/timedated/ , right? Yes, but having looked at that before it is also broken in exactly the same way. It's obvious it has never been properly integrated on a Debian system. It needs changing to allow for distro specific config scripts for setting the tz. It could also use some extra calls added like we have in ktimezoned. - John --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/#review45240 --- On Dec. 5, 2013, 11:24 p.m., Lukáš Tinkl wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/ --- (Updated Dec. 5, 2013, 11:24 p.m.) Review request for kde-workspace. Bugs: 159171 and 323511 http://bugs.kde.org/show_bug.cgi?id=159171 http://bugs.kde.org/show_bug.cgi?id=323511 Repository: kde-workspace Description --- - fix saving/loading of timezones in kcmclock - do not mark the module as changed right after the new timezone gets loaded back Besides the above mentioned bugs, it also fixes https://bugzilla.redhat.com/show_bug.cgi?id=990146 Diffs - kcontrol/dateandtime/dtime.cpp 518afe5 kcontrol/dateandtime/helper.cpp 9168db3 kcontrol/dateandtime/main.cpp 2fa0f3e Diff: http://git.reviewboard.kde.org/r/114321/diff/ Testing --- In the past, this would for some reason only work sporadically and make ktimezoned utterly confused; now it correctly sets a symlink (instead of copying the file over) from /etc/localtime to the respective file under /usr/share/zoneinfo (as described in man tzset). The symlink points to the right location after each save. Launching the module again, it shows the correct timezone, as previously saved. Thanks, Lukáš Tinkl
