date: improve timezone handling and UTC mode support by jadijadi · Pull Request #7597 · uutils/coreutils

jadijadi · 2025-03-28T00:38:07Z

Add proper UTC mode handling with -u flag
Improve TZ environment variable support
Add more TZ tests

github-actions · 2025-03-28T01:12:30Z

GNU testsuite comparison:

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

cakebaker · 2025-03-28T09:18:37Z

Can you please run cargo clippy? The corresponding checks fail in the CI :|

jadijadi · 2025-03-28T16:01:21Z

Can you please run cargo clippy? The corresponding checks fail in the CI :|

Done and Thanks.

github-actions · 2025-03-28T16:49:19Z

GNU testsuite comparison:

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

drinkcat · 2025-03-29T14:33:22Z

tests/by-util/test_date.rs

 }
+
+#[test]
+fn test_date_tz_utc() {


Can you do a blank TZ too?

Thanks for the comments. there was one test empty TZ and I add another one with .arg("@0") as an additional test.

drinkcat · 2025-03-29T14:38:23Z

tests/by-util/test_date.rs

+        .arg("+%Z")
+        .succeeds()
+        .stdout_only("UTC\n");
+}


Also, it'd be nice to have tests that actually test that the output time is shifted properly (since that's the issue in #7498). You should pass a fixed datetime as a parameter to date and check that the output is exactly what you expect.

Note that adding a full test for #7498 might be tricky, as it sort of relies on the localtime not being UTC (not sure how our CI runners are configured...)

Dear @drinkcat , I've added a test for this but it was not working correctly on CI/CD. Its kind of complicated. Will try again tomorrow.
Side note: I also did minor changes to other tests to respect the Day Light Saving changes.

jadijadi · 2025-04-01T01:48:17Z

Side note: it seems our cargo fmt disagree on some minor stuff. I'm on +stable.

github-actions · 2025-04-01T02:21:46Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

- Add proper UTC mode handling with -u flag - Improve TZ environment variable support - Add more TZ tests FIXES uutils#7497 FIXES uutils#7498

github-actions · 2025-04-01T03:10:00Z

GNU testsuite comparison:

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

drinkcat

Sorry for the delay, started looking at other timezone stuff and realized this was pending.

Still not 100% sure to understand everything, trying to simplify things where possible.

drinkcat · 2025-04-05T09:21:44Z

src/uu/date/src/date.rs

            match date {
                Ok(date) => {
-                    let format_string = custom_time_format(format_string);
+                    let format_string = if should_use_utc(settings.utc) {


Wouldn't it be better to move that logic to custom_time_format?

drinkcat · 2025-04-05T09:25:08Z

src/uu/date/src/date.rs

+/// Check if we should use UTC time based on the utc flag and TZ environment variable
+fn should_use_utc(utc: bool) -> bool {
+    // Either the -u flag is set or the TZ environment variable is empty
+    utc || matches!(std::env::var("TZ"), Ok(tz) if tz.is_empty())


Maybe this is slightly nicer: std::env::var("TZ").is_ok_and(|tz| tz.is_empty())

drinkcat · 2025-04-05T09:26:51Z

src/uu/date/src/date.rs


    let date_source = if let Some(date) = matches.get_one::<String>(OPT_DATE) {
-        let ref_time = Local::now();
+        let ref_time = get_current_time(matches.get_flag(OPT_UNIVERSAL));


I'd have a mild preference if you did this:

let utc = matches.get_flag(OPT_UNIVERSAL);

And used utc here and below when initializing settings.

drinkcat · 2025-04-05T09:32:15Z

src/uu/date/src/date.rs

-            now.with_timezone(&now.offset().fix())
+        let now = get_current_time(settings.utc);
+        // Convert to FixedOffset for consistent timezone handling
+        let now_fixed = if should_use_utc(settings.utc) {


Why aren't you moving the now_fixed logic into get_current_time? Seems like you output something with the wrong timezone in get_current_time that you now need to adjust again?

drinkcat · 2025-04-05T13:07:45Z

@jadijadi looking at this a bit more... I feel like this would be better fixed in chrono, not here (I was looking at #7504 and couldn't really fully grasp why uucore handling code needs to be so complicated...).

See chronotope/chrono#1690 . If you're interested to submit a patch for chrono I'm happy to leave it to you, just let me know.

If we have problems getting this fix into chrono (or if getting a new release takes time), we could think of some wrapper in uucore that fixes this for all the apps that handle dates (not just date, but also ls and du).

drinkcat · 2025-04-05T13:24:47Z

Argh, maybe I'm wrong, I think we still need some custom logic to handle timezone passed in TZ (i.e. to be able to read the abbreviation from the timezone database).

But... I think we can make things easier by having a wrapper function that returns something like this:

date.with_timezone(&tz)

(where tz comes from the code we already have in timezone_abbreviation)

sylvestre · 2025-04-17T19:51:13Z

@jadijadi are you still working on it ? thanks :)

drinkcat · 2025-04-17T20:05:04Z

(btw the empty TZ case is fixed in chrono, but we need to wait for a new release: chronotope/chrono#1691)

jadijadi · 2025-04-18T20:06:20Z

@jadijadi are you still working on it ? thanks :)

yes. had a busy week. will work on it during this long weekend

sylvestre

waiting for update

jadijadi · 2025-04-23T00:37:37Z

waiting for update

I'm a bit lost on this. As @drinkcat mentioned, its better to have most of these fixes in chrono. And they are already fixed there. So maybe its better to close this PR and wait for chrono changes to be merged and work on this again based on what they have.

I'm still having busy work days and wont have time to go deeper and see what is changed in upcomming days. If anyone else wants to finish this, I will be happy.

drinkcat · 2025-04-23T13:02:06Z

I... also lost all context... and if it's not clear from my comments, I was also a bit confused a few weeks ago when I tried to look at this ,-)

I pinged chrono maintainer here: TimeZone::from_posix_tz: Treat empty TZ variable as UTC chronotope/chrono#1691, would be good to at least have the empty TZ covered.
I'll resume my efforts on Significant increase of the ls & date binary size when built independantly #7504, hopefully in the process I'll understand better what needs to be done here (I think it's technically orthogonal, but some of the refactoring will probably conflict with this PR)

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)

jadijadi · 2025-06-10T23:06:15Z

I will close this because most fixed are already done in chronotope/chrono#1691 . Will open another PR if I work on the remaining issues.

jadijadi force-pushed the timezone_fixes branch from 2de96bd to 5792dd7 Compare March 28, 2025 16:00

drinkcat reviewed Mar 29, 2025

View reviewed changes

jadijadi force-pushed the timezone_fixes branch 3 times, most recently from 330bb67 to 7184adc Compare April 1, 2025 01:47

date: improve timezone handling and UTC mode support

63cf6bf

- Add proper UTC mode handling with -u flag - Improve TZ environment variable support - Add more TZ tests FIXES uutils#7497 FIXES uutils#7498

jadijadi force-pushed the timezone_fixes branch from 7184adc to 63cf6bf Compare April 1, 2025 02:34

drinkcat mentioned this pull request Apr 5, 2025

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

Closed

drinkcat reviewed Apr 5, 2025

View reviewed changes

drinkcat mentioned this pull request Apr 5, 2025

Significant increase of the ls & date binary size when built independantly #7504

Open

sylvestre requested changes Apr 21, 2025

View reviewed changes

drinkcat mentioned this pull request Apr 27, 2025

Use system timezone database when available #7849

Closed

drinkcat mentioned this pull request May 7, 2025

date/ls: Switch from chrono to jiff #7894

Merged

jadijadi closed this Jun 10, 2025

Uh oh!

Conversation

jadijadi commented Mar 28, 2025

Uh oh!

github-actions bot commented Mar 28, 2025

Uh oh!

cakebaker commented Mar 28, 2025

Uh oh!

jadijadi commented Mar 28, 2025

Uh oh!

github-actions bot commented Mar 28, 2025

Uh oh!

drinkcat Mar 29, 2025

Choose a reason for hiding this comment

Uh oh!

jadijadi Apr 1, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat Mar 29, 2025

Choose a reason for hiding this comment

Uh oh!

jadijadi Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jadijadi commented Apr 1, 2025

Uh oh!

github-actions bot commented Apr 1, 2025

Uh oh!

github-actions bot commented Apr 1, 2025

Uh oh!

drinkcat left a comment

Choose a reason for hiding this comment

Uh oh!

drinkcat Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat Apr 5, 2025

Choose a reason for hiding this comment

Uh oh!

drinkcat commented Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

drinkcat commented Apr 5, 2025

Uh oh!

sylvestre commented Apr 17, 2025

Uh oh!

drinkcat commented Apr 17, 2025

Uh oh!

jadijadi commented Apr 18, 2025

Uh oh!

sylvestre left a comment

Choose a reason for hiding this comment

Uh oh!

jadijadi commented Apr 23, 2025

Uh oh!

drinkcat commented Apr 23, 2025

Uh oh!

jadijadi commented Jun 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jadijadi Apr 1, 2025 •

edited

Loading

drinkcat commented Apr 5, 2025 •

edited

Loading