date/ls: Switch from chrono to jiff #7894

drinkcat · 2025-05-07T02:37:47Z

jiff handling of timezones makes our life so much easier. Also, jiff provides
a convenient feature to only embed the timezone database when necessary (e.g. Windows).

Fixes date: alphabetic time zone abbreviation (%Z) prints local timezone, not the one specified in TZ #7497, date: Does not correctly use UTC when TZ is empty #7498, date: Timezone abbreviation conversion should take into account the date to be parsed #7659.
Helps with Significant increase of the ls & date binary size when built independantly #7504, but still adds an extra ~300k compared to a chrono-only solution
(that did not fix all issues, FWIW):

-rwxr-xr-x 2 drinkcat drinkcat 4828144 Apr 27 13:57 target/release/date ## HEAD
-rwxr-xr-x 2 drinkcat drinkcat 3279432 Apr 28 15:42 target/release/date ## jiff
-rwxr-xr-x 2 drinkcat drinkcat 2949080 Apr 27 13:58 target/release/date ## with #7849

Part of the tests taken from #7597 (@jadijadi FYI), then refactored on top of those to add more test cases.

@BurntSushi thanks for your support!

--

Cross.toml: Install tzdata in container

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

test_date: Extend coverage to a lot more timezones

Also test %z/%Z formats.

date: Add more TZ tests

[drinkcat: separated test changes]

ls: switch to lenient formating configuration

ls: Avoid additional String creation/copy in display_date

From code provided in #7852 by @BurntSushi.

Depending on the benchmarks, there is still a small performance
difference (~4%) vs main, but it's seen mostly on small trees
getting printed repeatedly, which is probably not a terribly
interesting use case.

ls: cache recent time threshold in jiff implementation

ls: convert to jiff

Revert "ls: Optimize time formatting"

This reverts commit fc6b896.

This also reverts the change from new to new_lenient, we'll
recover that later as part of the jiff conversion.

date: switch from chrono to jiff

Also adds cargo dependency.

github-actions · 2025-05-07T03:11:26Z

GNU testsuite comparison:

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

Copilot

Pull Request Overview

This PR switches the time handling in both the date and ls utilities from chrono to jiff to improve timezone handling and offer optional timezone database embedding. Key changes include updated tests for expanded timezone coverage (in test_date.rs), a refactoring of ls’s time formatting logic to remove the legacy TimeStyler in favor of jiff’s API (in ls.rs), and corresponding dependency updates in the Cargo.toml files and related build configurations.

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/by-util/test_date.rs	Updated testing of timezone handling with various locales and UTC flags
src/uu/ls/src/ls.rs	Refactored time formatting code: removed TimeStyler and updated display_date usage
src/uu/ls/Cargo.toml	Added jiff dependency with new features
src/uu/date/src/date.rs	Migrated date parsing/formatting from chrono to jiff, with updated error handling
src/uu/date/Cargo.toml	Added jiff dependency with appropriate feature flags
Cross.toml	Added pre-build step to install tzdata
Cargo.toml	Updated global dependencies to include jiff’s libraries
.github/workflows/CICD.yml	Removed redundant Cross.toml creation step

Comments suppressed due to low confidence (1)

src/uu/ls/src/ls.rs:2889

[nitpick] Now that display_date writes directly to the provided buffer and returns a UResult, its naming might be misleading. Consider renaming it (e.g. to write_date) to clarify that it modifies the buffer in place.

output_display.extend(display_date(md, config, state, &mut output_display)?);

Copilot · 2025-05-09T08:54:04Z

src/uu/date/src/date.rs

+    match parse_datetime::parse_datetime(s.as_ref()) {
+        Ok(date) => {
+            let timestamp =
+                Timestamp::new(date.timestamp(), date.timestamp_subsec_nanos() as i32).unwrap();


Using unwrap() here may lead to a panic if Timestamp::new ever fails. Consider handling the potential error explicitly to make the code more robust.

Suggested change

Timestamp::new(date.timestamp(), date.timestamp_subsec_nanos() as i32).unwrap();

Timestamp::new(date.timestamp(), date.timestamp_subsec_nanos() as i32)?;

for once, it might make sense ?! :)

Ah, yeah, it does make some sort of sense... But:

The suggestion doesn't build:

? couldn't convert the error to (std::string::String, ParseDateTimeError)

The error cannot really happen anyway:

This returns an error if the given components would correspond to an instant outside the supported range. Also, nanosecond is limited to the range -999,999,999..=999,999,999.

So yes, technically I could handle the error and convert it, but since timestamp() and timestamp_subsec_nanos() both come from a good library (jiff), and go into another good library (chrono), I think we can just unwrap.

And we can get rid of this conversion once we switch parse_datetime to jiff (I haven't found time to look into that yet...).

drinkcat · 2025-05-25T14:15:45Z

Rebased, and also cleaned up custom_tz_fmt that's not needed anymore.

I'll try to continue this conversion (maybe will look at parse_datetime next).

github-actions · 2025-05-25T14:57:07Z

GNU testsuite comparison:

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

Also adds cargo dependency.

This reverts commit fc6b896. This also reverts the change from new to new_lenient, we'll recover that later as part of the jiff conversion.

@BurntSushi

From code provided in uutils#7852 by @BurntSushi. Depending on the benchmarks, there is _still_ a small performance difference (~4%) vs main, but it's seen mostly on small trees getting printed repeatedly, which is probably not a terribly interesting use case.

[drinkcat: separated test changes]

Also test %z/%Z formats.

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

Nobody needs it anymore.

drinkcat · 2025-05-29T12:03:07Z

Rebased. @sylvestre do you think we could go ahead with this part of the conversion? We get the most bang for the buck
with this one: Smaller binary, and correct timezone handling.

I'd like to look into parse_datetime as well, but as you could see, I hit other issues, and spent a silly amount of time trying to figure out what I did wrong -- while the original version was already broken ,-P

github-actions · 2025-05-29T12:45:45Z

GNU testsuite comparison:

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

tests/by-util/test_date.rs

src/uu/ls/src/ls.rs

Also, the comment does not fully apply anymore, so we can leave it more open-ended to figure out how to support locale.

Using the current time requires a bit of care, but it's nice to have a test that doesn't use a fixed date as input.

github-actions · 2025-06-01T18:48:31Z

GNU testsuite comparison:

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

cakebaker · 2025-06-02T07:36:39Z

Good work, thanks!

sylvestre · 2025-06-02T07:44:31Z

+1

sylvestre requested a review from Copilot May 9, 2025 08:53

Copilot AI reviewed May 9, 2025

View reviewed changes

drinkcat force-pushed the jiff-date-ls branch from f3db434 to ab42c92 Compare May 25, 2025 14:13

drinkcat and others added 10 commits May 29, 2025 14:00

date: switch from chrono to jiff

301e33c

Also adds cargo dependency.

Revert "ls: Optimize time formatting"

07c9205

This reverts commit fc6b896. This also reverts the change from new to new_lenient, we'll recover that later as part of the jiff conversion.

ls: convert to jiff

fc947ec

ls: cache recent time threshold in jiff implementation

c599363

ls: switch to lenient formating configuration

6031de5

date: Add more TZ tests

d1525e2

[drinkcat: separated test changes]

test_date: Extend coverage to a lot more timezones

dadda0d

Also test %z/%Z formats.

Cross.toml: Install tzdata in container

eb5fc4c

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

uucore: Remove custom_tz_fmt, cleanup dependencies

986bdf5

Nobody needs it anymore.

drinkcat force-pushed the jiff-date-ls branch from ab42c92 to 986bdf5 Compare May 29, 2025 12:00

cakebaker reviewed May 30, 2025

View reviewed changes

tests/by-util/test_date.rs Outdated Show resolved Hide resolved

cakebaker reviewed May 30, 2025

View reviewed changes

src/uu/ls/src/ls.rs Outdated Show resolved Hide resolved

drinkcat added 2 commits June 1, 2025 19:36

ls: Simplify TimeStyle::format

5d75e28

Also, the comment does not fully apply anymore, so we can leave it more open-ended to figure out how to support locale.

test_date: Expand on test_date_utc_time

66d1e8a

Using the current time requires a bit of care, but it's nice to have a test that doesn't use a fixed date as input.

drinkcat force-pushed the jiff-date-ls branch from fb884b4 to 66d1e8a Compare June 1, 2025 18:04

cakebaker merged commit dfc2e24 into uutils:main Jun 2, 2025
73 of 74 checks passed

This was referenced Jun 2, 2025

date: Does not correctly use UTC when TZ is empty #7498

Closed

date: Timezone abbreviation conversion should take into account the date to be parsed #7659

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

date/ls: Switch from chrono to jiff #7894

date/ls: Switch from chrono to jiff #7894

Uh oh!

drinkcat commented May 7, 2025

Uh oh!

github-actions bot commented May 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 9, 2025

Uh oh!

sylvestre May 9, 2025

Uh oh!

drinkcat May 14, 2025

Uh oh!

drinkcat commented May 25, 2025

Uh oh!

github-actions bot commented May 25, 2025

Uh oh!

drinkcat commented May 29, 2025

Uh oh!

github-actions bot commented May 29, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 1, 2025

Uh oh!

Uh oh!

cakebaker commented Jun 2, 2025

Uh oh!

sylvestre commented Jun 2, 2025

Uh oh!

Uh oh!

	Timestamp::new(date.timestamp(), date.timestamp_subsec_nanos() as i32).unwrap();
	Timestamp::new(date.timestamp(), date.timestamp_subsec_nanos() as i32)?;

Uh oh!

date/ls: Switch from chrono to jiff #7894

date/ls: Switch from chrono to jiff #7894

Uh oh!

Conversation

drinkcat commented May 7, 2025

Cross.toml: Install tzdata in container

test_date: Extend coverage to a lot more timezones

date: Add more TZ tests

ls: switch to lenient formating configuration

ls: Avoid additional String creation/copy in display_date

ls: cache recent time threshold in jiff implementation

ls: convert to jiff

Revert "ls: Optimize time formatting"

date: switch from chrono to jiff

Uh oh!

github-actions bot commented May 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 9, 2025

Choose a reason for hiding this comment

Uh oh!

sylvestre May 9, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat May 14, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat commented May 25, 2025

Uh oh!

github-actions bot commented May 25, 2025

Uh oh!

drinkcat commented May 29, 2025

Uh oh!

github-actions bot commented May 29, 2025

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jun 1, 2025

Uh oh!

Uh oh!

cakebaker commented Jun 2, 2025

Uh oh!

sylvestre commented Jun 2, 2025

Uh oh!

Uh oh!