From ff284f930541c32a303851ce50c096c19ce4c4f6 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 a3f2ad0426c..b1ed83b3d49 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 ac16fe83145..d922484dd0c 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")))] @@ -491,3 +494,44 @@ fn test_date_empty_tz() { .succeeds() .stdout_only("UTC\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 4136548e8fa0f1275ce77d977484ab6a06e52784 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 b1ed83b3d49..ac69cb398ab 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 d922484dd0c..fa867a9d056 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -535,3 +535,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 80ff205e9b0b447e5b5e7798bcf8f57e7f46f709 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 ac69cb398ab..1cf36e2ed75 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::custom_tz_fmt::custom_time_format; @@ -93,6 +93,7 @@ enum DateSource { Custom(String), File(PathBuf), Stdin, + Reference(PathBuf), Human(TimeDelta), } @@ -179,6 +180,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 }; @@ -261,6 +264,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 fa867a9d056..4166047c544 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); @@ -571,3 +577,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 7a0b556bf68b38070a4b4eeac04e616f983e1574 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 1cf36e2ed75..b6e3455e8fb 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 4166047c544..e7784da5e39 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -239,9 +239,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(); } @@ -268,6 +287,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 13e02c88d763491176c7acdffad1cfcd637a95e7 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 | 93 +++++++++++++++++++------------------- tests/by-util/test_date.rs | 6 +-- 2 files changed, 49 insertions(+), 50 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index b6e3455e8fb..83a9a15659f 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -204,27 +204,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); @@ -276,42 +277,42 @@ 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) => { - let format_string = custom_time_format(format_string); - // 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.as_str()); - 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) => { + let format_string = custom_time_format(format_string); + // 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.as_str()); + 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 e7784da5e39..515224e9234 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -243,8 +243,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(); } } @@ -260,8 +259,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 fb826a483e860da4864088f2b7d6c92a3c96dc82 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 | 22 ++++++++++++++++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 83a9a15659f..8543e5bd6bd 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -75,7 +75,7 @@ struct Settings { utc: bool, format: Format, date_source: DateSource, - set_to: Option>, + set_to: Option, } /// Various ways of displaying the date @@ -186,22 +186,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. @@ -214,7 +205,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 515224e9234..44ca53ca838 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -285,6 +285,28 @@ 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 the exact content: + assert!(!result.stdout_str().is_empty()); + 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 ff83a41fbf388004312daaf47c34b4b3a1356c11 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 8543e5bd6bd..d1c7244f96a 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 44ca53ca838..e4a5e24bf52 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -661,3 +661,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 a1ecea6feb2e0b1851924af69ba5541c456ed4dc 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 d1c7244f96a..2ac6c3d4bb9 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}; @@ -137,54 +137,65 @@ impl From<&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); @@ -206,6 +217,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 8f03ead5960ef23d216503d7e0dbcbc275f06d0b 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 | 119 +++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 56 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 2ac6c3d4bb9..814323c8054 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -25,6 +25,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"; @@ -187,6 +190,64 @@ 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() { + 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); + 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] @@ -230,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 +452,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 220bb1a5237fb2f5653ae05455629d7cabe8c90f 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 | 49 +++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/uu/date/src/date.rs b/src/uu/date/src/date.rs index 814323c8054..c723751ab63 100644 --- a/src/uu/date/src/date.rs +++ b/src/uu/date/src/date.rs @@ -250,6 +250,32 @@ impl DateSource { } } +fn print_date(format_string: &str, date: DateTime) -> UResult<()> { + let format_string = custom_time_format(format_string); + // 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.as_str()); + 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,28 +326,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { for date in dates { match date { Ok(date) => { - let format_string = custom_time_format(format_string); - // 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.as_str()); - 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,