Replace chrono with jiff #511

sharkdp · 2024-07-28T12:50:39Z

This PR ports date-time handling in Numbat from chrono to jiff.

General findings:

The documentation of jiff is just excellent!
The missing support for parsing/formatting of fractional seconds is the only blocker for us right now (see some tests that needed to be disabled).
I really like that fallible methods are marked as such and return Results, as appropriate (I still have some TODOs in the code here where plain values were returned earlier. I need to check whether we want to pass on errors or if they can be handled locally / ignored). Making all those potential errors visible in the API is great for Numbat since we want to be correct in all cases and often allow arbitrary inputs from users. It's also easy for us to handle errors since we turn them into appropriate Numbat errors for the user. On the other hand, I could also imagine that this is a bit too much in terms of being explicit about errors for developers that write applications that just do "normal" things with dates and times. They might prefer a library where they need to think less about all those edge cases (at the cost of possible panics). Anyway, as I said: personally, and for Numbat in particular: I like jiffs error strategy.
I was a bit confused by the (apparent) absence of RFC 3339 parsing. I searched the docs for "rfc" and only found the rfc2822 module. I did not do a full-text search, unfortunately. It was only until later when I saw the example in jiffs README that I noticed that support for RFC 3339 was indeed present... somehow? Maybe discoverability could be improved a bit. And currently, it's not yet clear to me if there is a way to strictly parse RFC 3339. But to be honest, I'm also not sure if I really want strict RFC 3339 parsing or if I should opt into the more general format that seems to be provided by jiff (https://docs.rs/jiff/0.1.1/jiff/fmt/temporal/index.html).
jiff::Error does not implement PartialEq. I'm not saying it needs to, but it would be convenient for our use case since we derive PartialEq on our own error types (mainly for easier unit testing). And this currently prevents us from embedding a jiff::Error inside our own error variants. It's not strictly necessary of course.
I never worked a lot with C, so I'm not super familar with the strptime/strftime naming and it always takes a second for me to remember which one is which. There might be a good reason why jiff adopted these names, but I personally like parse/format/to_string etc. better.

see: #323

numbat-cli/src/completer.rs

numbat/Cargo.toml

numbat/modules/datetime/human.nbt

sharkdp · 2024-07-28T12:55:29Z

numbat/src/datetime.rs

+    if let Ok(dt) = DateTime::strptime("%Y-%m-%dT%H:%M:%SZ", input) {
+        // RFC 3339
+        Some(dt.to_zoned(TimeZone::UTC).unwrap()) // TODO


RFC 3339 parsing could previously be done with parse_from rfc3339 in chrono. I used strptime with a fixed format string for now (instead of input.parse()), since the FromStr instance on Timestamp seems to support a couple of other formats as well (https://docs.rs/jiff/0.1.1/jiff/fmt/temporal/index.html). And I'm not sure if it conflicts with what we have below.

I think it is generally fine to think of jiff::fmt::temporal as "RFC 3339 support." That is, if you need to support RFC 3339, then that's what you should use. Now, if for some reason you actually need strict RFC 3339 checking (as in, do not support anything beyond it), then Jiff doesn't really support that, other than using the strptime APIs as you've done here.

The stuff that it supports beyond RFC 3339 come from RFC 9557 (time zone identifiers) and (parts of) ISO 8601. For example, 2024-07-27 is supported, although you can't parse a Zoned from it. Here are some others:

2024-07-27 2024-07-27 13:30 2024-07-27 13 2024-07-27T13 2024-07-27T13:30:29.123456789 2024-07-27 13[America/New_York] 2024-07-27 13-04[America/New_York] 2024-07-27 13-04:00[America/New_York]

Generally it's pretty flexible. But won't support things like "use slashes instead of dashes" as a delimiter.

The other main thing to remember is that in order to parse a Zoned, the interchange format must contain a bracketed time zone identifier like [America/New_York] or even [-04:00]. Otherwise, you can parse pretty much anything with a date into a jiff::civil::DateTime. For example, all of the above examples will parse into a jiff::civil::DateTime.

Let me know if this doesn't quite answer things for you. (I've only had a quick glance at the code below.)

At the risk of digressing completely... let me describe our use case. I think it might be something that other jiff users also need. But I'm not sure.

I want to provide a way for Numbat users (most of which will not have a detailed knowledge about timezones and UTC offsets) to enter datetimes as a string without having to provide a format string. I'm later planning to add strptime-functionality (i.e. with a format string) as well. But for convenience reasons, I want to provide a function that simply takes a string and returns a (zoned) date-time.

We provide a long list of supported formats in our documentation. But ideally, I want users to have something that just works. That means I want to support a large range of popular formats.

In Numbat, we don't provide all layers of safety that jiff provides. For example: we currently don't support datetimes without timezones. We do this by providing fallbacks/defaults: If a user does not supply a timezone (or offset) in the input string, we'll use their local timezone. If that is not available either, we'll assume UTC. I understand that this might sound like heresy to someone who just implemented a datetime library, but so far, this approach works fine for Numbat.

Now the problem I have is this: I don't know upfront if I am going to parse a jiff::civil::DateTime or a jiff::Zoned. So what I'm doing now is to try and parse into a jiff::Zoned at first (with formats that provide timezone information). And if that doesn't work, try to parse into a civil::DateTime (and add timezone information as described above).

And this is why I was a bit hesitant to use a "black box" like jiff::fmt::temporal at first. But now that I think about it some more, I guess I can use it in the same way that I do for the other strptime-based parsers. I need to try to parse into jiff::Zoned at first, and then fall back to parsing into a civil::DateTime afterwards.

The only thing that would be bad is if somehow, temporal would successfully parse something into a DateTime that could otherwise be parsed into a Zoned. But even then, I could try all parsers with Zoned at first, and only then attempt to parse civil::DateTimes.

Sorry for the long (and probably not every useful) answer. As you can see, I need(ed) to sort my own thoughts at first. I don't really know if there is anything that jiff can help with here. Having even more supported formats than just RFC 3339 should be a good thing for my use case.

Yeah I think you are on the right track here.

In terms of "provide a datetime and have it just work," I think the main problem there is that it's getting very close to wanting locale support, which is something I've tried hard to not solve in Jiff. (With strtime actually being the singular exception to that because it is just so useful.)

I am open to supporting something like this where you just try to parse anything and make sense of it, probably with some assumptions. But I'm not yet sure what that would look like and whether it would be free of footguns.

But yeah, in a general datetime library, "just assume local time if no TZ/offset is present" is a rather large footgun.

sharkdp · 2024-07-28T12:59:12Z

numbat/src/datetime.rs

    } else {
-        dt.to_rfc2822()
+        dt.strftime("%Y-%m-%d %H:%M:%S %Z (UTC %z)").to_string()


This is something that was not (easily) possible before, I think. We can now display clock times like CET or CEST instead of just the offsets. I chose to show both the clock time and the offset to the user for now. I personally don't know all of these abbreviations, so having the UTC offset in addition seems like a good idea.

Yes, this is right. The abbreviation comes directly from your time zone database (i.e. /usr/share/zoneinfo).

If an abbreviation isn't available, then the offset is used. For example:

let zdt = date(2024, 7, 28).intz("Antarctica/Troll")?; println!("{}", zdt.strftime("%F %Z"));

Gives this output:

$ cargo -q r 2024-07-28 +02

Antarctica/Troll

Ok, this is a really minor point.. but: I saw that in the docs as well, and initially interpreted it as some kind of non-existing timezone (troll as in: someone is trying to troll us). Might be just me, but could be worth using a different example (if one exists for the scenario in question).

Yes, this is right. The abbreviation comes directly from your time zone database (i.e. /usr/share/zoneinfo).

Oh, good point. In that case, I would probably rather append zoned_datetime.time_zone().iana_name(), in case it exists. I don't want to show the offset twice.

Ah I love Antarctica/Troll because it's notable for having a 2 hour DST transition! Hah. But yeah, I think I've heard someone else express confusion about that, so it's probably best to choose a different one. Thank you for the feedback.

Oh, good point. In that case, I would probably rather append zoned_datetime.time_zone().iana_name(), in case it exists. I don't want to show the offset twice.

So if there was something like %X (specifier name to be determined) that expanded to the IANA time zone ID (and parsed it), what would it do if the IANA time zone ID wasn't available? I guess it would do the same as the abbreviation right? Probably just expand to the offset?

So if there was something like %X (specifier name to be determined) that expanded to the IANA time zone ID (and parsed it), what would it do if the IANA time zone ID wasn't available? I guess it would do the same as the abbreviation right? Probably just expand to the offset?

I think that is reasonable, because %Z does the same.

That said, my intention here in Numbat (and I guess in some other programs as well) is to show a Zoned datetime in a way that it is easy for humans to understand. And neither UTC offsets nor time zone IDs serve that purpose alone, in my opinion. Even for my local timezone, I would have to look up the precise DST rules to determine whether 2024-03-31 12:00:00 Europe/Berlin would have a UTC offset of +01 or +02. So while this contains all information in principle, it's probably not ideal.

This is why I thought I'd maybe show both: timezone and UTC offset (and/or clock time). I don't care if it is verbose. Something like

2024-03-31 12:00:00 (UTC +0200, Europe/Berlin) or even 2024-03-31 12:00:00 (CEST, UTC +0200, Europe/Berlin)

And for this use case, a fallback to the offset would not be ideal for %Z/%X. Because I would show the offset twice for cases where the timezone is not known (only the offset) or has no name.

I'm not sure if that makes sense. And if this is even the best way to present timestamps to a user. And I realize that the strftime/strptime grammar is (and should stay) somewhat limited, so it does not need to support each and every use case. I can also glue pieces together from various sources (such as zoned.time_zone().iana_name()).

In terms of parsing, even with the time zone, the offset is actually required in order to losslessly format and parse the zoned datetime back into the same instant. So in terms of being verbose, I think you're heading in the right direction.

I'm not quite sure how to deal with "show offset and TZ id, but only show offset if there is no TZ id" in the context of the strtime APIs, since their expressivity is pretty limited. It would probably need a directive just for that very specific formatting choice. The RFC 9557 interchange format deals with this by accepting redundancy. That is, 2024-07-28T14:16-04[-04] is valid. I would be pretty tempted to ask you to consider redundancy to be okay in this case too. In general, it should be rare for a TZ identifier to just not exist at all. It's usually indicative of a system configuration problem or a bug in Jiff (e.g., Jiff not yet knowing how to detect the system default timezone on iOS or something).

@BurntSushi I now wrote a little bit of custom logic to format datetimes for Numbat. Which is completely fine for me. strftime doesn't need to support each and every use case. I wanted it to look like the following. Note that there is no clock time (timezone abbreviation?) for Asia/Kathmandu:

The only thing which is totally hacky (and maybe not correct) is how I get the abbreviation and how I avoid printing the offset twice in case there is no abbreviation. I basically use zoned.strftime("%Z") to get the abbreviation as a string and then check if it starts with + or - (in which case I omit printing it). I guess it would be nice to have a way to get to that information without this hack, but I'm not sure if that's really a common use case.

numbat/numbat/src/datetime.rs

Lines 71 to 95 in ad1543c

pub fn to_string(dt: &Zoned) -> String {

let tz = dt.time_zone();

if dt.time_zone() == &TimeZone::UTC {

dt.strftime("%Y-%m-%d %H:%M:%S UTC").to_string()

} else {

let offset = dt.offset();

let zone_abbreviation = dt.strftime("%Z").to_string();

let abbreviation_and_offset =

if zone_abbreviation.starts_with('+') || zone_abbreviation.starts_with('-') {

format!("(UTC {})", offset)

} else {

format!("{} (UTC {})", zone_abbreviation, offset)

};

let timezone_name = if let Some(iana_tz_name) = tz.iana_name() {

format!(", {iana_tz_name}")

} else {

"".into()

};

let dt_str = dt.strftime("%Y-%m-%d %H:%M:%S");

format!("{dt_str} {abbreviation_and_offset}{timezone_name}")

}

}

Yeah Asia/Kathmandu doesn't have any TZ abbreviations in tzdb:

[andrew@duff jiff]$ ./scripts/jiff-debug tzif /usr/share/zoneinfo/Asia/Kathmandu TIME ZONE NAME /usr/share/zoneinfo/Asia/Kathmandu TIME ZONE VERSION 2 LOCAL TIME TYPES 000: offset=+05:41:16 designation=LMT indicator=local/wall 001: offset=+05:30 designation=+0530 indicator=local/wall 002: offset=+05:45 designation=+0545 indicator=local/wall TRANSITIONS 0000: -009999-01-02T01:59:59Z unix=-377705023201 wall=-009999-01-02T07:41:15 unambiguous type=0 +05:41:16 LMT 0001: 1919-12-31T18:18:44Z unix=-1577943676 wall=1919-12-31T23:48:44 fold-until(1920-01-01T00:00:00) type=1 +05:30 +0530 0002: 1985-12-31T18:30:00Z unix=504901800 wall=1986-01-01T00:00:00 gap-until(1986-01-01T00:15:00) type=2 +05:45 +0545 POSIX TIME ZONE STRING <+0545>-5:45

I basically use zoned.strftime("%Z") to get the abbreviation as a string and then check if it starts with + or - (in which case I omit printing it). I guess it would be nice to have a way to get to that information without this hack, but I'm not sure if that's really a common use case.

Yeah that's the TimeZone::to_offset API.

It just returns a &str though. It is quite literally passing through the abbreviation from tzdb itself. It isn't Jiff that's giving you the offset if there is no abbreviation. That comes from tzdb. So you'll still need to do the "check for +/- dance." But you at least don't need to go through strftime.

Your code looks correct though. I think what you're doing is sound and I can't think of any footguns here.

I am likely to add a %V specifier (from Java's DateTimeFormatter) that will expand to an IANA tz identifier or the UTC offset if the IANA tz identifier isn't available. But I don't think that will help you here to avoid the redundancy. I'll think on that a little more because I do like the format you settled on. It displays nicely.

numbat/src/ffi/datetime.rs

numbat/src/vm.rs

numbat/tests/interpreter.rs

sharkdp · 2024-07-28T13:05:48Z

numbat/src/datetime.rs

                return Some(dt);
            }

            // Try to match the given format plus an additional timezone name (%Z).
-            // chrono does not support %Z, so we implement this ourselves. We were
+            // jiff does not support %Z, so we implement this ourselves. We were


Is is hard to support %Z when parsing datetimes? I know that chrono doesn't support this either, but it seems like jiff might be able to, since there is a tighter coupling to timezones and the timezone database?

It can't really be done, because the time zone abbreviations are ambiguous. CST for example is an abbreviation for Central Standard Time, China Standard Time and Cuba Standard Time. So there's just no way to parse it in a way that doesn't lead to a footgun.

The better way is to parse IANA time zone identifiers. The only way Jiff supports that currently is via the RFC3339/RFC9557/ISO-8601 format that looks like this: 2024-07-28T09:31-04[America/New_York]. The strtime module doesn't have a directive for parsing/formatting an IANA time zone identifier, yet. I don't think there is any specific roadblock to adding that support, but AIUI, there isn't a precedent for it in strtime APIs in other ecosystems. So I was hesitant to add it.

It can't really be done, because the time zone abbreviations are ambiguous. CST for example is an abbreviation for Central Standard Time, China Standard Time and Cuba Standard Time.

Right, that is unfortunate. As a user, I would still like to have that functionality for cases where it's not ambiguous. I would be okay with not being able to parse 2023-07-28 09:00:00 CST, but I would like to be able to parse 2023-07-28 09:00:00 US/Central.

So there's just no way to parse it in a way that doesn't lead to a footgun.

I'm definitely not saying this is important. But for our use case, it would certainly be great if I could just use %Z in a datetime format and get a result if it is unambiguous, and an error otherwise.

The better way is to parse IANA time zone identifiers. The only way Jiff supports that currently is via the RFC3339/RFC9557/ISO-8601 format that looks like this: 2024-07-28T09:31-04[America/New_York]. The strtime module doesn't have a directive for parsing/formatting an IANA time zone identifier, yet. I don't think there is any specific roadblock to adding that support, but AIUI, there isn't a precedent for it in strtime APIs in other ecosystems. So I was hesitant to add it.

Having a new/special %-identifier for that would be great, but again — we already have something that works ok for us, so please don't add this unless others find this useful as well.

All righty. When I get a chance, I'll make an issue for this on Jiff and work through some of the choices here.

And yeah, US/Central is an IANA time zone identifier. So if strptime had a switch for those, then US/Central would work. As would America/New_York among many others.

numbat/src/datetime.rs

BurntSushi · 2024-07-28T14:00:16Z

jiff::Error does not implement PartialEq.

Yeah I find this supremely annoying myself.

The first problem, as with most errors that don't implement PartialEq, is that it embeds a std::io::Error. I've considered trying to work-around it by boxing the std::io::Error and using pointer equality, converting it to a string first or even just considering them equal if their kinds are equivalent.

The second problem is that a jiff::Error is like a little mini-anyhow::Error internally (not actually using anyhow) in that one of its internal error types is just anything that implements Display:

struct AdhocError(Box<dyn core::fmt::Display + Send + Sync + 'static>);

And doing equality on this is difficult as well, for effectively the same reason that std::io::Error is itself not Eq. Pointer equality on this would be pretty precarious. But converting to strings first would be okay I think, if egregious.

See also BurntSushi/jiff#8 for more on Jiff's error design.

BurntSushi · 2024-07-28T14:04:58Z

I never worked a lot with C, so I'm not super familar with the strptime/strftime naming and it always takes a second for me to remember which one is which. There might be a good reason why jiff adopted these names, but I personally like parse/format/to_string etc. better.

Yeah I wanted to be very careful about using parse, format or to_string for those, because that would, I think, heavily bias them towards being the "standard" way to format/parse datetimes. And the main way to do it really should be the Temporal format, and that's why those are used for the Display and FromStr impls. The strptime and strftime functions are more like a utility knife.

I went with those names not just because they are the C names, but because they are also used in other ecosystems with essentially the same meaning. I'm worried I made the same mistake I did with the memchr crate, but I'm not sure yet.

A good mnemonic is that the "p" in strptime is for parsing, while the "f" in strftime is for formatting.

I don't really like the names either. Hell, I don't even like the format language either. It's a confusing mess IMO. But... it is what everyone seems to be used to.

examples/tests/human.nbt

sharkdp · 2024-07-31T20:45:32Z

Ok, this is interesting. I added some tests for the timezone handling and they fail on all Linux runners (as if no tzdb was not available). Locally, these tests run fine for me on Linux. Investigating…

BurntSushi · 2024-07-31T20:59:26Z

Ok, this is interesting. I added some tests for the timezone handling and they fail on all Linux runners (as if no tzdb was not available). Locally, these tests run fine for me on Linux. Investigating…

If you can enable trace level logging, Jiff should emit a lot of info about not finding tzdb or the system configured time zone.

BurntSushi · 2024-07-31T20:59:57Z

If you're running inside a sandboxed environment, that could be a reason why Jiff can't see it.

sharkdp · 2024-07-31T21:00:39Z

I'm using cross. And I just verified that the cross docker images don't contain /usr/share/zoneinfo

▶ docker run -it ghcr.io/cross-rs/x86_64-unknown-linux-gnu:main ls /usr/share
aclocal          debconf      glib-2.0     perl
aclocal-1.16     debianutils  info         perl5
adduser          dict         keyrings     pixmaps
apport           doc          libc-bin     pkg-config-crosswrapper
autoconf         doc-base     libtool      pkg-config-dpkghook
automake-1.16    dpkg         lintian      pkgconfig
base-files       emacs        locale       polkit-1
base-passwd      file         man          sensible-utils
bash-completion  gcc          menu         tabset
bug              gdb          misc         terminfo
ca-certificates  git-core     pam          zsh
common-licenses  gitweb       pam-configs

I saw you also use cross for jiff CI tests. I wonder how that works for you?

BurntSushi · 2024-07-31T21:03:12Z

Oof yeah that's annoying. I don't know enough about cross to know if there's an easy way to add a bind-mount to /usr/share/zoneinfo to the docker run command it executes. An alternative would be to enable the tzdb-bundle-always in tests, but that means tests won't cover the non-bundled case which is kind of a bummer.

BurntSushi · 2024-07-31T21:05:19Z

I'm using cross. And I just verified that the cross docker images don't contain /usr/share/zoneinfo

▶ docker run -it ghcr.io/cross-rs/x86_64-unknown-linux-gnu:main ls /usr/share
aclocal          debconf      glib-2.0     perl
aclocal-1.16     debianutils  info         perl5
adduser          dict         keyrings     pixmaps
apport           doc          libc-bin     pkg-config-crosswrapper
autoconf         doc-base     libtool      pkg-config-dpkghook
automake-1.16    dpkg         lintian      pkgconfig
base-files       emacs        locale       polkit-1
base-passwd      file         man          sensible-utils
bash-completion  gcc          menu         tabset
bug              gdb          misc         terminfo
ca-certificates  git-core     pam          zsh
common-licenses  gitweb       pam-configs

I saw you also use cross for jiff CI tests. I wonder how that works for you?

Tests that rely on tzdb are skipped if tzdb could not be found. But I only use cross for targets that don't easily work on native runners. So tests on macOS and Linux itself should cover the non-bundled use case. I guess the cross tests don't.

Tons of Jiff's doc tests rely on tzdb being available and those are only run with non-cross targets. (I don't think cross supports doc tests.)

sharkdp · 2024-07-31T21:45:05Z

Adding a volume mount did the trick. Can be configured from within Cross.toml.

numbat_wasm_bg.wasm (chrono):

2.12 MB (2,220,032 bytes)

numbat_wasm_bg.wasm (jiff)

1.72 MiB (1,802,240 bytes)

That's a 20% reduction in size — cool! Probably about 40% if we only compare the TZ data (chrono added about 1 MB)

sharkdp · 2024-07-31T21:53:51Z

https://numbat.dev/ now runs the latest version, powered by jiff! Here are some things Numbat can do with date times. It's pretty limited so far (I still use it fairly often). In particular, we don't have calendar arithmetic for units larger than days (but we could easily add something like a add_months function, which would use jiff to add one month instead of adding an average-sized month).

@BurntSushi Thank you very much for writing jiff and for your support here!

BurntSushi · 2024-07-31T23:27:15Z

w00t that's awesome! I tweeted about it. :-)

BurntSushi · 2024-07-31T23:37:32Z

Adding a volume mount did the trick. Can be configured from within Cross.toml.

numbat_wasm_bg.wasm (chrono):
2.12 MB (2,220,032 bytes)
numbat_wasm_bg.wasm (jiff)
1.72 MiB (1,802,240 bytes)
That's a 20% reduction in size — cool! Probably about 40% if we only compare the TZ data (chrono added about 1 MB)

I wasn't expecting this! I wonder if embedding the data as just a flat &'static [u8] ends up being more memory efficient than what chrono-tz does.