std.time: add Date #18272

Vexu · 2023-12-13T14:54:57Z

Implements #8396

InKryption · 2023-12-13T18:15:56Z

Haven't looked through thoroughly, but it seems the added Month enum is a duplicate of std.time.epoch.Month.

lib/std/io/Reader.zig

xdBronch · 2023-12-13T18:58:34Z

i dont have a strong opinion on it but recently there was some discussion about StackFallback having get instead of allocator and imo it makes sense for it to be different. afaik its the only allocator getter that affects the state of the allocator which can cause some unexpected behavior if youre not aware of that.

edit: i didnt realize this existed, see #16344. my thinking is that the function having a different name will possibly cause people to investigate why and learn what it does differently

Vexu · 2023-12-13T19:21:58Z

Haven't looked through thoroughly, but it seems the added Month enum is a duplicate of std.time.epoch.Month.

Looks like it does, that enum didn't exist when I started this. I'll combine them.

i dont have a strong opinion on it but recently there was some discussion about StackFallback having get instead of allocator and imo it makes sense for it to be different. afaik its the only allocator getter that affects the state of the allocator which can cause some unexpected behavior if youre not aware of that.

Oh right, didn't notice that despite it being documented. Maybe there should be an additional allocator function which doesn't reset it or a const allocator = @compileError("use get because XYZ");?

xdBronch · 2023-12-13T19:25:33Z

either sounds good, even get just not reinitializing the FBA like #16344 mentioned would probably be fine

Closes ziglang#12960

Vexu · 2023-12-13T20:46:27Z

I extracted the stackFallback change to a separate PR since it is breaking: #18275

daurnimator · 2023-12-14T01:04:13Z

lib/std/time.zig

+        pub const names = [_][]const u8{
+            "January",
+            "February",
+            "March",
+            "April",
+            "May",
+            "June",
+            "July",
+            "August",
+            "September",
+            "October",
+            "November",
+            "December",
+        };


I feel like these need to be off in some Locale specific area; not as a pub field in the Month enum.

Should these names constants be provided at all? They are specifically English and not localized and if you do not intend to localize your application to languages other than English it's not too difficult to do @tagName on the tag and then customize the capitalization as well. Maybe I want to capitalize it "january" or "JANUARY" instead.

lib/std/time.zig

FObersteiner · 2023-12-14T10:33:42Z

lib/std/time.zig

@@ -83,6 +83,12 @@ pub fn localtime(allocator: std.mem.Allocator) LocalTimeError!TimeZone {

 /// ISO 8601 compliant date and time representation.
 pub const DateTime = struct {
+    /// nanoseconds, range 0 to 999


thanks for the update :) Not sure though why we would need separate fields for ms, us and ns - a u30 nanosecond field [0-999999999] would allow any amount of fractional seconds, down to ns precision, no?

I was following the format for the other parts but using just one field for fractions does make more sense thinking again.

FObersteiner · 2023-12-14T10:37:22Z

lib/std/time.zig

+    day: u8,
+
+    /// day of the year, range 1 to 366
+    year_day: u16,


regarding year_day and week_day - what's the reason to carry those around with any datetime instance? why not calculate them "on demand"?

Why carry anything but the timestamp and the timezone offset? Either way removing them doesn't affect the size of the struct.

I think Go takes the timestamp approach. Rust's Chrono seems to be in the middle. I feel that one benefit of a timestamp approach is that it's easier to implement date arithmetic.

I didn't mean to advocate a purely timestamp-based approach. If there is a need to have fields like year_day stored in the structure then that's fine. I was just thinking that fields that are rarely required could be replaced by methods that calculate the value on request for given datetime.

FObersteiner · 2023-12-14T10:42:34Z

lib/std/time.zig

+
+    /// UNIX timestamp in nanoseconds
+    nano_timestamp: i128,
+    tt: TimeZone.TimeType,


what is the implication of "TimeType" here? Does this cover a rule associated with the TimeZone (POSIX or transition lookup from the tzfile)?

I don't like how this is currently either. I'll adjust it so that it is just the offset to the UNIX timestamp and a time zone name.

@leroycep is currently working on an abstraction for this in his zig-chrono library. Basically, I think you'd like to have an interface type that could be a fixed offset, a POSIX TZ rule or a IANA db tzfile time zone as concrete type. Any concrete type would have a method that gives you the offset from UTC (+ a DST indicator if appropriate) for a given datetime, tz and Unix timestamp.

Currently I have it organized like this:

tz.zig tz/ TZif.zig Posix.zig

tz/TZif.zig is the equivalent of the TimeZone file here, and tz/Posix.zig implements parsing and using POSIX TZ environment variable that is in the footer of most TZif files.

Like @FObersteiner mentioned, I have also been working on a TimeZone abstraction. I decided to go with an Allocator-like interface. Unfortunately this does make lifetimes of the TimeZones a bit more complex, which I'm handling at the moment by having a Database which handles their allocation.

FObersteiner · 2023-12-14T10:44:52Z

lib/std/time/TimeZone.zig

+    allocator.free(self.timetypes);
+}
+
+/// Project UTC timestamp to this time zone.


Does this cover a fall-back to LMT if the tzfile neither defines an offset for given Unix time (not UTC ;-)) nor a general POSIX rule? (v1 TZif)

No, currently it does only the most basic lookup for the offset. I was hoping for someone more familiar with the format to handle the edge cases so that I don't need to learn more about it.

Thinking this through once more, I think if you have a IANA tz db file loaded (TZif), it should specify an LMT offset as the first entry in the timetypes array. I'm not sure if this is historically "correct" since the concept of time zones as we have them today didn't exist a 150 years ago. But to me it seems "more" correct than extending a POSIX rule indefinitely into the past.
For the future, the story is a bit different; if the tzfile does not define any transitions after 2038, you end up with a fixed UTC offset, which doesn't seem right. But as for the past, extending a DST rule into the future indefinitely also seems... optimistic?

karlseguin · 2023-12-14T11:41:47Z

I don't mean to pick on this specific PR, since I think it's in-line with much of the standard lib, but I think this could have much more extensive testing. For example, each of these lines crash in a different way:

std.time.DateTime.fromTimestamp(170141183460469231731687303715884105727, std.time.TimeZone.TimeType.UTC);

std.time.DateTime.fromTimestamp(-1, std.time.TimeZone.TimeType.UTC);

std.time.DateTime.fromTimestamp(-323232223991999, std.time.TimeZone.TimeType.UTC);

Vexu · 2023-12-14T12:17:25Z

lib/std/time.zig

+                    @intCast(tzi.StandardName[4]),
+                    @intCast(tzi.StandardName[5]),
+                },
+                .offset = tzi.Bias,


The documentation doesn't mention if this includes leap seconds but I'd guess it does?

Not sure. If not, there is microsoft/STL#1624 and https://www.codeproject.com/Articles/8789/World-Clock-and-the-TimeZoneInformation-class on how to do it.

Bear in mind, that Linux may or may not be configured to use UTC or TAI directly without (from my understanding) a standard way to check the current config ie for folks hosting their own UTC servers (without standard query interface) see https://en.wikipedia.org/wiki/Unix_time#Variant_that_counts_leap_seconds.

Vexu · 2023-12-14T12:23:22Z

I don't mean to pick on this specific PR, since I think it's in-line with much of the standard lib, but I think this could have much more extensive testing. For example, each of these lines crash in a different way:

I fully expected a lot of comments on this considering the three previous PRs and the way time is in general.

I'll look more into the edge cases once we agree on the common cases.

matu3ba · 2023-12-14T14:02:51Z

lib/std/time.zig

+    /// nanoseconds, range 0 to 999 999 999
+    nanosecond: u32,
+
+    /// seconds, range 0 to 60


nit: could mention here 60 [leap second]

matu3ba · 2023-12-14T14:20:17Z

lib/std/time.zig

+                    @intCast(tzi.StandardName[4]),
+                    @intCast(tzi.StandardName[5]),
+                },
+                .offset = tzi.Bias,


Not sure. If not, there is microsoft/STL#1624 and https://www.codeproject.com/Articles/8789/World-Clock-and-the-TimeZoneInformation-class on how to do it.

Bear in mind, that Linux may or may not be configured to use UTC or TAI directly without (from my understanding) a standard way to check the current config ie for folks hosting their own UTC servers (without standard query interface) see https://en.wikipedia.org/wiki/Unix_time#Variant_that_counts_leap_seconds.

matu3ba · 2023-12-14T14:23:54Z

lib/std/time.zig

+    /// %M Minute (00-59)
+    /// %p AM or PM designation
+    /// %s Millisecond 891
+    /// %S Second (00-60)


60 [leap second]

matu3ba · 2023-12-14T14:26:33Z

lib/std/time/TimeZone.zig

+    while (i < header.counts.leapcnt) : (i += 1) {
+        const occur: i64 = if (legacy) try reader.readInt(i32, .big) else try reader.readInt(i64, .big);
+        if (occur < 0) return error.Malformed; // rfc8536: occur [...] MUST be nonnegative
+        if (i > 0 and leapseconds[i - 1].occurrence + 2419199 > occur) return error.Malformed; // rfc8536: occur [...] each later value MUST be at least 2419199 greater than the previous value


nit: const least_diff_leap_sec = 2419199 ?

leroycep · 2023-12-15T20:02:10Z

lib/std/time.zig

+    LocalTimeUnavailable,
+} || std.mem.Allocator.Error;
+
+pub fn localtime(allocator: std.mem.Allocator) LocalTimeError!TimeZone {


Are you planning to support the Posix style footers and/or TZ environment variable? I have support for Posix style TimeZone specifications (https://codeberg.org/geemili/chrono-zig/src/branch/master/src/tz/Posix.zig) and support for loading from the TZ environment variable: https://codeberg.org/geemili/chrono-zig/src/branch/master/src/tz.zig#L140-L154

wooster0 · 2023-12-17T14:52:57Z

lib/std/time.zig

+    };
+
+    /// Get current date and time in the system's local time.
+    /// On non-Windows systems this loads the system's local time


Suggested change

/// On non-Windows systems this loads the system's local time

/// On non-Windows systems this loads the system's local

wooster0 · 2023-12-17T14:53:15Z

lib/std/time.zig

+    /// Get current date and time in the system's local time.
+    /// On non-Windows systems this loads the system's local time
+    /// time zone database from the filesystem and may allocate
+    /// so when called repeated `nowTz` should be preferred.


Suggested change

/// so when called repeated `nowTz` should be preferred.

/// so when called repeatedly `nowTz` should be preferred.

FObersteiner · 2024-01-03T14:06:55Z

lib/std/time/TimeZone.zig

+    const leap_adjustment = leap: {
+        var i = self.leapseconds.len;
+        if (i == 0) break :leap 0;
+        while (i > 0) {
+            i -= 1;
+            if (self.leapseconds[i].occurrence < seconds) break;
+        }
+        break :leap self.leapseconds[i].correction;
+    };
+    const tt = self.transitions[index].timetype;
+    return .{
+        .offset = tt.offset + leap_adjustment,
+        .name_data = tt.name_data,


Not sure I get that one :) Are you proposing a leap second correction for a UTC offset? IIUC, time zones as defined in TZif specify offsets relative to a certain reference time (UTC).

FObersteiner · 2024-01-03T14:25:04Z

lib/std/time.zig

+    /// %s Millisecond 891
+    /// %S Second (00-60)


what about using %f for the fractional seconds part? That would make it independent of the resolution (ms, µs, ns).

FObersteiner · 2024-01-03T14:26:43Z

lib/std/time.zig

+    /// %S Second (00-60)
+    /// %y Year, last two digits (00-99)
+    /// %Y Year
+    /// %z UTC offset in the form ±HHMM


I think ISO8601 wants there to be a colon between hours and minutes, plus there might be non-zero seconds as well (current IANA db version does define such offsets for some time zones during some periods of time).

FObersteiner · 2024-01-03T14:29:09Z

lib/std/time.zig

+    /// %y Year, last two digits (00-99)
+    /// %Y Year
+    /// %z UTC offset in the form ±HHMM
+    /// %Z Time zone name.


%Z typically gives you the abbreviation, not the (up-to-date) IANA db identifier. While this is nice for output, parsing the abbreviations is a pain, if not impossible since there are many ambiguous ones. I think it would be great to have a directive that gives (or accepts) the IANA identifier.

andrewrk · 2024-01-04T07:49:48Z

I'm leaning towards rejecting #8396. Can you share why you personally want zig standard library to have date APIs in it, and maybe start by maintaining a third party API for some duration, and then we can think about upstreaming it to the standard library?

FObersteiner · 2024-01-04T12:54:38Z

As a side-note to Andrew's comment above; if you not only want datetime support but also IANA-db time zone support, you would also have to ship the database itself since not all distributions come with it. Windows for example does its own thing. Plus you would have to make sure that version of the database is up-to-date.

Vexu · 2024-01-04T14:12:16Z

I need the system's local time and a way to convert that into a textual representation in order to properly implement __DATE__, __TIME__ and __TIMESTAMP__ macros in Aro.

andrewrk · 2024-01-04T18:36:41Z

Note that timestamps are a colossal headache for the reproducible builds community. This spec for SOURCE_DATE_EPOCH suggests a neat idea: using the last modification time of the source if this environment variable is not provided.

Of course, for this use case, localizing the time is out of the question.

It seems such a shame to add a massive dependency to the zig standard library (timezone data in addition to code), which is also volatile (subject to changing in response to wars, politics, border disputes, etc.), which is then mainly used to cause reproducibility problems.

nektro · 2024-01-04T20:29:56Z

I used to be in the camp that this is something that would be essential for Zig to have eventually but I'm come around to agreeing that, just like with proper string support, users that need it would be better served by a separate module that's not tied to Zig's own release cadence and can update whenever the source data does

FObersteiner · 2024-01-05T19:29:23Z

Note that timestamps are a colossal headache for the reproducible builds community. This spec for SOURCE_DATE_EPOCH suggests a neat idea: using the last modification time of the source if this environment variable is not provided.

Is this to say "don't use local time"? I mean, IIUC they suggest Unix time in seconds resolution as the solution - I didn't see that coming ;-) Having an ISO8601 compatible date/time string with UTC offset would at least make it human readable - if that is a useful feature in this context I don't know. And it's convertible to UTC/Unix time, except for potential leap seconds. Plus, you wouldn't need "real" time zone support for this, just the UTC offset.
Anyways, I can see why you wouldn't want a half-featured datetime in your std lib (you still need a 3rd party lib for actual time zone support) or full-featured datetime with tz integration (dependencies mentioned above).

Vexu · 2024-01-06T17:10:19Z

I don't necessarily need the DateTime since Aro already has code to convert timestamps into human readable dates but what about time zones? How do you suggest I get the offset of the local time zone for those macros and what should be done about std.Tz?

andrewrk · 2024-01-06T20:57:32Z

I think if you made it so that Aro assumed that everyone's local time zone was UTC then you would be doing everyone a favor. It doesn't make any sense for the local time zone of the computer that compiled a piece of software to be present inside the executable. Nobody wants that.

edit:

what should be done about std.Tz?

It looks like it is unused by the zig project itself and should probably be deleted in favor of a third party package.

I'm open to reconsidering this later, but I'm pretty sure this is the best option for the time being.

Vexu · 2024-01-07T23:40:26Z

I think if you made it so that Aro assumed that everyone's local time zone was UTC then you would be doing everyone a favor. It doesn't make any sense for the local time zone of the computer that compiled a piece of software to be present inside the executable. Nobody wants that.

That's how it works currently since it uses the value from std.time.timestamp but it could lead to confusion since that's not how those macros are defined.

I'll open a new PR that removes std.Tz.

xdBronch reviewed Dec 13, 2023

View reviewed changes

lib/std/io/Reader.zig Show resolved Hide resolved

Vexu and others added 6 commits December 13, 2023 22:40

std.io.Reader: readStruct is only valid for extern structs

4ab2fd9

Closes ziglang#12960

move std.Tz to std.time.TimeZone

9667506

std.time: add localtime

4d09ffb

std.time.TimeZone: add project

f227d1a

std.time: add Date

00530b1

std.time.Date: store time type with the date

ad8fdf6

Vexu force-pushed the date branch from 27d490c to ad8fdf6 Compare December 13, 2023 20:45

std.time: implement Date.now for Windows

9034ea1

daurnimator reviewed Dec 14, 2023

View reviewed changes

FObersteiner reviewed Dec 14, 2023

View reviewed changes

lib/std/time.zig Outdated Show resolved Hide resolved

lib/std/time.zig Show resolved Hide resolved

rename Date to DateTime

f7b4297

Vexu force-pushed the date branch from 34c015b to 62c481f Compare December 14, 2023 09:42

FObersteiner reviewed Dec 14, 2023

View reviewed changes

std.time.DateTime: store fractions of second in nanoseconds

ea54ee9

Vexu force-pushed the date branch from 62c481f to 043150c Compare December 14, 2023 12:15

std.time: include leap seconds in timezone projection

f1effb2

Vexu force-pushed the date branch from 043150c to f1effb2 Compare December 14, 2023 12:16

Vexu commented Dec 14, 2023

View reviewed changes

matu3ba reviewed Dec 14, 2023

View reviewed changes

leroycep reviewed Dec 15, 2023

View reviewed changes

wooster0 reviewed Dec 17, 2023

View reviewed changes

FObersteiner reviewed Jan 3, 2024

View reviewed changes

Vexu closed this Jan 7, 2024

Vexu deleted the date branch January 7, 2024 23:40

FObersteiner mentioned this pull request Apr 5, 2024

Proposal: DateTime in std.time #8396

Open

clickingbuttons mentioned this pull request Apr 5, 2024

std: add generic date.Date, time.Time, and date_time.DateTime #19549

Closed

	/// On non-Windows systems this loads the system's local time
	/// On non-Windows systems this loads the system's local

	/// so when called repeated `nowTz` should be preferred.
	/// so when called repeatedly `nowTz` should be preferred.

std.time: add Date #18272

std.time: add Date #18272

Conversation

Vexu commented Dec 13, 2023

InKryption commented Dec 13, 2023

xdBronch commented Dec 13, 2023 • edited

Vexu commented Dec 13, 2023

xdBronch commented Dec 13, 2023

Vexu commented Dec 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FObersteiner Dec 14, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlseguin Dec 14, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FObersteiner Dec 14, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FObersteiner Dec 14, 2023 • edited

Choose a reason for hiding this comment

Vexu Dec 14, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karlseguin commented Dec 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vexu commented Dec 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FObersteiner Jan 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FObersteiner Jan 3, 2024 • edited

Choose a reason for hiding this comment

andrewrk commented Jan 4, 2024

FObersteiner commented Jan 4, 2024

Vexu commented Jan 4, 2024

andrewrk commented Jan 4, 2024

nektro commented Jan 4, 2024

FObersteiner commented Jan 5, 2024 • edited

Vexu commented Jan 6, 2024

andrewrk commented Jan 6, 2024 • edited

Vexu commented Jan 7, 2024

xdBronch commented Dec 13, 2023 •

edited

FObersteiner Dec 14, 2023 •

edited

karlseguin Dec 14, 2023 •

edited

FObersteiner Dec 14, 2023 •

edited

FObersteiner Dec 14, 2023 •

edited

Vexu Dec 14, 2023 •

edited

FObersteiner Jan 3, 2024 •

edited

FObersteiner Jan 3, 2024 •

edited

FObersteiner commented Jan 5, 2024 •

edited

andrewrk commented Jan 6, 2024 •

edited