Repository: uutils/coreutils (language: rust)

Issue:
ls: --time-style with format string %Z gives timezone offset instead of name
Environment: Ubuntu 20.04, uutils `main` branch (git commit 00d186606035876ad47afb82ad7d109a1868201b), GNU coreutils v8.30

Steps to reproduce:
```
TZ=UTC0 touch -d '1970-07-08 09:10:11' f
TZ=UTC0 ls -l --time-style="+%Z" f
```

What happens now: uutils `ls` shows a numeric offset for the timezone
```
-rw-rw-r-- 1 jeffrey jeffrey 0 +00:00 f
```

What I expected to happen: GNU `ls` shows the timezone name
```
-rw-rw-r-- 1 jeffrey jeffrey 0 UTC f
```

Notes: This causes a failure in the GNU test file `tests/ls/time-style.sh`.

We use the `chrono` package for time formatting. It seems that they have an open issue about this here https://github.com/chronotope/chrono/issues/288


Discussion / hints:
Related to #5164

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 1b2a67c13aa..29b14b9a820 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2871,9 +2871,11 @@ version = "0.0.29"
 dependencies = [
  "ansi-width",
  "chrono",
+ "chrono-tz",
  "clap",
  "glob",
  "hostname",
+ "iana-time-zone",
  "lscolors",
  "number_prefix",
  "once_cell",
diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml
index 17cef9b8aa4..0b60009e65b 100644
--- a/src/uu/ls/Cargo.toml
+++ b/src/uu/ls/Cargo.toml
@@ -18,13 +18,17 @@ path = "src/ls.rs"
 
 [dependencies]
 ansi-width = { workspace = true }
-clap = { workspace = true, features = ["env"] }
 chrono = { workspace = true }
-number_prefix = { workspace = true }
-uutils_term_grid = { workspace = true }
-terminal_size = { workspace = true }
+chrono-tz = { workspace = true }
+clap = { workspace = true, features = ["env"] }
 glob = { workspace = true }
+hostname = { workspace = true }
+iana-time-zone = { workspace = true }
 lscolors = { workspace = true }
+number_prefix = { workspace = true }
+once_cell = { workspace = true }
+selinux = { workspace = true, optional = true }
+terminal_size = { workspace = true }
 uucore = { workspace = true, features = [
   "colors",
   "entries",
@@ -34,9 +38,7 @@ uucore = { workspace = true, features = [
   "quoting-style",
   "version-cmp",
 ] }
-once_cell = { workspace = true }
-selinux = { workspace = true, optional = true }
-hostname = { workspace = true }
+uutils_term_grid = { workspace = true }
 
 [[bin]]
 name = "ls"
diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs
index 994eabc21b6..9aaa0d0a4e3 100644
--- a/src/uu/ls/src/ls.rs
+++ b/src/uu/ls/src/ls.rs
@@ -5,19 +5,9 @@
 
 // spell-checker:ignore (ToDO) somegroup nlink tabsize dired subdired dtype colorterm stringly
 
-use clap::{
-    builder::{NonEmptyStringValueParser, PossibleValue, ValueParser},
-    crate_version, Arg, ArgAction, Command,
-};
-use glob::{MatchOptions, Pattern};
-use lscolors::LsColors;
-
-use ansi_width::ansi_width;
-use std::{cell::OnceCell, num::IntErrorKind};
-use std::{collections::HashSet, io::IsTerminal};
-
 #[cfg(windows)]
 use std::os::windows::fs::MetadataExt;
+use std::{cell::OnceCell, num::IntErrorKind};
 use std::{
     cmp::Reverse,
     error::Error,
@@ -34,7 +24,20 @@ use std::{
     os::unix::fs::{FileTypeExt, MetadataExt},
     time::Duration,
 };
+use std::{collections::HashSet, io::IsTerminal};
+
+use ansi_width::ansi_width;
+use chrono::{DateTime, Local, TimeDelta, TimeZone, Utc};
+use chrono_tz::{OffsetName, Tz};
+use clap::{
+    builder::{NonEmptyStringValueParser, PossibleValue, ValueParser},
+    crate_version, Arg, ArgAction, Command,
+};
+use glob::{MatchOptions, Pattern};
+use iana_time_zone::get_timezone;
+use lscolors::LsColors;
 use term_grid::{Direction, Filling, Grid, GridOptions};
+
 use uucore::error::USimpleError;
 use uucore::format::human::{human_readable, SizeFormat};
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
@@ -67,10 +70,12 @@ use uucore::{
     version_cmp::version_cmp,
 };
 use uucore::{help_about, help_section, help_usage, parse_glob, show, show_error, show_warning};
+
 mod dired;
 use dired::{is_dired_arg_present, DiredOutput};
 mod colors;
 use colors::{color_name, StyleManager};
+
 #[cfg(not(feature = "selinux"))]
 static CONTEXT_HELP_TEXT: &str = "print any security context of each file (not enabled)";
 #[cfg(feature = "selinux")]
@@ -334,6 +339,58 @@ enum TimeStyle {
     Format(String),
 }
 
+/// Whether the given date is considered recent (i.e., in the last 6 months).
+fn is_recent(time: DateTime<Local>) -> bool {
+    // According to GNU a Gregorian year has 365.2425 * 24 * 60 * 60 == 31556952 seconds on the average.
+    time + TimeDelta::try_seconds(31_556_952 / 2).unwrap() > Local::now()
+}
+
+/// Get the alphabetic abbreviation of the current timezone.
+///
+/// For example, "UTC" or "CET" or "PDT".
+fn timezone_abbrev() -> String {
+    let tz = match std::env::var("TZ") {
+        // TODO Support other time zones...
+        Ok(s) if s == "UTC0" || s.is_empty() => Tz::Etc__UTC,
+        _ => match get_timezone() {
+            Ok(tz_str) => tz_str.parse().unwrap(),
+            Err(_) => Tz::Etc__UTC,
+        },
+    };
+    let offset = tz.offset_from_utc_date(&Utc::now().date_naive());
+    offset.abbreviation().unwrap_or("UTC").to_string()
+}
+
+/// Format the given time according to a custom format string.
+fn custom_time_format(fmt: &str, time: DateTime<Local>) -> String {
+    // TODO Refactor the common code from `ls` and `date` for rendering dates.
+    // TODO - Revisit when chrono 0.5 is released. https://github.com/chronotope/chrono/issues/970
+    // GNU `date` uses `%N` for nano seconds, however the `chrono` crate uses `%f`.
+    let fmt = fmt.replace("%N", "%f").replace("%Z", &timezone_abbrev());
+    time.format(&fmt).to_string()
+}
+
+impl TimeStyle {
+    /// Format the given time according to this time format style.
+    fn format(&self, time: DateTime<Local>) -> String {
+        let recent = is_recent(time);
+        match (self, recent) {
+            (Self::FullIso, _) => time.format("%Y-%m-%d %H:%M:%S.%f %z").to_string(),
+            (Self::LongIso, _) => time.format("%Y-%m-%d %H:%M").to_string(),
+            (Self::Iso, true) => time.format("%m-%d %H:%M").to_string(),
+            (Self::Iso, false) => time.format("%Y-%m-%d ").to_string(),
+            // spell-checker:ignore (word) datetime
+            //In this version of chrono translating can be done
+            //The function is chrono::datetime::DateTime::format_localized
+            //However it's currently still hard to get the current pure-rust-locale
+            //So it's not yet implemented
+            (Self::Locale, true) => time.format("%b %e %H:%M").to_string(),
+            (Self::Locale, false) => time.format("%b %e  %Y").to_string(),
+            (Self::Format(e), _) => custom_time_format(e, time),
+        }
+    }
+}
+
 fn parse_time_style(options: &clap::ArgMatches) -> Result<TimeStyle, LsError> {
     let possible_time_styles = vec![
         "full-iso".to_string(),
@@ -3115,31 +3172,7 @@ fn get_time(md: &Metadata, config: &Config) -> Option<chrono::DateTime<chrono::L
 
 fn display_date(metadata: &Metadata, config: &Config) -> String {
     match get_time(metadata, config) {
-        Some(time) => {
-            //Date is recent if from past 6 months
-            //According to GNU a Gregorian year has 365.2425 * 24 * 60 * 60 == 31556952 seconds on the average.
-            let recent = time + chrono::TimeDelta::try_seconds(31_556_952 / 2).unwrap()
-                > chrono::Local::now();
-
-            match &config.time_style {
-                TimeStyle::FullIso => time.format("%Y-%m-%d %H:%M:%S.%f %z"),
-                TimeStyle::LongIso => time.format("%Y-%m-%d %H:%M"),
-                TimeStyle::Iso => time.format(if recent { "%m-%d %H:%M" } else { "%Y-%m-%d " }),
-                TimeStyle::Locale => {
-                    let fmt = if recent { "%b %e %H:%M" } else { "%b %e  %Y" };
-
-                    // spell-checker:ignore (word) datetime
-                    //In this version of chrono translating can be done
-                    //The function is chrono::datetime::DateTime::format_localized
-                    //However it's currently still hard to get the current pure-rust-locale
-                    //So it's not yet implemented
-
-                    time.format(fmt)
-                }
-                TimeStyle::Format(e) => time.format(e),
-            }
-            .to_string()
-        }
+        Some(time) => config.time_style.format(time),
         None => "???".into(),
     }
 }


Test patch:
diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs
index 6ef7ac93a2e..715f18a1eaf 100644
--- a/tests/by-util/test_ls.rs
+++ b/tests/by-util/test_ls.rs
@@ -5628,3 +5628,14 @@ fn test_non_unicode_names() {
         .succeeds()
         .stdout_is_bytes(b"\xC0.dir\n\xC0.file\n");
 }
+
+#[test]
+fn test_time_style_timezone_name() {
+    let re_custom_format = Regex::new(r"[a-z-]* \d* [\w.]* [\w.]* \d* UTC f\n").unwrap();
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("f");
+    ucmd.env("TZ", "UTC0")
+        .args(&["-l", "--time-style=+%Z"])
+        .succeeds()
+        .stdout_matches(&re_custom_format);
+}