After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 644473 - g_date_time_get_utc_offset fails to read timezone data
g_date_time_get_utc_offset fails to read timezone data
Status: RESOLVED DUPLICATE of bug 631382
Product: glib
Classification: Platform
Component: general
2.34.x
Other Mac OS
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2011-03-11 06:28 UTC by Daniel Macks
Modified: 2012-10-12 16:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
add support for format 1 tzinfo files, as shipped by OS X (2.31 KB, patch)
2012-03-29 08:58 UTC, camillo.lugaresi+gnome
needs-work Details | Review
patch (2.34 KB, patch)
2012-04-14 05:01 UTC, camillo.lugaresi+gnome
needs-work Details | Review
add support for format 1 tzinfo files, as shipped by OS X (2.74 KB, patch)
2012-05-30 03:24 UTC, camillo.lugaresi+gnome
none Details | Review

Description Daniel Macks 2011-03-11 06:28:54 UTC
Building glib-2.28.2 on OS X 10.6, using "arch -i386" to force 32-bit-only build, 'make check' fails with:


TEST: gdatetime... (pid=78316)
  /GDateTime/add_days:                                                 OK
  /GDateTime/add_full:                                                 OK
  /GDateTime/add_hours:                                                OK
  /GDateTime/add_minutes:                                              OK
  /GDateTime/add_months:                                               OK
  /GDateTime/add_seconds:                                              OK
  /GDateTime/add_weeks:                                                OK
  /GDateTime/add_years:                                                OK
  /GDateTime/compare:                                                  OK
  /GDateTime/diff:                                                     OK
  /GDateTime/equal:                                                    **
ERROR:gdatetime.c:191:test_GDateTime_equal: assertion failed (g_date_time_get_utc_offset (dt1) / G_USEC_PER_SEC == (-3 * 3600)): (0 == -10800)
FAIL
GTester: last random seed: R02S17995b3d36fe2821bee875ea4a4d1f5a
/bin/sh: line 1: 77567 Terminated              MALLOC_CHECK_=2 MALLOC_PERTURB_=$((${RANDOM:-256} % 256)) ../../glib/gtester --verbose testing option-context keyfile fileutils printf protocol rand strfuncs string markup-parse markup-collect markup-escape markup-subparser array-test hostutils gvariant mem-overflow utils regex base64 sequence scannerapi shell collate utf8-pointer utf8-validate utf8-misc checksum hash date node convert list slist queue tree uri dir pattern logging error bookmarkfile gdatetime timeout environment

I didn't see any warnings related to gdatetime.c during the build, but I'm also not sure what specifically I should be looking for during the build or in platform configurations that would relate to this test-failure.
Comment 1 Daniel Macks 2011-03-13 22:15:55 UTC
Tried on 64-bit-only (arch x86_64), self-test failed even earlier:

TEST: collate... (pid=29257)
  /unicode/collate/0:                                                  OK
  /unicode/collate-key/0:                                              **
ERROR:collate.c:62:do_collate: assertion failed (str == test->sorted[i]): ("c" == "223")
FAIL
GTester: last random seed: R02S947f7b1011bee2466ad34318157ccfac
### execution of LANG=C failed, exit code 2
/bin/sh: line 1: 28787 Terminated              MALLOC_CHECK_=2 MALLOC_PERTURB_=$((${RANDOM:-256} % 256)) ../../glib/gtester --verbose testing option-context keyfile fileutils printf protocol rand strfuncs string markup-parse markup-collect markup-escape markup-subparser array-test hostutils gvariant mem-overflow utils regex base64 sequence scannerapi shell collate utf8-pointer utf8-validate utf8-misc checksum hash date node convert list slist queue tree uri dir pattern logging error bookmarkfile gdatetime timeout environment
Comment 2 Daniel Macks 2011-03-14 02:10:08 UTC
So right, Comment #1 should obviously be filed as a separate bug. But moving ahead, with the original one, but built from scratch in a 64-bit-only world,

$ ./gdatetime
/GDateTime/add_days: OK
/GDateTime/add_full: OK
/GDateTime/add_hours: OK
/GDateTime/add_minutes: OK
/GDateTime/add_months: OK
/GDateTime/add_seconds: OK
/GDateTime/add_weeks: OK
/GDateTime/add_years: OK
/GDateTime/compare: OK
/GDateTime/diff: OK
/GDateTime/equal: **
ERROR:gdatetime.c:191:test_GDateTime_equal: assertion failed (g_date_time_get_utc_offset (dt1) / G_USEC_PER_SEC == (-3 * 3600)): (0 == -10800)
Abort


so the failure at hand is not arch-dependent.
Comment 3 Daniel Macks 2011-05-19 09:01:14 UTC
Still busted as of 2.28.6

Debugging a bit (please help, this is completely unfamiliar code to me!), looks like g_time_zone_new() fails to find any timezone info from named timezones (EST, America/Recife, US/Eastern). They all give utc offsets of zero whereas timebased timezone data ("-03:00") give the correct offsets. I do have /usr/share/zoneinfo/ files for each of the named zones I tried, and they do start with the "TZif2" magic string that g_time_zone_new() looks for. I'm at the end of my knowledge here of both timezone files and GDateTime internals. Could a glib devel please speak up and help me debug this?
Comment 4 Peter Dyballa 2012-03-12 21:03:14 UTC
On Mac OS X 10.5.8, Leopard, PPC G4 (PowerPC 7447A) and with glib-2.29.92 the error still persists:

TEST: gdatetime... (pid=5182)
  /GDateTime/add_days:                                                 OK
  /GDateTime/add_full:                                                 OK
  /GDateTime/add_hours:                                                OK
  /GDateTime/add_minutes:                                              OK
  /GDateTime/add_months:                                               OK
  /GDateTime/add_seconds:                                              OK
  /GDateTime/add_weeks:                                                OK
  /GDateTime/add_years:                                                OK
  /GDateTime/compare:                                                  OK
  /GDateTime/diff:                                                     OK
  /GDateTime/equal:                                                    **
ERROR:gdatetime.c:192:test_GDateTime_equal: assertion failed (g_date_time_get_utc_offset (dt1) / G_USEC_PER_SEC == (-3 * 3600)): (0 == -10800)
FAIL
GTester: last random seed: R02Saf3ef6db6df6c40b49adb157ec518554
/bin/sh: line 1: 99855 Terminated              MALLOC_CHECK_=2 MALLOC_PERTURB_=$((${RANDOM:-256} % 256)) ../../glib/gtester --verbose testing option-context option-argv0 keyfile fileutils test-printf protocol rand strfuncs string markup-parse markup-collect markup-escape markup-subparser array-test hostutils gvariant mem-overflow utf8-performance utils regex base64 sequence scannerapi shell collate utf8-pointer utf8-validate utf8-misc unicode checksum hmac hash cache date node convert list slist queue tree uri dir pattern logging error bookmarkfile gdatetime timeout environment mappedfile dataset sort unix unix-nothreads bitlock
Comment 5 camillo.lugaresi+gnome 2012-03-29 08:58:32 UTC
Created attachment 210847 [details] [review]
add support for format 1 tzinfo files, as shipped by OS X

The problem is that glib only reads format 2 timezone files, while OS X ships with format 1 files. I have a patch that adds support for format 1.
Comment 6 Dan Winship 2012-03-30 14:17:48 UTC
Comment on attachment 210847 [details] [review]
add support for format 1 tzinfo files, as shipped by OS X

>+      if (tz->zoneinfo) {

glib uses gnu indent style:

  if (tz->zoneinfo)
    {
      ...
    }

>+          g_free (tz->trans);

that ought to cause a warning. You need to cast the "const" away.

>+          else	/* assume version 1 */

It would be better to check that explicitly. v1 apparently has a 0 byte after the "TZif".

>+              for (i = 0; i < tz->timecnt; i++) {

(indent/brace style is wrong again)

>+                gint64 tmp = GINT64_TO_BE (gint32_from_be (trans32[i]));
>+                memcpy (&trans64[i], &tmp, sizeof tmp);

It might be better to make a gint32_be_to_gint64_be() helper method to go along with the others. (All you need to do is clear the first 4 bytes of the gint64_be, and copy the gint32_be to the second 4 bytes.)
Comment 7 camillo.lugaresi+gnome 2012-04-11 08:26:50 UTC
Dan, I didn't see your reply until now, because bugzilla never emailed me about it. I guess you might miss mine, too, but:

> It would be better to check that explicitly. v1 apparently has a 0 byte after the "TZif".

V2 was defined in such a way as to be backwards-compatible with v1: it begins with a v1-format file, and then repeats the same data in v2 format, so that a v1-format reader can still use it. This was done because there is no version field in v1 (though there are reserved bytes set to 0), and so a reader was not supposed to check anything but the TZif code. I think it's reasonable to assume that any future versions would have the same backwards-compatibility approach as v2, and thus falling back on the v1 reader would still work.

> It might be better to make a gint32_be_to_gint64_be() helper method to go along
with the others.

It's already a single line of code, I don't think it needs a separate helper.


For the style issues and the const thing, please go ahead and fix them when you commit the patch.
Comment 8 Dan Winship 2012-04-11 14:08:25 UTC
(In reply to comment #7)
> Dan, I didn't see your reply until now, because bugzilla never emailed me about
> it.

Click "Preferences" at the top, then "Email Preferences" and make sure you didn't turn off notification at some point... Or check you spam folder...

> I think it's reasonable to assume
> that any future versions would have the same backwards-compatibility approach
> as v2, and thus falling back on the v1 reader would still work.

Hm. Well, ideally we'd use the v2 data rather than the v1 data in that case, but I guess it doesn't really matter until 2038 anyway. OK.
Comment 9 camillo.lugaresi+gnome 2012-04-14 05:01:11 UTC
Created attachment 212036 [details] [review]
patch

All new whitespace!
Comment 10 John Ralls 2012-05-30 00:16:20 UTC
Review of attachment 212036 [details] [review]:

Looks great... but could you make a new patch with `git format-patch`, so that you can provide a commit message and get credit as author?
Comment 11 camillo.lugaresi+gnome 2012-05-30 03:24:23 UTC
Created attachment 215232 [details] [review]
add support for format 1 tzinfo files, as shipped by OS X

Patch made with git format-patch.
Comment 12 Daniel Macks 2012-06-06 00:35:08 UTC
The patch in Comment #11, applied to glib-2.32.2 on my 32-bit system, gets me much further, but still not fully passing GDateTime_equal:

  /GDateTime/non_utf8_printf:                                          **
ERROR:gdatetime.c:894:test_non_utf8_printf: assertion failed (__p == ("10\346\234\210")): ("10" == "10\346\234\210")
Comment 13 camillo.lugaresi+gnome 2012-06-06 17:54:15 UTC
Daniel Macks: the failures in /GDateTime/non_utf8_printf and /GDateTime/strftime are unrelated to this bug. They are due to OS X's strftime producing different outputs for the Japanese locale than the ones hardcoded in the tests. The OS X outputs do seem strange, for example producing "PM" instead of "午後", so that should probably be counted as an OS X bug rather than a glib one.
Comment 14 Daniel Macks 2012-06-06 18:03:17 UTC
Alright. So then please consider my report as "patch works: timezone lookups are now working thanks!".
Comment 15 Daniel Macks 2012-09-24 17:24:54 UTC
glib-2.34.0 self-test is still failing and Comment #11 patch still applies cleanly and fixes most of it. It's been a whole cycle of development versions and now a new stable branch; could someone commit the patch or let us know what more work is needed?
Comment 16 John Ralls 2012-10-12 16:49:26 UTC

*** This bug has been marked as a duplicate of bug 631382 ***