Skip to content

Use system timezone database when available #7849

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

drinkcat
Copy link
Contributor

There are a lot of challenges around timezone handling, and IMHO some refactoring will be needed.

Decided to focus on #7504 for now: we use a system timezone database is available through tzfile.

Also, added some tests, and a bunch of TODO in the code to give pointers to the next person fixing this (maybe me...).

Shrinks date/ls by about 2 megabytes on Linux:

cargo build -r -p uu_date && cargo build -r -p uu_ls && ls -l target/release/{date,ls}
-rwxr-xr-x 2 drinkcat drinkcat 4828144 Apr 27 13:57 target/release/date
-rwxr-xr-x 2 drinkcat drinkcat 3769336 Apr 27 13:57 target/release/ls
## after
-rwxr-xr-x 2 drinkcat drinkcat 2949080 Apr 27 13:58 target/release/date
-rwxr-xr-x 2 drinkcat drinkcat 1894176 Apr 27 13:59 target/release/ls

uucore: custom_tz_fmt: test: Setting TZ is not reliable

Not 100% clear why, but setting TZ in CI is not fully reliable,
so let's pass a parameter instead.

Cross.toml: Install tzdata in container

Linux tests require that now, as we now assume /usr/share/zoneinfo
is present.

uucore: custom_tz_fmt: Set an embed_tz feature based on platform

Instead of repeating complex logic (and its inverse), set a
feature in build.rs.

fuzz/Cargo.lock: Update

uucore: Use tzfile instead of chrono-tz

Unless we're on Windows.

Fixes #7504, reduces ls/date executable size by about 2 MB.

uucore: custom_tz_fmt: use format("%Z")

uucore: custom_tz_fmt: Add abbreviation tests

This function will still need some love, but at least with this,
we have basic tests.

uucore: custom_tz_fmt: Improve TZ environment variable handling

iana_time_zone::get_timezone doesn't cover all cases, so we need
to handle some of the timezone parsing manually.

Also add a bunch of TODO, this function is by no means complete.

(change similar to #7597 for this file)

iana_time_zone::get_timezone doesn't cover all cases, so we need
to handle some of the timezone parsing manually.

Also add a bunch of TODO, this function is by no means complete.

(change similar to uutils#7597 for this file)
This function will still need some love, but at least with this,
we have basic tests.
Unless we're on Windows.

Fixes uutils#7504, reduces ls/date executable size by about 2 MB.
Instead of repeating complex logic (and its inverse), set a
feature in build.rs.
Linux tests require that now, as we now assume /usr/share/zoneinfo
is present.
Not 100% clear why, but setting TZ in CI is not fully reliable,
so let's pass a parameter instead.
///
/// We need this function even for local dates as chrono(_tz) does not provide a
/// way to convert Local to a fully specified timezone with abbreviation
/// (<https://github.com/chronotope/chrono-tz/issues/13>).
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should link to chronotope/chrono#960 here, instead.

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@BurntSushi
Copy link

BurntSushi commented Apr 27, 2025

Are you aware of jiff? (I am its author.) It does all of this for you. I did a quick look and I believe it fixes all of your todos as well.

@sylvestre
Copy link
Contributor

@BurntSushi of course, who does not know you, your great work and jiff ? :)
it is just our work predates jiff but I have been looking at your work and I think it would be indeed a nice work to migrate to jiff!

@BurntSushi
Copy link

Yeah it just looks like y'all are fighting with Chrono quite a bit.

I'm also working on making some extension points to Jiff's strptime and strftime to help with locale formatting (which is intended to be implemented with ICU4X).

@drinkcat
Copy link
Contributor Author

@BurntSushi thanks, looks like we've been fighting similar problems ,-) I'll try playing with jiff then, will let you know how that goes.

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.

Significant increase of the ls & date binary size when built independantly
3 participants