Skip to content

Fix getTimezoneOffset() when tm_gmtoff is not available #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Dec 24, 2023

Conversation

TooTallNate
Copy link
Contributor

In my environment, the build fails because tm_gmtoff is not available:

quickjs.c: In function 'getTimezoneOffset':
quickjs.c:40527:15: error: 'struct tm' has no member named 'tm_gmtoff'
40527 |     return -tm.tm_gmtoff / 60;
      |               ^

This approach attempts to reimplement the getTimezoneOffset() to not rely on that property. This change works for my build environment, but I am open to any suggestions for improvement.

I didn't test, but this might also work on Windows where this logic is currently not implemented.

In my environment, the build fails because `tm_gmtoff` is not available:

```
quickjs.c: In function 'getTimezoneOffset':
quickjs.c:40527:15: error: 'struct tm' has no member named 'tm_gmtoff'
40527 |     return -tm.tm_gmtoff / 60;
      |               ^
```

This approach attempts to reimplement the `getTimezoneOffset()` to not
rely on that property. This change works for my build environment, but
I am open to any suggestions for improvement.

I didn't test, but this might also work on Windows where this logic is
currently not implemented.
@TooTallNate
Copy link
Contributor Author

@bnoordhuis Please take another look. I get the same results now in qjs and Node.js:

if (typeof print === 'undefined') global.print = console.log;
const a = new Date('2023-06-19');
const b = new Date('2023-12-19');
print(a.toString());
print(a.getTimezoneOffset());
print(b.toString());
print(b.getTimezoneOffset());
$ node d.js 
Sun Jun 18 2023 17:00:00 GMT-0700 (Pacific Daylight Time)
420
Mon Dec 18 2023 16:00:00 GMT-0800 (Pacific Standard Time)
480

$ ./build/qjs d.js
Sun Jun 18 2023 17:00:00 GMT-0700
420
Mon Dec 18 2023 16:00:00 GMT-0800
480

@bnoordhuis
Copy link
Contributor

The patch itself LGTM. But.

There are a bunch of TZ-related tests in test262/implementation-contributed that aren't run as part of the CI. When I run them manually, the results become significantly worse with this change.

$ cat assertEquals.js
function assertEquals(a, b) {
    if (String(a) !== String(b)) print(`${a} !== ${b}`)
}
$ cat test.sh
make -s -C build
for FILE in test262/implementation-contributed/v8/mjsunit/tzoffset-*; do
    export TZ=`perl -ne 'print "$1\n" if /Environment Variables: TZ=(.+)/' $FILE`
    ./build/qjs -I assertEquals.js $FILE;
done | wc -l
$ bash test.sh # HEAD
65
$ bash test.sh # this PR
109

As a diff:

--- before.txt	2023-12-19 13:18:54.411810798 +0100
+++ after.txt	2023-12-19 13:19:05.075402301 +0100
@@ -1,44 +1,93 @@
 Sat Sep 24 2011 04:00:00 GMT-1000 !== Sat Sep 24 2011 05:00:00 GMT-1000
 Sat Sep 24 2011 04:30:00 GMT-1000 !== Sat Sep 24 2011 05:30:00 GMT-1000
-Sat Dec 31 2011 10:00:00 GMT+1400 !== Thu Dec 29 2011 10:00:00 GMT-1000
-Sat Dec 31 2011 10:30:00 GMT+1400 !== Thu Dec 29 2011 10:30:00 GMT-1000
-Sat Dec 31 2011 11:00:00 GMT+1400 !== Thu Dec 29 2011 11:00:00 GMT-1000
-Sat Dec 31 2011 11:30:00 GMT+1400 !== Thu Dec 29 2011 11:30:00 GMT-1000
-Sat Dec 31 2011 12:00:00 GMT+1400 !== Thu Dec 29 2011 12:00:00 GMT-1000
-Sat Dec 31 2011 12:30:00 GMT+1400 !== Thu Dec 29 2011 12:30:00 GMT-1000
-Sat Dec 31 2011 13:00:00 GMT+1400 !== Thu Dec 29 2011 13:00:00 GMT-1000
-Sat Dec 31 2011 13:30:00 GMT+1400 !== Thu Dec 29 2011 13:30:00 GMT-1000
-Sat Dec 31 2011 14:00:00 GMT+1400 !== Thu Dec 29 2011 14:00:00 GMT-1000
-Sat Dec 31 2011 14:30:00 GMT+1400 !== Thu Dec 29 2011 14:30:00 GMT-1000
-Sat Dec 31 2011 15:00:00 GMT+1400 !== Thu Dec 29 2011 15:00:00 GMT-1000
-Sat Dec 31 2011 15:30:00 GMT+1400 !== Thu Dec 29 2011 15:30:00 GMT-1000
-Sat Dec 31 2011 16:00:00 GMT+1400 !== Thu Dec 29 2011 16:00:00 GMT-1000
-Sat Dec 31 2011 16:30:00 GMT+1400 !== Thu Dec 29 2011 16:30:00 GMT-1000
-Sat Dec 31 2011 17:00:00 GMT+1400 !== Thu Dec 29 2011 17:00:00 GMT-1000
-Sat Dec 31 2011 17:30:00 GMT+1400 !== Thu Dec 29 2011 17:30:00 GMT-1000
-Sat Dec 31 2011 18:00:00 GMT+1400 !== Thu Dec 29 2011 18:00:00 GMT-1000
-Sat Dec 31 2011 18:30:00 GMT+1400 !== Thu Dec 29 2011 18:30:00 GMT-1000
-Sat Dec 31 2011 19:00:00 GMT+1400 !== Thu Dec 29 2011 19:00:00 GMT-1000
-Sat Dec 31 2011 19:30:00 GMT+1400 !== Thu Dec 29 2011 19:30:00 GMT-1000
-Sat Dec 31 2011 20:00:00 GMT+1400 !== Thu Dec 29 2011 20:00:00 GMT-1000
-Sat Dec 31 2011 20:30:00 GMT+1400 !== Thu Dec 29 2011 20:30:00 GMT-1000
-Sat Dec 31 2011 21:00:00 GMT+1400 !== Thu Dec 29 2011 21:00:00 GMT-1000
-Sat Dec 31 2011 21:30:00 GMT+1400 !== Thu Dec 29 2011 21:30:00 GMT-1000
-Sat Dec 31 2011 22:00:00 GMT+1400 !== Thu Dec 29 2011 22:00:00 GMT-1000
-Sat Dec 31 2011 22:30:00 GMT+1400 !== Thu Dec 29 2011 22:30:00 GMT-1000
-Sat Dec 31 2011 23:00:00 GMT+1400 !== Thu Dec 29 2011 23:00:00 GMT-1000
-Sat Dec 31 2011 23:30:00 GMT+1400 !== Thu Dec 29 2011 23:30:00 GMT-1000
+Sat Dec 31 2011 01:00:00 GMT+1500 !== Wed Dec 28 2011 23:00:00 GMT-1000
+Sat Dec 31 2011 01:30:00 GMT+1500 !== Wed Dec 28 2011 23:30:00 GMT-1000
+Sat Dec 31 2011 01:00:00 GMT+1500 !== Sat Dec 31 2011 00:00:00 GMT-1000
+Sat Dec 31 2011 01:30:00 GMT+1500 !== Sat Dec 31 2011 00:30:00 GMT-1000
+Sat Dec 31 2011 02:00:00 GMT+1500 !== Thu Dec 29 2011 00:00:00 GMT-1000
+Sat Dec 31 2011 02:30:00 GMT+1500 !== Thu Dec 29 2011 00:30:00 GMT-1000
+Sat Dec 31 2011 02:00:00 GMT+1500 !== Sat Dec 31 2011 01:00:00 GMT-1000
+Sat Dec 31 2011 02:30:00 GMT+1500 !== Sat Dec 31 2011 01:30:00 GMT-1000
+Sat Dec 31 2011 03:00:00 GMT+1500 !== Thu Dec 29 2011 01:00:00 GMT-1000
+Sat Dec 31 2011 03:30:00 GMT+1500 !== Thu Dec 29 2011 01:30:00 GMT-1000
+Sat Dec 31 2011 03:00:00 GMT+1500 !== Sat Dec 31 2011 02:00:00 GMT-1000
+Sat Dec 31 2011 03:30:00 GMT+1500 !== Sat Dec 31 2011 02:30:00 GMT-1000
+Sat Dec 31 2011 04:00:00 GMT+1500 !== Thu Dec 29 2011 02:00:00 GMT-1000
+Sat Dec 31 2011 04:30:00 GMT+1500 !== Thu Dec 29 2011 02:30:00 GMT-1000
+Sat Dec 31 2011 04:00:00 GMT+1500 !== Sat Dec 31 2011 03:00:00 GMT-1000
+Sat Dec 31 2011 04:30:00 GMT+1500 !== Sat Dec 31 2011 03:30:00 GMT-1000
+Sat Dec 31 2011 05:00:00 GMT+1500 !== Thu Dec 29 2011 03:00:00 GMT-1000
+Sat Dec 31 2011 05:30:00 GMT+1500 !== Thu Dec 29 2011 03:30:00 GMT-1000
+Fri Dec 30 2011 05:00:00 GMT-0900 !== Sun Jan 01 2012 04:00:00 GMT+1400
+Fri Dec 30 2011 05:30:00 GMT-0900 !== Sun Jan 01 2012 04:30:00 GMT+1400
+Fri Dec 30 2011 06:00:00 GMT-0900 !== Fri Dec 30 2011 05:00:00 GMT-0900
+Fri Dec 30 2011 06:30:00 GMT-0900 !== Fri Dec 30 2011 05:30:00 GMT-0900
+Fri Dec 30 2011 06:00:00 GMT-0900 !== Sun Jan 01 2012 05:00:00 GMT+1400
+Fri Dec 30 2011 06:30:00 GMT-0900 !== Sun Jan 01 2012 05:30:00 GMT+1400
+Fri Dec 30 2011 07:00:00 GMT-0900 !== Fri Dec 30 2011 06:00:00 GMT-0900
+Fri Dec 30 2011 07:30:00 GMT-0900 !== Fri Dec 30 2011 06:30:00 GMT-0900
+Fri Dec 30 2011 07:00:00 GMT-0900 !== Sun Jan 01 2012 06:00:00 GMT+1400
+Fri Dec 30 2011 07:30:00 GMT-0900 !== Sun Jan 01 2012 06:30:00 GMT+1400
+Fri Dec 30 2011 08:00:00 GMT-0900 !== Fri Dec 30 2011 07:00:00 GMT-0900
+Fri Dec 30 2011 08:30:00 GMT-0900 !== Fri Dec 30 2011 07:30:00 GMT-0900
+Fri Dec 30 2011 08:00:00 GMT-0900 !== Sun Jan 01 2012 07:00:00 GMT+1400
+Fri Dec 30 2011 08:30:00 GMT-0900 !== Sun Jan 01 2012 07:30:00 GMT+1400
+Fri Dec 30 2011 09:00:00 GMT-0900 !== Fri Dec 30 2011 08:00:00 GMT-0900
+Fri Dec 30 2011 09:30:00 GMT-0900 !== Fri Dec 30 2011 08:30:00 GMT-0900
+Fri Dec 30 2011 09:00:00 GMT-0900 !== Sun Jan 01 2012 08:00:00 GMT+1400
+Fri Dec 30 2011 09:30:00 GMT-0900 !== Sun Jan 01 2012 08:30:00 GMT+1400
+Fri Dec 30 2011 10:00:00 GMT-0900 !== Fri Dec 30 2011 09:00:00 GMT-0900
+Fri Dec 30 2011 10:30:00 GMT-0900 !== Fri Dec 30 2011 09:30:00 GMT-0900
+Fri Dec 30 2011 10:00:00 GMT-0900 !== Sun Jan 01 2012 09:00:00 GMT+1400
+Fri Dec 30 2011 10:30:00 GMT-0900 !== Sun Jan 01 2012 09:30:00 GMT+1400
+Fri Dec 30 2011 11:00:00 GMT-0900 !== Fri Dec 30 2011 10:00:00 GMT-0900
+Fri Dec 30 2011 11:30:00 GMT-0900 !== Fri Dec 30 2011 10:30:00 GMT-0900
+Fri Dec 30 2011 11:00:00 GMT-0900 !== Sun Jan 01 2012 10:00:00 GMT+1400
+Fri Dec 30 2011 11:30:00 GMT-0900 !== Sun Jan 01 2012 10:30:00 GMT+1400
+Fri Dec 30 2011 12:00:00 GMT-0900 !== Fri Dec 30 2011 11:00:00 GMT-0900
+Fri Dec 30 2011 12:30:00 GMT-0900 !== Fri Dec 30 2011 11:30:00 GMT-0900
+Fri Dec 30 2011 12:00:00 GMT-0900 !== Sun Jan 01 2012 11:00:00 GMT+1400
+Fri Dec 30 2011 12:30:00 GMT-0900 !== Sun Jan 01 2012 11:30:00 GMT+1400
+Fri Dec 30 2011 13:00:00 GMT-0900 !== Fri Dec 30 2011 12:00:00 GMT-0900
+Fri Dec 30 2011 13:30:00 GMT-0900 !== Fri Dec 30 2011 12:30:00 GMT-0900
+Fri Dec 30 2011 13:00:00 GMT-0900 !== Sun Jan 01 2012 12:00:00 GMT+1400
+Fri Dec 30 2011 13:30:00 GMT-0900 !== Sun Jan 01 2012 12:30:00 GMT+1400
+Fri Dec 30 2011 14:00:00 GMT-0900 !== Fri Dec 30 2011 13:00:00 GMT-0900
+Fri Dec 30 2011 14:30:00 GMT-0900 !== Fri Dec 30 2011 13:30:00 GMT-0900
+Sat Dec 31 2011 14:00:00 GMT+1500 !== Sun Jan 01 2012 13:00:00 GMT+1400
+Sat Dec 31 2011 14:30:00 GMT+1500 !== Sun Jan 01 2012 13:30:00 GMT+1400
+Fri Dec 30 2011 14:00:00 GMT-1000 !== Sat Dec 31 2011 14:00:00 GMT+1500
+Fri Dec 30 2011 14:30:00 GMT-1000 !== Thu Dec 29 2011 13:30:00 GMT-1000
+Fri Dec 30 2011 15:00:00 GMT-1000 !== Fri Dec 30 2011 15:00:00 GMT+1500
+Fri Dec 30 2011 15:30:00 GMT-1000 !== Fri Dec 30 2011 15:30:00 GMT+1500
+Fri Dec 30 2011 16:00:00 GMT-1000 !== Fri Dec 30 2011 16:00:00 GMT+1500
+Fri Dec 30 2011 16:30:00 GMT-1000 !== Fri Dec 30 2011 16:30:00 GMT+1500
+Fri Dec 30 2011 17:00:00 GMT-1000 !== Fri Dec 30 2011 17:00:00 GMT+1500
+Fri Dec 30 2011 17:30:00 GMT-1000 !== Fri Dec 30 2011 17:30:00 GMT+1500
+Fri Dec 30 2011 18:00:00 GMT-1000 !== Fri Dec 30 2011 18:00:00 GMT+1500
+Fri Dec 30 2011 18:30:00 GMT-1000 !== Fri Dec 30 2011 18:30:00 GMT+1500
+Fri Dec 30 2011 19:00:00 GMT-1000 !== Fri Dec 30 2011 19:00:00 GMT+1500
+Fri Dec 30 2011 19:30:00 GMT-1000 !== Fri Dec 30 2011 19:30:00 GMT+1500
+Fri Dec 30 2011 20:00:00 GMT-1000 !== Fri Dec 30 2011 20:00:00 GMT+1500
+Fri Dec 30 2011 20:30:00 GMT-1000 !== Fri Dec 30 2011 20:30:00 GMT+1500
+Fri Dec 30 2011 21:00:00 GMT-1000 !== Fri Dec 30 2011 21:00:00 GMT+1500
+Fri Dec 30 2011 21:30:00 GMT-1000 !== Fri Dec 30 2011 21:30:00 GMT+1500
+Fri Dec 30 2011 22:00:00 GMT-1000 !== Fri Dec 30 2011 22:00:00 GMT+1500
+Fri Dec 30 2011 22:30:00 GMT-1000 !== Fri Dec 30 2011 22:30:00 GMT+1500
+Fri Dec 30 2011 23:00:00 GMT-1000 !== Fri Dec 30 2011 23:00:00 GMT+1500
+Fri Dec 30 2011 23:30:00 GMT-1000 !== Fri Dec 30 2011 23:30:00 GMT+1500
 Sun Apr 01 2012 03:00:00 GMT+1400 !== Sun Apr 01 2012 03:00:00 GMT+1300
 Sun Apr 01 2012 03:30:00 GMT+1400 !== Sun Apr 01 2012 03:30:00 GMT+1300
 Sun Apr 01 2012 03:59:00 GMT+1400 !== Sun Apr 01 2012 03:59:00 GMT+1300
-Sun Apr 02 2017 01:29:00 GMT+1100 !== Sun Apr 02 2017 01:59:00 GMT+1100
-Sun Apr 02 2017 01:30:00 GMT+1100 !== Sun Apr 02 2017 01:30:00 GMT+1030
-Sun Apr 02 2017 01:45:00 GMT+1100 !== Sun Apr 02 2017 01:45:00 GMT+1030
-Sun Apr 02 2017 01:59:00 GMT+1100 !== Sun Apr 02 2017 01:59:00 GMT+1030
-Sun Oct 01 2017 01:59:00 GMT+1030 !== Sun Oct 01 2017 01:29:00 GMT+1030
-Sun Oct 01 2017 02:30:00 GMT+1100 !== Sun Oct 01 2017 01:30:00 GMT+1030
-Sun Oct 01 2017 02:45:00 GMT+1100 !== Sun Oct 01 2017 01:45:00 GMT+1030
--660 !== -630
+Sun Apr 02 2017 01:59:00 GMT+1130 !== Sun Apr 02 2017 02:29:00 GMT+1130
+Sun Apr 02 2017 02:00:00 GMT+1130 !== Sun Apr 02 2017 01:30:00 GMT+1030
+Sun Apr 02 2017 02:15:00 GMT+1130 !== Sun Apr 02 2017 01:45:00 GMT+1030
+Sun Apr 02 2017 02:29:00 GMT+1130 !== Sun Apr 02 2017 01:59:00 GMT+1030
+Sun Oct 01 2017 01:59:00 GMT+1030 !== Sun Oct 01 2017 00:59:00 GMT+1030
+Sun Oct 01 2017 03:00:00 GMT+1130 !== Sun Oct 01 2017 01:00:00 GMT+1030
+Sun Oct 01 2017 03:15:00 GMT+1130 !== Sun Oct 01 2017 01:15:00 GMT+1030
+Sun Oct 01 2017 03:00:00 GMT+1130 !== Sun Oct 01 2017 01:30:00 GMT+1030
+Sun Oct 01 2017 03:15:00 GMT+1130 !== Sun Oct 01 2017 01:45:00 GMT+1030
 Sun Mar 28 2010 01:59:00 GMT+0300 !== Sun Mar 28 2010 00:59:00 GMT+0300
 Sun Mar 28 2010 03:00:00 GMT+0400 !== Sun Mar 28 2010 01:00:00 GMT+0300
 Sun Mar 28 2010 03:30:00 GMT+0400 !== Sun Mar 28 2010 01:30:00 GMT+0300
@@ -47,14 +96,9 @@
 Sun Oct 31 2010 02:00:00 GMT+0400 !== Sun Oct 31 2010 02:00:00 GMT+0300
 Sun Oct 31 2010 02:30:00 GMT+0400 !== Sun Oct 31 2010 02:30:00 GMT+0300
 Sun Oct 31 2010 02:59:00 GMT+0400 !== Sun Oct 31 2010 02:59:00 GMT+0300
-Sun Mar 27 2011 01:59:00 GMT+0300 !== Sun Mar 27 2011 00:59:00 GMT+0300
-Sun Mar 27 2011 03:00:00 GMT+0400 !== Sun Mar 27 2011 01:00:00 GMT+0300
-Sun Mar 27 2011 03:30:00 GMT+0400 !== Sun Mar 27 2011 01:30:00 GMT+0300
--240 !== -180
-Sun Oct 26 2014 00:59:00 GMT+0400 !== Sun Oct 26 2014 01:59:00 GMT+0400
-Sun Oct 26 2014 01:00:00 GMT+0400 !== Sun Oct 26 2014 01:00:00 GMT+0300
-Sun Oct 26 2014 01:30:00 GMT+0400 !== Sun Oct 26 2014 01:30:00 GMT+0300
-Sun Oct 26 2014 01:59:00 GMT+0400 !== Sun Oct 26 2014 01:59:00 GMT+0300
+Sun Mar 27 2011 02:00:00 GMT+0300 !== Tue Mar 29 2011 09:04:50 GMT+0400
+Sun Mar 27 2011 02:30:00 GMT+0300 !== Tue Mar 29 2011 09:04:50 GMT+0400
+-180 !== -240
 Sun Mar 12 2017 03:00:00 GMT-0400 !== Sun Mar 12 2017 04:00:00 GMT-0400
 Sun Mar 12 2017 03:30:00 GMT-0400 !== Sun Mar 12 2017 04:30:00 GMT-0400
 Sun Nov 05 2017 02:00:00 GMT-0500 !== Sun Nov 05 2017 01:00:00 GMT-0500

@TooTallNate
Copy link
Contributor Author

TooTallNate commented Dec 21, 2023

Ok, I switched to a different approach yet again. Using CMake to check if tm_gmtoff exists and add a define when it does, so the same existing logic will be used for that case. Now the tests have the same result when tm_gmtoff exists:

$ ./test.sh # `tm_gmtoff` exists
65

I also removed the manual one hour timezone adjustment in favor of relying on difftime() to do the right thing, which gives better results than the 109 failures before:

$ ./test.sh # `tm_gmtoff` does not exist
82

Let me know if anything else is needed.

@saghul
Copy link
Contributor

saghul commented Dec 21, 2023

What are those new 17 failures related to?

@saghul
Copy link
Contributor

saghul commented Dec 21, 2023

I'm not a fan of adding this config.h file because it adds one extra step to those who just want to grab the bunch of files and add them to an existing project.

@TooTallNate
Copy link
Contributor Author

TooTallNate commented Dec 21, 2023 via email

@bnoordhuis
Copy link
Contributor

A define works for me.

Quick question, something I forgot to ask earlier: are you sure tm_gmtoff isn't hidden behind e.g. _POSIX_C_SOURCE/_BSD_SOURCE/_GNU_SOURCE/etc.?

@TooTallNate
Copy link
Contributor Author

The struct tm that newlib has is here: https://github.com/devkitPro/newlib/blob/master/newlib/libc/include/time.h#L37-L54

There is a __TM_GMTOFF define, but based on the commit, it seems like it's only active for Cygwin? Or at least it's not clear to me how to use it. I'm guessing the toolchain was built without that defined.

@saghul
Copy link
Contributor

saghul commented Dec 21, 2023

Weird, it does look cygwin only...

This looks like an implementation that could work: https://github.com/util-linux/util-linux/blob/8a7c8803d60f5131b123fd17d9dc422f2f1dcffd/lib/timeutils.c#L416 If it wasn't GPL :-/

@TooTallNate
Copy link
Contributor Author

TooTallNate commented Dec 21, 2023

What are those new 17 failures related to?

With tm_gmtoff
test262/implementation-contributed/v8/mjsunit/tzoffset-seoul-noi18n.js

test262/implementation-contributed/v8/mjsunit/tzoffset-seoul.js

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-apia.js
Sat Sep 24 2011 04:00:00 GMT-1000 !== Sat Sep 24 2011 05:00:00 GMT-1000
Sat Sep 24 2011 04:30:00 GMT-1000 !== Sat Sep 24 2011 05:30:00 GMT-1000
Sat Dec 31 2011 10:00:00 GMT+1400 !== Thu Dec 29 2011 10:00:00 GMT-1000
Sat Dec 31 2011 10:30:00 GMT+1400 !== Thu Dec 29 2011 10:30:00 GMT-1000
Sat Dec 31 2011 11:00:00 GMT+1400 !== Thu Dec 29 2011 11:00:00 GMT-1000
Sat Dec 31 2011 11:30:00 GMT+1400 !== Thu Dec 29 2011 11:30:00 GMT-1000
Sat Dec 31 2011 12:00:00 GMT+1400 !== Thu Dec 29 2011 12:00:00 GMT-1000
Sat Dec 31 2011 12:30:00 GMT+1400 !== Thu Dec 29 2011 12:30:00 GMT-1000
Sat Dec 31 2011 13:00:00 GMT+1400 !== Thu Dec 29 2011 13:00:00 GMT-1000
Sat Dec 31 2011 13:30:00 GMT+1400 !== Thu Dec 29 2011 13:30:00 GMT-1000
Sat Dec 31 2011 14:00:00 GMT+1400 !== Thu Dec 29 2011 14:00:00 GMT-1000
Sat Dec 31 2011 14:30:00 GMT+1400 !== Thu Dec 29 2011 14:30:00 GMT-1000
Sat Dec 31 2011 15:00:00 GMT+1400 !== Thu Dec 29 2011 15:00:00 GMT-1000
Sat Dec 31 2011 15:30:00 GMT+1400 !== Thu Dec 29 2011 15:30:00 GMT-1000
Sat Dec 31 2011 16:00:00 GMT+1400 !== Thu Dec 29 2011 16:00:00 GMT-1000
Sat Dec 31 2011 16:30:00 GMT+1400 !== Thu Dec 29 2011 16:30:00 GMT-1000
Sat Dec 31 2011 17:00:00 GMT+1400 !== Thu Dec 29 2011 17:00:00 GMT-1000
Sat Dec 31 2011 17:30:00 GMT+1400 !== Thu Dec 29 2011 17:30:00 GMT-1000
Sat Dec 31 2011 18:00:00 GMT+1400 !== Thu Dec 29 2011 18:00:00 GMT-1000
Sat Dec 31 2011 18:30:00 GMT+1400 !== Thu Dec 29 2011 18:30:00 GMT-1000
Sat Dec 31 2011 19:00:00 GMT+1400 !== Thu Dec 29 2011 19:00:00 GMT-1000
Sat Dec 31 2011 19:30:00 GMT+1400 !== Thu Dec 29 2011 19:30:00 GMT-1000
Sat Dec 31 2011 20:00:00 GMT+1400 !== Thu Dec 29 2011 20:00:00 GMT-1000
Sat Dec 31 2011 20:30:00 GMT+1400 !== Thu Dec 29 2011 20:30:00 GMT-1000
Sat Dec 31 2011 21:00:00 GMT+1400 !== Thu Dec 29 2011 21:00:00 GMT-1000
Sat Dec 31 2011 21:30:00 GMT+1400 !== Thu Dec 29 2011 21:30:00 GMT-1000
Sat Dec 31 2011 22:00:00 GMT+1400 !== Thu Dec 29 2011 22:00:00 GMT-1000
Sat Dec 31 2011 22:30:00 GMT+1400 !== Thu Dec 29 2011 22:30:00 GMT-1000
Sat Dec 31 2011 23:00:00 GMT+1400 !== Thu Dec 29 2011 23:00:00 GMT-1000
Sat Dec 31 2011 23:30:00 GMT+1400 !== Thu Dec 29 2011 23:30:00 GMT-1000
Sun Apr 01 2012 03:00:00 GMT+1400 !== Sun Apr 01 2012 03:00:00 GMT+1300
Sun Apr 01 2012 03:30:00 GMT+1400 !== Sun Apr 01 2012 03:30:00 GMT+1300
Sun Apr 01 2012 03:59:00 GMT+1400 !== Sun Apr 01 2012 03:59:00 GMT+1300

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-lord-howe.js
Sun Apr 02 2017 01:29:00 GMT+1100 !== Sun Apr 02 2017 01:59:00 GMT+1100
Sun Apr 02 2017 01:30:00 GMT+1100 !== Sun Apr 02 2017 01:30:00 GMT+1030
Sun Apr 02 2017 01:45:00 GMT+1100 !== Sun Apr 02 2017 01:45:00 GMT+1030
Sun Apr 02 2017 01:59:00 GMT+1100 !== Sun Apr 02 2017 01:59:00 GMT+1030
Sun Oct 01 2017 01:59:00 GMT+1030 !== Sun Oct 01 2017 01:29:00 GMT+1030
Sun Oct 01 2017 02:30:00 GMT+1100 !== Sun Oct 01 2017 01:30:00 GMT+1030
Sun Oct 01 2017 02:45:00 GMT+1100 !== Sun Oct 01 2017 01:45:00 GMT+1030
-660 !== -630

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-moscow.js
Sun Mar 28 2010 01:59:00 GMT+0300 !== Sun Mar 28 2010 00:59:00 GMT+0300
Sun Mar 28 2010 03:00:00 GMT+0400 !== Sun Mar 28 2010 01:00:00 GMT+0300
Sun Mar 28 2010 03:30:00 GMT+0400 !== Sun Mar 28 2010 01:30:00 GMT+0300
-240 !== -180
Sun Oct 31 2010 01:59:00 GMT+0400 !== Sun Oct 31 2010 02:59:00 GMT+0400
Sun Oct 31 2010 02:00:00 GMT+0400 !== Sun Oct 31 2010 02:00:00 GMT+0300
Sun Oct 31 2010 02:30:00 GMT+0400 !== Sun Oct 31 2010 02:30:00 GMT+0300
Sun Oct 31 2010 02:59:00 GMT+0400 !== Sun Oct 31 2010 02:59:00 GMT+0300
Sun Mar 27 2011 01:59:00 GMT+0300 !== Sun Mar 27 2011 00:59:00 GMT+0300
Sun Mar 27 2011 03:00:00 GMT+0400 !== Sun Mar 27 2011 01:00:00 GMT+0300
Sun Mar 27 2011 03:30:00 GMT+0400 !== Sun Mar 27 2011 01:30:00 GMT+0300
-240 !== -180
Sun Oct 26 2014 00:59:00 GMT+0400 !== Sun Oct 26 2014 01:59:00 GMT+0400
Sun Oct 26 2014 01:00:00 GMT+0400 !== Sun Oct 26 2014 01:00:00 GMT+0300
Sun Oct 26 2014 01:30:00 GMT+0400 !== Sun Oct 26 2014 01:30:00 GMT+0300
Sun Oct 26 2014 01:59:00 GMT+0400 !== Sun Oct 26 2014 01:59:00 GMT+0300

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-new-york-noi18n.js
Sun Mar 12 2017 03:00:00 GMT-0400 !== Sun Mar 12 2017 04:00:00 GMT-0400
Sun Mar 12 2017 03:30:00 GMT-0400 !== Sun Mar 12 2017 04:30:00 GMT-0400
Sun Nov 05 2017 02:00:00 GMT-0500 !== Sun Nov 05 2017 01:00:00 GMT-0500
Sun Nov 05 2017 03:00:00 GMT-0500 !== Sun Nov 05 2017 02:00:00 GMT-0500

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-new-york.js
Sun Mar 12 2017 03:00:00 GMT-0400 !== Sun Mar 12 2017 04:00:00 GMT-0400
Sun Mar 12 2017 03:30:00 GMT-0400 !== Sun Mar 12 2017 04:30:00 GMT-0400
Sun Nov 05 2017 02:00:00 GMT-0500 !== Sun Nov 05 2017 01:00:00 GMT-0500
Sun Nov 05 2017 03:00:00 GMT-0500 !== Sun Nov 05 2017 02:00:00 GMT-0500
Without tm_gmtoff
test262/implementation-contributed/v8/mjsunit/tzoffset-seoul-noi18n.js

test262/implementation-contributed/v8/mjsunit/tzoffset-seoul.js

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-apia.js
Sat Sep 24 2011 04:00:00 GMT-1000 !== Sat Sep 24 2011 05:00:00 GMT-1000
Sat Sep 24 2011 04:30:00 GMT-1000 !== Sat Sep 24 2011 05:30:00 GMT-1000
Sat Dec 27 2053 20:00:00 GMT+36812200 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 27 2053 21:00:00 GMT+36812230 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 27 2053 22:00:00 GMT+36812300 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 27 2053 23:00:00 GMT+36812330 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 00:00:00 GMT+36812400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 01:00:00 GMT+36812430 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 02:00:00 GMT+36812500 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 03:00:00 GMT+36812530 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 04:00:00 GMT+36812600 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 05:00:00 GMT+36812630 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 06:00:00 GMT+36812700 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 07:00:00 GMT+36812730 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 08:00:00 GMT+36812800 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 09:00:00 GMT+36812830 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 10:00:00 GMT+36812900 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 11:00:00 GMT+36812930 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 12:00:00 GMT+36813000 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 13:00:00 GMT+36813030 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 14:00:00 GMT+36813100 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 15:00:00 GMT+36813130 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 16:00:00 GMT+36813200 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 17:00:00 GMT+36813230 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 18:00:00 GMT+36813300 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 19:00:00 GMT+36813330 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 20:00:00 GMT+36813400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 21:00:00 GMT+36813430 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 22:00:00 GMT+36813500 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Dec 28 2053 23:00:00 GMT+36813530 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 14:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 14:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 15:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 15:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 16:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 16:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 17:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 17:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 18:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 18:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 19:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 19:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 20:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 20:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 21:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 21:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 22:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 22:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 23:00:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sat Dec 31 2011 23:30:00 GMT+1400 !== Sun Jan 22 2012 01:21:34 GMT+1400
Sun Apr 01 2012 03:00:00 GMT+1400 !== Sun Apr 01 2012 03:00:00 GMT+1300
Sun Apr 01 2012 03:30:00 GMT+1400 !== Sun Apr 01 2012 03:30:00 GMT+1300
Sun Apr 01 2012 03:59:00 GMT+1400 !== Sun Apr 01 2012 03:59:00 GMT+1300

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-lord-howe.js
Sun Apr 02 2017 01:29:00 GMT+1100 !== Sun Apr 02 2017 01:59:00 GMT+1100
Sun Apr 02 2017 01:30:00 GMT+1100 !== Sun Apr 02 2017 01:30:00 GMT+1030
Sun Apr 02 2017 01:45:00 GMT+1100 !== Sun Apr 02 2017 01:45:00 GMT+1030
Sun Apr 02 2017 01:59:00 GMT+1100 !== Sun Apr 02 2017 01:59:00 GMT+1030
Sun Oct 01 2017 01:59:00 GMT+1030 !== Sun Oct 01 2017 01:29:00 GMT+1030
Sun Oct 01 2017 02:30:00 GMT+1100 !== Sun Oct 01 2017 01:30:00 GMT+1030
Sun Oct 01 2017 02:45:00 GMT+1100 !== Sun Oct 01 2017 01:45:00 GMT+1030
-660 !== -630

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-moscow.js
Sun Mar 28 2010 01:59:00 GMT+0300 !== Sun Mar 28 2010 00:59:00 GMT+0300
Sun Mar 28 2010 03:00:00 GMT+0400 !== Sun Mar 28 2010 01:00:00 GMT+0300
Sun Mar 28 2010 03:30:00 GMT+0400 !== Sun Mar 28 2010 01:30:00 GMT+0300
-240 !== -180
Sun Oct 31 2010 01:59:00 GMT+0400 !== Sun Oct 31 2010 02:59:00 GMT+0400
Sun Oct 31 2010 02:00:00 GMT+0400 !== Sun Oct 31 2010 02:00:00 GMT+0300
Sun Oct 31 2010 02:30:00 GMT+0400 !== Sun Oct 31 2010 02:30:00 GMT+0300
Sun Oct 31 2010 02:59:00 GMT+0400 !== Sun Oct 31 2010 02:59:00 GMT+0300
Sun Mar 27 2011 02:00:00 GMT+0300 !== Tue Mar 29 2011 09:04:50 GMT+0400
Sun Mar 27 2011 02:30:00 GMT+0300 !== Tue Mar 29 2011 09:04:50 GMT+0400
-180 !== -240
Sun Oct 26 2014 02:30:00 GMT+0500 !== Sun Oct 26 2014 02:30:00 GMT+0400
Sun Oct 26 2014 02:59:00 GMT+0500 !== Sun Oct 26 2014 02:59:00 GMT+0400

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-new-york-noi18n.js
Sun Mar 12 2017 03:00:00 GMT-0400 !== Sun Mar 12 2017 04:00:00 GMT-0400
Sun Mar 12 2017 03:30:00 GMT-0400 !== Sun Mar 12 2017 04:30:00 GMT-0400
Sun Nov 05 2017 02:00:00 GMT-0500 !== Sun Nov 05 2017 01:00:00 GMT-0500
Sun Nov 05 2017 03:00:00 GMT-0500 !== Sun Nov 05 2017 02:00:00 GMT-0500

test262/implementation-contributed/v8/mjsunit/tzoffset-transition-new-york.js
Sun Mar 12 2017 03:00:00 GMT-0400 !== Sun Mar 12 2017 04:00:00 GMT-0400
Sun Mar 12 2017 03:30:00 GMT-0400 !== Sun Mar 12 2017 04:30:00 GMT-0400
Sun Nov 05 2017 02:00:00 GMT-0500 !== Sun Nov 05 2017 01:00:00 GMT-0500
Sun Nov 05 2017 03:00:00 GMT-0500 !== Sun Nov 05 2017 02:00:00 GMT-0500

Looks like the "without" case has new failures with strange offsets calculated such as Sat Dec 27 2053 20:00:00 GMT+36812200 !== Sun Jan 22 2012 01:21:34 GMT+1400 from the tzoffset-transition-apia.js tests. Interestingly, the without case also has 3 less failures in the tzoffset-transition-moscow.js tests (13 vs. 16).

@TooTallNate
Copy link
Contributor Author

I switched to adding a -DHAVE_TM_GMTOFF define instead of config.h.

@TooTallNate
Copy link
Contributor Author

Looks like those GMT+36812200 failures are because mktime(&gmt) is returning -1 after setting that gmt.tm_isdst = tm.tm_isdst line :/

@TooTallNate
Copy link
Contributor Author

I "fixed" the GMT+36812200 issue by disabling DST on the local struct instead of matching the DST value on the GMT struct. The same 82 tests are still failing, but at least it's a reasonable, albeit incorrect date now.

This might be the best we can do without pulling in a full TZ library like ICU.

CMakeLists.txt Outdated
Comment on lines 67 to 70
check_struct_has_member("struct tm" tm_gmtoff "time.h" HAVE_TM_GMTOFF LANGUAGE C)
if(HAVE_TM_GMTOFF)
add_compile_definitions(HAVE_TM_GMTOFF)
endif()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is something we want to auto-detect but rather have it as a build option that one opts into explicitly.

Maybe it's better to incorporate the tzdata directly because that gives a consistent experience everywhere (been looking into that) but that doesn't have to hold up this PR.

Copy link
Contributor Author

@TooTallNate TooTallNate Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An explicit option would be fine to me as well, but I'm curious why we wouldn't want to leverage CMake's detection as a default?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it would be best to set the define for the special case, so the build just works if you grab the bunch of files and create a simple make file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason is that we'd have to mimic it if we added e.g. a meson or autotools build script. Another is that it affects runtime semantics; that should be a conscious decision, not something the build system does behind your back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I buy it.

  • If other build systems are implemented, they may or may not have equivalent features. Doesn't seem like a requirement to me.
  • Regarding runtime semantics, we're already at the mercy of the behavior of the libc implementation. Like you said, for total control, we should incorporate tzdata directly. What I'm going for here is a "best effort" with room for improvement.

But anyways, I'm also fine with removing the check_struct_has_member if that's what gets this PR over the finish line. Just let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kind of auto-detection I could live with it are#ifdef __NEWLIB__ guards in quickjs.c, assuming it's actually possible to feature-detect newlib at compile time1. Otherwise I'd like to see an explicit build flag.

1 I'm fairly sure #if defined(_NEWLIB_STDIO_H) would work although it's maybe not the best way

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@TooTallNate
Copy link
Contributor Author

TooTallNate commented Dec 23, 2023

  • Removed CMake check_struct_has_member check
  • Added explicit cmake -DHAVE_TM_GMTOFF=OFF option (defaults to ON)
  • Disabled by default for newlib

CMakeLists.txt Outdated
@@ -85,6 +85,7 @@ xoption(BUILD_STATIC_QJS_EXE "Build a static qjs executable" OFF)
xoption(CONFIG_ASAN "Enable AddressSanitizer (ASan)" OFF)
xoption(CONFIG_MSAN "Enable MemorySanitizer (MSan)" OFF)
xoption(CONFIG_UBSAN "Enable UndefinedBehaviorSanitizer (UBSan)" OFF)
xoption(HAVE_TM_GMTOFF "Enable when `tm_gmtoff` exists on `struct tm`" ON)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we were going to drop this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an explicit option to disable, defaults to ON.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but if you just copy the C files over to a project with an existing build system it won't work.

Ideally updating from QuickJS to our NG is a simple as overwriting the files, without the need to add extra build time flags.

@saghul saghul merged commit 440fc1b into quickjs-ng:master Dec 24, 2023
@saghul
Copy link
Contributor

saghul commented Dec 24, 2023

Thank you @TooTallNate !

@TooTallNate TooTallNate deleted the getTimezoneOffset branch December 24, 2023 08:45
@saghul
Copy link
Contributor

saghul commented Jan 11, 2024

QuickJS did this: bellard/quickjs@e66ce48

I'll hopefully have some time this week to sync with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants