proposal: time: Add function to generate a Location struct

[FlorianUekermann]

If I am not mistaken it is currently not possible (without using unsafe) to generate a time.Location. This is a bit of a problem because whether LoadLocation works is quite pretty platform/instance dependent. A simple solution could be to add:
func NewLocation(name string, zone []Zone, tx []ZoneTrans) *Location
which would require making Zone(Trans)? and theyr fields public.

I guess Zone and ZoneTrans could also go into a subpackage to avoid cluttering the documentation of the time package.

[bradfitz]

If I am not mistaken it is currently not possible (without using unsafe) to generate a time.Location.

You're jumping to a solution here.

This is a bit of a problem because whether LoadLocation works is quite pretty platform/instance dependent

Is that the actual problem? If so, we can discuss that problem.

I guess Zone and ZoneTrans could also go into a subpackage to avoid cluttering the documentation of the time package.

We definitely don't want any types or zone constructor functions in the time package.

If we're going to add a new package, I'd rather see something like "time/zones" and then zones.Load(zoneName), where the "time/zones" package documents that has all the tzdata available (perhaps we bundle the tzdata zip file into the archive file; we still don't have an efficient way to do that without duplicating the content into a Go string file)

In any case, I'm against your proposed solution. But maybe you want to restate the problem. (Please don't edit the top comment, though.)

[FlorianUekermann]

Not surprised, I am not too happy about the proposal either. I'm thinking about portability and custom locations.

If I understand the current situation correctly (not 100% sure about that after your reply), the return values of LoadLocation depend on the local zonefile, which may be outdated for example.
Making your own locations may sound silly, but it would allow shipping locations without depending on a particular environment. Another problematic case is GopherJS as platform, which doesn't support any locations (I do realize that this may not be the place to fix that though).

[bradfitz]

I'm thinking about portability and custom locations.

What do you mean by portability? And what is a custom location? Is there missing data in tzdata?

Another problematic case is GopherJS as platform, which doesn't support any locations

What is the GopherJS bug URL for this issue? I'm curious what the options to address it are. Presumably they can just bundle the whole tzdata zip file into the Javascript if they really wanted everything. But perhaps that's too big.

[FlorianUekermann]

By portability I mean the possibility of writing code that involves time.Location instances and produces the same results independent of environment details (like OS, version/presence of tzdata).
By custom location I mean a developer defined instance of time.Location (either to ensure portability, or because a not/recently standardized timezone combination is needed). I am not sure if there is missing data in tzdata beyond the outdated version issue.

Note: Countries/states change daylight saving details quite quickly sometimes. So tzdata being out of date is not as weird as it may sound.

The GopherJS issue is gopherjs/gopherjs#64. Yes, size is a problem that is interesting. A size efficient solution that was discussed years ago is using momentsJS as a source of location data.

[rsc]

We definitely don't want to expose Zone, ZoneTrans, and so on. Location is where the API stops.

I do understand wanting to get the location info from somewhere else (a []byte in tzdata format), but the solution would be an API solving that problem.

[rsc]

Note that any solution here should probably address #20211 as well (closed as dup of this one). The theory is that if you can override where location data comes from, the override can provide its own listing function separate from package time.

[FlorianUekermann]

That sounds a lot better. So I guess a solution to both issues could look like this?
NewLocation(name string, tzdata []byte) (*Location, error)
I'm not sure if that is a good name or if it should explicitly state the input format (maybe "LoadTzif"), which would also come in handy if we ever want to add other formats that may not have magic bytes.

Looking at the code this should be a tiny patch. I'm happy to submit some code if everyone is happy with this.

Btw.: What's the deal with not importing os (mentioned in #20211)? Is that a general rule?

[dmitshur]

Btw.: What's the deal with not importing os (mentioned in #20211)? Is that a general rule?

Packages in standard library have tiers, and higher level tiers are not allowed to import lower level ones. This is to prevent import cycles, and per policy, to keep package size smaller.

See here for full details.

[rsc]

We're not sure what the API here should be. Does anyone want to make any suggestions?

[dmitshur]

Looking for clarification/confirmation, but what would be the uses/users of this new API? Is it needed by anyone/anything other than to resolve the underlying mentioned GopherJS issue?

Because I'm quite confident the GopherJS issue could be resolved by modifying the time package internally in its overrides, without having to add to upstream time API. So I want to confirm this is a generally useful change in the Go standard library that warrants increasing API size.

[FlorianUekermann]

What about the scenarios outlined above?
The gopherjs issue just highlights the problems you can run into with LoadLocation. I definitely would like to define my own Locations as well as ensure that the locations I use are available independent of the target environment, which is not an issue that is exclusive to gopherjs.
See issue #20211 for another usecase.