> On Dec. 6, 2013, 12:22 a.m., Thomas Lübking wrote: > > kcontrol/dateandtime/helper.cpp, line 189 > > <http://git.reviewboard.kde.org/r/114321/diff/1/?file=222931#file222931line189> > > > > 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? > > Lukáš Tinkl wrote: > No issue, the /etc/timezone handling is still there, a few lines below
I meant handling of the symlink ./. inode type of /etc/localtime - nothing at all about /etc/timezone See comments by Martin and John. > On Dec. 6, 2013, 12:22 a.m., Thomas Lübking wrote: > > kcontrol/dateandtime/helper.cpp, line 204 > > <http://git.reviewboard.kde.org/r/114321/diff/1/?file=222931#file222931line204> > > > > this looks like it'll break compilation on solaris? > > Lukáš Tinkl wrote: > 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 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. > On Dec. 6, 2013, 12:22 a.m., Thomas Lübking wrote: > > kcontrol/dateandtime/helper.cpp, line 206 > > <http://git.reviewboard.kde.org/r/114321/diff/1/?file=222931#file222931line206> > > > > iff ASCII is wrong, this should be rather ::toLocal8Bit(), yesno? > > Lukáš Tinkl wrote: > Hmm, maybe, but I think utf8 is safer here 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. > On Dec. 6, 2013, 12:22 a.m., Thomas Lübking wrote: > > kcontrol/dateandtime/helper.cpp, line 180 > > <http://git.reviewboard.kde.org/r/114321/diff/1/?file=222931#file222931line180> > > > > What's the problem about using zic? > > If zic is broken (distro related?) zic should be fixed, yesno? > > Lukáš Tinkl wrote: > This step is not needed at all 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. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114321/#review45241 ----------------------------------------------------------- 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 > >
