Skip to content
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.

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

Return file creation times on Linux (using statx) #248

Merged
merged 9 commits into from
Jun 4, 2022
22 changes: 0 additions & 22 deletions cap-primitives/src/fs/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,35 +48,13 @@ impl Metadata {

#[inline]
fn from_parts(std: fs::Metadata, ext: MetadataExt, file_type: FileType) -> Self {
// TODO: Initialize `created` on Linux with `std.created().ok()` once we
// make use of `statx`.
Self {
file_type,
len: std.len(),
permissions: Permissions::from_std(std.permissions()),
modified: std.modified().ok().map(SystemTime::from_std),
accessed: std.accessed().ok().map(SystemTime::from_std),

#[cfg(any(
target_os = "freebsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios",
target_os = "netbsd",
windows,
))]
created: std.created().ok().map(SystemTime::from_std),

#[cfg(not(any(
target_os = "freebsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios",
target_os = "netbsd",
windows,
)))]
created: None,

ext,
}
}
Expand Down
4 changes: 3 additions & 1 deletion cap-primitives/src/rustix/fs/metadata_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
use crate::fs::PermissionsExt;
use crate::fs::{FileTypeExt, Metadata};
use crate::time::{Duration, SystemClock, SystemTime};
// TODO: update all these to
// #[cfg(any(target_os = "android", target_os = "linux"))]
// once we're on restix >= v0.34.3.
#[cfg(all(target_os = "linux", target_env = "gnu"))]
use rustix::fs::{makedev, Statx};
use rustix::fs::{RawMode, Stat};
Expand Down Expand Up @@ -222,7 +225,6 @@ impl MetadataExt {
/// Constructs a new instance of `Metadata` from the given `Statx`.
#[cfg(all(target_os = "linux", target_env = "gnu"))]
#[inline]
#[allow(dead_code)] // TODO: use `statx` when possible.
pub(crate) fn from_rustix_statx(statx: Statx) -> Metadata {
Metadata {
file_type: FileTypeExt::from_raw_mode(RawMode::from(statx.stx_mode)),
Expand Down
29 changes: 29 additions & 0 deletions cap-primitives/src/rustix/fs/stat_unchecked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ use rustix::fs::{statat, AtFlags};
use std::path::Path;
use std::{fs, io};

// TODO: update all these to
// #[cfg(any(target_os = "android", target_os = "linux"))]
// once we're on restix >= v0.34.3.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any idea when the rustix version will be updated? I don't have a sense of how big that is, so I didn't want to add it to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

The biggest thing for cap-std is that there are some semver incompatibilities between versions of io-lifetimes, which is tracking changes from I/O safety stabilization. Fortunately, the FCP looks to be almost done. Once it's done, I plan to do an io-lifetimes release and then a rustix 0.35 release, and then update cap-std and various other things.

#[cfg(all(target_os = "linux", target_env = "gnu"))]
use rustix::fs::{statx, StatxFlags};
#[cfg(all(target_os = "linux", target_env = "gnu"))]
use std::sync::atomic::{AtomicBool, Ordering};

/// *Unsandboxed* function similar to `stat`, but which does not perform
/// sandboxing.
pub(crate) fn stat_unchecked(
Expand All @@ -15,5 +23,26 @@ pub(crate) fn stat_unchecked(
FollowSymlinks::No => AtFlags::SYMLINK_NOFOLLOW,
};

// `statx` is preferred on Linux because it can return creation times. Linux
// kernels prior to 4.11 don't have `statx` and return `ENOSYS`. We store the
// availability in a global to avoid unnecessary syscalls.
#[cfg(all(target_os = "linux", target_env = "gnu"))]
static HAS_STATX: AtomicBool = AtomicBool::new(true);

#[cfg(all(target_os = "linux", target_env = "gnu"))]
if HAS_STATX.load(Ordering::Relaxed) {
let statx_result = statx(
start,
path,
atflags,
StatxFlags::BASIC_STATS | StatxFlags::BTIME,
);
if let Err(rustix::io::Error::NOSYS) = statx_result {
HAS_STATX.store(false, Ordering::Relaxed);
} else {
return Ok(statx_result.map(MetadataExt::from_rustix_statx)?);
}
ongardie marked this conversation as resolved.
Show resolved Hide resolved
}

Ok(statat(start, path, atflags).map(MetadataExt::from_rustix)?)
}
52 changes: 52 additions & 0 deletions tests/metadata-ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,55 @@ fn test_metadata_ext() {
);
}
}

#[test]
fn test_metadata_ext_created() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this test in the right / a reasonable place? I wasn't sure.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, good question. metadata-ext.rs is for MetadataExt APIs, but the created() field is a regular Metadata API. This can go in fs_additional.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

let tmpdir = tmpdir();
let a = check!(tmpdir.create("a"));
let a_metadata = check!(a.metadata());

let modified = check!(a_metadata.modified());
let tolerance = std::time::Duration::from_secs(10);
let expected = (modified - tolerance)..(modified + tolerance);

// If the standard library supports file creation times, then cap-std
// should too.
sunfishcode marked this conversation as resolved.
Show resolved Hide resolved
let std_supports_created = matches!(
a.into_std().metadata(),
Ok(m) if m.created().is_ok(),
);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of using the modified() timestamp for expected, could you use the created() timestamp for expected? And for the directories, could we always call std::fs::metadata so that we always get the created() timestamp from std? It seems like that should eliminate the need for a tolerance, because the creation time from cap-std should be exactly the same as the creation time from std.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that makes more sense than what I had done. Updated.


if std_supports_created {
let created = check!(a_metadata.created());
assert!(
expected.contains(&created),
"expected File creation time near {:#?} but got {:#?}",
modified,
created,
);

let tmpdir_metadata = check!(tmpdir.dir_metadata());
let created = check!(tmpdir_metadata.created());
assert!(
expected.contains(&created),
"expected Dir creation time near {:#?} but got {:#?}",
modified,
created,
);

let mut entries = check!(tmpdir.entries());
if let Some(a) = entries.next() {
let a = check!(a);
assert_eq!(a.file_name(), "a");
let metadata = check!(a.metadata());
let created = check!(metadata.created());
assert!(
expected.contains(&created),
"expected DirEntry creation time near {:#?} but got {:#?}",
modified,
created,
);
}
assert!(entries.next().is_none(), "unexpected dir entry");
}
}