From 703cf62fd2cb981a8037945f42bf3163a9b8a238 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 05:47:10 +0100 Subject: [PATCH 01/10] date: mark unfixable argument handlers, prepare tests --- src/uu/date/src/date.rs | 2 ++ tests/by-util/test_date.rs | 44 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 02737dca28..1d8af024af 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -313,6 +313,8 @@ pub fn uu_app() -> Command { .about(ABOUT) .override_usage(format_usage(USAGE)) .infer_long_args(true) + // Must not use .args_override_self(true)! + // Some flags like --rfc-email do NOT override themselves. .arg( Arg::new(OPT_DATE) .short('d') diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 553414af85..64adf0d05c 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -2,6 +2,9 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. + +// spell-checker:ignore (arguments) Idate Idefinitely + use crate::common::util::TestScenario; use regex::Regex; #[cfg(all(unix, not(target_os = "macos")))] @@ -478,3 +481,44 @@ fn test_date_from_stdin() { Sat Apr 15 18:30:00 2023\n", ); } + +#[test] +#[ignore = "known issue https://github.com/uutils/coreutils/issues/4254#issuecomment-2026446634"] +fn test_format_conflict_self() { + for param in ["-I", "-Idate", "-R", "--rfc-3339=date"] { + new_ucmd!() + .arg(param) + .arg(param) + .fails() + .stderr_contains("multiple output formats specified"); + } +} + +#[test] +#[ignore = "known issue https://github.com/uutils/coreutils/issues/4254#issuecomment-2026446634"] +fn test_format_conflict_other() { + new_ucmd!() + .args(&["-I", "-Idate", "-R", "--rfc-3339=date"]) + .fails() + .stderr_contains("multiple output formats specified"); +} + +#[test] +#[ignore = "known issue https://github.com/uutils/coreutils/issues/4254#issuecomment-2026446634"] +fn test_format_error_priority() { + // First, try to parse the value to "-I", even though it cannot be useful: + new_ucmd!() + .args(&["-R", "-Idefinitely_invalid"]) + .fails() + .stderr_contains("definitely_invalid"); + // And then raise an error: + new_ucmd!() + .args(&["-R", "-R"]) + .fails() + .stderr_contains("multiple output formats specified"); + // Even if a later argument would be "even more invalid": + new_ucmd!() + .args(&["-R", "-R", "-Idefinitely_invalid"]) + .fails() + .stderr_contains("multiple output formats specified"); +} From 5f5b04dfe2fdb0dc7c770239a19c739dd7a4654f Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 06:02:57 +0100 Subject: [PATCH 02/10] date: enable repeating trivial flags --- src/uu/date/src/date.rs | 4 ++++ tests/by-util/test_date.rs | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 1d8af024af..c107d3f1a2 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -320,6 +320,7 @@ pub fn uu_app() -> Command { .short('d') .long(OPT_DATE) .value_name("STRING") + .overrides_with(OPT_DATE) .help("display time described by STRING, not 'now'"), ) .arg( @@ -328,6 +329,7 @@ pub fn uu_app() -> Command { .long(OPT_FILE) .value_name("DATEFILE") .value_hint(clap::ValueHint::FilePath) + .overrides_with(OPT_FILE) .help("like --date; once for each line of DATEFILE"), ) .arg( @@ -360,6 +362,7 @@ pub fn uu_app() -> Command { Arg::new(OPT_DEBUG) .long(OPT_DEBUG) .help("annotate the parsed date, and warn about questionable usage to stderr") + .overrides_with(OPT_DEBUG) .action(ArgAction::SetTrue), ) .arg( @@ -383,6 +386,7 @@ pub fn uu_app() -> Command { .long(OPT_UNIVERSAL) .alias(OPT_UNIVERSAL_2) .help("print or set Coordinated Universal Time (UTC)") + .overrides_with(OPT_UNIVERSAL) .action(ArgAction::SetTrue), ) .arg(Arg::new(OPT_FORMAT)) diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 64adf0d05c..293e9537ef 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -522,3 +522,39 @@ fn test_format_error_priority() { .fails() .stderr_contains("multiple output formats specified"); } + +#[test] +fn test_pick_last_date() { + new_ucmd!() + .arg("-d20020304") + .arg("-d20010203") + .arg("-I") + .succeeds() + .stdout_only("2001-02-03\n") + .no_stderr(); +} + +#[test] +fn test_repeat_from_file() { + const FILE1: &str = "file1"; + const FILE2: &str = "file2"; + let (at, mut ucmd) = at_and_ucmd!(); + at.write( + FILE1, + "2001-01-01 13:30:00+08:00\n2010-01-10 13:30:00+08:00\n", + ); + at.write(FILE2, "2020-03-12 13:30:00+08:00\n"); + ucmd.args(&["-I", "-f", FILE1, "-f", FILE2]) + .succeeds() + .stdout_only("2020-03-12\n") + .no_stderr(); +} + +#[test] +fn test_repeat_flags() { + new_ucmd!() + .args(&["-d20010203", "-I", "--debug", "--debug", "-u", "-u"]) + .succeeds() + .stdout_only("2001-02-03\n"); + // stderr may or may not contain something. +} From dcf5b669a2bf87428a20e3ea3b08bf9386dab044 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 14:58:13 +0100 Subject: [PATCH 03/10] date: implement and test reference resolution-order --- src/uu/date/src/date.rs | 14 +++++++++++++- tests/by-util/test_date.rs | 38 +++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index c107d3f1a2..2653c8d7d4 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -12,7 +12,7 @@ use chrono::{Datelike, Timelike}; use clap::{crate_version, Arg, ArgAction, Command}; #[cfg(all(unix, not(target_os = "macos"), not(target_os = "redox")))] use libc::{clock_settime, timespec, CLOCK_REALTIME}; -use std::fs::File; +use std::fs::{metadata, File}; use std::io::{BufRead, BufReader}; use std::path::PathBuf; use uucore::display::Quotable; @@ -92,6 +92,7 @@ enum DateSource { Custom(String), File(PathBuf), Stdin, + Reference(PathBuf), Human(TimeDelta), } @@ -178,6 +179,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { "-" => DateSource::Stdin, _ => DateSource::File(file.into()), } + } else if let Some(file) = matches.get_one::(OPT_REFERENCE) { + DateSource::Reference(file.into()) } else { DateSource::Now }; @@ -260,6 +263,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let iter = lines.map_while(Result::ok).map(parse_date); Box::new(iter) } + DateSource::Reference(ref path) => { + let metadata = metadata(path)?; + // TODO: "This field might not be available on all platforms" → which ones? + let date_utc: DateTime = metadata.modified()?.into(); + let date: DateTime = date_utc.into(); + let iter = std::iter::once(Ok(date)); + Box::new(iter) + } DateSource::Now => { let iter = std::iter::once(Ok(now)); Box::new(iter) @@ -371,6 +382,7 @@ pub fn uu_app() -> Command { .long(OPT_REFERENCE) .value_name("FILE") .value_hint(clap::ValueHint::AnyPath) + .overrides_with(OPT_REFERENCE) .help("display the last modification time of FILE"), ) .arg( diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 293e9537ef..25bdffbadc 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -5,11 +5,17 @@ // spell-checker:ignore (arguments) Idate Idefinitely -use crate::common::util::TestScenario; +use crate::common::util::{AtPath, TestScenario}; +use filetime::{set_file_times, FileTime}; use regex::Regex; #[cfg(all(unix, not(target_os = "macos")))] use uucore::process::geteuid; +fn set_file_times_unix(at: &AtPath, path: &str, secs_since_epoch: i64) { + let time = FileTime::from_unix_time(secs_since_epoch, 0); + set_file_times(at.plus_as_string(path), time, time).expect("touch failed"); +} + #[test] fn test_invalid_arg() { new_ucmd!().arg("--definitely-invalid").fails().code_is(1); @@ -558,3 +564,33 @@ fn test_repeat_flags() { .stdout_only("2001-02-03\n"); // stderr may or may not contain something. } + +#[test] +fn test_repeat_reference_newer_last() { + const FILE1: &str = "file1"; + const FILE2: &str = "file2"; + let (at, mut ucmd) = at_and_ucmd!(); + at.touch(FILE1); + at.touch(FILE2); + set_file_times_unix(&at, FILE1, 981_203_696); // 2001-02-03 12:34:56 + set_file_times_unix(&at, FILE2, 1_323_779_696); // 2011-12-13 12:34:56 + ucmd.args(&["-I", "-r", FILE1, "-r", FILE2]) + .succeeds() + .stdout_only("2011-12-13\n") + .no_stderr(); +} + +#[test] +fn test_repeat_reference_older_last() { + const FILE1: &str = "file1"; + const FILE2: &str = "file2"; + let (at, mut ucmd) = at_and_ucmd!(); + at.touch(FILE1); + at.touch(FILE2); + set_file_times_unix(&at, FILE1, 1_323_779_696); // 2011-12-13 12:34:56 + set_file_times_unix(&at, FILE2, 981_203_696); // 2001-02-03 12:34:56 + ucmd.args(&["-I", "-r", FILE1, "-r", FILE2]) + .succeeds() + .stdout_only("2001-02-03\n") + .no_stderr(); +} From 37004749e682681b0ae1c7b1f8cf20376e10b08c Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 16:10:22 +0100 Subject: [PATCH 04/10] date: enable repeating --set --- src/uu/date/src/date.rs | 1 + tests/by-util/test_date.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 2653c8d7d4..be6359c046 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -390,6 +390,7 @@ pub fn uu_app() -> Command { .short('s') .long(OPT_SET) .value_name("STRING") + .overrides_with(OPT_SET) .help(OPT_SET_HELP_STRING), ) .arg( diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 25bdffbadc..eec369e33e 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -238,9 +238,28 @@ fn test_date_format_literal() { fn test_date_set_valid() { if geteuid() == 0 { new_ucmd!() + .arg("-I") .arg("--set") .arg("2020-03-12 13:30:00+08:00") .succeeds() + // FIXME: .stdout_only("2020-03-12") + .no_stdout() + .no_stderr(); + } +} + +#[test] +#[cfg(all(unix, not(target_os = "macos")))] +fn test_date_set_valid_repeated() { + if geteuid() == 0 { + new_ucmd!() + .arg("-I") + .arg("--set") + .arg("2021-03-12 13:30:00+08:00") + .arg("--set") + .arg("2022-03-12 13:30:00+08:00") + .succeeds() + // FIXME: .stdout_only("2022-03-12") .no_stdout() .no_stderr(); } @@ -267,6 +286,21 @@ fn test_date_set_permissions_error() { } } +#[test] +#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] +fn test_date_set_permissions_error_repeated() { + if !(geteuid() == 0 || uucore::os::is_wsl_1()) { + let result = new_ucmd!() + .arg("--set") + .arg("2020-03-11 21:45:00+08:00") + .arg("--set") + .arg("2021-03-11 21:45:00+08:00") + .fails(); + result.no_stdout(); + assert!(result.stderr_str().starts_with("date: cannot set date: ")); + } +} + #[test] #[cfg(target_os = "macos")] fn test_date_set_mac_unavailable() { From 21876ab54f737d084fc02f91225be762dbf9f0a5 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 16:22:10 +0100 Subject: [PATCH 05/10] date: produce output on --set --- src/uu/date/src/date.rs | 95 +++++++++++++++++++------------------- tests/by-util/test_date.rs | 6 +-- 2 files changed, 50 insertions(+), 51 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index be6359c046..5e8aa4a5e6 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -203,27 +203,28 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { set_to, }; - if let Some(date) = settings.set_to { + // Get the current time, either in the local time zone or UTC. + let now: DateTime = if settings.utc { + let now = Utc::now(); + now.with_timezone(&now.offset().fix()) + } else { + let now = Local::now(); + now.with_timezone(now.offset()) + }; + + // Iterate over all dates - whether it's a single date or a file. + let dates: Box> = if let Some(date) = settings.set_to { // All set time functions expect UTC datetimes. let date: DateTime = if settings.utc { date.with_timezone(&Utc) } else { date.into() }; - - return set_system_datetime(date); + set_system_datetime(date)?; + let date_fixed: DateTime = date.into(); + Box::new(std::iter::once(Ok(date_fixed))) } else { - // Get the current time, either in the local time zone or UTC. - let now: DateTime = if settings.utc { - let now = Utc::now(); - now.with_timezone(&now.offset().fix()) - } else { - let now = Local::now(); - now.with_timezone(now.offset()) - }; - - // Iterate over all dates - whether it's a single date or a file. - let dates: Box> = match settings.date_source { + match settings.date_source { DateSource::Custom(ref input) => { let date = parse_date(input.clone()); let iter = std::iter::once(date); @@ -275,43 +276,43 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let iter = std::iter::once(Ok(now)); Box::new(iter) } - }; + } + }; - let format_string = make_format_string(&settings); + let format_string = make_format_string(&settings); - // Format all the dates - for date in dates { - match date { - Ok(date) => { - // GNU `date` uses `%N` for nano seconds, however crate::chrono uses `%f` - let format_string = &format_string.replace("%N", "%f"); - // Refuse to pass this string to chrono as it is crashing in this crate - if format_string.contains("%#z") { - return Err(USimpleError::new( - 1, - format!("invalid format {}", format_string.replace("%f", "%N")), - )); - } - // Hack to work around panic in chrono, - // TODO - remove when a fix for https://github.com/chronotope/chrono/issues/623 is released - let format_items = StrftimeItems::new(format_string); - if format_items.clone().any(|i| i == Item::Error) { - return Err(USimpleError::new( - 1, - format!("invalid format {}", format_string.replace("%f", "%N")), - )); - } - let formatted = date - .format_with_items(format_items) - .to_string() - .replace("%f", "%N"); - println!("{formatted}"); + // Format all the dates + for date in dates { + match date { + Ok(date) => { + // GNU `date` uses `%N` for nano seconds, however crate::chrono uses `%f` + let format_string = &format_string.replace("%N", "%f"); + // Refuse to pass this string to chrono as it is crashing in this crate + if format_string.contains("%#z") { + return Err(USimpleError::new( + 1, + format!("invalid format {}", format_string.replace("%f", "%N")), + )); + } + // Hack to work around panic in chrono, + // TODO - remove when a fix for https://github.com/chronotope/chrono/issues/623 is released + let format_items = StrftimeItems::new(format_string); + if format_items.clone().any(|i| i == Item::Error) { + return Err(USimpleError::new( + 1, + format!("invalid format {}", format_string.replace("%f", "%N")), + )); } - Err((input, _err)) => show!(USimpleError::new( - 1, - format!("invalid date {}", input.quote()) - )), + let formatted = date + .format_with_items(format_items) + .to_string() + .replace("%f", "%N"); + println!("{formatted}"); } + Err((input, _err)) => show!(USimpleError::new( + 1, + format!("invalid date {}", input.quote()) + )), } } diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index eec369e33e..8c3bde5beb 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -242,8 +242,7 @@ fn test_date_set_valid() { .arg("--set") .arg("2020-03-12 13:30:00+08:00") .succeeds() - // FIXME: .stdout_only("2020-03-12") - .no_stdout() + .stdout_only("2020-03-12\n") .no_stderr(); } } @@ -259,8 +258,7 @@ fn test_date_set_valid_repeated() { .arg("--set") .arg("2022-03-12 13:30:00+08:00") .succeeds() - // FIXME: .stdout_only("2022-03-12") - .no_stdout() + .stdout_only("2022-03-12\n") .no_stderr(); } } From 08bf2c747dfc99f7bf01f8c83a049226ce9bd0ac Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 16:47:17 +0100 Subject: [PATCH 06/10] date: improve range of accepted values for --set --- src/uu/date/src/date.rs | 19 ++++++------------- tests/by-util/test_date.rs | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 5e8aa4a5e6..b944b8e4c5 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -74,7 +74,7 @@ struct Settings { utc: bool, format: Format, date_source: DateSource, - set_to: Option>, + set_to: Option, } /// Various ways of displaying the date @@ -185,22 +185,13 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { DateSource::Now }; - let set_to = match matches.get_one::(OPT_SET).map(parse_date) { - None => None, - Some(Err((input, _err))) => { - return Err(USimpleError::new( - 1, - format!("invalid date {}", input.quote()), - )); - } - Some(Ok(date)) => Some(date), - }; + let set_to = matches.get_one::(OPT_SET); let settings = Settings { utc: matches.get_flag(OPT_UNIVERSAL), format, date_source, - set_to, + set_to: set_to.cloned(), }; // Get the current time, either in the local time zone or UTC. @@ -213,7 +204,9 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; // Iterate over all dates - whether it's a single date or a file. - let dates: Box> = if let Some(date) = settings.set_to { + let dates: Box> = if let Some(date_string) = &settings.set_to { + let date = parse_datetime::parse_datetime_at_date(now.into(), date_string) + .map_err(|_| USimpleError::new(1, format!("invalid date {}", date_string.quote())))?; // All set time functions expect UTC datetimes. let date: DateTime = if settings.utc { date.with_timezone(&Utc) diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 8c3bde5beb..6506df45e7 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -284,6 +284,27 @@ fn test_date_set_permissions_error() { } } +#[test] +#[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] +fn test_date_set_permissions_error_interpreted() { + // This implicitly tests that the given strings are interpreted as valid dates, + // because parsing errors would have been discovered earlier in the process. + if !(geteuid() == 0 || uucore::os::is_wsl_1()) { + for date_string in [ + "yesterday", + // TODO "a fortnight ago", + "42 days", + "2001-02-03", + "20010203", + // TODO "02/03/2001", + ] { + let result = new_ucmd!().arg("-s").arg(date_string).fails(); + // stdout depends on the specific date; don't check it. + assert!(result.stderr_str().starts_with("date: cannot set date: ")); + } + } +} + #[test] #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] fn test_date_set_permissions_error_repeated() { From 9a740cad737c7d732f1c7af765571dfb205e3611 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Fri, 29 Mar 2024 17:13:51 +0100 Subject: [PATCH 07/10] date: forbid conflicting options --- src/uu/date/src/date.rs | 9 +++++++++ tests/by-util/test_date.rs | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index b944b8e4c5..f469b5c430 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -326,6 +326,8 @@ pub fn uu_app() -> Command { .long(OPT_DATE) .value_name("STRING") .overrides_with(OPT_DATE) + .conflicts_with(OPT_FILE) + .conflicts_with(OPT_REFERENCE) .help("display time described by STRING, not 'now'"), ) .arg( @@ -335,6 +337,7 @@ pub fn uu_app() -> Command { .value_name("DATEFILE") .value_hint(clap::ValueHint::FilePath) .overrides_with(OPT_FILE) + .conflicts_with(OPT_REFERENCE) .help("like --date; once for each line of DATEFILE"), ) .arg( @@ -347,6 +350,8 @@ pub fn uu_app() -> Command { ])) .num_args(0..=1) .default_missing_value(OPT_DATE) + .conflicts_with(OPT_RFC_EMAIL) + .conflicts_with(OPT_RFC_3339) .help(ISO_8601_HELP_STRING), ) .arg( @@ -354,6 +359,7 @@ pub fn uu_app() -> Command { .short('R') .long(OPT_RFC_EMAIL) .help(RFC_5322_HELP_STRING) + .conflicts_with(OPT_RFC_3339) .action(ArgAction::SetTrue), ) .arg( @@ -385,6 +391,9 @@ pub fn uu_app() -> Command { .long(OPT_SET) .value_name("STRING") .overrides_with(OPT_SET) + .conflicts_with(OPT_DATE) + .conflicts_with(OPT_FILE) + .conflicts_with(OPT_REFERENCE) .help(OPT_SET_HELP_STRING), ) .arg( diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 6506df45e7..fdf28a4e34 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -647,3 +647,27 @@ fn test_repeat_reference_older_last() { .stdout_only("2001-02-03\n") .no_stderr(); } + +#[test] +fn test_incompatible_args() { + for args in [ + // Input with other input + vec!["-d", "now", "-f", "foo"], + vec!["-d", "now", "-r", "foo"], + vec!["-f", "foo", "-r", "foo"], + // Format with other format + vec!["-I", "-R"], + vec!["-I", "--rfc-3339=date"], + vec!["-R", "--rfc-3339=date"], + // Input with --set + vec!["-d", "now", "-s", "now"], + vec!["-r", "foo", "-s", "now"], + vec!["-f", "foo", "-s", "now"], + ] { + new_ucmd!() + .args(&args) + .fails() + .no_stdout() + .stderr_contains(" cannot be used with "); + } +} From 4326e3ed06618b757a53215987f401c1af7a82e5 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 11 Aug 2024 22:57:10 +0200 Subject: [PATCH 08/10] date: refactor argument parsing into their own methods --- src/uu/date/src/date.rs | 100 ++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 44 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index f469b5c430..c8b8131aa9 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -9,7 +9,7 @@ use chrono::format::{Item, StrftimeItems}; use chrono::{DateTime, FixedOffset, Local, Offset, TimeDelta, Utc}; #[cfg(windows)] use chrono::{Datelike, Timelike}; -use clap::{crate_version, Arg, ArgAction, Command}; +use clap::{crate_version, Arg, ArgAction, ArgMatches, Command}; #[cfg(all(unix, not(target_os = "macos"), not(target_os = "redox")))] use libc::{clock_settime, timespec, CLOCK_REALTIME}; use std::fs::{metadata, File}; @@ -136,54 +136,65 @@ impl<'a> From<&'a str> for Rfc3339Format { } } +impl Format { + fn extract_from(matches: &ArgMatches) -> UResult { + if let Some(form) = matches.get_one::(OPT_FORMAT) { + if let Some(stripped_form) = form.strip_prefix('+') { + Ok(Self::Custom(stripped_form.to_string())) + } else { + Err(USimpleError::new( + 1, + format!("invalid date {}", form.quote()), + )) + } + } else if let Some(fmt) = matches + .get_many::(OPT_ISO_8601) + .map(|mut iter| iter.next().unwrap_or(&DATE.to_string()).as_str().into()) + { + Ok(Self::Iso8601(fmt)) + } else if matches.get_flag(OPT_RFC_EMAIL) { + Ok(Self::Rfc5322) + } else if let Some(fmt) = matches + .get_one::(OPT_RFC_3339) + .map(|s| s.as_str().into()) + { + Ok(Self::Rfc3339(fmt)) + } else { + Ok(Self::Default) + } + } +} + +impl DateSource { + fn extract_from(matches: &ArgMatches) -> Self { + if let Some(date) = matches.get_one::(OPT_DATE) { + let ref_time = Local::now(); + if let Ok(new_time) = parse_datetime::parse_datetime_at_date(ref_time, date.as_str()) { + let duration = new_time.signed_duration_since(ref_time); + Self::Human(duration) + } else { + Self::Custom(date.into()) + } + } else if let Some(file) = matches.get_one::(OPT_FILE) { + match file.as_ref() { + "-" => Self::Stdin, + _ => Self::File(file.into()), + } + } else if let Some(file) = matches.get_one::(OPT_REFERENCE) { + Self::Reference(file.into()) + } else { + Self::Now + } + } +} + #[uucore::main] -#[allow(clippy::cognitive_complexity)] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; - let format = if let Some(form) = matches.get_one::(OPT_FORMAT) { - if !form.starts_with('+') { - return Err(USimpleError::new( - 1, - format!("invalid date {}", form.quote()), - )); - } - let form = form[1..].to_string(); - Format::Custom(form) - } else if let Some(fmt) = matches - .get_many::(OPT_ISO_8601) - .map(|mut iter| iter.next().unwrap_or(&DATE.to_string()).as_str().into()) - { - Format::Iso8601(fmt) - } else if matches.get_flag(OPT_RFC_EMAIL) { - Format::Rfc5322 - } else if let Some(fmt) = matches - .get_one::(OPT_RFC_3339) - .map(|s| s.as_str().into()) - { - Format::Rfc3339(fmt) - } else { - Format::Default - }; + let format = Format::extract_from(&matches)?; - let date_source = if let Some(date) = matches.get_one::(OPT_DATE) { - let ref_time = Local::now(); - if let Ok(new_time) = parse_datetime::parse_datetime_at_date(ref_time, date.as_str()) { - let duration = new_time.signed_duration_since(ref_time); - DateSource::Human(duration) - } else { - DateSource::Custom(date.into()) - } - } else if let Some(file) = matches.get_one::(OPT_FILE) { - match file.as_ref() { - "-" => DateSource::Stdin, - _ => DateSource::File(file.into()), - } - } else if let Some(file) = matches.get_one::(OPT_REFERENCE) { - DateSource::Reference(file.into()) - } else { - DateSource::Now - }; + let date_source = DateSource::extract_from(&matches); let set_to = matches.get_one::(OPT_SET); @@ -205,6 +216,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { // Iterate over all dates - whether it's a single date or a file. let dates: Box> = if let Some(date_string) = &settings.set_to { + // Ignore settings.date_source, as it should not be able to be set on the command-line. let date = parse_datetime::parse_datetime_at_date(now.into(), date_string) .map_err(|_| USimpleError::new(1, format!("invalid date {}", date_string.quote())))?; // All set time functions expect UTC datetimes. From e7d618ff4956531b109bdb6060baf5f2694fff85 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 11 Aug 2024 23:25:31 +0200 Subject: [PATCH 09/10] date: refactor date computation into its own method --- src/uu/date/src/date.rs | 120 +++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 56 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index c8b8131aa9..93cf2934b4 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -24,6 +24,9 @@ use windows_sys::Win32::{Foundation::SYSTEMTIME, System::SystemInformation::SetS use uucore::shortcut_value_parser::ShortcutValueParser; +type MaybeParsedDatetime = + Result, (String, parse_datetime::ParseDateTimeError)>; + // Options const DATE: &str = "date"; const HOURS: &str = "hours"; @@ -186,6 +189,65 @@ impl DateSource { Self::Now } } + + // The hideous error-type stems from `parse_date`. + fn try_into_iterator( + &self, + now: &DateTime, + ) -> UResult>> { + match self { + Self::Custom(ref input) => { + let date = parse_date(input.clone()); + let iter = std::iter::once(date); + Ok(Box::new(iter)) + } + Self::Human(relative_time) => { + // Double-check the result is overflow or not of the current_time + relative_time + // it may cause a panic of chrono::datetime::DateTime add + match now.checked_add_signed(*relative_time) { + Some(date) => { + let iter = std::iter::once(Ok(date)); + Ok(Box::new(iter)) + } + None => Err(USimpleError::new( + 1, + format!("invalid date {}", relative_time), + )), + } + } + Self::Stdin => { + let lines = BufReader::new(std::io::stdin()).lines(); + let iter = lines.map_while(Result::ok).map(parse_date); + Ok(Box::new(iter)) + } + Self::File(ref path) => { + if path.is_dir() { + Err(USimpleError::new( + 2, + format!("expected file, got directory {}", path.quote()), + )) + } else { + let file = File::open(path) + .map_err_context(|| path.as_os_str().to_string_lossy().to_string())?; + let lines = BufReader::new(file).lines(); + let iter = lines.map_while(Result::ok).map(parse_date); + Ok(Box::new(iter)) + } + } + Self::Reference(ref path) => { + let metadata = metadata(path)?; + // TODO: "This field might not be available on all platforms" → which ones? + let date_utc: DateTime = metadata.modified()?.into(); + let date: DateTime = date_utc.into(); + let iter = std::iter::once(Ok(date)); + Ok(Box::new(iter)) + } + Self::Now => { + let iter = std::iter::once(Ok(*now)); + Ok(Box::new(iter)) + } + } + } } #[uucore::main] @@ -229,59 +291,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let date_fixed: DateTime = date.into(); Box::new(std::iter::once(Ok(date_fixed))) } else { - match settings.date_source { - DateSource::Custom(ref input) => { - let date = parse_date(input.clone()); - let iter = std::iter::once(date); - Box::new(iter) - } - DateSource::Human(relative_time) => { - // Double check the result is overflow or not of the current_time + relative_time - // it may cause a panic of chrono::datetime::DateTime add - match now.checked_add_signed(relative_time) { - Some(date) => { - let iter = std::iter::once(Ok(date)); - Box::new(iter) - } - None => { - return Err(USimpleError::new( - 1, - format!("invalid date {}", relative_time), - )); - } - } - } - DateSource::Stdin => { - let lines = BufReader::new(std::io::stdin()).lines(); - let iter = lines.map_while(Result::ok).map(parse_date); - Box::new(iter) - } - DateSource::File(ref path) => { - if path.is_dir() { - return Err(USimpleError::new( - 2, - format!("expected file, got directory {}", path.quote()), - )); - } - let file = File::open(path) - .map_err_context(|| path.as_os_str().to_string_lossy().to_string())?; - let lines = BufReader::new(file).lines(); - let iter = lines.map_while(Result::ok).map(parse_date); - Box::new(iter) - } - DateSource::Reference(ref path) => { - let metadata = metadata(path)?; - // TODO: "This field might not be available on all platforms" → which ones? - let date_utc: DateTime = metadata.modified()?.into(); - let date: DateTime = date_utc.into(); - let iter = std::iter::once(Ok(date)); - Box::new(iter) - } - DateSource::Now => { - let iter = std::iter::once(Ok(now)); - Box::new(iter) - } - } + settings.date_source.try_into_iterator(&now)? }; let format_string = make_format_string(&settings); @@ -443,9 +453,7 @@ fn make_format_string(settings: &Settings) -> &str { /// Parse a `String` into a `DateTime`. /// If it fails, return a tuple of the `String` along with its `ParseError`. -fn parse_date + Clone>( - s: S, -) -> Result, (String, parse_datetime::ParseDateTimeError)> { +fn parse_date + Clone>(s: S) -> MaybeParsedDatetime { parse_datetime::parse_datetime(s.as_ref()).map_err(|e| (s.as_ref().into(), e)) } From d3be43e2ca4cf3989c5611e65534438f9d5b3664 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sun, 11 Aug 2024 23:29:15 +0200 Subject: [PATCH 10/10] date: refactor date printing into its own method --- src/uu/date/src/date.rs | 51 ++++++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 93cf2934b4..bb727d5bcd 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -250,6 +250,33 @@ impl DateSource { } } +fn print_date(format_string: &str, date: DateTime) -> UResult<()> { + // GNU `date` uses `%N` for nano seconds, however crate::chrono uses `%f` + let format_string = &format_string.replace("%N", "%f"); + // Refuse to pass this string to chrono as it is crashing in this crate + if format_string.contains("%#z") { + return Err(USimpleError::new( + 1, + format!("invalid format {}", format_string.replace("%f", "%N")), + )); + } + // Hack to work around panic in chrono, + // TODO - remove when a fix for https://github.com/chronotope/chrono/issues/623 is released + let format_items = StrftimeItems::new(format_string); + if format_items.clone().any(|i| i == Item::Error) { + return Err(USimpleError::new( + 1, + format!("invalid format {}", format_string.replace("%f", "%N")), + )); + } + let formatted = date + .format_with_items(format_items) + .to_string() + .replace("%f", "%N"); + println!("{formatted}"); + Ok(()) +} + #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; @@ -300,29 +327,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { for date in dates { match date { Ok(date) => { - // GNU `date` uses `%N` for nano seconds, however crate::chrono uses `%f` - let format_string = &format_string.replace("%N", "%f"); - // Refuse to pass this string to chrono as it is crashing in this crate - if format_string.contains("%#z") { - return Err(USimpleError::new( - 1, - format!("invalid format {}", format_string.replace("%f", "%N")), - )); - } - // Hack to work around panic in chrono, - // TODO - remove when a fix for https://github.com/chronotope/chrono/issues/623 is released - let format_items = StrftimeItems::new(format_string); - if format_items.clone().any(|i| i == Item::Error) { - return Err(USimpleError::new( - 1, - format!("invalid format {}", format_string.replace("%f", "%N")), - )); - } - let formatted = date - .format_with_items(format_items) - .to_string() - .replace("%f", "%N"); - println!("{formatted}"); + print_date(format_string, date)?; } Err((input, _err)) => show!(USimpleError::new( 1,