From e81f172113c44742c9e096c296f3055abd2dfa0b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 27 May 2021 14:47:30 -0500 Subject: [PATCH] Avoid zero-mtime files (#250) With rust-lang/cargo#9512 discovered this is, uh, odd fallout of the "deterministic" header mode in this crate. This is an unintended consequence of the deterministic header mode which should ideally be fixable. This commit changes the `tar` crate to use a different constant than 0 when creating archives with the deterministic header mode (specifically 123456789: Nov 29, 1973). It also will now refuse to create files with a 0 mtime, instead resetting the mtime to 1 which should help the mtime be nonzero so tools like lldb don't accidentally think it's zero. --- src/entry.rs | 7 +++++++ src/header.rs | 11 ++++++++++- tests/all.rs | 23 ++++++++++++++++++++++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/entry.rs b/src/entry.rs index e02ecbe0..f99c897b 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -610,6 +610,13 @@ impl<'a> EntryFields<'a> { if self.preserve_mtime { if let Ok(mtime) = self.header.mtime() { + // For some more information on this see the comments in + // `Header::fill_platform_from`, but the general idea is that + // we're trying to avoid 0-mtime files coming out of archives + // since some tools don't ingest them well. Perhaps one day + // when Cargo stops working with 0-mtime archives we can remove + // this. + let mtime = if mtime == 0 { 1 } else { mtime }; let mtime = FileTime::from_unix_time(mtime as i64, 0); filetime::set_file_handle_times(&f, Some(mtime), Some(mtime)).map_err(|e| { TarError::new(&format!("failed to set mtime for `{}`", dst.display()), e) diff --git a/src/header.rs b/src/header.rs index beb5836d..ec04bba3 100644 --- a/src/header.rs +++ b/src/header.rs @@ -734,7 +734,16 @@ impl Header { self.set_mode(meta.mode() as u32); } HeaderMode::Deterministic => { - self.set_mtime(0); + // We could in theory set the mtime to zero here, but not all + // tools seem to behave well when ingesting files with a 0 + // timestamp. For example rust-lang/cargo#9512 shows that lldb + // doesn't ingest files with a zero timestamp correctly. + // + // We just need things to be deterministic here so just pick + // something that isn't zero. This time, chosen after careful + // deliberation, corresponds to Nov 29, 1973. + self.set_mtime(123456789); + self.set_uid(0); self.set_gid(0); diff --git a/tests/all.rs b/tests/all.rs index 93ec3e23..37816875 100644 --- a/tests/all.rs +++ b/tests/all.rs @@ -11,7 +11,7 @@ use std::iter::repeat; use std::path::{Path, PathBuf}; use filetime::FileTime; -use tar::{Archive, Builder, EntryType, Header}; +use tar::{Archive, Builder, EntryType, Header, HeaderMode}; use tempfile::{Builder as TempBuilder, TempDir}; macro_rules! t { @@ -645,6 +645,27 @@ fn file_times() { assert_eq!(atime.nanoseconds(), 0); } +#[test] +fn zero_file_times() { + let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); + + let mut ar = Builder::new(Vec::new()); + ar.mode(HeaderMode::Deterministic); + let path = td.path().join("tmpfile"); + t!(File::create(&path)); + t!(ar.append_path_with_name(&path, "a")); + + let data = t!(ar.into_inner()); + let mut ar = Archive::new(&data[..]); + assert!(ar.unpack(td.path()).is_ok()); + + let meta = fs::metadata(td.path().join("a")).unwrap(); + let mtime = FileTime::from_last_modification_time(&meta); + let atime = FileTime::from_last_access_time(&meta); + assert!(mtime.unix_seconds() != 0); + assert!(atime.unix_seconds() != 0); +} + #[test] fn backslash_treated_well() { // Insert a file into an archive with a backslash