From 7bcb8551474ef01085ad0cf0cacdbbe3520a2135 Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Fri, 7 Jan 2022 23:45:19 +1100 Subject: [PATCH 1/3] Make the dependency filetime optional --- Cargo.toml | 4 ++-- src/entry.rs | 2 ++ tests/all.rs | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bcafdbd7..3aec4eac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ contents are never required to be entirely resident in memory all at once. """ [dependencies] -filetime = "0.2.8" +filetime = { version = "0.2.8", optional = true } [dev-dependencies] tempfile = "3" @@ -29,4 +29,4 @@ xattr = { version = "0.2", optional = true } libc = "0.2" [features] -default = ["xattr"] +default = ["filetime", "xattr"] diff --git a/src/entry.rs b/src/entry.rs index 8f0b62ac..1b34ff48 100644 --- a/src/entry.rs +++ b/src/entry.rs @@ -7,6 +7,7 @@ use std::io::{self, Error, ErrorKind, SeekFrom}; use std::marker; use std::path::{Component, Path, PathBuf}; +#[cfg(feature = "filetime")] use filetime::{self, FileTime}; use crate::archive::ArchiveInner; @@ -617,6 +618,7 @@ impl<'a> EntryFields<'a> { ) })?; + #[cfg(feature = "filetime")] if self.preserve_mtime { if let Ok(mtime) = self.header.mtime() { // For some more information on this see the comments in diff --git a/tests/all.rs b/tests/all.rs index 11103bd6..95a80168 100644 --- a/tests/all.rs +++ b/tests/all.rs @@ -1,3 +1,4 @@ +#[cfg(feature = "filetime")] extern crate filetime; extern crate tar; extern crate tempfile; @@ -10,6 +11,7 @@ use std::io::{self, Cursor}; use std::iter::repeat; use std::path::{Path, PathBuf}; +#[cfg(feature = "filetime")] use filetime::FileTime; use tar::{Archive, Builder, Entries, EntryType, Header, HeaderMode}; use tempfile::{Builder as TempBuilder, TempDir}; @@ -687,6 +689,7 @@ fn empty_filename() { } #[test] +#[cfg(feature = "filetime")] fn file_times() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); let rdr = Cursor::new(tar!("file_times.tar")); @@ -703,6 +706,7 @@ fn file_times() { } #[test] +#[cfg(feature = "filetime")] fn zero_file_times() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); From 2fb74968201f6f424d731bbf45331fbdc05da73d Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Sat, 8 Jan 2022 00:15:47 +1100 Subject: [PATCH 2/3] Split Builder functionality into a feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Or, “the outworking of making libc optional in order to reach zero dependencies, even though you almost certainly depend on libc by some other path already”. This one is rather more extreme, but I was curious and it happened, and frankly I think the end result is pretty decent, so that if I were maintaining this crate I’d ship it. There are plenty of people who won’t use Builder functionality, being able to turn it off is a fairly trivial nice thing. And yeah, I just like being able to have no dependencies sometimes. Along the way I noticed wasm32 has a big ol’ unimplemented!() on Header::fill_platform_from. I should learn what WASI does for a file system. Probably there should be a simple fallback implementation that only knows about files, directories and symlinks. --- Cargo.toml | 13 +++++++++++-- README.md | 8 ++++++++ src/header.rs | 10 +++++++--- src/lib.rs | 2 ++ tests/all.rs | 36 ++++++++++++++++++++++++++++++++++-- tests/header/mod.rs | 2 ++ 6 files changed, 64 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3aec4eac..f1c4edf4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,16 @@ tempfile = "3" [target."cfg(unix)".dependencies] xattr = { version = "0.2", optional = true } -libc = "0.2" +libc = { version = "0.2", optional = true } [features] -default = ["filetime", "xattr"] +default = ["builder", "filetime", "xattr"] +builder = ["libc"] + +[[test]] +name = "entry" +required-features = ["builder"] + +[[example]] +name = "write" +required-features = ["builder"] diff --git a/README.md b/README.md index 25291e83..9265c041 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,14 @@ fn main() { } ``` +# Cargo features + +- **builder** (enabled by default): provides tools for creating tarballs: + `Builder` and `Header::{set_metadata, set_metadata_in_mode}`. Depends on + libc on unix, and methods touching `std::fs` are unimplemented on wasm. +- **filetime** (enabled by default): write file times when unpacking a tarball. +- **xattr** (enabled by defeult): write xattrs when unpacking a tarball on unix. + # License This project is licensed under either of diff --git a/src/header.rs b/src/header.rs index 7e507fc7..a5a9be7c 100644 --- a/src/header.rs +++ b/src/header.rs @@ -5,6 +5,7 @@ use std::os::windows::prelude::*; use std::borrow::Cow; use std::fmt; +#[cfg(feature = "builder")] use std::fs; use std::io; use std::iter; @@ -279,12 +280,14 @@ impl Header { /// This is useful for initializing a `Header` from the OS's metadata from a /// file. By default, this will use `HeaderMode::Complete` to include all /// metadata. + #[cfg(feature = "builder")] pub fn set_metadata(&mut self, meta: &fs::Metadata) { self.fill_from(meta, HeaderMode::Complete); } /// Sets only the metadata relevant to the given HeaderMode in this header /// from the metadata argument provided. + #[cfg(feature = "builder")] pub fn set_metadata_in_mode(&mut self, meta: &fs::Metadata, mode: HeaderMode) { self.fill_from(meta, mode); } @@ -714,6 +717,7 @@ impl Header { .fold(0, |a, b| a + (*b as u32)) } + #[cfg(feature = "builder")] fn fill_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { self.fill_platform_from(meta, mode); // Set size of directories to zero @@ -732,13 +736,13 @@ impl Header { } } - #[cfg(target_arch = "wasm32")] + #[cfg(all(target_arch = "wasm32", feature = "builder"))] #[allow(unused_variables)] fn fill_platform_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { unimplemented!(); } - #[cfg(unix)] + #[cfg(all(unix, feature = "builder"))] fn fill_platform_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { match mode { HeaderMode::Complete => { @@ -797,7 +801,7 @@ impl Header { } } - #[cfg(windows)] + #[cfg(all(windows, feature = "builder"))] fn fill_platform_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { // There's no concept of a file mode on Windows, so do a best approximation here. match mode { diff --git a/src/lib.rs b/src/lib.rs index 52251cd2..efad8096 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,6 +24,7 @@ use std::io::{Error, ErrorKind}; pub use crate::archive::{Archive, Entries}; +#[cfg(feature = "builder")] pub use crate::builder::Builder; pub use crate::entry::{Entry, Unpacked}; pub use crate::entry_type::EntryType; @@ -32,6 +33,7 @@ pub use crate::header::{GnuHeader, GnuSparseHeader, Header, HeaderMode, OldHeade pub use crate::pax::{PaxExtension, PaxExtensions}; mod archive; +#[cfg(feature = "builder")] mod builder; mod entry; mod entry_type; diff --git a/tests/all.rs b/tests/all.rs index 95a80168..8b6b83b2 100644 --- a/tests/all.rs +++ b/tests/all.rs @@ -8,12 +8,17 @@ extern crate xattr; use std::fs::{self, File}; use std::io::prelude::*; use std::io::{self, Cursor}; +#[cfg(feature = "builder")] use std::iter::repeat; -use std::path::{Path, PathBuf}; +use std::path::Path; +#[cfg(feature = "builder")] +use std::path::PathBuf; #[cfg(feature = "filetime")] use filetime::FileTime; -use tar::{Archive, Builder, Entries, EntryType, Header, HeaderMode}; +use tar::{Archive, Entries, Header}; +#[cfg(feature = "builder")] +use tar::{Builder, EntryType, HeaderMode}; use tempfile::{Builder as TempBuilder, TempDir}; macro_rules! t { @@ -132,6 +137,7 @@ fn reading_files() { } #[test] +#[cfg(feature = "builder")] fn writing_files() { let mut ar = Builder::new(Vec::new()); let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -156,6 +162,7 @@ fn writing_files() { } #[test] +#[cfg(feature = "builder")] fn large_filename() { let mut ar = Builder::new(Vec::new()); let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -405,6 +412,7 @@ fn no_xattrs() { } #[test] +#[cfg(feature = "builder")] fn writing_and_extracting_directories() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -423,6 +431,7 @@ fn writing_and_extracting_directories() { } #[test] +#[cfg(feature = "builder")] fn writing_directories_recursively() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -454,6 +463,7 @@ fn writing_directories_recursively() { } #[test] +#[cfg(feature = "builder")] fn append_dir_all_blank_dest() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -485,6 +495,7 @@ fn append_dir_all_blank_dest() { } #[test] +#[cfg(feature = "builder")] fn append_dir_all_does_not_work_on_non_directory() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); let path = td.path().join("test"); @@ -507,6 +518,7 @@ fn extracting_duplicate_dirs() { } #[test] +#[cfg(feature = "builder")] fn unpack_old_style_bsd_dir() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -533,6 +545,7 @@ fn unpack_old_style_bsd_dir() { } #[test] +#[cfg(feature = "builder")] fn handling_incorrect_file_size() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -560,6 +573,7 @@ fn handling_incorrect_file_size() { } #[test] +#[cfg(feature = "builder")] fn extracting_malicious_tarball() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -658,6 +672,7 @@ fn octal_spaces() { } #[test] +#[cfg(feature = "builder")] fn extracting_malformed_tar_null_blocks() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -706,6 +721,7 @@ fn file_times() { } #[test] +#[cfg(feature = "builder")] #[cfg(feature = "filetime")] fn zero_file_times() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -728,6 +744,7 @@ fn zero_file_times() { } #[test] +#[cfg(feature = "builder")] fn backslash_treated_well() { // Insert a file into an archive with a backslash let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -763,6 +780,7 @@ fn backslash_treated_well() { #[cfg(unix)] #[test] +#[cfg(feature = "builder")] fn nul_bytes_in_path() { use std::ffi::OsStr; use std::os::unix::prelude::*; @@ -869,6 +887,7 @@ fn pax_linkpath() { } #[test] +#[cfg(feature = "builder")] fn long_name_trailing_nul() { let mut b = Builder::new(Vec::::new()); @@ -894,6 +913,7 @@ fn long_name_trailing_nul() { } #[test] +#[cfg(feature = "builder")] fn long_linkname_trailing_nul() { let mut b = Builder::new(Vec::::new()); @@ -919,6 +939,7 @@ fn long_linkname_trailing_nul() { } #[test] +#[cfg(feature = "builder")] fn long_linkname_gnu() { for t in [tar::EntryType::Symlink, tar::EntryType::Link] { let mut b = Builder::new(Vec::::new()); @@ -940,6 +961,7 @@ fn long_linkname_gnu() { } #[test] +#[cfg(feature = "builder")] fn linkname_literal() { for t in [tar::EntryType::Symlink, tar::EntryType::Link] { let mut b = Builder::new(Vec::::new()); @@ -962,6 +984,7 @@ fn linkname_literal() { } #[test] +#[cfg(feature = "builder")] fn encoded_long_name_has_trailing_nul() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); let path = td.path().join("foo"); @@ -1090,6 +1113,7 @@ fn sparse_with_trailing() { } #[test] +#[cfg(feature = "builder")] fn path_separators() { let mut ar = Builder::new(Vec::new()); let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -1133,6 +1157,7 @@ fn path_separators() { #[test] #[cfg(unix)] +#[cfg(feature = "builder")] fn append_path_symlink() { use std::borrow::Cow; use std::env; @@ -1187,6 +1212,7 @@ fn append_path_symlink() { } #[test] +#[cfg(feature = "builder")] fn name_with_slash_doesnt_fool_long_link_and_bsd_compat() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -1220,6 +1246,7 @@ fn name_with_slash_doesnt_fool_long_link_and_bsd_compat() { } #[test] +#[cfg(feature = "builder")] fn insert_local_file_different_name() { let mut ar = Builder::new(Vec::new()); let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -1242,6 +1269,7 @@ fn insert_local_file_different_name() { #[test] #[cfg(unix)] +#[cfg(feature = "builder")] fn tar_directory_containing_symlink_to_directory() { use std::os::unix::fs::symlink; @@ -1279,6 +1307,7 @@ fn unpack_path_larger_than_windows_max_path() { } #[test] +#[cfg(feature = "builder")] fn append_long_multibyte() { let mut x = tar::Builder::new(Vec::new()); let mut name = String::new(); @@ -1292,6 +1321,7 @@ fn append_long_multibyte() { } #[test] +#[cfg(feature = "builder")] fn read_only_directory_containing_files() { let td = t!(TempBuilder::new().prefix("tar-rs").tempdir()); @@ -1320,6 +1350,7 @@ fn read_only_directory_containing_files() { // This test was marked linux only due to macOS CI can't handle `set_current_dir` correctly #[test] #[cfg(target_os = "linux")] +#[cfg(feature = "builder")] fn tar_directory_containing_special_files() { use std::env; use std::ffi::CString; @@ -1351,6 +1382,7 @@ fn tar_directory_containing_special_files() { } #[test] +#[cfg(feature = "builder")] fn header_size_overflow() { // maximal file size doesn't overflow anything let mut ar = Builder::new(Vec::new()); diff --git a/tests/header/mod.rs b/tests/header/mod.rs index 86692e33..dcaff964 100644 --- a/tests/header/mod.rs +++ b/tests/header/mod.rs @@ -1,3 +1,4 @@ +#![cfg_attr(not(feature = "builder"), allow(unused_imports))] use std::fs::{self, File}; use std::io::{self, Write}; use std::path::Path; @@ -177,6 +178,7 @@ fn set_ustar_path_hard() { } #[test] +#[cfg(feature = "builder")] fn set_metadata_deterministic() { let td = t!(Builder::new().prefix("tar-rs").tempdir()); let tmppath = td.path().join("tmpfile"); From b031dee050b6efb242b14f93b27a9734d0c334fb Mon Sep 17 00:00:00 2001 From: Chris Morgan Date: Sat, 8 Jan 2022 01:02:10 +1100 Subject: [PATCH 3/3] Fix Builder on cfg(not(any(unix, windows))) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cfg(target_arch = "wasm32") would panic if you tried to do any std::fs Builder stuff, which was fine for wasm32-unknown-unknown and wasm32-unknown-emscripten, but not good for wasm32-wasi, which has a file system. (Might still want a specialised implementation for wasm32-wasi: std::os::wasi::fs::FileTypeExt is currently unstable, but offers is_block_device, is_character_device, is_socket_dgram and is_socket_stream. But this one is good to start with.) - cfg(not(any(unix, windows, target_arch = "wasm32"))) would fail to compile (… unless you disabled the builder feature, since my previous commit). Now it all works pretty decently. --- src/header.rs | 70 ++++++++++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/src/header.rs b/src/header.rs index a5a9be7c..c4010c6d 100644 --- a/src/header.rs +++ b/src/header.rs @@ -736,12 +736,6 @@ impl Header { } } - #[cfg(all(target_arch = "wasm32", feature = "builder"))] - #[allow(unused_variables)] - fn fill_platform_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { - unimplemented!(); - } - #[cfg(all(unix, feature = "builder"))] fn fill_platform_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { match mode { @@ -801,39 +795,53 @@ impl Header { } } - #[cfg(all(windows, feature = "builder"))] + #[cfg(all(not(unix), feature = "builder"))] + #[allow(unused_variables)] fn fill_platform_from(&mut self, meta: &fs::Metadata, mode: HeaderMode) { - // There's no concept of a file mode on Windows, so do a best approximation here. - match mode { + // Fallback implementation: approximate modes, and get as good an mtime as we can. + let mtime = match mode { + #[cfg(windows)] HeaderMode::Complete => { - self.set_uid(0); - self.set_gid(0); // The dates listed in tarballs are always seconds relative to // January 1, 1970. On Windows, however, the timestamps are returned as // dates relative to January 1, 1601 (in 100ns intervals), so we need to // add in some offset for those dates. - let mtime = (meta.last_write_time() / (1_000_000_000 / 100)) - 11644473600; - self.set_mtime(mtime); - let fs_mode = { - const FILE_ATTRIBUTE_READONLY: u32 = 0x00000001; - let readonly = meta.file_attributes() & FILE_ATTRIBUTE_READONLY; - match (meta.is_dir(), readonly != 0) { - (true, false) => 0o755, - (true, true) => 0o555, - (false, false) => 0o644, - (false, true) => 0o444, - } - }; - self.set_mode(fs_mode); + (meta.last_write_time() / (1_000_000_000 / 100)) - 11644473600 } - HeaderMode::Deterministic => { - self.set_uid(0); - self.set_gid(0); - self.set_mtime(123456789); // see above in unix - let fs_mode = if meta.is_dir() { 0o755 } else { 0o644 }; - self.set_mode(fs_mode); + #[cfg(not(windows))] + HeaderMode::Complete => { + meta.modified() + .ok() + .and_then(|time| time.duration_since(std::time::SystemTime::UNIX_EPOCH).ok()) + .map(|duration| duration.as_secs()) + .unwrap_or(1) // avoiding zero mtimes, see above in unix } - } + HeaderMode::Deterministic => 123456789, // see above in unix + }; + let fs_mode = match mode { + #[cfg(windows)] + HeaderMode::Complete => { + const FILE_ATTRIBUTE_READONLY: u32 = 0x00000001; + let readonly = meta.file_attributes() & FILE_ATTRIBUTE_READONLY; + match (meta.is_dir(), readonly != 0) { + (true, false) => 0o755, + (true, true) => 0o555, + (false, false) => 0o644, + (false, true) => 0o444, + } + } + _ => { + if meta.is_dir() { + 0o755 + } else { + 0o644 + } + } + }; + self.set_uid(0); + self.set_gid(0); + self.set_mtime(mtime); + self.set_mode(fs_mode); let ft = meta.file_type(); self.set_entry_type(if ft.is_dir() {