Use correct timezone information on Solaris

Use correct timezone information on Solaris

`timezone` variable contains the difference, in seconds, between UTC and
local standard time (see `man 3 localtime` on Solaris).

Call to `tzset` is required to apply contents of `TZ` variable to
`timezone` variable.

BUG=v8:2064
Committed: https://code.google.com/p/v8/source/detail?r=12802

[Yang, 2012-09-24 07:43:55]

LocalTimeOffset is supposed to take daylight saving time into account. tzset()
seems to set the 'daylight' variable accordingly, but it's not being used in
this CL.

[aleksander.adamowski, 2012-10-13 18:51:31]

On 2012/09/24 07:43:55, Yang wrote:
> LocalTimeOffset is supposed to take daylight saving time into account. tzset()
> seems to set the 'daylight' variable accordingly, but it's not being used in
> this CL.

Looking at date.h, it seems that daylight saving is supposed to be taken into
account somewhat later (somewhat reordered, to visualize the order in which
functions would return - travelling up the call stack):


  virtual int GetLocalOffsetFromOS() {
    double offset = OS::LocalTimeOffset();
    ASSERT(offset < kInvalidLocalOffsetInMs);
    return static_cast<int>(offset);
  }
...
  // ECMA 262 - 15.9.1.7.
  int LocalOffsetInMs() {
    if (local_offset_ms_ == kInvalidLocalOffsetInMs)  {
      local_offset_ms_ = GetLocalOffsetFromOS();
    }
    return local_offset_ms_;
  } 
...
  // ECMA 262 - 15.9.1.9
  int64_t ToLocal(int64_t time_ms) {
    return time_ms + LocalOffsetInMs() + DaylightSavingsOffsetInMs(time_ms);
  } 


My tests on SmartOS are consistent with this (unfortunately, I cannot easily
test it with winter time since this is a virtual hosting instance on Joyent, and
it doesn't allow me to set an arbitrary system date).

With Maciej Małecki's patch, in timezone Europe/Warsaw, I can currently see a
UTC+2 offset, which is correct. Without the patch, I can see a UTC+1 offset,
which is incorrect - I suppose that it only takes daylight saving offset into
account.

I've also slightly modified Maciej's patch and retested:


diff --git a/deps/v8/src/platform-solaris.cc b/deps/v8/src/platform-solaris.cc
index 07718fe..3fa64c2 100644
--- a/deps/v8/src/platform-solaris.cc
+++ b/deps/v8/src/platform-solaris.cc
@@ -126,7 +126,7 @@ const char* OS::LocalTimezone(double time) {
 
 double OS::LocalTimeOffset() {
   tzset();
-  return -static_cast<double>(timezone * msPerSecond);
+  return -static_cast<double>((daylight ? altzone : timezone) * msPerSecond);
 }


The altzone variable on Solaris returns the composite offset - timezone and DST
combined. With this patch, I can see an offset of UTC+3 in Europe/Warsaw, which
is clearly incorrect and should never occur (as you can check on
http://www.timeanddate.com/worldclock/timezone.html?n=262). That's why I suppose
that DST offset is already handled correctly on Solaris and there's no need to
incorporate it in this part of code.

[aleksander.adamowski, 2012-10-13 18:55:28]

I'm downloading a SmartOS ISO image so that I can test my theory with arbutrary
dates, but my test results suggest that if we account for DST offset in
OS::LocalTimeOffset(), it will be taken into account twice, thus exaggerating it
by a factor of two - hence the UTC+3 offset in Europe/Warsaw summer time.

[aleksander.adamowski, 2012-10-15 22:31:21]

Some more test results - they suggest that all is OK with Maciej Malecki's
patch. I've chosen 2 dates - the first during summer time (2012-08-23 08:40:00)
which should exhibit daylight saving correction (UTC+2), and the second during
winter time (2012-10-30 08:25:00) which should not exhibit daylight saving
(UTC+1).

I'm using 2 Node.JS binaries: /opt/nodejs/v0.6/bin/node is the old release
without the patch, while /home/node/src/GIT/node.git/node is built from sources
with Maciej's patch applied.

Here's the log from tests I've performed. As can be seen, the version with
Maciej's patch behaves correctly for both dates (with DST and no DST). It can
also be seen that my previous hypothesis has been correct - the unpatched
version already applies daylight saving offset, but it fails to apply the
timezone offset:


####### The date is now: #######
15 października 2012 21:33:29 UTC
Mon, 15 Oct 2012 21:33:29 +0000
2012-10-15 21:33:29+00:00
################################
======== Binary /opt/nodejs/v0.6/bin/node without TZ in env ========
hex timestamp: 13952a35700
process.env.TZ: undefined
comb formatted local: 2012-08-23 08:40:00
toString(): Thu Aug 23 2012 08:40:00 GMT+0000 (UTC)
getTimezoneOffset(): 0
JSON: "2012-08-23T08:40:00.000Z"
======== Binary /opt/nodejs/v0.6/bin/node with TZ=Europe/Warsaw in env ========
hex timestamp: 13952a35700
process.env.TZ: Europe/Warsaw
comb formatted local: 2012-08-23 09:40:00
toString(): Thu Aug 23 2012 09:40:00 GMT+0100 (CET)
getTimezoneOffset(): -60
JSON: "2012-08-23T08:40:00.000Z"
======== Binary /home/node/src/GIT/node.git/node without TZ in env ========
hex timestamp: 13952a35700
process.env.TZ: undefined
comb formatted local: 2012-08-23 08:40:00
toString(): Thu Aug 23 2012 08:40:00 GMT+0000 (UTC)
getTimezoneOffset(): 0
JSON: "2012-08-23T08:40:00.000Z"
======== Binary /home/node/src/GIT/node.git/node with TZ=Europe/Warsaw in env
========
hex timestamp: 13952a35700
process.env.TZ: Europe/Warsaw
comb formatted local: 2012-08-23 10:40:00
toString(): Thu Aug 23 2012 10:40:00 GMT+0200 (CET)
getTimezoneOffset(): -120
JSON: "2012-08-23T08:40:00.000Z"
======== Binary /opt/nodejs/v0.6/bin/node without TZ in env ========
hex timestamp: 13ab0c60b60
process.env.TZ: undefined
comb formatted local: 2012-10-30 08:25:00
toString(): Tue Oct 30 2012 08:25:00 GMT+0000 (UTC)
getTimezoneOffset(): 0
JSON: "2012-10-30T08:25:00.000Z"
======== Binary /opt/nodejs/v0.6/bin/node with TZ=Europe/Warsaw in env ========
hex timestamp: 13ab0c60b60
process.env.TZ: Europe/Warsaw
comb formatted local: 2012-10-30 08:25:00
toString(): Tue Oct 30 2012 08:25:00 GMT+0000 (CET)
getTimezoneOffset(): 0
JSON: "2012-10-30T08:25:00.000Z"
======== Binary /home/node/src/GIT/node.git/node without TZ in env ========
hex timestamp: 13ab0c60b60
process.env.TZ: undefined
comb formatted local: 2012-10-30 08:25:00
toString(): Tue Oct 30 2012 08:25:00 GMT+0000 (UTC)
getTimezoneOffset(): 0
JSON: "2012-10-30T08:25:00.000Z"
======== Binary /home/node/src/GIT/node.git/node with TZ=Europe/Warsaw in env
========
hex timestamp: 13ab0c60b60
process.env.TZ: Europe/Warsaw
comb formatted local: 2012-10-30 09:25:00
toString(): Tue Oct 30 2012 09:25:00 GMT+0100 (CET)
getTimezoneOffset(): -60
JSON: "2012-10-30T08:25:00.000Z"


So Maciej's patch LGTM and I vote for commiting it to the source repository.

[Sven Panne, 2012-10-16 06:40:11]

Some general drive-by comments: The whole time zone/DST code seems to be a mess
for several reasons:

   * tzset()/_tzset() changes the global state of the whole application, not
only v8's state, so it gives me a very uneasy feeling. Why do we use it only on
some platforms? If we really need it somewhere, we should use it consistently on
all platforms at the same places.

   * Handling the time zone in one place and handling DST somewhere else looks
wrong, unless the spec somehow forces us to do it this confusing way.

[aleksander.adamowski, 2012-10-16 07:41:25]

On 2012/10/16 06:40:11, Sven Panne wrote:
>    * Handling the time zone in one place and handling DST somewhere else looks
> wrong, unless the spec somehow forces us to do it this confusing way.

I think that this has been decomposed into two operations precisely because of
code quality/single responsibility and deduplication.

On a given platform, you can imagine having to use platform-specific method of
getting the time zone offset, but standard POSIX approach for getting the DST
offset.

[Yang, 2012-10-23 13:03:57]

On 2012/10/16 07:41:25, aleksander.adamowski wrote:
> On 2012/10/16 06:40:11, Sven Panne wrote:
> >    * Handling the time zone in one place and handling DST somewhere else
looks
> > wrong, unless the spec somehow forces us to do it this confusing way.
> 
> I think that this has been decomposed into two operations precisely because of
> code quality/single responsibility and deduplication.
> 
> On a given platform, you can imagine having to use platform-specific method of
> getting the time zone offset, but standard POSIX approach for getting the DST
> offset.

LGTM. Landing...