Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tail: Refactor handling of warnings and early exits #4135

Merged
merged 4 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/uu/tail/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ memchr = "2.5.0"
notify = { version = "=5.0.0", features=["macos_kqueue"]}
uucore = { version=">=0.0.16", package="uucore", path="../../uucore", features=["ringbuffer", "lines"] }
same-file = "1.0.6"
atty = "0.2"

[target.'cfg(windows)'.dependencies]
windows-sys = { version = "0.42.0", default-features = false, features = ["Win32_System_Threading", "Win32_Foundation"] }
Expand Down
119 changes: 85 additions & 34 deletions src/uu/tail/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@

use crate::paths::Input;
use crate::{parse, platform, Quotable};
use atty::Stream;
use clap::crate_version;
use clap::{parser::ValueSource, Arg, ArgAction, ArgMatches, Command};
use same_file::Handle;
use std::collections::VecDeque;
use std::ffi::OsString;
use std::time::Duration;
Expand Down Expand Up @@ -113,6 +115,13 @@ pub enum FollowMode {
Name,
}

#[derive(Debug)]
pub enum VerificationResult {
Ok,
CannotFollowStdinByName,
NoOutput,
}

#[derive(Debug, Default)]
pub struct Settings {
pub follow: Option<FollowMode>,
Expand Down Expand Up @@ -149,10 +158,6 @@ impl Settings {
settings.retry =
matches.get_flag(options::RETRY) || matches.get_flag(options::FOLLOW_RETRY);

if settings.retry && settings.follow.is_none() {
show_warning!("--retry ignored; --retry is useful only when following");
}

if let Some(s) = matches.get_one::<String>(options::SLEEP_INT) {
settings.sleep_sec = match s.parse::<f32>() {
Ok(s) => Duration::from_secs_f32(s),
Expand Down Expand Up @@ -194,14 +199,8 @@ impl Settings {
format!("invalid PID: {}", pid_str.quote()),
));
}

settings.pid = pid;
if settings.follow.is_none() {
show_warning!("PID ignored; --pid=PID is useful only when following");
}
if !platform::supports_pid_checks(settings.pid) {
show_warning!("--pid=PID is not supported on this system");
settings.pid = 0;
}
}
Err(e) => {
return Err(USimpleError::new(
Expand All @@ -214,16 +213,6 @@ impl Settings {

settings.mode = FilterMode::from(matches)?;

// Mimic GNU's tail for -[nc]0 without -f and exit immediately
if settings.follow.is_none()
&& matches!(
settings.mode,
FilterMode::Lines(Signum::MinusZero, _) | FilterMode::Bytes(Signum::MinusZero)
)
{
std::process::exit(0)
}

let mut inputs: VecDeque<Input> = matches
.get_many::<String>(options::ARG_FILES)
.map(|v| v.map(|string| Input::from(string.clone())).collect())
Expand All @@ -243,6 +232,81 @@ impl Settings {

Ok(settings)
}

pub fn has_only_stdin(&self) -> bool {
self.inputs.iter().all(|input| input.is_stdin())
}

pub fn has_stdin(&self) -> bool {
self.inputs.iter().any(|input| input.is_stdin())
}

pub fn num_inputs(&self) -> usize {
self.inputs.len()
}

/// Check [`Settings`] for problematic configurations of tail originating from user provided
/// command line arguments and print appropriate warnings.
pub fn check_warnings(&self) {
if self.retry {
if self.follow.is_none() {
show_warning!("--retry ignored; --retry is useful only when following");
} else if self.follow == Some(FollowMode::Descriptor) {
show_warning!("--retry only effective for the initial open");
}
}

if self.pid != 0 {
if self.follow.is_none() {
show_warning!("PID ignored; --pid=PID is useful only when following");
} else if !platform::supports_pid_checks(self.pid) {
show_warning!("--pid=PID is not supported on this system");
}
}

// This warning originates from gnu's tail implementation of the equivalent warning. If the
// user wants to follow stdin, but tail is blocking indefinitely anyways, because of stdin
// as `tty` (but no otherwise blocking stdin), then we print a warning that `--follow`
// cannot be applied under these circumstances and is therefore ineffective.
if self.follow.is_some() && self.has_stdin() {
let blocking_stdin = self.pid == 0
&& self.follow == Some(FollowMode::Descriptor)
&& self.num_inputs() == 1
&& Handle::stdin().map_or(false, |handle| {
handle
.as_file()
.metadata()
.map_or(false, |meta| !meta.is_file())
});

if !blocking_stdin && atty::is(Stream::Stdin) {
show_warning!("following standard input indefinitely is ineffective");
}
}
}

/// Verify [`Settings`] and try to find unsolvable misconfigurations of tail originating from
/// user provided command line arguments. In contrast to [`Settings::check_warnings`] these
/// misconfigurations usually lead to the immediate exit or abortion of the running `tail`
/// process.
pub fn verify(&self) -> VerificationResult {
tertsdiepraam marked this conversation as resolved.
Show resolved Hide resolved
// Mimic GNU's tail for `tail -F`
if self.inputs.iter().any(|i| i.is_stdin()) && self.follow == Some(FollowMode::Name) {
return VerificationResult::CannotFollowStdinByName;
}

// Mimic GNU's tail for -[nc]0 without -f and exit immediately
if self.follow.is_none()
&& matches!(
self.mode,
FilterMode::Lines(Signum::MinusZero, _) | FilterMode::Bytes(Signum::MinusZero)
)
{
return VerificationResult::NoOutput;
}

VerificationResult::Ok
}
}

pub fn arg_iterate<'a>(
Expand Down Expand Up @@ -298,19 +362,6 @@ fn parse_num(src: &str) -> Result<Signum, ParseSizeError> {
})
}

pub fn stdin_is_pipe_or_fifo() -> bool {
#[cfg(unix)]
{
platform::stdin_is_pipe_or_fifo()
}
#[cfg(windows)]
{
winapi_util::file::typ(winapi_util::HandleRef::stdin())
.map(|t| t.is_disk() || t.is_pipe())
.unwrap_or(false)
}
}

pub fn parse_args(args: impl uucore::Args) -> UResult<Settings> {
let matches = uu_app().try_get_matches_from(arg_iterate(args)?)?;
Settings::from(&matches)
Expand Down
2 changes: 2 additions & 0 deletions src/uu/tail/src/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
//! or at the end of piped stdin with [`LinesChunk`] or [`BytesChunk`].
//!
//! Use [`ReverseChunks::new`] to create a new iterator over chunks of bytes from the file.

// spell-checker:ignore (ToDO) filehandle BUFSIZ

use std::collections::VecDeque;
use std::fs::File;
use std::io::{BufRead, Read, Seek, SeekFrom, Write};
Expand Down
1 change: 0 additions & 1 deletion src/uu/tail/src/follow/files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use std::collections::hash_map::Keys;
use std::collections::HashMap;
use std::fs::{File, Metadata};
use std::io::{stdout, BufRead, BufReader, BufWriter};

use std::path::{Path, PathBuf};
use uucore::error::UResult;

Expand Down
2 changes: 1 addition & 1 deletion src/uu/tail/src/follow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
mod files;
mod watch;

pub use watch::{follow, WatcherService};
pub use watch::{follow, Observer};
61 changes: 34 additions & 27 deletions src/uu/tail/src/follow/watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use notify::{RecommendedWatcher, RecursiveMode, Watcher, WatcherKind};
use std::collections::VecDeque;
use std::io::BufRead;
use std::path::{Path, PathBuf};
use std::sync::mpsc;
use std::sync::mpsc::{channel, Receiver};
use std::sync::mpsc::{self, channel, Receiver};
use uucore::display::Quotable;
use uucore::error::{set_exit_code, UResult, USimpleError};
use uucore::show_error;
Expand Down Expand Up @@ -81,7 +80,7 @@ impl WatcherRx {
}
}

pub struct WatcherService {
pub struct Observer {
/// Whether --retry was given on the command line
pub retry: bool,

Expand All @@ -92,25 +91,36 @@ pub struct WatcherService {
/// platform specific event driven method. Since `use_polling` is subject to
/// change during runtime it is moved out of [`Settings`].
pub use_polling: bool,

pub watcher_rx: Option<WatcherRx>,
pub orphans: Vec<PathBuf>,
pub files: FileHandling,

pub pid: platform::Pid,
}

impl WatcherService {
impl Observer {
tertsdiepraam marked this conversation as resolved.
Show resolved Hide resolved
pub fn new(
retry: bool,
follow: Option<FollowMode>,
use_polling: bool,
files: FileHandling,
pid: platform::Pid,
) -> Self {
let pid = if platform::supports_pid_checks(pid) {
pid
} else {
0
};

Self {
retry,
follow,
use_polling,
watcher_rx: None,
orphans: Vec::new(),
files,
pid,
}
}

Expand All @@ -120,6 +130,7 @@ impl WatcherService {
settings.follow,
settings.use_polling,
FileHandling::from(settings),
settings.pid,
)
}

Expand Down Expand Up @@ -460,14 +471,12 @@ impl WatcherService {
}
}

pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResult<()> {
if watcher_service.files.no_files_remaining(settings)
&& !watcher_service.files.only_stdin_remaining()
{
pub fn follow(mut observer: Observer, settings: &Settings) -> UResult<()> {
if observer.files.no_files_remaining(settings) && !observer.files.only_stdin_remaining() {
return Err(USimpleError::new(1, text::NO_FILES_REMAINING.to_string()));
}

let mut process = platform::ProcessChecker::new(settings.pid);
let mut process = platform::ProcessChecker::new(observer.pid);

let mut _event_counter = 0;
let mut _timeout_counter = 0;
Expand All @@ -478,7 +487,7 @@ pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResu

// If `--pid=p`, tail checks whether process p
// is alive at least every `--sleep-interval=N` seconds
if settings.follow.is_some() && settings.pid != 0 && process.is_dead() {
if settings.follow.is_some() && observer.pid != 0 && process.is_dead() {
tertsdiepraam marked this conversation as resolved.
Show resolved Hide resolved
// p is dead, tail will also terminate
break;
}
Expand All @@ -487,22 +496,20 @@ pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResu
// If a path becomes an orphan during runtime, it will be added to orphans.
// To be able to differentiate between the cases of test_retry8 and test_retry9,
// here paths will not be removed from orphans if the path becomes available.
if watcher_service.follow_name_retry() {
for new_path in &watcher_service.orphans {
if observer.follow_name_retry() {
for new_path in &observer.orphans {
if new_path.exists() {
let pd = watcher_service.files.get(new_path);
let pd = observer.files.get(new_path);
let md = new_path.metadata().unwrap();
if md.is_tailable() && pd.reader.is_none() {
show_error!(
"{} has appeared; following new file",
pd.display_name.quote()
);
watcher_service.files.update_metadata(new_path, Some(md));
watcher_service.files.update_reader(new_path)?;
_read_some = watcher_service
.files
.tail_file(new_path, settings.verbose)?;
watcher_service
observer.files.update_metadata(new_path, Some(md));
observer.files.update_reader(new_path)?;
_read_some = observer.files.tail_file(new_path, settings.verbose)?;
observer
.watcher_rx
.as_mut()
.unwrap()
Expand All @@ -514,7 +521,7 @@ pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResu

// With -f, sleep for approximately N seconds (default 1.0) between iterations;
// We wake up if Notify sends an Event or if we wait more than `sleep_sec`.
let rx_result = watcher_service
let rx_result = observer
.watcher_rx
.as_mut()
.unwrap()
Expand All @@ -529,9 +536,9 @@ pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResu
match rx_result {
Ok(Ok(event)) => {
if let Some(event_path) = event.paths.first() {
if watcher_service.files.contains_key(event_path) {
if observer.files.contains_key(event_path) {
// Handle Event if it is about a path that we are monitoring
paths = watcher_service.handle_event(&event, settings)?;
paths = observer.handle_event(&event, settings)?;
}
}
}
Expand All @@ -540,8 +547,8 @@ pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResu
paths,
})) if e.kind() == std::io::ErrorKind::NotFound => {
if let Some(event_path) = paths.first() {
if watcher_service.files.contains_key(event_path) {
let _ = watcher_service
if observer.files.contains_key(event_path) {
let _ = observer
.watcher_rx
.as_mut()
.unwrap()
Expand All @@ -566,16 +573,16 @@ pub fn follow(mut watcher_service: WatcherService, settings: &Settings) -> UResu
Err(e) => return Err(USimpleError::new(1, format!("RecvTimeoutError: {}", e))),
}

if watcher_service.use_polling && settings.follow.is_some() {
if observer.use_polling && settings.follow.is_some() {
// Consider all files to potentially have new content.
// This is a workaround because `Notify::PollWatcher`
// does not recognize the "renaming" of files.
paths = watcher_service.files.keys().cloned().collect::<Vec<_>>();
paths = observer.files.keys().cloned().collect::<Vec<_>>();
}

// main print loop
for path in &paths {
_read_some = watcher_service.files.tail_file(path, settings.verbose)?;
_read_some = observer.files.tail_file(path, settings.verbose)?;
}

if _timeout_counter == settings.max_unchanged_stats {
Expand Down
Loading