Merge pull request #8396 from drinkcat/du-bigtime

`du`/`ls`: Unify file metadata time handling

src/uu/du/Cargo.toml

@@ -21,7 +21,7 @@ path = "src/du.rs"
 # For the --exclude & --exclude-from options
 glob = { workspace = true }
 clap = { workspace = true }
-uucore = { workspace = true, features = ["format", "parser", "time"] }
+uucore = { workspace = true, features = ["format", "fsext", "parser", "time"] }
 thiserror = { workspace = true }
 
 [target.'cfg(target_os = "windows")'.dependencies]

src/uu/du/src/du.rs

@@ -7,24 +7,21 @@ use clap::{Arg, ArgAction, ArgMatches, Command, builder::PossibleValue};
 use glob::Pattern;
 use std::collections::{HashMap, HashSet};
 use std::env;
-#[cfg(not(windows))]
 use std::fs::Metadata;
 use std::fs::{self, DirEntry, File};
 use std::io::{BufRead, BufReader, stdout};
 #[cfg(not(windows))]
 use std::os::unix::fs::MetadataExt;
 #[cfg(windows)]
-use std::os::windows::fs::MetadataExt;
-#[cfg(windows)]
 use std::os::windows::io::AsRawHandle;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::sync::mpsc;
 use std::thread;
-use std::time::{Duration, UNIX_EPOCH};
 use thiserror::Error;
 use uucore::display::{Quotable, print_verbatim};
 use uucore::error::{FromIo, UError, UResult, USimpleError, set_exit_code};
+use uucore::fsext::{MetadataTimeField, metadata_get_time};
 use uucore::line_ending::LineEnding;
 use uucore::locale::{get_message, get_message_with_args};
 use uucore::parser::parse_glob;
@@ -87,7 +84,7 @@ struct StatPrinter {
     threshold: Option<Threshold>,
     apparent_size: bool,
     size_format: SizeFormat,
-    time: Option<Time>,
+    time: Option<MetadataTimeField>,
     time_format: String,
     line_ending: LineEnding,
     summarize: bool,
@@ -101,13 +98,6 @@ enum Deref {
     None,
 }
 
-#[derive(Clone, Copy)]
-enum Time {
-    Accessed,
-    Modified,
-    Created,
-}
-
 #[derive(Clone)]
 enum SizeFormat {
     HumanDecimal,
@@ -123,14 +113,11 @@ struct FileInfo {
 
 struct Stat {
     path: PathBuf,
-    is_dir: bool,
     size: u64,
     blocks: u64,
     inodes: u64,
     inode: Option<FileInfo>,
-    created: Option<u64>,
-    accessed: u64,
-    modified: u64,
+    metadata: Metadata,
 }
 
 impl Stat {
@@ -157,69 +144,27 @@ impl Stat {
             fs::symlink_metadata(path)
         }?;
 
-        #[cfg(not(windows))]
-        {
-            let file_info = FileInfo {
-                file_id: metadata.ino() as u128,
-                dev_id: metadata.dev(),
-            };
+        let file_info = get_file_info(path, &metadata);
+        let blocks = get_blocks(path, &metadata);
 
-            Ok(Self {
-                path: path.to_path_buf(),
-                is_dir: metadata.is_dir(),
-                size: if metadata.is_dir() { 0 } else { metadata.len() },
-                blocks: metadata.blocks(),
-                inodes: 1,
-                inode: Some(file_info),
-                created: birth_u64(&metadata),
-                accessed: metadata.atime() as u64,
-                modified: metadata.mtime() as u64,
-            })
-        }
-
-        #[cfg(windows)]
-        {
-            let size_on_disk = get_size_on_disk(path);
-            let file_info = get_file_info(path);
-
-            Ok(Self {
-                path: path.to_path_buf(),
-                is_dir: metadata.is_dir(),
-                size: if metadata.is_dir() { 0 } else { metadata.len() },
-                blocks: size_on_disk / 1024 * 2,
-                inodes: 1,
-                inode: file_info,
-                created: windows_creation_time_to_unix_time(metadata.creation_time()),
-                accessed: windows_time_to_unix_time(metadata.last_access_time()),
-                modified: windows_time_to_unix_time(metadata.last_write_time()),
-            })
-        }
+        Ok(Self {
+            path: path.to_path_buf(),
+            size: if metadata.is_dir() { 0 } else { metadata.len() },
+            blocks,
+            inodes: 1,
+            inode: file_info,
+            metadata,
+        })
     }
 }
 
-#[cfg(windows)]
-/// <https://doc.rust-lang.org/std/os/windows/fs/trait.MetadataExt.html#tymethod.last_access_time>
-/// "The returned 64-bit value [...] which represents the number of 100-nanosecond intervals since January 1, 1601 (UTC)."
-/// "If the underlying filesystem does not support last access time, the returned value is 0."
-fn windows_time_to_unix_time(win_time: u64) -> u64 {
-    (win_time / 10_000_000).saturating_sub(11_644_473_600)
-}
-
-#[cfg(windows)]
-fn windows_creation_time_to_unix_time(win_time: u64) -> Option<u64> {
-    (win_time / 10_000_000).checked_sub(11_644_473_600)
-}
-
 #[cfg(not(windows))]
-fn birth_u64(meta: &Metadata) -> Option<u64> {
-    meta.created()
-        .ok()
-        .and_then(|t| t.duration_since(UNIX_EPOCH).ok())
-        .map(|e| e.as_secs())
+fn get_blocks(_path: &Path, metadata: &Metadata) -> u64 {
+    metadata.blocks()
 }
 
 #[cfg(windows)]
-fn get_size_on_disk(path: &Path) -> u64 {
+fn get_blocks(path: &Path, _metadata: &Metadata) -> u64 {
     let mut size_on_disk = 0;
 
     // bind file so it stays in scope until end of function
@@ -244,11 +189,19 @@ fn get_size_on_disk(path: &Path) -> u64 {
         }
     }
 
-    size_on_disk
+    size_on_disk / 1024 * 2
+}
+
+#[cfg(not(windows))]
+fn get_file_info(_path: &Path, metadata: &Metadata) -> Option<FileInfo> {
+    Some(FileInfo {
+        file_id: metadata.ino() as u128,
+        dev_id: metadata.dev(),
+    })
 }
 
 #[cfg(windows)]
-fn get_file_info(path: &Path) -> Option<FileInfo> {
+fn get_file_info(path: &Path, _metadata: &Metadata) -> Option<FileInfo> {
     let mut result = None;
 
     let Ok(file) = File::open(path) else {
@@ -306,7 +259,7 @@ fn du(
     seen_inodes: &mut HashSet<FileInfo>,
     print_tx: &mpsc::Sender<UResult<StatPrintInfo>>,
 ) -> Result<Stat, Box<mpsc::SendError<UResult<StatPrintInfo>>>> {
-    if my_stat.is_dir {
+    if my_stat.metadata.is_dir() {
         let read = match fs::read_dir(&my_stat.path) {
             Ok(read) => read,
             Err(e) => {
@@ -367,7 +320,7 @@ fn du(
                                 seen_inodes.insert(inode);
                             }
 
-                            if this_stat.is_dir {
+                            if this_stat.metadata.is_dir() {
                                 if options.one_file_system {
                                     if let (Some(this_inode), Some(my_inode)) =
                                         (this_stat.inode, my_stat.inode)
@@ -435,9 +388,6 @@ enum DuError {
     ])))]
     InvalidTimeStyleArg(String),
 
-    #[error("{}", get_message("du-error-invalid-time-arg"))]
-    InvalidTimeArg,
-
     #[error("{}", get_message_with_args("du-error-invalid-glob", HashMap::from([("error".to_string(), _0.to_string())])))]
     InvalidGlob(String),
 }
@@ -448,7 +398,6 @@ impl UError for DuError {
             Self::InvalidMaxDepthArg(_)
             | Self::SummarizeDepthConflict(_)
             | Self::InvalidTimeStyleArg(_)
-            | Self::InvalidTimeArg
             | Self::InvalidGlob(_) => 1,
         }
     }
@@ -577,11 +526,13 @@ impl StatPrinter {
     fn print_stat(&self, stat: &Stat, size: u64) -> UResult<()> {
         print!("{}\t", self.convert_size(size));
 
-        if let Some(time) = self.time {
-            let secs = get_time_secs(time, stat)?;
-            let time = UNIX_EPOCH + Duration::from_secs(secs);
-            uucore::time::format_system_time(&mut stdout(), time, &self.time_format, true)?;
-            print!("\t");
+        if let Some(md_time) = &self.time {
+            if let Some(time) = metadata_get_time(&stat.metadata, *md_time) {
+                uucore::time::format_system_time(&mut stdout(), time, &self.time_format, true)?;
+                print!("\t");
+            } else {
+                print!("???\t");
+            }
         }
 
         print_verbatim(&stat.path).unwrap();
@@ -697,12 +648,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     };
 
     let time = matches.contains_id(options::TIME).then(|| {
-        match matches.get_one::<String>(options::TIME).map(AsRef::as_ref) {
-            None | Some("ctime" | "status") => Time::Modified,
-            Some("access" | "atime" | "use") => Time::Accessed,
-            Some("birth" | "creation") => Time::Created,
-            _ => unreachable!("should be caught by clap"),
-        }
+        matches
+            .get_one::<String>(options::TIME)
+            .map_or(MetadataTimeField::Modification, |s| s.as_str().into())
     });
 
     let size_format = if matches.get_flag(options::HUMAN_READABLE) {
@@ -853,14 +801,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     Ok(())
 }
 
-fn get_time_secs(time: Time, stat: &Stat) -> Result<u64, DuError> {
-    match time {
-        Time::Modified => Ok(stat.modified),
-        Time::Accessed => Ok(stat.accessed),
-        Time::Created => stat.created.ok_or(DuError::InvalidTimeArg),
-    }
-}
-
 fn parse_time_style(s: Option<&str>) -> UResult<&str> {
     match s {
         Some(s) => match s {

src/uu/ls/Cargo.toml

@@ -33,6 +33,7 @@ uucore = { workspace = true, features = [
   "entries",
   "format",
   "fs",
+  "fsext",
   "fsxattr",
   "parser",
   "quoting-style",

src/uu/ls/src/ls.rs

@@ -39,6 +39,7 @@ use uucore::entries;
 use uucore::error::USimpleError;
 use uucore::format::human::{SizeFormat, human_readable};
 use uucore::fs::FileInformation;
+use uucore::fsext::{MetadataTimeField, metadata_get_time};
 #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
 use uucore::fsxattr::has_acl;
 #[cfg(unix)]
@@ -248,13 +249,6 @@ enum Files {
     Normal,
 }
 
-enum Time {
-    Modification,
-    Access,
-    Change,
-    Birth,
-}
-
 fn parse_time_style(options: &clap::ArgMatches) -> Result<(String, Option<String>), LsError> {
     const TIME_STYLES: [(&str, (&str, Option<&str>)); 4] = [
         ("full-iso", ("%Y-%m-%d %H:%M:%S.%f %z", None)),
@@ -332,7 +326,7 @@ pub struct Config {
     ignore_patterns: Vec<Pattern>,
     size_format: SizeFormat,
     directory: bool,
-    time: Time,
+    time: MetadataTimeField,
     #[cfg(unix)]
     inode: bool,
     color: Option<LsColors>,
@@ -467,23 +461,16 @@ fn extract_sort(options: &clap::ArgMatches) -> Sort {
 ///
 /// # Returns
 ///
-/// A Time variant representing the time to use.
-fn extract_time(options: &clap::ArgMatches) -> Time {
+/// A `MetadataTimeField` variant representing the time to use.
+fn extract_time(options: &clap::ArgMatches) -> MetadataTimeField {
     if let Some(field) = options.get_one::<String>(options::TIME) {
-        match field.as_str() {
-            "ctime" | "status" => Time::Change,
-            "access" | "atime" | "use" => Time::Access,
-            "mtime" | "modification" => Time::Modification,
-            "birth" | "creation" => Time::Birth,
-            // below should never happen as clap already restricts the values.
-            _ => unreachable!("Invalid field for --time"),
-        }
+        field.as_str().into()
     } else if options.get_flag(options::time::ACCESS) {
-        Time::Access
+        MetadataTimeField::Access
     } else if options.get_flag(options::time::CHANGE) {
-        Time::Change
+        MetadataTimeField::Change
     } else {
-        Time::Modification
+        MetadataTimeField::Modification
     }
 }
 
@@ -2099,7 +2086,7 @@ fn sort_entries(entries: &mut [PathData], config: &Config, out: &mut BufWriter<S
         Sort::Time => entries.sort_by_key(|k| {
             Reverse(
                 k.get_metadata(out)
-                    .and_then(|md| get_system_time(md, config))
+                    .and_then(|md| metadata_get_time(md, config.time))
                     .unwrap_or(UNIX_EPOCH),
             )
         }),
@@ -2685,7 +2672,7 @@ fn display_grid(
 /// * `group` ([`display_group`], config-optional)
 /// * `author` ([`display_uname`], config-optional)
 /// * `size / rdev` ([`display_len_or_rdev`])
-/// * `system_time` ([`get_system_time`])
+/// * `system_time` ([`display_date`])
 /// * `item_name` ([`display_item_name`])
 ///
 /// This function needs to display information in columns:
@@ -2963,35 +2950,13 @@ fn display_group(_metadata: &Metadata, _config: &Config, _state: &mut ListState)
     "somegroup"
 }
 
-// The implementations for get_system_time are separated because some options, such
-// as ctime will not be available
-#[cfg(unix)]
-fn get_system_time(md: &Metadata, config: &Config) -> Option<SystemTime> {
-    match config.time {
-        Time::Change => Some(UNIX_EPOCH + Duration::new(md.ctime() as u64, md.ctime_nsec() as u32)),
-        Time::Modification => md.modified().ok(),
-        Time::Access => md.accessed().ok(),
-        Time::Birth => md.created().ok(),
-    }
-}
-
-#[cfg(not(unix))]
-fn get_system_time(md: &Metadata, config: &Config) -> Option<SystemTime> {
-    match config.time {
-        Time::Modification => md.modified().ok(),
-        Time::Access => md.accessed().ok(),
-        Time::Birth => md.created().ok(),
-        Time::Change => None,
-    }
-}
-
 fn display_date(
     metadata: &Metadata,
     config: &Config,
     state: &mut ListState,
     out: &mut Vec<u8>,
 ) -> UResult<()> {
-    let Some(time) = get_system_time(metadata, config) else {
+    let Some(time) = metadata_get_time(metadata, config.time) else {
         out.extend(b"???");
         return Ok(());
     };

src/uucore/src/lib/features/fsext.rs

@@ -69,9 +69,15 @@ use std::io::Error as IOError;
 use std::mem;
 #[cfg(windows)]
 use std::path::Path;
-use std::time::UNIX_EPOCH;
+use std::time::{SystemTime, UNIX_EPOCH};
 use std::{borrow::Cow, ffi::OsString};
 
+use std::fs::Metadata;
+#[cfg(unix)]
+use std::os::unix::fs::MetadataExt;
+#[cfg(unix)]
+use std::time::Duration;
+
 #[cfg(any(
     target_os = "linux",
     target_os = "android",
@@ -112,7 +118,6 @@ pub trait BirthTime {
     fn birth(&self) -> Option<(u64, u32)>;
 }
 
-use std::fs::Metadata;
 impl BirthTime for Metadata {
     fn birth(&self) -> Option<(u64, u32)> {
         self.created()
@@ -122,6 +127,51 @@ impl BirthTime for Metadata {
     }
 }
 
+#[derive(Debug, Copy, Clone)]
+pub enum MetadataTimeField {
+    Modification,
+    Access,
+    Change,
+    Birth,
+}
+
+impl From<&str> for MetadataTimeField {
+    /// Get a `MetadataTimeField` from a string, we expect the value
+    /// to come from clap, and be constrained there (e.g. if Modification is
+    /// not supported), and the default branch should not be reached.
+    fn from(value: &str) -> Self {
+        match value {
+            "ctime" | "status" => MetadataTimeField::Change,
+            "access" | "atime" | "use" => MetadataTimeField::Access,
+            "mtime" | "modification" => MetadataTimeField::Modification,
+            "birth" | "creation" => MetadataTimeField::Birth,
+            // below should never happen as clap already restricts the values.
+            _ => unreachable!("Invalid metadata time field."),
+        }
+    }
+}
+
+#[cfg(unix)]
+fn metadata_get_change_time(md: &Metadata) -> Option<SystemTime> {
+    // TODO: This is incorrect for negative timestamps.
+    Some(UNIX_EPOCH + Duration::new(md.ctime() as u64, md.ctime_nsec() as u32))
+}
+
+#[cfg(not(unix))]
+fn metadata_get_change_time(_md: &Metadata) -> Option<SystemTime> {
+    // Not available.
+    None
+}
+
+pub fn metadata_get_time(md: &Metadata, md_time: MetadataTimeField) -> Option<SystemTime> {
+    match md_time {
+        MetadataTimeField::Change => metadata_get_change_time(md),
+        MetadataTimeField::Modification => md.modified().ok(),
+        MetadataTimeField::Access => md.accessed().ok(),
+        MetadataTimeField::Birth => md.created().ok(),
+    }
+}
+
 // TODO: Types for this struct are probably mostly wrong. Possibly, most of them
 // should be OsString.
 #[derive(Debug, Clone)]

tests/by-util/test_du.rs

@@ -594,6 +594,8 @@ fn test_du_h_precision() {
 #[cfg(feature = "touch")]
 #[test]
 fn test_du_time() {
+    use regex::Regex;
+
     let ts = TestScenario::new(util_name!());
 
     // du --time formats the timestamp according to the local timezone. We set the TZ
@@ -634,21 +636,18 @@ fn test_du_time() {
         result.stdout_only("0\t2015-05-15 00:00\tdate_test\n");
     }
 
-    let result = ts
-        .ucmd()
-        .env("TZ", "UTC")
-        .arg("--time=ctime")
-        .arg("date_test")
-        .succeeds();
-    result.stdout_only("0\t2016-06-16 00:00\tdate_test\n");
+    // Change (and birth) times can't be easily modified, so we just do a regex
+    let re_change_birth =
+        Regex::new(r"0\t[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}\tdate_test").unwrap();
+    let result = ts.ucmd().arg("--time=ctime").arg("date_test").succeeds();
+    #[cfg(windows)]
+    result.stdout_only("0\t???\tdate_test\n"); // ctime not supported on Windows
+    #[cfg(not(windows))]
+    result.stdout_matches(&re_change_birth);
 
     if birth_supported() {
-        use regex::Regex;
-
-        let re_birth =
-            Regex::new(r"0\t[0-9]{4}-[0-9]{2}-[0-9]{2} [0-9]{2}:[0-9]{2}\tdate_test").unwrap();
         let result = ts.ucmd().arg("--time=birth").arg("date_test").succeeds();
-        result.stdout_matches(&re_birth);
+        result.stdout_matches(&re_change_birth);
     }
 }