Skip to content

date: improve timezone handling and UTC mode support #7597

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
Open

date: improve timezone handling and UTC mode support #7597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 48 additions & 10 deletions src/uu/date/src/date.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// spell-checker:ignore (chrono) Datelike Timelike ; (format) DATEFILE MMDDhhmm ; (vars) datetime datetimes

use chrono::format::{Item, StrftimeItems};
use chrono::{DateTime, FixedOffset, Local, Offset, TimeDelta, Utc};
use chrono::{DateTime, FixedOffset, Local, TimeDelta, Utc};
#[cfg(windows)]
use chrono::{Datelike, Timelike};
use clap::{Arg, ArgAction, Command};
Expand Down Expand Up @@ -136,6 +136,24 @@ impl From<&str> for Rfc3339Format {
}
}

/// 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())
Copy link
Contributor

Choose a reason for hiding this comment

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

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

}

/// Get the current time, considering the utc flag and TZ environment variable
fn get_current_time(utc: bool) -> DateTime<Local> {
if should_use_utc(utc) {
// When -u flag is used or TZ is empty, we should use UTC time
// Simply convert UTC to Local without adjusting the time value
Utc::now().into()
} else {
// Use local time when neither condition is met
Local::now()
}
}

#[uucore::main]
#[allow(clippy::cognitive_complexity)]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
Expand Down Expand Up @@ -167,7 +185,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
};

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));
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

if let Ok(new_time) = parse_datetime::parse_datetime_at_date(ref_time, date.as_str()) {
let duration = new_time.signed_duration_since(ref_time);
DateSource::Human(duration)
Expand Down Expand Up @@ -212,11 +230,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
return set_system_datetime(date);
} else {
// Get the current time, either in the local time zone or UTC.
let now: DateTime<FixedOffset> = if settings.utc {
let now = Utc::now();
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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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?

// When -u flag is used or TZ is empty, use actual UTC time with zero offset
let utc_now = Utc::now();
utc_now.with_timezone(&FixedOffset::east_opt(0).unwrap())
} else {
let now = Local::now();
// Otherwise use the local timezone's offset
now.with_timezone(now.offset())
};

Expand All @@ -230,7 +251,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
DateSource::Human(relative_time) => {
// Double check the result is overflow or not of the current_time + relative_time
// it may cause a panic of chrono::datetime::DateTime add
match now.checked_add_signed(relative_time) {
match now_fixed.checked_add_signed(relative_time) {
Some(date) => {
let iter = std::iter::once(Ok(date));
Box::new(iter)
Expand Down Expand Up @@ -262,7 +283,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
Box::new(iter)
}
DateSource::Now => {
let iter = std::iter::once(Ok(now));
let iter = std::iter::once(Ok(now_fixed));
Box::new(iter)
}
};
Expand All @@ -273,7 +294,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
for date in dates {
match date {
Ok(date) => {
let format_string = custom_time_format(format_string);
let format_string = if should_use_utc(settings.utc) {
Copy link
Contributor

Choose a reason for hiding this comment

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

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

// When -u flag is used or TZ is empty, replace %Z with UTC directly
format_string.replace("%Z", "UTC")
} else {
custom_time_format(format_string)
};

// Refuse to pass this string to chrono as it is crashing in this crate
if format_string.contains("%#z") {
return Err(USimpleError::new(
Expand All @@ -290,10 +317,21 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
format!("invalid format {}", format_string.replace("%f", "%N")),
));
}
let formatted = date

// When -u flag is used or TZ is empty, ensure we format using UTC time
let date_to_format = if should_use_utc(settings.utc) {
// Convert the date to UTC to ensure correct time display
date.with_timezone(&Utc)
.with_timezone(&FixedOffset::east_opt(0).unwrap())
} else {
date
};

let formatted = date_to_format
.format_with_items(format_items)
.to_string()
.replace("%f", "%N");

println!("{formatted}");
}
Err((input, _err)) => show!(USimpleError::new(
Expand Down
10 changes: 7 additions & 3 deletions src/uucore/src/lib/features/custom_tz_fmt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ use iana_time_zone::get_timezone;
///
/// For example, "UTC" or "CET" or "PDT"
fn timezone_abbreviation() -> String {
// Check if we're in UTC mode (either through TZ=UTC0 or -u flag)
let tz = match std::env::var("TZ") {
// TODO Support other time zones...
// If TZ is set to UTC0 or empty, use UTC
Ok(s) if s == "UTC0" || s.is_empty() => Tz::Etc__UTC,
_ => match get_timezone() {
Ok(tz_str) => tz_str.parse().unwrap(),
// If TZ is set to any other value, try to parse it
Ok(tz_str) => tz_str.parse().unwrap_or(Tz::Etc__UTC),
// If TZ is not set, try to get the system timezone
Err(_) => match get_timezone() {
Ok(tz_str) => tz_str.parse().unwrap_or(Tz::Etc__UTC),
Err(_) => Tz::Etc__UTC,
},
};
Expand Down
110 changes: 110 additions & 0 deletions tests/by-util/test_date.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,3 +492,113 @@ fn test_date_empty_tz() {
.succeeds()
.stdout_only("UTC\n");
}

#[test]
fn test_date_tz_utc() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you do a blank TZ too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

new_ucmd!()
.env("TZ", "UTC0")
.arg("+%Z")
.succeeds()
.stdout_only("UTC\n");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

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...)

Copy link
Contributor Author

@jadijadi jadijadi Apr 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.


#[test]
fn test_date_tz_berlin() {
new_ucmd!()
.env("TZ", "Europe/Berlin")
.arg("+%Z")
.succeeds()
.stdout_matches(&Regex::new(r"^(CET|CEST)\n$").unwrap());
}

#[test]
fn test_date_tz_vancouver() {
new_ucmd!()
.env("TZ", "America/Vancouver")
.arg("+%Z")
.succeeds()
.stdout_matches(&Regex::new(r"^(PDT|PST)\n$").unwrap());
}

#[test]
fn test_date_tz_invalid() {
new_ucmd!()
.env("TZ", "Invalid/Timezone")
.arg("+%Z")
.succeeds()
.stdout_only("UTC\n");
}

#[test]
fn test_date_tz_with_format() {
new_ucmd!()
.env("TZ", "Europe/Berlin")
.arg("+%Y-%m-%d %H:%M:%S %Z")
.succeeds()
.stdout_matches(
&Regex::new(r"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} (CET|CEST)\n$").unwrap(),
);
}

#[test]
fn test_date_tz_with_utc_flag() {
new_ucmd!()
.env("TZ", "Europe/Berlin")
.arg("-u")
.arg("+%Z")
.succeeds()
.stdout_only("UTC\n");
}

#[test]
fn test_date_tz_with_date_string() {
new_ucmd!()
.env("TZ", "Asia/Tokyo")
.arg("-d")
.arg("2024-01-01 12:00:00")
.arg("+%Y-%m-%d %H:%M:%S %Z")
.succeeds()
.stdout_only("2024-01-01 12:00:00 JST\n");
}

#[test]
fn test_date_tz_with_relative_time() {
new_ucmd!()
.env("TZ", "America/Vancouver")
.arg("-d")
.arg("1 hour ago")
.arg("+%Y-%m-%d %H:%M:%S %Z")
.succeeds()
.stdout_matches(&Regex::new(r"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} PDT\n$").unwrap());
}

#[test]
fn test_date_utc_time() {
// Test that -u flag shows correct UTC time
new_ucmd!().arg("-u").arg("+%H:%M").succeeds();

// Test that -u flag shows UTC timezone
new_ucmd!()
.arg("-u")
.arg("+%Z")
.succeeds()
.stdout_only("UTC\n");

// Test that -u flag with specific timestamp shows correct UTC time
new_ucmd!()
.arg("-u")
.arg("-d")
.arg("@0")
.succeeds()
.stdout_only("Thu Jan 1 00:00:00 UTC 1970\n");
}

#[test]
fn test_date_empty_tz_time() {
new_ucmd!()
.env("TZ", "")
.arg("-d")
.arg("@0")
.succeeds()
.stdout_only("Thu Jan 1 00:00:00 UTC 1970\n");
}
Loading