r-hub/r-minimal

make check failure due to timezone issue

bastistician opened this issue · 5 comments

bastistician commented

I've tried a make check between make && make install. This fails because

difftime(
    as.POSIXct(c("1970-01-01 00:00:00", "1970-01-01 12:00:00"), tz="EST5EDT"),
    as.POSIXct(c("1970-01-01 00:00:00", "1970-01-01 00:00:00"), tz="UTC"))
## Time differences in hours
## [1]  5 16

returns a wrong result (should be [1] 5 17).

This problem has been reported long ago at https://bugs.r-project.org/bugzilla/show_bug.cgi?id=16843 and I've just verified that the issue persists even with latest Alpine 3.12.0. help(timezones) is pretty comprehensive and talks about the possibility of using R's internal zoneinfo database.

If the Dockerfile would

  1. configure --with-internal-tzcode
  2. set ENV TZ=UTC (which is used in plain Alpine Linux anyway) to suppress R's warning "system timezone name is unknown: set environment variable TZ".

time zone computations would be correct but the R installation would grow in size: share/zoneinfo has 2.4 MB.
Other ideas?

gaborcsardi commented

Thanks!

So this is an Alpine bug in the end? I installed tzdata, but that did not change anything. We should report it to Alpine then?

As I understand, if we set TZ to something, e.g. Europe/London, then make check passes, right? Do you think this bug causes trouble in practice?

gaborcsardi commented

Btw. it seems that if we use --with-internal-tzcode and set the time zone to UTC, then we can actually delete most of share/zoneinfo and just document that this image does not know about other time zones?

This seems like a good solution to me. What do you think?

bastistician commented

So this is an Alpine bug in the end? I installed tzdata, but that did not change anything. We should report it to Alpine then?

I don't know where in the system the bug sits (maybe in musl). From reading help(timezones), I have the impression that time zone management is relatively chaotic. The configuration option --with-internal-tzcode is used by default on Windows & macOS, and recommended for Solaris. This builds R with the tweaked tzcode implementation from R's src/extra/tzone, which includes localtime.c and strftime.c. Otherwise the native services are used (that means from musl in Alpine).

As I understand, if we set TZ to something, e.g. Europe/London, then make check passes, right? Do you think this bug causes trouble in practice?

To clarify, even if we setup a time zone in Alpine Linux by setting TZ, installing tzdata, and linking /etc/localtime to /usr/share/zoneinfo/${TZ}, R's make check will fail. Currently --with-internal-tzcode is needed for correct interpretation of "EST5EDT" in Alpine Linux.
Yes, I think this bug can cause trouble for people using the EST5EDT time zone name instead of the preferred "America/New_York" name (after installing tzdata). In current rhub/r-minimal:

apk add --no-cache tzdata
R --quiet -e 'as.POSIXct("2020-01-01 12:00:00", tz="EST5EDT"); as.POSIXct("2020-01-01 12:00:00", tz="America/New_York")'

gives

[1] "2020-01-01 12:00:00 EDT"
[1] "2020-01-01 12:00:00 EST"

The first one is wrong and should be identical to the second. Note that the second result would have time zone UTC (without warning) if tzdata was not installed.

Btw. it seems that if we use --with-internal-tzcode and set the time zone to UTC, then we can actually delete most of share/zoneinfo and just document that this image does not know about other time zones?

Maybe. It seems that at least the America/New_York and UTC files need to be kept. Warnings will then appear if any other timezone is used (as in many of R's tests). I'm not sure if this wouldn't also break some of the tests...
An advantage of this approach is that if I want to build something on top of rhub/r-minimal which does know about time zones, I can simply enrich the existing R installation through

apk add --no-cache tzdata
export TZDIR=/usr/share/zoneinfo
gaborcsardi commented

It seems that at least the America/New_York and UTC files need to be kept.

The former to fix the bug, right?

Warnings will then appear if any other timezone is used (as in many of R's tests). I'm not sure if this wouldn't also break some of the tests...

TBH I am not super concerned about the tests. I found them quite fragile.

if I want to build something on top of rhub/r-minimal which does know about time zones, I can simply enrich the existing R installation through

Yeah, this sounds great I think. We can remove most time zones, and keep UTC and the problematic ones. And document how to install the rest, should you need it.

bastistician commented

Without "America/New_York" warnings will always appear, even for

Sys.timezone()
## [1] "UTC"
as.POSIXct("2020-01-01 12:00:00")
## [1] "2020-01-01 12:00:00 UTC"
## Warning messages:
## 1: In strptime(xx, f, tz = tz) : unknown timezone 'America/New_York'
## 2: In format.POSIXlt(as.POSIXlt(x, tz), format, usetz, ...) :
##   unknown timezone 'America/New_York'

probably because of

src/extra/tzone/tzfile.h:33:#define TZDEFRULES	"America/New_York"

BTW, reporting the time zone error for Alpine's own R package built from https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/community/R/APKBUILD (also not using --with-internal-tzcode) might help to involve people who know more about these things.