Fix timezone saving in System Settings -> Date & Time

Review Request #114321 - Created Dec. 5, 2013 and updated

Information
Lukáš Tinkl
kde-workspace
KDE/4.11
159171, 323511
Reviewers
kde-workspace
- 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
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. 

Issues

  • 0
  • 4
  • 0
  • 4
Description From Last Updated
Martin Klapetek
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.
  1. 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?
    
    Hmmmm, I almost find myself wishing systemd would take care of all this for us :-)
  2. You mean http://www.freedesktop.org/wiki/Software/systemd/timedated/ , right?
  3. 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.
  4. Yeah I tried using timedated as well but it fails spectacularly to update the Timezone property when you change it from outside (like on RHEL/Fedora) using system-config-date. This way it works using all the testcases I tried
Thomas Lübking

   
kcontrol/dateandtime/helper.cpp (Diff revision 1)
 
 
What's the problem about using zic?
If zic is broken (distro related?) zic should be fixed, yesno?
  1. This step is not needed at all
  2. Because?
    Do you know it's not there, because distros handle(d) their specifics through zic patches?
    
    Eg. plain "zic -l" prefers a hardlink and resorts to a symlink only. Eventually coyping as last resort (not checked)
    
    I will admit that i cannot say whether that's good or bad, but if you want to change it, i suggest to reason that by some deeper knowledge.
kcontrol/dateandtime/helper.cpp (Diff revision 1)
 
 
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?
  1. No issue, the /etc/timezone handling is still there, a few lines below
  2. I meant handling of the symlink ./. inode type of /etc/localtime - nothing at all about /etc/timezone
    See comments by Martin and John.
kcontrol/dateandtime/helper.cpp (Diff revision 1)
 
 
this looks like it'll break compilation on solaris?
  1. I don't think so, previously the code was wrong:
    
    QString val = ':' + tz;
    #endif // !USE_SOLARIS
    setenv("TZ", val.toAscii(), 1);
    
    so it would be setting the TZ from an uninit value on Solaris, I don't know how this could ever compile
  2. There're now two "QString val" declarations #if defined(USE_SOLARIS) (the other one is right above the #else)
    
    So let me rephrase it:
    It *will* break compilation on Solaris - for sure.
kcontrol/dateandtime/helper.cpp (Diff revision 1)
 
 
iff ASCII is wrong, this should be rather ::toLocal8Bit(), yesno?
  1. Hmm, maybe, but I think utf8 is safer here
  2. And you base that "think" upon what?
    - Either the names (and so it seems) are by contract limited to ASII
      Then using ASCII or UTF-8 is equal, and i'd tend to use ASCII to stress that limitation
    - Or it's not and then the correct format severely matters or the running process will fail to resolve the filename.
    
    Seriously:
    i18n and timehandling suck badly and if you intend to bring a "dunno why, but works for me" patch, I cannot support that at all, sorry.
Loading...