espressif / newlib-esp32

Version of newlib used in ESP32 ROM and ESP-IDF
GNU General Public License v2.0
31 stars 18 forks source link

localtime() misses support for <+/-nn> timezone names #8

Closed BlueAndi closed 2 years ago

BlueAndi commented 3 years ago

The same problem as described here: https://github.com/esp8266/Arduino/issues/4637 The fix for the newlib can found here: https://github.com/earlephilhower/newlib-xtensa/pull/14

@igrr You were involved in the original issue too, would it be possible to integrate here, to get it for the esp32 too?

igrr commented 3 years ago

@BlueAndi We can integrate it, although I would prefer if the patch was accepted upstream. @earlephilhower Looking at the discussion at http://sourceware-org.1504.n7.nabble.com/PATCH-Add-support-for-TZ-names-with-lt-gt-in-tzset-td654031.html, seems like the only thing remaining is to send a git format-patch -formatted patch. I understand this was a while ago, but would you mind sending that to the list?

Edit: above link is broken, see https://sourceware.org/pipermail/newlib/2020/018144.html instead.

earlephilhower commented 3 years ago

@igrr I will try and resubmit it this weekend. Before the holidays came up and it just fell to the wayside.

trombik commented 3 years ago

for those who wonder what the issue means:

in esp32 arduino (confirmed with 1.0.6), timezone is broken. TZ in quoted format, such as <+07>-7, does not work as expected. the quoted format is what you usually find on unix machine under /usr/share/zoneinfo.

additional confusion arises from the official doc which says:

The format of the time string is the same as described in the GNU libc documentation

which is not true. Asia/Tokyo does not work either. the doc does not say what actually the libc implementation accepts.

    time_t now = time(NULL);
    setenv("TZ", "<+07>-7", 1); 
    tzset();
    Serial.printf("%s", ctime(&now));
Wed Dec 31 17:00:00 1969

-7 does not work either.

    time_t now = time(NULL);
    setenv("TZ", "-7", 1); 
    tzset();
    Serial.printf("%s", ctime(&now));
Thu Jan  1 00:00:00 1970

workaround: prefix characters to the offset.

    time_t now = time(NULL);
    setenv("TZ", "FOO-7", 1); 
    tzset();
    Serial.printf("%s", ctime(&now));
Thu Jan  1 07:00:00 1970

even a space works:

    time_t now = time(NULL);
    setenv("TZ", " -7", 1); 
    tzset();
    Serial.printf("%s", ctime(&now));

see if you are affected at:

https://github.com/nayarsystems/posix_tz_db/blob/master/zones.csv

BlueAndi commented 2 years ago

@earlephilhower Did you have time to integrate it?

pablo-mendoza commented 2 years ago

I just got hit by this. Are the changes going to be integrated? Or anyone has a pointer that doesn't use the non supported format?

BlueAndi commented 2 years ago

@igrr You mentioned it should be integrated upstream, can you please point the repository/branch exactly?

igrr commented 2 years ago

@BlueAndi The upstream repository is https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git. Newlib accepts patches via their mailing list, details are here: https://sourceware.org/mailman/listinfo/newlib/.

BlueAndi commented 2 years ago

@igrr I posted to the mailing list and it seems they are already working on it: https://sourceware.org/pipermail/newlib/2022/019434.html

igrr commented 2 years ago

The patch for angle brackets support has now been merged upstream: https://github.com/bminor/newlib/commit/539ac66ffa868762fbe43dd2e9b40d1a5407b53d. We'll pick it up into our fork next time we sync with the upstream (cc @antmak)

antmak commented 2 years ago

Fixed in esp-2022r1-RC1 toolchain