Repository: alexcrichton/tar-rs (language: rust)

Issue:
HeaderMode::Deterministic differs in behaviour on Unix and Windows platforms
On Windows the `mtime` is set to the dummy value of https://github.com/alexcrichton/tar-rs/blob/master/src/header.rs#L828

```rust
                self.set_mtime(123456789); // see above in unix
```
but on Unix it is set to https://github.com/alexcrichton/tar-rs/blob/master/src/header.rs#L760

```rust
                // 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 Jul 23, 2006 -- the date of the
                // first commit for what would become Rust.
                self.set_mtime(1153704088);
```

I don't see why these should be different, and it causes an annoyance in one of our use-cases.

Is there a reason why these two dummy values should be different?




Discussion / hints:

Fix patch:
diff --git a/src/header.rs b/src/header.rs
index 7e507fc7..65f665e0 100644
--- a/src/header.rs
+++ b/src/header.rs
@@ -16,6 +16,14 @@ use std::str;
 use crate::other;
 use crate::EntryType;
 
+/// A deterministic, arbitrary, non-zero timestamp that use used as `mtime`
+/// of headers when [`HeaderMode::Deterministic`] is used.
+///
+/// This value, chosen after careful deliberation, corresponds to _Jul 23, 2006_,
+/// which is the date of the first commit for what would become Rust.
+#[cfg(any(unix, windows))]
+const DETERMINISTIC_TIMESTAMP: u64 = 1153704088;
+
 /// Representation of the header of an entry in an archive
 #[repr(C)]
 #[allow(missing_docs)]
@@ -748,16 +756,11 @@ impl Header {
                 self.set_mode(meta.mode() as u32);
             }
             HeaderMode::Deterministic => {
-                // 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 Jul 23, 2006 -- the date of the
-                // first commit for what would become Rust.
-                self.set_mtime(1153704088);
+                // 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.
+                self.set_mtime(DETERMINISTIC_TIMESTAMP);
 
                 self.set_uid(0);
                 self.set_gid(0);
@@ -825,7 +828,7 @@ impl Header {
             HeaderMode::Deterministic => {
                 self.set_uid(0);
                 self.set_gid(0);
-                self.set_mtime(123456789); // see above in unix
+                self.set_mtime(DETERMINISTIC_TIMESTAMP); // see above in unix
                 let fs_mode = if meta.is_dir() { 0o755 } else { 0o644 };
                 self.set_mode(fs_mode);
             }


Test patch:
diff --git a/tests/header/mod.rs b/tests/header/mod.rs
index 86692e33..14865b86 100644
--- a/tests/header/mod.rs
+++ b/tests/header/mod.rs
@@ -204,6 +204,7 @@ fn set_metadata_deterministic() {
 
     // Would not match without `Deterministic`.
     assert_eq!(t!(one.mtime()), t!(two.mtime()));
+    assert_eq!(t!(one.mtime()), 1153704088);
     // TODO: No great way to validate that these would not be filled, but
     // check them anyway.
     assert_eq!(t!(one.uid()), t!(two.uid()));