Skip to content
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: List timezones #20211

Closed
Merovius opened this issue May 2, 2017 · 12 comments
Closed

proposal: time: List timezones #20211

Merovius opened this issue May 2, 2017 · 12 comments
Labels
Milestone

Comments

@Merovius
Copy link

@Merovius Merovius commented May 2, 2017

I'd like to propose to add a function to the time package that lists the set of installed timezones. For a good user experience, I want my service to provide auto completion or a drop-down for a user to select their timezone; currently, the time package only supports probing, which would make for a bad user-experience.

In theory, this could be provided outside of the stdlib by walking the corresponding directory trees; however, for different systems, different directories are used, so this would require duplicating those lists and keeping them in sync with the stdlib manually (otherwise you might claim to support a timezone, which you then not actually support; again, a bad user experience).

I'd write this myself, but wanted to first get an opinion on whether this would be accepted.

System details

go version go1.8.1 linux/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GORACE=""
GOROOT="/home/axelw/go"
GOTOOLDIR="/home/axelw/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build049658130=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
GOROOT/bin/go version: go version go1.8.1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.8.1 X:framepointer
uname -sr: Linux 4.4.0-72-generic
Distributor ID:	Ubuntu
Description:	Ubuntu 14.04.5 LTS
Release:	14.04
Codename:	trusty
/lib/x86_64-linux-gnu/libc.so.6: GNU C Library (Ubuntu EGLIBC 2.19-0ubuntu6.11) stable release version 2.19, by Roland McGrath et al.
gdb --version: GNU gdb (GDB) 7.9-gg19
@bradfitz bradfitz changed the title time: List timezones proposal: time: List timezones May 2, 2017
@gopherbot gopherbot added this to the Proposal milestone May 2, 2017
@gopherbot gopherbot added the Proposal label May 2, 2017
@rsc
Copy link
Contributor

@rsc rsc commented May 15, 2017

Someone would have to put readdir etc into package time (which cannot import os), at the bare minimum. Then the implementation would have to read the TZ database directory and open and read every file? Or maybe just list them and let the caller open them?

It's true that package time knows best where it will search from LoadLocation, and returning the directory is not super-helpful on Windows or other systems that have it in the zip file in GOROOT. So you can't write this function outside the standard library.

On the other hand it seems very specialized.

@Merovius
Copy link
Author

@Merovius Merovius commented May 16, 2017

I did not think about the restrictions about importing packages; that, indeed, complicates things and changes the tradeoffs significantly. TBH, I don't find it that specialized. I'd argue that most software that exposes timezones to users probably wants this, or risk being very frustrating to users. I know I often don't know what to call the timezone I'm about; what major city to use (and daylight savings time makes zone abbreviations pretty useless too, AIUI). But yeah, I can see how it's probably specialized enough to not justify the tradeoffs under these circumstances.

I'd still love to see this (maybe in x/, which doesn't have the restriction on imports but would still have reasonable guarantees to stay in sync with the stdlib?), but understand if you decide to not do it.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jun 12, 2017

Related issue: #20629 ("time: Add function to generate a Location struct")

@rsc
Copy link
Contributor

@rsc rsc commented Jun 12, 2017

Note that if we support listing timezones by default that complicates #20629. On the other hand, if we support #20629, maybe developers can use that and their own "list their source" instead of having listing be part of the standard time API.

@rsc
Copy link
Contributor

@rsc rsc commented Jun 12, 2017

Will close this in favor of #20629.

@rsc rsc closed this Jun 12, 2017
@golang golang locked and limited conversation to collaborators Jun 12, 2018
@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Aug 19, 2020

@rsc

On the other hand, if we support #20629, maybe developers can use that and their own "list their source" instead of having listing be part of the standard time API

I'm not sure that's possible with the eventually chosen solution to #20629, is it? The tzdata package doesn't seem to provide any way to acquire any of the actual data or enumerate the possible locations. Should this issue be reopened, or am I missing something?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Aug 19, 2020

As far as I can see basically the only thing people need to implement this is the list of directories, which on Unix systems is:

	"/usr/share/zoneinfo/",
	"/usr/share/lib/zoneinfo/",
	"/usr/lib/locale/TZ/",
	runtime.GOROOT() + "/lib/time/zoneinfo.zip",

I think the question is: is it useful for the time package to export that list?

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Aug 20, 2020

As far as I can see basically the only thing people need to implement this is the list of directories

That's not quite true, at least any more, because the data can also be in the tzdata package. I see the problem with adding directory-reading functionality to the time package, and adding some kind of listing functionality to tzdata seems wrong because it's important that it's only imported at the top level and we wouldn't want the "list time zones" functionality to have the side effect of ignoring system-supplied time zones.

I wonder about a new package (time/zones? time/tz?) that exports just a single function, List, that lists the names of all the available time zones.

I'll reopen this issue for now, as it's clearly not entirely resolved.

@rogpeppe rogpeppe reopened this Aug 20, 2020
@golang golang unlocked this conversation Aug 20, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Aug 28, 2020

@rogpeppe, I believe that what I meant back in #20211 (comment) is that once you have LoadLocationFromTZData you can have your own set of time zone data, iterate over it, create locations from it, and so on.

For the original scenario above, namely a web server offering a timezone drop-down, it seems like you'd almost certainly want to be using your own specific set of timezone data so that you have control over exactly which version you are using, instead of relying on whatever is built-in to Go or shipped with the operating system (especially on Windows).

So I'm not convinced this proposal needs to be reopened, at least not without a new concrete use case to think about.

It sounds like maybe you have a new need that might be addressed by List? Can you say more about what it is?

@Merovius
Copy link
Author

@Merovius Merovius commented Aug 28, 2020

The argument that it's more robust to be explicit about the set of timezones to include is convincing to me.
I guess the advantage of not doing that (and instead use what ships with Go) is that you don't have to update it yourself when things change (say, countries abolish DST or something). No idea how frequently that happens. And even then, that's probably not enough of a reason.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Oct 14, 2020

@Merovius It sounds like you are retracting this proposal. Should we close the issue?

@Merovius
Copy link
Author

@Merovius Merovius commented Oct 14, 2020

Sure.

@Merovius Merovius closed this Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.