Add baseline ZoneInfo64 data provider by nekevss · Pull Request #354 · boa-dev/temporal

nekevss · 2025-06-18T00:53:10Z

This adds a ZoneInfo64Provider that directly uses zoneinfo64.res bytes. This does not implement any resolution for now for the actual provider.

Ideally, long term, it may be best for this to take in bytes and complete some zerocopy deserialization, but there's some real difficulty around handling zoneinfo64's u16 strings that may require some custom types.

I think this may also require us to carry a Unicode license in the root for timezone_provider due to the res files. CC: @Manishearth for confirmation on that

This adds a ZoneInfo64Provider that directly uses zoneinfo64.res bytes. This does not implement any resolution for now for the actual provider. Ideally, long term, it may be best for this to take in bytes and complete some zerocopy deserialization, but there's some real difficulty around handling zoneinfo64's u16 strings that may require some custom types.

HalidOdat

Looks good on my side, just some minor non-blocking nitpicks 😄

HalidOdat · 2025-06-21T15:19:07Z

+# ZoneInfo64 dependencies
+resb = { git = "https://github.com/unicode-org/icu4x.git", optional = true }
+rustc-hash = { workspace = true, optional = true }
+writeable = { workspace = true, optional = true }


Whitespace 👮‍♂️

Suggested change

writeable = { workspace = true, optional = true }

writeable = { workspace = true, optional = true }

HalidOdat · 2025-06-21T15:21:12Z

+    #[serde(borrow)]
+    pub final_rule: Option<ZoneInfo64String<'data>>,
+    pub final_raw: Option<i32>,
+    pub final_year: Option<i32>,


Remark: Seems like from the data that the year can fit in a i16

HalidOdat · 2025-06-21T15:42:53Z

+
+#[derive(Debug, Clone)]
+pub enum Zone<'data> {
+    Table(ZoneTable<'data>),


Since ZoneTable is 108 bytes bigger than the Link variant maybe we should wrap it in a Box.

Suggested change

Table(ZoneTable<'data>),

Table(Box<ZoneTable<'data>>),

Not useful if it's being zero-copy deserialized. Could be a Cow though.

Manishearth · 2025-06-21T22:36:51Z

I think this may also require us to carry a Unicode license in the root for timezone_provider due to the res files. CC: @Manishearth for confirmation on that

Probably, yes. Make sure it's included in the published package!

Manishearth

Yeah, looking at this ZeroVec won't help for the variable sized stuff (the three toplevel Vecs)

We can make it work by adding resb-specific Array, Array16, IntVector types and giving them custom from_bytes behavior.

Manishearth · 2025-06-21T22:40:07Z

 serde_json = { version = "1.0.140", optional = true }

+# ZoneInfo64 dependencies
+resb = { git = "https://github.com/unicode-org/icu4x.git", optional = true }


We should cut a release; make a PR to ICU4X that finalizes the resb Cargo.toml (make it publish = false), write a changelog entry, and ping me for a release.

Manishearth · 2025-06-21T22:41:55Z

+
+#[derive(Debug, Clone)]
+pub enum Zone<'data> {
+    Table(ZoneTable<'data>),


Not useful if it's being zero-copy deserialized. Could be a Cow though.

Manishearth · 2025-06-21T22:56:17Z

More thoughts on zerocopy stuff:

Really I think there's not as much benefit doing this through the serde model if you want to do it zerocopy, doing zerocopy in the serde model is easier if you control the data format itself, but here we do not. If we want to do 100% zerocopy stuff we should write a custom parser with the right intermediate types.

But it's not super necessary, a handful of allocations isn't a big deal here.

Manishearth · 2025-06-21T22:58:57Z

+        #[cfg(target_endian = "little")]
+        let bytes = include_bytes!("./data/zoneinfo64/2025b/le/zoneinfo64.res");
+        #[cfg(target_endian = "big")]
+        let bytes = include_bytes!("./data/zoneinfo64/2025b/be/zoneinfo64.res");


hm. This won't actually work.

ZeroVec does two things: it allows data to be unaligned, and it consistently serializes in little-endian format. You only need the first one if your parsers are different for LE/BE.

Either the resb parser needs to be written to allow selecting endianness in the caller, OR you should exclusively access the ZeroVec data here in a way that inverts the numbers on BE.

Probably something to be fixed later.

Yeah, I added it initially, and then realized the day after that the BE wouldn't work with ZeroVec. I just haven't gotten around to fixing it just yet.

If the resb data that is available from platforms uses native endianness, the resb crate should just not use ZeroVec.

Agreed. In this case, the zoneinfo file is provided, so the BE zoneinfo file just needs to be removed.

robertbastian

Would it make sense to move some of the logic, and more importantly the data and tests into the resb crate? This should also solve the license problem as resb is already Unicode-licensed.

robertbastian · 2025-06-27T10:59:20Z

+        #[cfg(target_endian = "little")]
+        let bytes = include_bytes!("./data/zoneinfo64/2025b/le/zoneinfo64.res");
+        #[cfg(target_endian = "big")]
+        let bytes = include_bytes!("./data/zoneinfo64/2025b/be/zoneinfo64.res");


If the resb data that is available from platforms uses native endianness, the resb crate should just not use ZeroVec.

nekevss · 2025-06-27T13:43:07Z

Would it make sense to move some of the logic, and more importantly the data and tests into the resb crate

Yeah, I think there's a good argument here for moving it into the resb crate.

The only issue is that I'm not entirely convinced that the resb crate should own the zoneinfo64 structs and data. It could probably be feature flagged ... Or maybe an intermediate zoneinfo64 crate that reexports resb and zoneinfo64 data + structs?

robertbastian · 2025-06-27T13:48:41Z

It should definitely be feature-flagged for the scenario where there's already a zoneinfo64 available from somewhere else.

nekevss · 2025-08-26T19:38:20Z

Closing as this is being implemented in a different approach

nekevss requested review from HalidOdat, jasonwilliams and jedel1043 June 18, 2025 00:53

nekevss added 2 commits June 17, 2025 19:58

Remove zoneinfo64 from default list

3c3fcd6

Make deps optional

1d97066

HalidOdat approved these changes Jun 21, 2025

View reviewed changes

Manishearth approved these changes Jun 21, 2025

View reviewed changes

Manishearth reviewed Jun 21, 2025

View reviewed changes

nekevss added providers Related to time zone providers labels Jun 26, 2025

robertbastian reviewed Jun 27, 2025

View reviewed changes

nekevss mentioned this pull request Jun 27, 2025

Add ZoneInfo64 structs and data to resb crate unicode-org/icu4x#6714

Closed

nekevss closed this Aug 26, 2025

nekevss deleted the impl-zoneinfo64-provider branch September 2, 2025 00:45

	writeable = { workspace = true, optional = true }
	writeable = { workspace = true, optional = true }

Uh oh!

Conversation

nekevss commented Jun 18, 2025

Uh oh!

HalidOdat left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manishearth commented Jun 21, 2025

Uh oh!

Manishearth left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manishearth commented Jun 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertbastian left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nekevss commented Jun 27, 2025

Uh oh!

robertbastian commented Jun 27, 2025

Uh oh!

nekevss commented Aug 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HalidOdat left a comment •

edited

Loading

Manishearth left a comment •

edited

Loading