tail/args: Fix parsing when -F is used together with --retry or --follow

This commit is contained in:
Joining7943 2023-04-09 16:19:57 +02:00
parent 08c5f3b61b
commit 20e32971dd
5 changed files with 91 additions and 22 deletions

1
Cargo.lock generated
View file

@ -3179,6 +3179,7 @@ dependencies = [
"libc",
"memchr",
"notify",
"rstest",
"same-file",
"uucore",
"winapi-util",

View file

@ -311,6 +311,7 @@ rand_core = "0.6"
rayon = "1.7"
redox_syscall = "0.2"
regex = "1.7.3"
rstest = "0.17.0"
rust-ini = "0.18.0"
same-file = "1.0.6"
selinux = "0.4"

View file

@ -29,6 +29,9 @@ fundu = { workspace=true }
windows-sys = { workspace=true, features = ["Win32_System_Threading", "Win32_Foundation"] }
winapi-util = { workspace=true }
[dev-dependencies]
rstest = { workspace=true }
[[bin]]
name = "tail"
path = "src/main.rs"

View file

@ -8,7 +8,7 @@
use crate::paths::Input;
use crate::{parse, platform, Quotable};
use clap::crate_version;
use clap::{parser::ValueSource, Arg, ArgAction, ArgMatches, Command};
use clap::{Arg, ArgAction, ArgMatches, Command};
use fundu::DurationParser;
use is_terminal::IsTerminal;
use same_file::Handle;
@ -182,19 +182,44 @@ impl Settings {
}
pub fn from(matches: &clap::ArgMatches) -> UResult<Self> {
let mut settings: Self = Self {
follow: if matches.get_flag(options::FOLLOW_RETRY) {
Some(FollowMode::Name)
} else if matches.value_source(options::FOLLOW) != Some(ValueSource::CommandLine) {
None
} else if matches.get_one::<String>(options::FOLLOW)
== Some(String::from("name")).as_ref()
// We're parsing --follow, -F and --retry under the following conditions:
// * -F sets --retry and --follow=name
// * plain --follow or short -f is the same like specifying --follow=descriptor
// * All these options and flags can occur multiple times as command line arguments
let follow_retry = matches.get_flag(options::FOLLOW_RETRY);
// We don't need to check for occurrences of --retry if -F was specified which already sets
// retry
let retry = follow_retry || matches.get_flag(options::RETRY);
let follow = match (
follow_retry,
matches
.get_one::<String>(options::FOLLOW)
.map(|s| s.as_str()),
) {
// -F and --follow if -F is specified after --follow. We don't need to care about the
// value of --follow.
(true, Some(_))
// It's ok to use `index_of` instead of `indices_of` since -F and --follow
// overwrite themselves (not only the value but also the index).
if matches.index_of(options::FOLLOW_RETRY) > matches.index_of(options::FOLLOW) =>
{
Some(FollowMode::Name)
} else {
Some(FollowMode::Descriptor)
},
retry: matches.get_flag(options::RETRY) || matches.get_flag(options::FOLLOW_RETRY),
}
// * -F and --follow=name if --follow=name is specified after -F
// * No occurrences of -F but --follow=name
// * -F and no occurrences of --follow
(_, Some("name")) | (true, None) => Some(FollowMode::Name),
// * -F and --follow=descriptor (or plain --follow, -f) if --follow=descriptor is
// specified after -F
// * No occurrences of -F but --follow=descriptor, --follow, -f
(_, Some(_)) => Some(FollowMode::Descriptor),
// The default for no occurrences of -F or --follow
(false, None) => None,
};
let mut settings: Self = Self {
follow,
retry,
use_polling: matches.get_flag(options::USE_POLLING),
mode: FilterMode::from(matches)?,
verbose: matches.get_flag(options::verbosity::VERBOSE),
@ -469,7 +494,7 @@ pub fn uu_app() -> Command {
Arg::new(options::FOLLOW)
.short('f')
.long(options::FOLLOW)
.default_value("descriptor")
.default_missing_value("descriptor")
.num_args(0..=1)
.require_equals(true)
.value_parser(["descriptor", "name"])
@ -544,13 +569,14 @@ pub fn uu_app() -> Command {
Arg::new(options::RETRY)
.long(options::RETRY)
.help("Keep trying to open a file if it is inaccessible")
.overrides_with(options::RETRY)
.action(ArgAction::SetTrue),
)
.arg(
Arg::new(options::FOLLOW_RETRY)
.short('F')
.help("Same as --follow=name --retry")
.overrides_with_all([options::RETRY, options::FOLLOW])
.overrides_with(options::FOLLOW_RETRY)
.action(ArgAction::SetTrue),
)
.arg(
@ -570,6 +596,8 @@ pub fn uu_app() -> Command {
#[cfg(test)]
mod tests {
use rstest::rstest;
use crate::parse::ObsoleteArgs;
use super::*;
@ -616,4 +644,39 @@ mod tests {
let result = Settings::from_obsolete_args(&args, Some(&"file".into()));
assert_eq!(result.follow, Some(FollowMode::Name));
}
#[rstest]
#[case::default(vec![], None, false)]
#[case::retry(vec!["--retry"], None, true)]
#[case::multiple_retry(vec!["--retry", "--retry"], None, true)]
#[case::follow_long(vec!["--follow"], Some(FollowMode::Descriptor), false)]
#[case::follow_short(vec!["-f"], Some(FollowMode::Descriptor), false)]
#[case::follow_long_with_retry(vec!["--follow", "--retry"], Some(FollowMode::Descriptor), true)]
#[case::follow_short_with_retry(vec!["-f", "--retry"], Some(FollowMode::Descriptor), true)]
#[case::follow_overwrites_previous_selection_1(vec!["--follow=name", "--follow=descriptor"], Some(FollowMode::Descriptor), false)]
#[case::follow_overwrites_previous_selection_2(vec!["--follow=descriptor", "--follow=name"], Some(FollowMode::Name), false)]
#[case::big_f(vec!["-F"], Some(FollowMode::Name), true)]
#[case::multiple_big_f(vec!["-F", "-F"], Some(FollowMode::Name), true)]
#[case::big_f_with_retry_then_does_not_change(vec!["-F", "--retry"], Some(FollowMode::Name), true)]
#[case::big_f_with_follow_descriptor_then_change(vec!["-F", "--follow=descriptor"], Some(FollowMode::Descriptor), true)]
#[case::multiple_big_f_with_follow_descriptor_then_no_change(vec!["-F", "--follow=descriptor", "-F"], Some(FollowMode::Name), true)]
#[case::big_f_with_follow_short_then_change(vec!["-F", "-f"], Some(FollowMode::Descriptor), true)]
#[case::follow_descriptor_with_big_f_then_change(vec!["--follow=descriptor", "-F"], Some(FollowMode::Name), true)]
#[case::follow_short_with_big_f_then_change(vec!["-f", "-F"], Some(FollowMode::Name), true)]
#[case::big_f_with_follow_name_then_not_change(vec!["-F", "--follow=name"], Some(FollowMode::Name), true)]
#[case::follow_name_with_big_f_then_not_change(vec!["--follow=name", "-F"], Some(FollowMode::Name), true)]
#[case::big_f_with_multiple_long_follow(vec!["--follow=name", "-F", "--follow=descriptor"], Some(FollowMode::Descriptor), true)]
#[case::big_f_with_multiple_long_follow_name(vec!["--follow=name", "-F", "--follow=name"], Some(FollowMode::Name), true)]
#[case::big_f_with_multiple_short_follow(vec!["-f", "-F", "-f"], Some(FollowMode::Descriptor), true)]
#[case::multiple_big_f_with_multiple_short_follow(vec!["-f", "-F", "-f", "-F"], Some(FollowMode::Name), true)]
fn test_parse_settings_follow_mode_and_retry(
#[case] args: Vec<&str>,
#[case] expected_follow_mode: Option<FollowMode>,
#[case] expected_retry: bool,
) {
let settings =
Settings::from(&uu_app().no_binary_name(true).get_matches_from(args)).unwrap();
assert_eq!(settings.follow, expected_follow_mode);
assert_eq!(settings.retry, expected_retry);
}
}

View file

@ -4358,14 +4358,7 @@ fn test_follow_when_file_and_symlink_are_pointing_to_same_file_and_append_data()
.stdout_only(expected_stdout);
}
// Fails with:
// 'Assertion failed. Expected 'tail' to be running but exited with status=exit status: 1.
// stdout:
// stderr: tail: warning: --retry ignored; --retry is useful only when following
// tail: error reading 'dir': Is a directory
// '
#[test]
#[cfg(disabled_until_fixed)]
fn test_args_when_directory_given_shorthand_big_f_together_with_retry() {
let scene = TestScenario::new(util_name!());
let fixtures = &scene.fixtures;
@ -4377,9 +4370,17 @@ fn test_args_when_directory_given_shorthand_big_f_together_with_retry() {
tail: {0}: cannot follow end of this type of file\n",
dirname
);
let mut child = scene.ucmd().args(&["-F", "--retry", "dir"]).run_no_wait();
child.make_assertion_with_delay(500).is_alive();
child
.kill()
.make_assertion()
.with_current_output()
.stderr_only(&expected_stderr);
let mut child = scene.ucmd().args(&["--retry", "-F", "dir"]).run_no_wait();
child.make_assertion_with_delay(500).is_alive();
child
.kill()