From 71893aafeb1e8badd787408f350a90cd15ed0285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Tue, 11 Oct 2022 21:22:45 -0300 Subject: [PATCH 1/3] Refactor the way we set last modified times Last modified time is a piece of metadata that is available when decompressing an archive --- Cargo.lock | 16 +++++++++++++ Cargo.toml | 3 ++- src/archive/zip.rs | 58 +++++++++++----------------------------------- 3 files changed, 31 insertions(+), 46 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e893c68f..a41d737a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -393,6 +393,12 @@ dependencies = [ "either", ] +[[package]] +name = "itoa" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4217ad341ebadf8d8e724e264f13e593e0648f5b3e94b3896a5df283be015ecc" + [[package]] name = "jobserver" version = "0.1.25" @@ -521,6 +527,7 @@ dependencies = [ "clap", "clap_complete", "clap_mangen", + "filetime", "flate2", "fs-err", "ignore", @@ -913,10 +920,18 @@ version = "0.3.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d634a985c4d4238ec39cacaed2e7ae552fbd3c476b552c1deac3021b7d7eaf0c" dependencies = [ + "itoa", "libc", "num_threads", + "time-macros", ] +[[package]] +name = "time-macros" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42657b1a6f4d817cda8e7a0ace261fe0cc946cf3a80314390b22cc61ae080792" + [[package]] name = "unicode-ident" version = "1.0.5" @@ -1075,6 +1090,7 @@ dependencies = [ "crc32fast", "crossbeam-utils", "flate2", + "time", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index b7928597c..701d46109 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,11 +25,12 @@ once_cell = "1.15.0" snap = "1.0.5" tar = "0.4.38" xz2 = "0.1.7" -zip = { version = "0.6.2", default-features = false } +zip = { version = "0.6.2", default-features = false, features = ["time"] } zstd = { version = "0.11.2", default-features = false } tempfile = "3.3.0" ignore = "0.4.18" indicatif = "0.17.1" +filetime = "0.2.17" [target.'cfg(unix)'.dependencies] time = { version = "0.3.15", default-features = false } diff --git a/src/archive/zip.rs b/src/archive/zip.rs index 77b9684e1..b3e329be7 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -10,6 +10,7 @@ use std::{ thread, }; +use filetime::{set_file_mtime, FileTime}; use fs_err as fs; use zip::{self, read::ZipFile, ZipArchive}; @@ -72,13 +73,12 @@ where let mut output_file = fs::File::create(file_path)?; io::copy(&mut file, &mut output_file)?; - #[cfg(unix)] - set_last_modified_time(&output_file, &file)?; + set_last_modified_time(&file, file_path)?; } } #[cfg(unix)] - __unix_set_permissions(&file_path, &file)?; + unix_set_permissions(&file_path, &file)?; unpacked_files.push(file_path); } @@ -228,55 +228,23 @@ fn display_zip_comment_if_exists(file: &ZipFile) { } } -#[cfg(unix)] -/// Attempts to convert a [`zip::DateTime`] to a [`libc::timespec`]. -fn convert_zip_date_time(date_time: zip::DateTime) -> Option { - use time::{Date, Month, PrimitiveDateTime, Time}; - - // Safety: time::Month is repr(u8) and goes from 1 to 12 - let month: Month = unsafe { std::mem::transmute(date_time.month()) }; - - let date = Date::from_calendar_date(date_time.year() as _, month, date_time.day()).ok()?; - - let time = Time::from_hms(date_time.hour(), date_time.minute(), date_time.second()).ok()?; - - let date_time = PrimitiveDateTime::new(date, time); - let timestamp = date_time.assume_utc().unix_timestamp(); - - Some(libc::timespec { - tv_sec: timestamp, - tv_nsec: 0, - }) -} - -#[cfg(unix)] -fn set_last_modified_time(file: &fs::File, zip_file: &ZipFile) -> crate::Result<()> { - use std::os::unix::prelude::AsRawFd; - - use libc::UTIME_NOW; - - let now = libc::timespec { - tv_sec: 0, - tv_nsec: UTIME_NOW, - }; - - let last_modified = zip_file.last_modified(); - let last_modified = convert_zip_date_time(last_modified).unwrap_or(now); - - // The first value is the last accessed time, which we'll set as being right now. - // The second value is the last modified time, which we'll copy over from the zip archive - let times = [now, last_modified]; +fn set_last_modified_time(zip_file: &ZipFile, path: &Path) -> crate::Result<()> { + let modification_time_in_seconds = zip_file + .last_modified() + .to_time() + .expect("Zip archive contains a file with broken 'last modified time'") + .unix_timestamp(); - let output_fd = file.as_raw_fd(); + // Zip does not support nanoseconds, so we can assume zero here + let modification_time = FileTime::from_unix_time(modification_time_in_seconds, 0); - // TODO: check for -1 - unsafe { libc::futimens(output_fd, × as *const _) }; + set_file_mtime(path, modification_time)?; Ok(()) } #[cfg(unix)] -fn __unix_set_permissions(file_path: &Path, file: &ZipFile) -> crate::Result<()> { +fn unix_set_permissions(file_path: &Path, file: &ZipFile) -> crate::Result<()> { use std::fs::Permissions; if let Some(mode) = file.unix_mode() { From a2c91c42483b15ff0ed9246dd1a6bfff5839fec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Tue, 11 Oct 2022 21:24:04 -0300 Subject: [PATCH 2/3] minor tweaks --- src/archive/tar.rs | 8 ++++++-- src/archive/zip.rs | 7 ++++++- src/utils/question.rs | 2 -- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/archive/tar.rs b/src/archive/tar.rs index acf0908c4..85b41b794 100644 --- a/src/archive/tar.rs +++ b/src/archive/tar.rs @@ -9,7 +9,6 @@ use std::{ }; use fs_err as fs; -use tar; use crate::{ error::FinalError, @@ -40,7 +39,12 @@ pub fn unpack_archive( // spoken text for users using screen readers, braille displays // and so on - info!(@display_handle, inaccessible, "{:?} extracted. ({})", utils::strip_cur_dir(&output_folder.join(file.path()?)), Bytes::new(file.size())); + info!( + @display_handle, + inaccessible, + "{:?} extracted. ({})", + utils::strip_cur_dir(&output_folder.join(file.path()?)), Bytes::new(file.size()) + ); files_unpacked.push(file_path); } diff --git a/src/archive/zip.rs b/src/archive/zip.rs index b3e329be7..bae50a3e4 100644 --- a/src/archive/zip.rs +++ b/src/archive/zip.rs @@ -68,7 +68,12 @@ where let file_path = strip_cur_dir(file_path.as_path()); // same reason is in _is_dir: long, often not needed text - info!(@display_handle, inaccessible, "{:?} extracted. ({})", file_path.display(), Bytes::new(file.size())); + info!( + @display_handle, + inaccessible, + "{:?} extracted. ({})", + file_path.display(), Bytes::new(file.size()) + ); let mut output_file = fs::File::create(file_path)?; io::copy(&mut file, &mut output_file)?; diff --git a/src/utils/question.rs b/src/utils/question.rs index f8009addc..84b0fb91f 100644 --- a/src/utils/question.rs +++ b/src/utils/question.rs @@ -60,8 +60,6 @@ pub fn create_or_ask_overwrite(path: &Path, question_policy: QuestionPolicy) -> Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => { if user_wants_to_overwrite(path, question_policy)? { if path.is_dir() { - // We can't just use `fs::File::create(&path)` because it would return io::ErrorKind::IsADirectory - // ToDo: Maybe we should emphasise that `path` is a directory and everything inside it will be gone? fs::remove_dir_all(path)?; } Ok(Some(fs::File::create(path)?)) From acb756671a001c3435e8cc0cc248d112d0a16290 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20M=2E=20Bezerra?= Date: Tue, 11 Oct 2022 21:26:01 -0300 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e7a00374..361542cfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,7 @@ Categories Used: - Respect file permissions when compressing zip files [\#271](https://github.com/ouch-org/ouch/pull/271) ([figsoda](https://github.com/figsoda)) - Apply clippy lints [\#273](https://github.com/ouch-org/ouch/pull/273) ([figsoda](https://github.com/figsoda)) - Warn user if file extension is passed as file name [\#277](https://github.com/ouch-org/ouch/pull/277) ([marcospb19](https://github.com/marcospb19)) +- Check for errors when setting the last modified time [\#278](https://github.com/ouch-org/ouch/pull/278) ([marcospb19](https://github.com/marcospb19)) ### Tweaks @@ -93,6 +94,7 @@ Categories Used: - Update dependencies [\#257](https://github.com/ouch-org/ouch/pull/257) ([Artturin](https://github.com/Artturin)) - Add pull request template [\#263](https://github.com/ouch-org/ouch/pull/263) ([figsoda](https://github.com/figsoda)) - Clean up the description for the `-d/--dir` argument to `decompress` [\#264](https://github.com/ouch-org/ouch/pull/264) ([hivehand](https://github.com/hivehand)) +- Show subcommand aliases on --help [\#275](https://github.com/ouch-org/ouch/pull/275) ([marcospb19](https://github.com/marcospb19)) - Update dependencies [\#276](https://github.com/ouch-org/ouch/pull/276) ([figsoda](https://github.com/figsoda)) ### New Contributors