From 843d15a5de127acb4e11f503344c44a5bacce9d0 Mon Sep 17 00:00:00 2001 From: Marek Kaput Date: Mon, 13 Nov 2023 11:15:34 +0100 Subject: [PATCH] Use constant `mtime` for packaged tarballs Although the `set_metadata_in_mode` call should set `mtime` to a deterministic value, it fails to do so due to https://github.com/alexcrichton/tar-rs/issues/341. Also, the constant value used there is funky and I do not feel convinced about its stability. Therefore, we use our own `mtime` value explicitly here. fix #883 Signed-off-by: Marek Kaput --- scarb/src/ops/package.rs | 18 ++++++++++++++---- scarb/tests/local_registry.rs | 14 +++++--------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/scarb/src/ops/package.rs b/scarb/src/ops/package.rs index 6a8287654..6bf284f87 100644 --- a/scarb/src/ops/package.rs +++ b/scarb/src/ops/package.rs @@ -385,6 +385,19 @@ fn tar( .with_context(|| format!("failed to stat: {disk_path}"))?; header.set_metadata_in_mode(&metadata, tar::HeaderMode::Deterministic); + + // Although the `set_metadata_in_mode` call above should set `mtime` to a + // deterministic value, it fails to do so due to + // https://github.com/alexcrichton/tar-rs/issues/341. + // Also, the constant value used there is funky and I do not feel convinced about + // its stability. Therefore, we use our own `mtime` value explicitly here. + // + // From `set_metadata_in_mode` implementation in `tar` crate: + // > 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. + header.set_mtime(1); + header.set_cksum(); ar.append_data(&mut header, &archive_path, &mut file) @@ -400,10 +413,7 @@ fn tar( header.set_mode(0o644); header.set_size(contents.len() as u64); - // From `set_metadata_in_mode` implementation in `tar` crate: - // 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. + // Same as above. header.set_mtime(1); header.set_cksum(); diff --git a/scarb/tests/local_registry.rs b/scarb/tests/local_registry.rs index 49c7c0c57..740109a10 100644 --- a/scarb/tests/local_registry.rs +++ b/scarb/tests/local_registry.rs @@ -127,9 +127,7 @@ fn url_pointing_to_file() { drop(registry_t); } -// FIXME(#883): Unignore these tests. #[test] -#[cfg_attr(target_os = "windows", ignore = "ignored on windows as of #883")] fn publish() { let t = TempDir::new().unwrap(); let index = t.child("index"); @@ -185,7 +183,7 @@ fn publish() { { "v": "1.0.0", "deps": [], - "cksum": "sha256:d891504afc86fc0a7a9f38533a66ef2763990a1ff4be3eb9d5836d32a9bd9ad3", + "cksum": "sha256:3771f51595f7df697a822823989714b3c9a83764a67fa517ac6d8df3cfc642bf", } ]) ); @@ -198,20 +196,18 @@ fn publish() { { "v": "1.0.0", "deps": [], - "cksum": "sha256:d05d4c524aa0136e42df6138f8e97f8b2b7fc946911cef8ae40baf38acf87ef6", + "cksum": "sha256:07db55042bb0be41eb957d649960cd16764426ce9df1cd0729d408acdf3c63cb", }, { "v": "1.1.0", "deps": [], - "cksum": "sha256:ec55410dac39c63ea1372f44f05b74bcf14ec6305749d80bd607be0603271ef1", + "cksum": "sha256:bd36828fa2134ec3ce9599511404f85958d8ed091fc3b03a3f3d8426d0a0f720", } ]) ); } -// FIXME(#883): Unignore these tests. #[test] -#[cfg_attr(target_os = "windows", ignore = "ignored on windows as of #883")] fn publish_overwrites_existing() { let index = TempDir::new().unwrap(); @@ -238,7 +234,7 @@ fn publish_overwrites_existing() { { "v": "1.0.0", "deps": [], - "cksum": "sha256:49bb7566594c89da4603578aebe812d750d1fefa1fccc532461963d813093b11", + "cksum": "sha256:b34e1202407e1a9b743f261cdc27723d0344619a6dc3058bdacd9b17f6106027", } ]) ); @@ -266,7 +262,7 @@ fn publish_overwrites_existing() { { "v": "1.0.0", "deps": [], - "cksum": "sha256:f6555b5b27327d40196578005de811158a3ac7401c36c13ee02b27afe7aab00f", + "cksum": "sha256:4682a7645acf0b342556778a0544e2871ae910efc609d89ab3bedf9491219ab5", } ]) );