Can't you load the Location from VTIMEZONE.TZID? loc,err := time.LoadLocation(VTIMEZONE.TZID) or if your system does not contain a timezone database you could use the loc := time.FixedZone("locationName", secondsEastOfUTC)

Can't you load the Location from VTIMEZONE.TZID? loc,err := time.LoadLocation(VTIMEZONE.TZID)

Regrettably, it's not that simple. This works for components created by some applications, but not all. Looking at my personal calendar, about 30% fail to parse when using this assumption. Some just have a TZID of local, other have the city name in another format, and a few are simply GMT-0300.

Some of the ones that say local include DST transitions too, so they retained the entire local TZ data, without serialising the name.

Aside from the anecdotal data, it's not safe to assume the TZID will match the timezone identifier; from RF5545:

    Note: This document does not define a naming convention for
    time zone identifiers.  Implementers may want to use the naming
    conventions defined in existing time zone specifications such
    as the public-domain TZ database [TZDB].  The specification of
    globally unique time zone identifiers is not addressed by this
    document and is left for future study.

FWIW, this approach you're suggesting is the one we initially took, and that's how we discovered this problem, apologies for not mentioning it initially.

if your system does not contain a timezone database you could use the loc := time.FixedZone("locationName", secondsEastOfUTC)

This won't yield the right results for repeating events (e.g.: a repeating event that repeats every day at 10:00, for a timezone with some DST).

It's also a bit problematic in that de-serialising the data, and the serialising it again would result in some data loss, and if the date for the iCalendar component is changed the timezone needs to be re-read since it's DST data has been lost.

CC @ianlancetaylor who seems to have modified this relatively recently.

Maybe this should be a proposal? @WhyNotHugo if you have a particular API in mind that would serve your use-case, then proposing that and having a discussion on that could be fruitful.

The Zone and ZoneTrans structs are currently private, and making them public seems like the easiest way to create a public constructor for Location:

Rename the current zone to Zone.
Rename the current zoneTrans to ZoneTrans.

With both structs being publicly usable, add a new method to create a Location instance:

func NewLocationFromZoneData(name string, zones []Zone, zoneTrans []ZoneTrans) Location...

I tried thinking of alternative APIs without using zone and zoneTrans, but they all end up basically re-implementing those two structs, since they represent the minimal amount of data required to create a Location instance.

This seems like a reasonable thing to do, but we probably want to define new structs that we're more willing to live with long term. The current zone and zoneTrans are clearly not ready for public consumption. Just to start things rolling:

func NewLocation(name string, zones []Zone, changes []ZoneChange) *Location

type Zone struct {
    Name string
    Offset Duration 
    IsDST bool
}

type ZoneChange struct {
    Start Time // first instant when this change is in effect
    Index int // index into zones list
}

Does this make sense? Should we just have a single slice with a Zone pointer inside the ZoneChange?
Also this does not include the 'extend' string that we use from the tz database.
So probably this is not quite right.

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

As a reference, here's a referrence VTIMEZONE (anecdotally, this particular one matches one returned by LoadLocation, but that's not necessarily the case for any valid VTIMEZONE):

BEGIN:VTIMEZONE
TZID:Europe/Amsterdam
BEGIN:DAYLIGHT
DTSTART:19810329T020000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3
TZNAME:CEST
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
END:DAYLIGHT
BEGIN:STANDARD
DTSTART:19961027T030000
RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10
TZNAME:CET
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
END:STANDARD
END:VTIMEZONE

There's a key trait about it: the fact that is starts on the last sunday of a given month, but I'm not sure the above API can actually represent that. I've found others that start on the first and second sunday of the month.

Potentially any valid RRULE could be used, although in practice I'm only seeing samples that repeat early on the Nth sunday of a specific month, so to be pragmatical, I think it's safe to assume that they all repeat yearly, and, for example, if the first one is on the Xth sunday, then all subsequent ones are too.

Trying to support something as flexible as icalendar might be too much complexity, and it's not worth it unless it's not needed (and I hope someone will correct me if any super-werid DST exists out here that breaks my assumptions).

For the record, #20629 is the proposal that led to adding time.LoadLocationFromTZData, some parts of that discussion might be relevant here as well.

If we expose structs with zone transition information to construct new Locations, we should consider whether/how to allow reading the zone transition information from Locations. Even if we only want to add an API to create new locations as part of this proposal, an API to read the transitions would probably use the same zone types if we add such API in the future. Use-cases for reading include things like "When is the next DST transition?", but I don't know how often this is needed. I also have some unfinished and unpublished experimental code for strict alternative to time.Parse that could benefit from reading the zone transitions as I wouldn't need to parse the tzdata files myself (but I'm not convinced that it's worth any changes in the standard library).

The current zone and zoneTrans are clearly not ready for public consumption.

I agree with this.

Just to start things rolling:

func NewLocation(name string, zones []Zone, changes []ZoneChange) *Location

What would the be the requirements for the parameters? I think NewLocation should return an error if some conditions are not satisfied. For example, if index of zone in ZoneChange is out of bounds. LoadLocationFromTZData currently checks the input data, which prevents panics when using the Location.

Should we just have a single slice with a Zone pointer inside the ZoneChange?

Advantages of this seem to be that:

• We can access the information directly, without needing the slice for indexing.
• We are not restricted to continuous block of memory for the zones.

On the other hand, currently some zones don't need to be referenced by a transition. In particular, if the first zone is not used by a transition, it is used for times before the first transition time:

We'd probably need a way to specify the zone to be used before the first transition time explicitly.

Also this does not include the 'extend' string that we use from the tz database. So probably this is not quite right.

If we are going to add an API to create new Locations programmatically, I think we need to support everything that LoadLocationFromTZData supports.

There's a key trait about it: the fact that is starts on the last sunday of a given month, but I'm not sure the above API can actually represent that. I've found others that start on the first and second sunday of the month.

Potentially any valid RRULE could be used, although in practice I'm only seeing samples that repeat early on the Nth sunday of a specific month, so to be pragmatical, I think it's safe to assume that they all repeat yearly, and, for example, if the first one is on the Xth sunday, then all subsequent ones are too.

Trying to support something as flexible as icalendar might be too much complexity, and it's not worth it unless it's not needed (and I hope someone will correct me if any super-werid DST exists out here that breaks my assumptions).

The extend string rule in tzdata supports things like last sunday of a given month. There is one start rule and one end rule in the extend string. So it might be possible to build such rule from the VTIMEZONE in practice, although theoretically VTIMEZONE can contain multiple rules.

It would be appropriate to produce an extend string for any entry in VTIMEZONE that does not have a termination date. Hopefully there would be at most two such entries, one for standard time and one for daylight time.

Turning the VTIMEZONE start and end times into a time.Time isn't straightforward, since they seem to be specified as local times in the timezone being defined. But I guess that is not something the time package needs to worry about.

Turning the VTIMEZONE start and end times into a time.Time isn't straightforward, since they seem to be specified as local times in the timezone being defined. But I guess that is not something the time package needs to worry about.

There is a TZOFFSETFROM field in each VTIMEZONE that specifies the offset from UTC prior to the transition, so I think using time.ParseInLocation with time.FixedZone should do the trick. The end date (UNTIL in RRULE) is in UTC. In any case, as you say, this is outside the scope of the time package.

Another use-case for reading the zone data from *time.Location is writing a VTIMEZONE to an iCalendar file.

It seems like we need some form for the future extension string, if we are to add this API.
It's unclear whether we can design something simpler than the TZdata encoding.
If we make the new API take a TZdata extension string,
perhaps it would make sense instead to write a VTIMEZONE-to-TZdata converter (maybe one already exists?)
and then use LoadLocationFromTZData, and then we don't need a new API?

Or is VTIMEZONE the only other possible format, and we should add it directly?
It's an RFC (https://datatracker.ietf.org/doc/html/rfc5545) so it doesn't seem too beyond the pale to add LocationFromVTIMEZONE, as long as there won't be more of these.
And it would be a top-level function, so it would be dropped from binaries that don't use it.

What APIs do other languages provide for this kind of need?

I've read through the code and spec a few time, and I'm not sure what the extend string means.

Is it the name of the zone to which we transition when there's no other explicit zone with a matching rule?

If we make the new API take a TZdata extension string, perhaps it would make sense instead to write a VTIMEZONE-to-TZdata converter (maybe one already exists?) and then use LoadLocationFromTZData, and then we don't need a new API?

I've mixed feeling about this approach. It sounds like a lot of "noise" to convert to an entirely different format to create a Location instance, and it's weird that this struct can't be instantiated without converting its data into a binary format, only for it to be parsed again.

I worry a bit that the extra conversion might add quite a bit of unnecessary overhead. For example, a cli calendar app I'm porting to golang reads 3k icalendar entries, so this extra conversion for each item doesn't sound minimal.

It might be worth giving a shot though, at least to have some understanding of how much overhead this is.

The "extend string" is roughly documented at https://man7.org/linux/man-pages/man3/tzset.3.html. It is the "first format" permitted in the TZ environment variable. If there is a more precise spec, I'm not aware of it.

Ah, thanks for the clarification.

Still wondering about:

Or is VTIMEZONE the only other possible format, and we should add it directly?

What APIs do other languages provide for this kind of need?

What APIs do other languages provide for this kind of need?

Rust uses a Trait for timezones (kinda like an interface), so anyone can create their own timezone implementations and use that. (note: this is the de-facto standard, but not shipped as part of stdlib).

Python is duck-typed, so some libraries just have their own timezone implementation that matches the interface of the regular one, and they can be used interchangeably.

This does shed light onto an entirely different alternative: It's possible to create a Timezone interface (of which Location is an implementer), and update all signatures that currently take a Location to take a Timezone. This should be pretty backwards compatible, but allow a third party to create a CustomLocation which can have any special behaviour or constructor. I haven't fully looked at all the internals yet to understand how feasible this is.

Or is VTIMEZONE the only other possible format, and we should add it directly?

I believe so. JMAP calendar uses just IANA names. I can't think of many other specs around calendaring / timezones.

This does shed light onto an entirely different alternative: It's possible to create a Timezone interface (of which Location is an implementer), and update all signatures that currently take a Location to take a Timezone. This should be pretty backwards compatible, but allow a third party to create a CustomLocation which can have any special behaviour or constructor. I haven't fully looked at all the internals yet to understand how feasible this is.

Backwards-compatibility concerns aside, even if you change all functions that take *Location in the standard library to an interface, there are many instances of *Location in user programs/libraries, which you can't update. So the utility of an interface that *Location implements would be limited. It might be possible to add a constructor that will create a *Location from an interface though.

But we need to keep in mind that we also need to be able to convert a *Location to VTIMEZONE when writing iCalendar files. Building a RRULE for the VTIMEZONE is easier if you have the extend string (or the same information exposed in a different way) rather than just a function that does lookups. We should also consider #50062 as well as it is somehow related. So it seems we need to expose the zone transitions and some form for the future extension string, even if we add an interface.

So I tried writing some code for testing.

https://github.com/martin-sucha/timezones creates new *time.Location using time.LoadLocationFromTZData.

When converting to tzif data, we need zone indexes, not pointers because there is a limit for how many zones can be present.

Benchmarks:

• LocationTemplate_NewLocation benchmarks allocation and filling of template with 100 changes all the way to converting it to *time.Location
• AllocTemplate is just filling of template with 100 changes
• LocationTemplate_tzdata is conversion from the template to tzif data
• LoadLocation is conversion from tzif data to *time.Location (time.LoadLocationFromTZData)

name                            time/op
AllocTemplate-4                 2.20µs ± 4%
LocationTemplate_NewLocation-4  5.38µs ± 1%
LocationTemplate_tzdata-4       1.37µs ± 1%
LoadLocation-4                  1.73µs ± 1%

name                            alloc/op
AllocTemplate-4                 3.26kB ± 0%
LocationTemplate_NewLocation-4  6.62kB ± 0%
LocationTemplate_tzdata-4       1.35kB ± 0%
LoadLocation-4                  2.01kB ± 0%

name                            allocs/op
AllocTemplate-4                   2.00 ± 0%
LocationTemplate_NewLocation-4    11.0 ± 0%
LocationTemplate_tzdata-4         3.00 ± 0%
LoadLocation-4                    6.00 ± 0%

And https://github.com/martin-sucha/vtimezone2tzif converts a VTIMEZONE using the above package to tzdata output. Although vtimezone2tzif is incomplete, probably buggy (as there is only one test case so far) and maybe could be somehow optimized.

Benchmark:

ToLocationTemplate parses VTIMEZONE and builds timezones.Template from it.

name                  time/op
ToLocationTemplate-4  7.41µs ± 1%

name                  alloc/op
ToLocationTemplate-4  8.91kB ± 0%

name                  allocs/op
ToLocationTemplate-4     111 ± 0%

@WhyNotHugo please try if something like that could work for you. Also I think it would be good to have some testing corpus of iCalendar files to see if all can be supported by the tzif format.

There is another argument against using pointers in ZoneChange since it is possible to do a mistake like

var zones []Zone
var changes []Change
for i := 0; i < 100; i++ {
    zones = append(zones, Zone{...})
    changes = append(changes, Change{Start: t, &zones[i]})
}

In this case, when append to zones reallocates, the old copy of zones will be still alive because of pointers stored in changes, so the memory usage would be essentially double of what it could be with indexes.

Backwards-compatibility concerns aside, even if you change all functions that take *Location in the standard library to an interface, there are many instances of *Location in user programs/libraries, which you can't update. So the utility of an interface that *Location implements would be limited. It might be possible to add a constructor that will create a *Location from an interface though.

Yeah, libraries would need to kind of "opt-in" into the new interface by updating functions that receive *Location to *Timezone. I don't think this is the best idea anyway.

I think this Template implementation is very interesting and useful. Something that I can't help noticed, is that its fields are almost the same as Location, and, in all honesty, it sounds like a candidate of what Location could look like if it had a public API.

Do you think it's sensible to try and copy this implementation/API into Location? This would allow constructing instances without going through tzif as an intermediate format at all and reading all the data that would be needed to serialise a Locatino into a VTIMEZONE too.

Regarding sample data: I've extracted VTIMEZONEs from my personal calendars and have 311 distinct VTIMEZONEs (different clients over the years, plus invites from third parties). I'll try to distil those further into those that are really different and worth testing individually.

If you have a vtimezone2tzdata, then it seems like that could be a 3rd-party package used in concert with time.LoadLocationFromTZData, and then we don't need to add code to package time at all.

Do I have that right?

Seems like we are still waiting to find the answer to:

If you have a vtimezone2tzdata, then it seems like that could be a 3rd-party package used in concert with time.LoadLocationFromTZData, and then we don't need to add code to package time at all.

Do I have that right?

Do you think it's sensible to try and copy this implementation/API into Location? This would allow constructing instances without going through tzif as an intermediate format at all and reading all the data that would be needed to serialise a Locatino into a VTIMEZONE too.

There are arguments against exporting the extend string, see #50062 (comment)

So it seems we'd need a different API if we wanted to expose it.

Also, the *Location caches the current zone, so we probably don't want to allow constructing new instances outside of the time package.

If you have a vtimezone2tzdata, then it seems like that could be a 3rd-party package used in concert with time.LoadLocationFromTZData, and then we don't need to add code to package time at all.

Note that vtimezone2tzif is not complete yet and we don't know if all VTIMEZONEs can be converted to tzif.
However, I think we don't want to have iCalendar specific code in time package as you need to implement full RRULE support to be able to do the conversion. And if there is some exotic VTIMEZONE that cannot be converted to tzdata without loss of precision (for example with more than two transitions per year in the "extend" part), you could always just generate a list of transitions for the next hundred years as a fallback. So it seems converting a VTIMEZONE to a list of zone transitions should be handled in an external package.

That leaves the question whether to expose Zone/ZoneChange in time package. The original proposal stated two reasons why we should do this:

(1) it's very complex to convert VTIMEZONE data into that binary format, and also, needless complexity (2) it's very inefficient, since every entry read has to be converted into an intermediate representation, to then be parsed again and then converted into the final one.

I think (1) is solved because https://github.com/martin-sucha/timezones exists now (and the code is ~250 lines of code).

As for (2), it seems that the performance of going through tzdata will be only about 2.5 times (5.38µs versus 2.20µs in the benchmark I posted previously) slower than allocating a *Location directly if we changed the time API. So it seems that the performance argument is not strong either. (Disclaimer: I don't work with iCalendar files regularly, so I won't be affected by the difference in performance)

There is another argument, which are the limitations imposed by the tzif file format. Currently it allows only a small number of zone names (all names must fit into a 255 byte buffer) so the timezones package must deduplicate them when writing the data. It is not clear whether this will ever be a problem in practice though.

So for the use case of converting a VTIMEZONE into a *time.Location I don't think we need to modify time package.

It seems that code to convert a *time.Location into a VTIMEZONE may need some way to access the information in the extend string. You could use ZoneBounds to dump for example the next 100 transitions to the VTIMEZONE, but is not clear whether that is acceptable or not as it may increase the size of the file.

So for the use case of converting a VTIMEZONE into a *time.Location I don't think we need to modify time package.

Great, thanks.

For the reverse, it seems like if you have a database of VTIMEZONEs you want (or you grab the timezone database and use github.com/martin-sucha/timezones to get VTIMEZONES), then you can just use time.Locations constructed from it and not worry about converting the "native" time.Locations back to VTIMEZONE. Note that we also don't provide a way to convert the "native" time.Locations to tzdata format. Especially on Windows, that would be difficult.

It sounds like we probably do not need to make modifications to the time package here.

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

I've been looking more into github.com/martin-sucha/timezones, and it seems it may be a feasible workaround. It does seem like there's going to be a lot more data conversion and parsing that necessary, but seems usable enough.

I'm not opposed to dropping this proposal.

No change in consensus, so declined.
— rsc for the proposal review group