[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: to brane about Windows builds

From: Nuutti Kotivuori <naked_at_iki.fi>
Date: 2002-06-04 23:52:00 CEST

brane@xbc.nu wrote:
> Quoting Nuutti Kotivuori <naked@iki.fi>:
>> ,----[ #svn ]
>> | 01:42 brane:#svn => time-test is failing on Windows
>> | 01:42 brane:#svn => looks like an apr error again
>> | 01:43 brane:#svn => in this case, gmt_offset doesn't get adjusted
>> | for DST
>> | 01:44 brane:#svn => gmt_offset should be the offset of the
>> | timezone, not the local ime.
>> | 01:45 brane:#svn => so the time test (using the old format, which
>> | is in local time) is off by 1 hour
>> `----
>>
>> Yes, I left that old format there intentionally to be in local time
>> just to catch regression of that timezone handling.
>
> Oh, that's fine. I even looked into having _two_ tests, one with DST
> on and one without.

Well hopefully all that is going to be extinct with the next tarball.

> I only meant to ask if time-test was failing on Unix or not, to
> confirm my theory that it was an APR error.

Nods.

>> But, if only the old format time test fails - and the time
>> read-write invariant works - it's not so bad. The timestamp
>> generated after my patch (rev 2012) are not in local time anymore -
>> and apparently apr seems to work fine for timestamps that are not
>> in local time.
>
> APR is only broken on Windows. And if we can't convert correctly
> between old and new format timestamps (even if it's just a question
> of being off by one hour), then we'll have weird timestamps in the
> working copies.

Well yes, the read-write invariant for apr_time_t is obviously broken
- so what happens is that that an apr_time_t written and read back is
off by one hour each time.

And doing something about it seems a bit difficult - except for just
fixing the damn thing and hoping things go well.

> Anyway, I didn't mean to imply that _you_ had done something
> wrong. I'm just a bit sad because I'd fixed that very code in APR
> before, and somebody went and hosed it again. Maybe the APR tests
> don't tickle this particular problem.

> I may look into it once I finish the delta combiner, but I don't
> have time now. Volunteers welcome. :-)

I took a peek and didn't understand shit about the code.

I'll try a little logic talk here.

First in apr:

  apr_time_exp_t that is in UTC:
    time is gotten as UTC time from system
    tm_isdst and tm_gmtoff are zero

  apr_time_exp_t that is in specified timezone:
    time is gotten as UTC time from system
    time is adjusted for the given offset
    tm_gmtoff is set to given offset
    tm_isdst is zero?

  apr_time_exp_t that is in local time:
    time is gotten as local time from system
    tm_isdst just informs if it's DST or not
    tm_gmtoff just tells the offset to UTC, including current DST

Then in Windows:

  apr_time_t is converted to FILETIME
  FILETIME is converted to SYSTEMTIME
  SYSTEMTIME is converted to apr_time_exp_t
  -> since apr_time_t is in UTC, the generated SYSTEMTIME is in UTC

  apr_time_exp_t that is in UTC:
    apr_time_t is converted to FILETIME
    FILETIME is converted to SYSTEMTIME
    SYSTEMTIME fields are already in UTC, just copy them
    tm_isdst and tm_gmtoff are zero

  apr_time_exp_t that is in specified timezone:
    apr_time_t is converted to FILETIME
    FILETIME is converted to SYSTEMTIME
    SYSTEMTIME fields are in UTC
    time needs to be adjusted for the given offset
    tm_gmtoff set to given offset
    tm_isdst is zero

  apr_time_exp_t that is in local time:
    apr_time_t is converted to FILETIME
    FILETIME is converted to _local_ FILETIME
    FILETIME is converted to SYSTEMTIME
    SYSTEMTIME fields are in _local time_
    tm_gmtoff is calculated from TIME_ZONE_INFORMATION.Bias
    tm_isdst assigned from TIME_ZONE_INFORMATION
* if we are in dst, tm_gmtoff needs to be updated as well for the DST

It would appear that there is only that one problem spot. Everything
else seems to be acting exactly like it should.

Except ofcourse apr_implode_gmt - that seems to assume that tm_isdst
means that the offset needs to be adjusted by one hour - so that needs
to be fixed as well.

Puuh, I'm tired, enough Windows code for me tonight.

> No, if a test that used to succeed starts failing, the reason should
> be analysed every time. It's dangerous to rely on results of
> previous analyses too much.

So obvious when one thinks of it. Thanks.

-- Naked

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jun 4 23:53:54 2002

This is an archived mail posted to the Subversion Dev mailing list.