Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tzif: support pathological transitions #164

Merged
merged 3 commits into from
Nov 30, 2024
Merged

Conversation

BurntSushi
Copy link
Owner

It turns out that some TZif files seem to have "pathological"
transitions that are very far in the past. So far, in fact, that they
are outside the supported range of Jiff's Timestamp (that is,
-9999-01-01 to 9999-12-31). This in turn caused parsing such TZif
files to fail.

But they are technically valid TZif files. There's nothing that says,
e.g., i64::MIN is an illegal transition value.

So we support them here by simply clamping out-of-range time zone
transitions to Jiff's range. While this could in theory be invalid for
some possible TZif files, they would need to be specifically constructed
to be invalid. In practice, there are no DST rules at the boundaries of
what Jiff supports, so this should be fine.

Fixes #163

It turns out that some TZif files seem to have "pathological"
transitions that are very far in the past. So far, in fact, that they
are outside the supported range of Jiff's `Timestamp` (that is,
`-9999-01-01` to `9999-12-31`). This in turn caused parsing such TZif
files to fail.

But they are technically valid TZif files. There's nothing that says,
e.g., `i64::MIN` is an illegal transition value.

So we support them here by simply clamping out-of-range time zone
transitions to Jiff's range. While this could in theory be invalid for
some possible TZif files, they would need to be specifically constructed
to be invalid. In practice, there are no DST rules at the boundaries of
what Jiff supports, so this should be fine.

Fixes #163
@BurntSushi BurntSushi force-pushed the ag/fix-older-tzif branch 2 times, most recently from 239bd42 to e259a0f Compare November 30, 2024 15:04
I have no idea what's going on here. But WASM tests are currently
[failing on CI] with an unscrutable error. What's worse is that I cannot
reproduce it locally. Sigh.

So for now, we rollback the Rust toolchain to an older version. I
suppose this is just kicking the can down the road, but it follows in
the footsteps of numbat (which Jiff's WASM tests were inspired by).
I found Rust 1.81 by just going backwards from the current release
(1.83).

[failing on CI]: https://github.com/BurntSushi/jiff/actions/runs/12097259497/job/33732443120?pr=164
[footsteps of `numbat`]: https://github.com/sharkdp/numbat/pull/563/files
@BurntSushi BurntSushi merged commit 711c528 into master Nov 30, 2024
17 checks passed
@BurntSushi BurntSushi deleted the ag/fix-older-tzif branch November 30, 2024 15:13
@BurntSushi
Copy link
Owner Author

BurntSushi commented Nov 30, 2024

This PR also fixed CI. For WASM, I kicked the can down the road and pinned Rust to 1.81. I have no idea what newer versions are failing. It doesn't fail for me locally.

@BurntSushi
Copy link
Owner Author

This PR is on crates.io in jiff 0.1.15.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jiff fails to determine the timezone using the system tzdata
1 participant