-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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
proposal: time: handle time zones better in Parse and LoadLocation #63345
Comments
An alternative would be:
This approach keeps the concepts of 'time zone location' and 'time zone' separate within the API, at least at the function level. In the IETF database, some strings (such as "CET") serve as both a time zone location and a time zone. By providing two functions, users can decide whether they want to look up "CET" as a time zone location or "CET" as an actual time zone. |
I'm concerned about the ambiguities. For example, Running a little program over the timezone database turns up these duplicate identifiers:
|
@ianlancetaylor As I said in the proposal text, the offset would only be created for unique names, although there are more duplicates than I suspected. @fzipp That does seem like a clean proposal, but in practice I suspect the pattern would be to call LoadLocation and if that fails, LoadFixedZone. Putting the functionality into LoadLocation seems worthwhile. |
@robpike Yeah, I understood the intended handling of duplicate names, but it troubles me that |
I'm not sure what the use case is for this kind of string, but it seems like the caller is the only one in a position to resolve the ambiguity. To that end, maybe there should be another flavor of (Of course, in the simplest case, they could just parse the timezone themselves beforehand and use |
It's also possible to disambiguate the names using the installed location, as hinted by @rittneje but more automatically. First, though, there needs to be some acknowledgement that there is a problem that needs addressing. To me, that's clear, but maybe not to others, or maybe not of sufficient importance. But that typescript above is not a happy thing. |
@robpike can you elaborate on what you mean by this? |
This proposal has been added to the active column of the proposals project |
I would suggest that Go handles the TZ environment variable like libc/tzcode does, so that it's consistent with the rest of the system. When TZ is set to
What is currently lacking is parsing POSIX-style TZ strings, as described in the newtzset(3) man page (e.g., However,
I kinda understand the desire to handle timezone abbreviations in a best-effort manner in My suggestion would be for
There's a comment by @rsc in
There are a couple of fine points about handling of TZ that I'm not sure about, like whether POSIX-style TZ strings should be handled on OSes other than Unix at all. Should I submit another proposal or keep the discussion here?
A timezone abbreviation may also be useful in However, if there's no time corresponding to such string exactly, this may be tricky. E.g., |
I tried implementing this proposal, scanning the database to get all the timezone initialisms, and there are far more duplicates than I imagined. Therefore although I'd love to find a solution to the problem, which I think is a problem, my naive suggestion is certainly not it. |
Based on the discussion above, this proposal seems like a likely decline. |
No change in consensus, so declined. |
Consider this unedited typescript running on my Mac with macOS and a recent Go distribution:
There are a few mysteries here for the uninitiated.
First, the time zone in the result - which is printed by
time.Time.String
so is actually what is held in the time value - always reads CEST. That seems good for starters.Yet second, only the last of the three runs prints a correct time zone offset for CEST. The first two have it at zero, which is always wrong for CEST.
Third, even when I explicitly set the time zone through the TZ variable, it still doesn't get the offset right.
Fourth, even in the last run, the time zone prints as CEST although one might expect Europe/Berlin.
Fifth, perhaps strangest of all, this is working correctly according to the documentation in the time package.
This is a proposal to address, if not completely clear up, these mysteries.
Allow me to explain. (If you don't know much about time zones and how they work in modern computing, I suggest reading this IETF document first.)
The concept of time zone is not a first-class citizen in the time package. Instead, "location" is paramount. To get the correct rendering of a time, we must in effect ask for the time in Berlin, not in Central European Standard Time. In library terms, we call
LoadLocation
, notLoadTimeZone
. There are a number of reasons for this, and it is confusing, but the fundamental one is that is how the IETF time zone (ha!) database works. That is why the only run above that gets everything right is the one where we set the location, even though that is ironically, incorrectly and confusingly done through the legacy TZ environment variable, whose initials of course stand for "time zone".What happens in that third run is that because our location is Berlin, we have loaded the data for Berlin, and in that data is a description of the time zone called CEST. There is no time zone file for CEST itself. The only description of CEST is inside the data for Berlin. (Have a look for yourself; the data is in
/usr/share/zoneinfo/europe/Berlin
on Unix, although it's in binary; you need a program called zdump to examine it.)In the first run, my location is not in central Europe, the data for CEST is never loaded, and
time.Parse
humors me but delivers an unsatisfactory result. I'll explain why in a minute. On the other hand, if you happen to live in central Europe, that first run will in fact work properly for you.In the second run, a reasonable attempt to address this issue through the time-zone-looking TZ variable still fails, because CEST is not a location and therefore cannot be
LoadLocationed
.You might ask, "Why doesn't
time.Parse
give an error then, if CEST is not a valid location?" It's because of this paragraph, the last in the doc comment fortime.Parse
:I claim that this is an unfortunate design decision, albeit one made in good faith with a tinge of ignorance back in the early days of Go's development.
(You might also ask, "Why does
time.Parse
accept time zone strings and not locations?" There the answer is simpler: That's what time values look like, confusing though it is in this context.)Compatibility concerns might argue we can't fix this behavior, but I believe it's confusing and results in apparently correct but actually wrong results, and should be fixed. But we can't just switch the whole thing to time zones for the very reason that locations were chosen as the fundamental unit. But we can do something about time zones.
Here is the proposal.
If you look in the time zone database manually, you'll find that all the time zone abbreviations like EST and AEST and CEST are unique, or at least nearly all are. In the data we can see the offset, which is a fixed value. For instance, EST is always offset by 5 hours from UTC, while EDT is always offset by 4 hours. I propose:
Parse
orLoadLocation
is given a location/time zone that is not loadable, instead of incorrectly using a zero offset, we look up the name in that table. If it exists, we build a nonceLocation
with the provided name and fixed offset.The properties of these nonce locations are worth enumerating:
I believe this approach is easy, reasonable, less confusing, and worth doing.
(Developed in conversation with @rsc).
The text was updated successfully, but these errors were encountered: