David Reid wrote:
What's with the HTML email????
Nothing. Check your mail client.
Content-Type: text/plain; charset=ISO-8859-2; format=flowed
Hmm, good point. But that's easily fixed, I think.
Why should it be fixed???
I think we're having a bit of a misunderstanding here. My fault; I should have got some sleep before posting.
So, let me try again.
First, some history: Subversion uses the APR time functions to record the time stamps of revision-controlled files in the working copy. It does this by statting the file, using apr_explode_localtime and writing a text representation of the result to disk (including the tm_gmtoff field). Later on it uses that timestamp to check if the file has changed: it reads the timestamp back in, scans it into an apr_exploded_time_t and uses apr_implode_time to get an apr_time_t, which it compares to the one coming from apr_stat. Obviously, this didn't work, because the apr_time_t we got from apr_implode_time wasn't UTC.
So I looked at apr_implode_time, and thought, "Aha, that'a a bug, we're ignoring tm_gmtoff." Then I was told, by Ryan and you, that this was intentional. I was confused by two things: first, I understood that the reason for this was that we can't calculate the GMT offset on Solaris; and second, that having an apr_time_t that's /not/ UTC is valid:
Actually, what was happening is something like this:
apr_time_t old, new; apr_exploded_time_t xt; apr_time_now(&old); apr_explode_localtime(&xt, old); apr_implode_time(&new, &xt); if (old != new) it's a bug!
Yeah, but that's what we expect! It's not a bug! If you call apr_time_now() you get GMT! That's the basic problem in your logic - apr will normally work in GMT as that's a sensible (the only really sensible) starting point. What you're saying is
- get GMT time as old - get localtime based on GMT - if gmt != localtime - it's a bug!
Unless you live in gmt_offset = 0 then it'll always be different! In fact most places in the gmt_offset use DST and so it'll be different for half the year anyway!
What I expected was that the apr_time_t coming from apr_implode_time would be UTC, just like the one coming from apr_time_now. That's why I talked about fixing the problem; after all, just making sure that the tm_gmtoff field is always correct after an explode, and accounting for it in an implode, would be enough.
This was what we decided at Santa Clara was the way we'd do the time zones.
When I raised the question there were probably 50% of the active APR folks in the room :)
Hm. Good thing I wasn't there, then. :-)
Why? You don't like participating in discussions? :)
Not at all, but I'd have opposed the decision most loudly. :-)
I think what this boils down to is that the explode/implode implementation is wrong. The result of an implode should always be the same as the original time before the explode, regardless of what we told explode to use as the offset.
Nope. Sorry but in that case we wouldn't need 3 different implementations would we? apr_time_now gives back GMT. apr_explode_localtime does just that - gives localtime. If you want GMT use apr_explode_gmt (as testtime does) and it'll work as you expect.
It's not apr_explode_* I'm ranting about, it's apr_implode_time not giving me an apr_time_t that's UTC, even though it could. I understand now that this is intentional (although I still don't understand why).
I would now like to formally suggest that we consider changing the behaviour of apr_implode_time. It seems to be the only function in APR that can produce an apr_time_t that's not UTC, which is confusing. If that can't be done, i.e., if there is a platform where we truly can't compute tm_gmtoff correctly in the apr_explode_* functions, then we should document that.
Brane
--
Brane Čibej
home: <[EMAIL PROTECTED]> http://www.xbc.nu/brane/
work: <[EMAIL PROTECTED]> http://www.hermes-softlab.com/
ACM: <[EMAIL PROTECTED]> http://www.acm.org/