Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Nov. 12, 2013, 10:39 a.m., Commit Hook wrote: This review has been submitted with commit 53e8e439af2483c86b21ad4d53ffe4da622e8c44 by Martin Klapetek to branch frameworks. Christoph Feck wrote: Locally, I get this error: AUTOMOC: error: process for /local/build/kf5/runtime/ktimezoned/ktimezoned.moc failed: /local/git/KDE/base/kde-runtime-frameworks/ktimezoned/ktimezoned.cpp:35: Error: Plugin Metadata file ktimezoned.json does not exist. Declaration will be ignored moc failed... make[2]: *** [ktimezoned/CMakeFiles/kded_ktimezoned_automoc] Error 1 make[2]: Target `ktimezoned/CMakeFiles/kded_ktimezoned_automoc.dir/build' not remade because of errors. make[1]: *** [ktimezoned/CMakeFiles/kded_ktimezoned_automoc.dir/all] Error 2 Any idea? I know other folks seen this too, last time I've heard it might be a race condition issue with -j1 build (the json file gets generated only after building the file that's actually including it). I think Aurelien was looking into that, dunno if he made any progress though. - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review43507 --- On Nov. 12, 2013, 10:39 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Nov. 12, 2013, 10:39 a.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Nov. 12, 2013, 10:39 a.m., Commit Hook wrote: This review has been submitted with commit 53e8e439af2483c86b21ad4d53ffe4da622e8c44 by Martin Klapetek to branch frameworks. Christoph Feck wrote: Locally, I get this error: AUTOMOC: error: process for /local/build/kf5/runtime/ktimezoned/ktimezoned.moc failed: /local/git/KDE/base/kde-runtime-frameworks/ktimezoned/ktimezoned.cpp:35: Error: Plugin Metadata file ktimezoned.json does not exist. Declaration will be ignored moc failed... make[2]: *** [ktimezoned/CMakeFiles/kded_ktimezoned_automoc] Error 1 make[2]: Target `ktimezoned/CMakeFiles/kded_ktimezoned_automoc.dir/build' not remade because of errors. make[1]: *** [ktimezoned/CMakeFiles/kded_ktimezoned_automoc.dir/all] Error 2 Any idea? Martin Klapetek wrote: I know other folks seen this too, last time I've heard it might be a race condition issue with -j1 build (the json file gets generated only after building the file that's actually including it). I think Aurelien was looking into that, dunno if he made any progress though. It's indeed a use before it's built, because when I run try to build it again (on the same build dir) it works. A clean build, however, reliably reproduces this error, even without using -j make option. - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review43507 --- On Nov. 12, 2013, 10:39 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Nov. 12, 2013, 10:39 a.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Nov. 12, 2013, 10:39 a.m., Commit Hook wrote: This review has been submitted with commit 53e8e439af2483c86b21ad4d53ffe4da622e8c44 by Martin Klapetek to branch frameworks. Locally, I get this error: AUTOMOC: error: process for /local/build/kf5/runtime/ktimezoned/ktimezoned.moc failed: /local/git/KDE/base/kde-runtime-frameworks/ktimezoned/ktimezoned.cpp:35: Error: Plugin Metadata file ktimezoned.json does not exist. Declaration will be ignored moc failed... make[2]: *** [ktimezoned/CMakeFiles/kded_ktimezoned_automoc] Error 1 make[2]: Target `ktimezoned/CMakeFiles/kded_ktimezoned_automoc.dir/build' not remade because of errors. make[1]: *** [ktimezoned/CMakeFiles/kded_ktimezoned_automoc.dir/all] Error 2 Any idea? - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review43507 --- On Nov. 12, 2013, 10:39 a.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Nov. 12, 2013, 10:39 a.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review43507 --- This review has been submitted with commit 53e8e439af2483c86b21ad4d53ffe4da622e8c44 by Martin Klapetek to branch frameworks. - Commit Hook On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Nov. 12, 2013, 10:39 a.m.) Status -- This change has been marked as submitted. Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review43323 --- Ship it! OK. I only meant don't remove existing code that might make sense for some users. But if the real solution for Solaris support is to do it in Qt rather than here, then this code doesn't make sense anymore, remove it. - David Faure On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review43331 --- Ship it! Ship It! - John Layt On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Nov. 4, 2013, 4:12 p.m., John Layt wrote: I've asked on the Qt Development list about Qt 5 Solaris support. I'm told it builds and works to some extent, and patches are welcome, but not without having been tested on a real Solaris build first, which I have no desire to do. I don't think much is required, Solaris uses the Olsen tz files, just with different patches and possibly a different zonetab layout, but I don't want to code blind. So we have two options, one is not worry about Solaris support for now, and if anyone (Ade?) complains then ask them to contribute upstream (with my help). The alternative is to keep the Solaris code in ktimezoned, including calls to return the current system time zone and the list of available time zones, and on other platforms just wrap the Qt calls. Opinions? John Layt wrote: s/patches/paths Martin Klapetek wrote: I'd like to reiterate the (imho) bigger issue here - there's no effort at all to have KF5 and/or PW2 running on Solaris (unlike the Windows effort), so I wouldn't push it (only) to the smallest component in the workspace, it makes no sense at this time. Agreed. - Kevin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review42961 --- On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review42954 --- Looks OK to me, but I'd like John to give it a last look. - Kevin Ottens On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Nov. 4, 2013, 4:12 p.m., John Layt wrote: I've asked on the Qt Development list about Qt 5 Solaris support. I'm told it builds and works to some extent, and patches are welcome, but not without having been tested on a real Solaris build first, which I have no desire to do. I don't think much is required, Solaris uses the Olsen tz files, just with different patches and possibly a different zonetab layout, but I don't want to code blind. So we have two options, one is not worry about Solaris support for now, and if anyone (Ade?) complains then ask them to contribute upstream (with my help). The alternative is to keep the Solaris code in ktimezoned, including calls to return the current system time zone and the list of available time zones, and on other platforms just wrap the Qt calls. Opinions? s/patches/paths - John --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review42961 --- On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Nov. 4, 2013, 4:12 p.m., John Layt wrote: I've asked on the Qt Development list about Qt 5 Solaris support. I'm told it builds and works to some extent, and patches are welcome, but not without having been tested on a real Solaris build first, which I have no desire to do. I don't think much is required, Solaris uses the Olsen tz files, just with different patches and possibly a different zonetab layout, but I don't want to code blind. So we have two options, one is not worry about Solaris support for now, and if anyone (Ade?) complains then ask them to contribute upstream (with my help). The alternative is to keep the Solaris code in ktimezoned, including calls to return the current system time zone and the list of available time zones, and on other platforms just wrap the Qt calls. Opinions? John Layt wrote: s/patches/paths I'd like to reiterate the (imho) bigger issue here - there's no effort at all to have KF5 and/or PW2 running on Solaris (unlike the Windows effort), so I wouldn't push it (only) to the smallest component in the workspace, it makes no sense at this time. - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review42961 --- On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review42743 --- Ping, this seems forgotten :) - Martin Klapetek On Oct. 22, 2013, 4:49 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 22, 2013, 4:49 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Changes --- * Watch the whole timezone database dir * Rename signal to timeZoneDatabaseUpdated() * Windows support is to be added later by John (I have no way to test) * The Solaris support remains removed. Yes, Solaris still has its users, but they are using KDE4 and will keep so for some time. IF PlasmaWorkspaces2 will get Solaris support (so that it all builds and runs), I hereby commit to add proper Solaris support to ktimezoned at that time, otherwise there's no point in keeping old (now untested) code around to bitrot and add maintenance burden, especially since there is 0 effort to have other, more important components work on Solaris (including Frameworks). Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs (updated) - CMakeLists.txt a5ec93d ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Oct. 16, 2013, 12:15 p.m., John Layt wrote: Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. Then again, it's not something that will happen very often, and what will the apps do in response? If they refresh their cache the shared copies in QDateTime won't update. I guess QTimeZone will need a reloadData() method added instead. I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. Martin Klapetek wrote: Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. Part of that is support for Solaris, but I see Solaris is not supported by Qt anymore [1][2], so I just removed it altogether. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. I think it might be worth having it...I'll add it back, can't hurt :) I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. Ah yes, I forgot to add that to the patch, sorry. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? I wanted to add it, but that's in kdelibs. Question is, should there be a guide for runtime/workspace too? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. Ok, keep us posted. (It would be cool to have cross-desktop solution for this too...or system-wide spec at least, so we could use non-qt/-kde apps still supporting timezone changes, but oh well). [1] - http://qt-project.org/doc/qt-5.1/qtdoc/supported-platforms.html [2] - http://qt.digia.com/Product/Supported-Platforms/ There are still people around with a Solaris system. Not supported doesn't mean we should actively remove existing support, IMHO. About the porting guide: the distinction between kdelibs and kde-runtime will disappear, given the framework-ification. So treat it as a porting guide from kde 4 to kde-frameworks, i.e. it's fine to include runtime stuff in there. About a solution in Qt --- that means each and every Qt app will have to monitor the same timezone file(s), isn't that a bit too expensive? (not sure, just wondering) - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review41816 --- On Oct. 18, 2013, 1 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 18, 2013, 1 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros)
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Oct. 16, 2013, 12:15 p.m., John Layt wrote: Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. Then again, it's not something that will happen very often, and what will the apps do in response? If they refresh their cache the shared copies in QDateTime won't update. I guess QTimeZone will need a reloadData() method added instead. I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. Martin Klapetek wrote: Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. Part of that is support for Solaris, but I see Solaris is not supported by Qt anymore [1][2], so I just removed it altogether. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. I think it might be worth having it...I'll add it back, can't hurt :) I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. Ah yes, I forgot to add that to the patch, sorry. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? I wanted to add it, but that's in kdelibs. Question is, should there be a guide for runtime/workspace too? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. Ok, keep us posted. (It would be cool to have cross-desktop solution for this too...or system-wide spec at least, so we could use non-qt/-kde apps still supporting timezone changes, but oh well). [1] - http://qt-project.org/doc/qt-5.1/qtdoc/supported-platforms.html [2] - http://qt.digia.com/Product/Supported-Platforms/ David Faure wrote: There are still people around with a Solaris system. Not supported doesn't mean we should actively remove existing support, IMHO. About the porting guide: the distinction between kdelibs and kde-runtime will disappear, given the framework-ification. So treat it as a porting guide from kde 4 to kde-frameworks, i.e. it's fine to include runtime stuff in there. About a solution in Qt --- that means each and every Qt app will have to monitor the same timezone file(s), isn't that a bit too expensive? (not sure, just wondering) There are still people around with a Solaris system. Not supported doesn't mean we should actively remove existing support, IMHO. Is there any attempt to make sure KF5/PW2 is working with Solaris? Because if the underlying components won't work on Solaris, there's no point in keeping Solaris code in one tiny module sitting on top of the bigger blocks. it's fine to include runtime stuff in there. Cool, will add it then :) - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review41816 --- On Oct. 18, 2013, 1 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 18, 2013, 1 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review42017 --- ktimezoned/ktimezonedbase.h http://git.reviewboard.kde.org/r/113260/#comment30665 Not generic enough, is Unix specific. Perhaps timeZoneDatabaseUpdated() without the file path? ktimezoned/org.kde.KTimeZoned.xml http://git.reviewboard.kde.org/r/113260/#comment30664 You need to keep this, or a variant of it. I don't think this is generic enough, it only applies to the zonetab file on Unix, and the data files could be updated without the zonetab being changed, and it doesn't apply to Windows at all. I'd suggest a generic timeZoneDatabaseUpdated() signal instead that triggers on Unix if any file in the zone path tree (/usr/share/zoneinfo or wherever) is updated, or for Windows if any of the registry database is updated (I can do that later). - John Layt On Oct. 18, 2013, 1 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 18, 2013, 1 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned.h ff21807 ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 18, 2013, 1 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Changes --- Use zoneTabChanged dbus signal for zone.tab file changes Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs (updated) - ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned.h ff21807 ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
On Oct. 16, 2013, 12:15 p.m., John Layt wrote: Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. Then again, it's not something that will happen very often, and what will the apps do in response? If they refresh their cache the shared copies in QDateTime won't update. I guess QTimeZone will need a reloadData() method added instead. I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. Part of that is support for Solaris, but I see Solaris is not supported by Qt anymore [1][2], so I just removed it altogether. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. I think it might be worth having it...I'll add it back, can't hurt :) I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. Ah yes, I forgot to add that to the patch, sorry. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? I wanted to add it, but that's in kdelibs. Question is, should there be a guide for runtime/workspace too? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. Ok, keep us posted. (It would be cool to have cross-desktop solution for this too...or system-wide spec at least, so we could use non-qt/-kde apps still supporting timezone changes, but oh well). [1] - http://qt-project.org/doc/qt-5.1/qtdoc/supported-platforms.html [2] - http://qt.digia.com/Product/Supported-Platforms/ - Martin --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review41816 --- On Oct. 15, 2013, 8:09 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 15, 2013, 8:09 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 17, 2013, 9:13 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Changes --- Bring back the detection of zone.tab file and watching its changes + emitting zoneDefinitionChanged(..) on changes. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs (updated) - ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/#review41816 --- Wow, there sure is a lot of code in there catering for all the possible corner cases :-) QTimeZone has a lot less places it checks, so I'll need to do an in-depth comparison, but given Qt5 will only support modern distros I think most of it is redundant. One thought is if we still want to have a signal that the system database has been updated? While Qt doesn't cache the time zone data, the apps probably will cache the QTimeZone instances themselves and may need to be told that the database has changed so they can take action. Then again, it's not something that will happen very often, and what will the apps do in response? If they refresh their cache the shared copies in QDateTime won't update. I guess QTimeZone will need a reloadData() method added instead. I've looked at the Windows code and it mostly looks OK, all it does is monitor the registry for changes. What you do need to change is the DBus signal from configChanged to your new timeZoneChanged signal. With regards the signal name change, does it need to go in the porting guide or somewhere so people know it has changed? In Qt 5.3 I will try adding a QEvent for TimeZoneChanged and TimeZoneDatabaseChanged, but I can't guarantee it will get in so we still need the daemon for now. - John Layt On Oct. 15, 2013, 8:09 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 15, 2013, 8:09 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
Re: Review Request 113260: Port KTimeZoned to Qt5/KF5
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113260/ --- (Updated Oct. 15, 2013, 8:09 p.m.) Review request for KDE Runtime, KDE Frameworks, Plasma, and John Layt. Repository: kde-runtime Description (updated) --- Originally I wanted to port KTimeZoned 1:1 to Qt5/KF5, but then I found out that all the stuff KTZD was doing was added in QTimeZone, that includes reading correct system files/env variables to obtain the timezone and returning the proper system zone (KTZD did all this by itself). It also doesn't need to parse the timezone files itself or watch for their changes as QTimeZone objects are not stored. So now it's just a thin module watching /etc/timezone (used by Debian-based distros) and /etc/localtime (used by eg. Fedora or Suse, but also by Debian in conjunction with /etc/timezone) for changes and if it detects a change, it checks if the new timezone is really different and if it is, it sends out a DBus signal timeZoneChange. I changed it from configChanged as I think timeZoneChanged makes way more sense. I didn't touch the Windows part as I have no way to test, would be nice if someone could help with that. EDIT: I removed the other two DBus signals which were not used and I'm unsure KTZD is the correct place for that now anyway. The only usage in KSystemTimeZone can be replaced by own KDirWatch instance. Diffs - ktimezoned/CMakeLists.txt bafc85e ktimezoned/ktimezoned.h ff21807 ktimezoned/ktimezoned.cpp f380c09 ktimezoned/ktimezoned_win.h 26e21cc ktimezoned/ktimezoned_win.cpp cadfe3a ktimezoned/ktimezonedbase.h ca00aca ktimezoned/org.kde.KTimeZoned.xml daaa0b7 Diff: http://git.reviewboard.kde.org/r/113260/diff/ Testing --- Tested by changing the timezone in different ways, change was detected and signalled out. Thanks, Martin Klapetek
