Fix statx created times when unavailable (#268)

* Fix statx created times when unavailable

This previously returned `Some(UNIX_EPOCH)` instead of `None` when `statx` did
not return a created time.

* Loosen fs_additional test assertion for created times

This makes it OK to return file created times even if std doesn't,
which is currently the case for musl builds.

* Check `statx` mask to see if `mtime` and `atime` were returned, too

`std` does these checks, so cap-std should too. `std` only does the checks for
32-bit targets, but they don't seem harmful to enable for all targets.

Author: ongardie

cap-primitives/src/rustix/fs/metadata_ext.rs

@@ -3,7 +3,7 @@
 use crate::fs::{FileTypeExt, Metadata, PermissionsExt};
 use crate::time::{Duration, SystemClock, SystemTime};
 #[cfg(any(target_os = "android", target_os = "linux"))]
-use rustix::fs::{makedev, Statx};
+use rustix::fs::{makedev, Statx, StatxFlags};
 use rustix::fs::{RawMode, Stat};
 use std::convert::{TryFrom, TryInto};
 use std::{fs, io};
@@ -226,9 +226,21 @@ impl MetadataExt {
             file_type: FileTypeExt::from_raw_mode(RawMode::from(statx.stx_mode)),
             len: u64::try_from(statx.stx_size).unwrap(),
             permissions: PermissionsExt::from_raw_mode(RawMode::from(statx.stx_mode)),
-            modified: system_time_from_rustix(statx.stx_mtime.tv_sec, statx.stx_mtime.tv_nsec as _),
-            accessed: system_time_from_rustix(statx.stx_atime.tv_sec, statx.stx_atime.tv_nsec as _),
-            created: system_time_from_rustix(statx.stx_btime.tv_sec, statx.stx_btime.tv_nsec as _),
+            modified: if statx.stx_mask & StatxFlags::MTIME.bits() != 0 {
+                system_time_from_rustix(statx.stx_mtime.tv_sec, statx.stx_mtime.tv_nsec as _)
+            } else {
+                None
+            },
+            accessed: if statx.stx_mask & StatxFlags::ATIME.bits() != 0 {
+                system_time_from_rustix(statx.stx_atime.tv_sec, statx.stx_atime.tv_nsec as _)
+            } else {
+                None
+            },
+            created: if statx.stx_mask & StatxFlags::BTIME.bits() != 0 {
+                system_time_from_rustix(statx.stx_btime.tv_sec, statx.stx_btime.tv_nsec as _)
+            } else {
+                None
+            },
 
             ext: Self {
                 dev: makedev(statx.stx_dev_major, statx.stx_dev_minor),

tests/fs_additional.rs

@@ -6,6 +6,7 @@
 mod sys_common;
 
 use cap_std::fs::{DirBuilder, OpenOptions};
+use cap_std::time::SystemClock;
 use std::io::{self, Read, Write};
 use std::path::Path;
 use std::str;
@@ -936,24 +937,51 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) {
 
     // If the standard library supports file modified/accessed/created times,
     // then cap-std should too.
-    if let Ok(expected) = std.modified() {
-        assert_eq!(expected, check!(cap.modified()).into_std());
+    match std.modified() {
+        Ok(expected) => assert_eq!(expected, check!(cap.modified()).into_std()),
+        Err(e) => assert!(
+            cap.modified().is_err(),
+            "modified time should be error ({}), got {:#?}",
+            e,
+            cap.modified()
+        ),
     }
     // The access times might be a little different due to either our own
     // or concurrent accesses.
     const ACCESS_TOLERANCE_SEC: u32 = 60;
-    if let Ok(expected) = std.accessed() {
-        let access_tolerance = std::time::Duration::from_secs(ACCESS_TOLERANCE_SEC.into());
-        assert!(
-            ((expected - access_tolerance)..(expected + access_tolerance))
-                .contains(&check!(cap.accessed()).into_std()),
-            "std accessed {:#?}, cap accessed {:#?}",
-            expected,
+    match std.accessed() {
+        Ok(expected) => {
+            let access_tolerance = std::time::Duration::from_secs(ACCESS_TOLERANCE_SEC.into());
+            assert!(
+                ((expected - access_tolerance)..(expected + access_tolerance))
+                    .contains(&check!(cap.accessed()).into_std()),
+                "std accessed {:#?}, cap accessed {:#?}",
+                expected,
+                cap.accessed()
+            );
+        }
+        Err(e) => assert!(
+            cap.accessed().is_err(),
+            "accessed time should be error ({}), got {:#?}",
+            e,
             cap.accessed()
-        );
+        ),
     }
-    if let Ok(expected) = std.created() {
-        assert_eq!(expected, check!(cap.created()).into_std());
+    match std.created() {
+        Ok(expected) => assert_eq!(expected, check!(cap.created()).into_std()),
+        Err(e) => {
+            // An earlier bug returned the Unix epoch instead of `None` when
+            // created times were unavailable. This tries to catch such errors,
+            // while also allowing some targets to return valid created times
+            // even when std doesn't.
+            if let Ok(actual) = cap.created() {
+                println!(
+                    "std returned error for created time ({}) but got {:#?}",
+                    e, actual
+                );
+                assert_ne!(actual, SystemClock::UNIX_EPOCH);
+            }
+        }
     }
 
     #[cfg(unix)]