timezones.phpt test fails with v8 6.7+

phpv8 / v8js

V8 Javascript Engine for PHP — This PHP extension embeds the Google V8 Javascript Engine

http://pecl.php.net/package/v8js

MIT License

1.84k stars 200 forks source link

timezones.phpt test fails with v8 6.7+ #378

Closed sactor closed 2 years ago

sactor commented 6 years ago

Starting with v8 6.7 timezone don't seem to be updated when TZ environment variable is changed.

In addition timezones.phpt is testing for old style toString() in which timezone is abbreviated.

The passing output of the test with v8 6.6 and below is:

Thu Mar 20 2014 11:03:24 GMT+0200 (EET)
Thu Mar 20 2014 05:03:24 GMT-0400 (EDT)
Thu Mar 20 2014 11:03:24 GMT+0200 (EET)

The failing output of the test with v8 6.7 and above is:

Thu Mar 20 2014 11:03:24 GMT+0200 (Eastern European Standard Time)
Thu Mar 20 2014 11:03:24 GMT+0200 (Eastern European Standard Time)
Thu Mar 20 2014 11:03:24 GMT+0200 (Eastern European Standard Time)

sactor commented 6 years ago

I attempted to fix the timezone refresh problem myself by using a similar approach as this: https://github.com/bnoordhuis/io.js/commit/2eb2b970a58af4483e498d2ffa3de80b82ee309e

I couldn't get it to work even with using tzset() before v8::Date::DateTimeConfigurationChangeNotification(c->isolate) in v8js_v8.cc. Does anyone have an idea why this used to work (the test passes with v8 6.6) but now fails? Abbreviation problem is secondary, it's important that it's possible to switch the timezone on the fly.

There are ways to work around the problem, but there is a lot of legacy code that depends on it working like it used to. This is blocking from moving above v8 6.6.

treyhan commented 5 years ago

I'm working on this issue.

small V8 and V8Js modifications are needed to get this fixed
review for the needed V8 modification is open
as soon as it gets merged in, I will create PR for the V8Js modifications

stesie commented 2 years ago

closed via #481