tail: add test_follow_name_move_retry

* add fixes to pass test_follow_name_move_retry
* fix test_follow_name_remove

* bump notify to 5.0.0-pre.15
* adjust PollWatcher::with_delay -> PollWatcher::with_config
This commit is contained in:
Jan Scheer 2022-05-23 00:51:02 +02:00
parent a62f71f93e
commit 6a7b6ccdbe
No known key found for this signature in database
GPG key ID: C62AD4C29E2B9828
5 changed files with 127 additions and 45 deletions

5
Cargo.lock generated
View file

@ -1221,8 +1221,9 @@ dependencies = [
[[package]]
name = "notify"
version = "5.0.0-pre.14"
source = "git+https://github.com/notify-rs/notify#8399e4195b31f6f188109363292ed220226146f4"
version = "5.0.0-pre.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "553f9844ad0b0824605c20fb55a661679782680410abfb1a8144c2e7e437e7a7"
dependencies = [
"bitflags",
"crossbeam-channel",

View file

@ -18,8 +18,8 @@ path = "src/tail.rs"
[dependencies]
clap = { version = "3.1", features = ["wrap_help", "cargo"] }
libc = "0.2.126"
# notify = { version = "5.0.0-pre.14", features=["macos_kqueue"]}
notify = { git = "https://github.com/notify-rs/notify", features=["macos_kqueue"]}
notify = { version = "5.0.0-pre.15", features=["macos_kqueue"]}
# notify = { git = "https://github.com/notify-rs/notify", features=["macos_kqueue"]}
uucore = { version=">=0.0.11", package="uucore", path="../../uucore", features=["ringbuffer", "lines"] }
[target.'cfg(windows)'.dependencies]

View file

@ -8,8 +8,8 @@
* file that was distributed with this source code.
*/
// spell-checker:ignore (ToDO) errno EPERM ENOSYS
// spell-checker:ignore (options) GETFD
// spell-checker:ignore (ToDO) stdlib
// spell-checker:ignore (options) GETFD EPERM ENOSYS
use std::io::{stdin, Error};
@ -67,7 +67,7 @@ pub fn stdin_is_pipe_or_fifo() -> bool {
// FIXME: Detect a closed file descriptor, e.g.: `tail <&-`
pub fn stdin_is_bad_fd() -> bool {
let fd = stdin().as_raw_fd();
// this is never `true`, even with `<&-` because stdlib is reopening fds as /dev/null
// this is never `true`, even with `<&-` because Rust's stdlib is reopening fds as /dev/null
// see also: https://github.com/uutils/coreutils/issues/2873
// (gnu/tests/tail-2/follow-stdin.sh fails because of this)
unsafe { libc::fcntl(fd, libc::F_GETFD) == -1 && get_errno() == libc::EBADF }

View file

@ -445,7 +445,11 @@ fn uu_tail(mut settings: Settings) -> UResult<()> {
}
}
// dbg!(files.map.is_empty(), files.files_remaining(), files.only_stdin_remaining());
// dbg!(
// files.map.is_empty(),
// files.files_remaining(),
// files.only_stdin_remaining()
// );
// for k in files.map.keys() {
// dbg!(k);
// }
@ -650,7 +654,11 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
let mut watcher: Box<dyn Watcher>;
if settings.use_polling || RecommendedWatcher::kind() == WatcherKind::PollWatcher {
watcher = Box::new(notify::PollWatcher::with_delay(tx, settings.sleep_sec).unwrap());
let config = notify::poll::PollWatcherConfig {
poll_interval: settings.sleep_sec,
..Default::default()
};
watcher = Box::new(notify::PollWatcher::with_config(tx, config).unwrap());
} else {
let tx_clone = tx.clone();
match notify::RecommendedWatcher::new(tx) {
@ -664,9 +672,11 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
text::BACKEND
);
settings.return_code = 1;
watcher = Box::new(
notify::PollWatcher::with_delay(tx_clone, settings.sleep_sec).unwrap(),
);
let config = notify::poll::PollWatcherConfig {
poll_interval: settings.sleep_sec,
..Default::default()
};
watcher = Box::new(notify::PollWatcher::with_config(tx_clone, config).unwrap());
}
Err(e) => panic!("called `Result::unwrap()` on an `Err` value: {:?}", &e),
};
@ -679,6 +689,8 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
let mut orphans = Vec::new();
for path in files.map.keys() {
if path.is_tailable() {
// TODO: [2021-10; jhscheer] also add `file` to please inotify-rotate-resourced.sh
// because it is looking for 2x inotify_add_watch and 1x inotify_rm_watch
let path = get_path(path, settings);
watcher
.watch(&path.canonicalize().unwrap(), RecursiveMode::NonRecursive)
@ -686,7 +698,6 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
} else if settings.follow.is_some() && settings.retry {
if path.is_orphan() {
orphans.push(path.to_path_buf());
// TODO: [2021-10; jhscheer] add test for this
} else {
let parent = path.parent().unwrap();
watcher
@ -715,7 +726,6 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
if new_path.exists() {
let display_name = files.map.get(new_path).unwrap().display_name.to_path_buf();
if new_path.is_file() && files.map.get(new_path).unwrap().metadata.is_none() {
// TODO: [2021-10; jhscheer] add test for this
show_error!("{} has appeared; following new file", display_name.quote());
if let Ok(new_path_canonical) = new_path.canonicalize() {
files.update_metadata(&new_path_canonical, None);
@ -775,31 +785,29 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
}
match rx_result {
Ok(Ok(event)) => {
// eprintln!("=={:=>3}===========================", _event_counter);
// eprintln!("=={:=>3}=====================dbg===", _event_counter);
// dbg!(&event);
// dbg!(files.map.keys());
// eprintln!("=={:=>3}===========================", _event_counter);
// eprintln!("=={:=>3}=====================dbg===", _event_counter);
handle_event(&event, files, settings, &mut watcher, &mut orphans);
}
Ok(Err(notify::Error {
kind: notify::ErrorKind::Io(ref e),
paths,
})) if e.kind() == std::io::ErrorKind::NotFound => {
// TODO: [2021-10; jhscheer] is this case still needed ?
if let Some(event_path) = paths.first() {
if files.map.contains_key(event_path) {
// TODO: [2021-10; jhscheer] handle this case for --follow=name --retry
let _ = watcher.unwatch(event_path);
// TODO: [2021-10; jhscheer] add test for this
show_error!(
"{}: {}",
files.map.get(event_path).unwrap().display_name.display(),
text::NO_SUCH_FILE
);
if !files.files_remaining() && !settings.retry {
// TODO: [2021-10; jhscheer] add test for this
crash!(1, "{}", text::NO_FILES_REMAINING);
}
// TODO: [2021-10; jhscheer] still needed? if yes, add test for this:
// show_error!(
// "{}: {}",
// files.map.get(event_path).unwrap().display_name.display(),
// text::NO_SUCH_FILE
// );
// TODO: [2021-10; jhscheer] still needed? if yes, add test for this:
// if !files.files_remaining() && !settings.retry {
// crash!(1, "{}", text::NO_FILES_REMAINING);
// }
}
}
}
@ -944,7 +952,9 @@ fn handle_event(
}
}
}
EventKind::Remove(RemoveKind::File) | EventKind::Remove(RemoveKind::Any) => {
EventKind::Remove(RemoveKind::File) | EventKind::Remove(RemoveKind::Any)
// | EventKind::Modify(ModifyKind::Name(RenameMode::Any))
| EventKind::Modify(ModifyKind::Name(RenameMode::From)) => {
if settings.follow == Some(FollowMode::Name) {
if settings.retry {
if let Some(old_md) = &files.map.get(event_path).unwrap().metadata {
@ -979,7 +989,7 @@ fn handle_event(
);
} else {
show_error!("{}: {}", display_name.display(), text::NO_SUCH_FILE);
if !files.files_remaining() {
if !files.files_remaining() && settings.use_polling {
crash!(1, "{}", text::NO_FILES_REMAINING);
}
}
@ -989,12 +999,9 @@ fn handle_event(
files.map.remove(event_path).unwrap();
}
}
EventKind::Modify(ModifyKind::Name(RenameMode::Any))
| EventKind::Modify(ModifyKind::Name(RenameMode::From)) => {
if settings.follow == Some(FollowMode::Name) {
show_error!("{}: {}", display_name.display(), text::NO_SUCH_FILE);
}
}
// EventKind::Modify(ModifyKind::Name(RenameMode::Any))
// | EventKind::Modify(ModifyKind::Name(RenameMode::From)) => {
// }
EventKind::Modify(ModifyKind::Name(RenameMode::Both)) => {
// NOTE: For `tail -f a`, keep tracking additions to b after `mv a b`
// (gnu/tests/tail-2/descriptor-vs-rename.sh)
@ -1437,7 +1444,6 @@ pub fn stdin_is_pipe_or_fifo() -> bool {
}
#[cfg(windows)]
{
use winapi_util;
winapi_util::file::typ(winapi_util::HandleRef::stdin())
.map(|t| t.is_disk() || t.is_pipe())
.unwrap_or(false)

View file

@ -36,6 +36,7 @@ static FOLLOW_NAME_SHORT_EXP: &str = "follow_name_short.expected";
static FOLLOW_NAME_EXP: &str = "follow_name.expected";
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_stdin_default() {
new_ucmd!()
.pipe_in_fixture(FOOBAR_TXT)
@ -45,6 +46,7 @@ fn test_stdin_default() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_stdin_explicit() {
new_ucmd!()
.pipe_in_fixture(FOOBAR_TXT)
@ -55,7 +57,7 @@ fn test_stdin_explicit() {
}
#[test]
#[cfg(target_os = "linux")]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_stdin_redirect_file() {
// $ echo foo > f
@ -389,6 +391,7 @@ fn test_follow_name_multiple() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_follow_stdin_pipe() {
new_ucmd!()
.arg("-f")
@ -493,6 +496,7 @@ fn test_bytes_single() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_bytes_stdin() {
new_ucmd!()
.pipe_in_fixture(FOOBAR_TXT)
@ -741,6 +745,7 @@ fn test_sleep_interval() {
/// Test for reading all but the first NUM bytes: `tail -c +3`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_positive_bytes() {
new_ucmd!()
.args(&["-c", "+3"])
@ -751,6 +756,7 @@ fn test_positive_bytes() {
/// Test for reading all bytes, specified by `tail -c +0`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_positive_zero_bytes() {
new_ucmd!()
.args(&["-c", "+0"])
@ -761,6 +767,7 @@ fn test_positive_zero_bytes() {
/// Test for reading all but the first NUM lines: `tail -n +3`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_positive_lines() {
new_ucmd!()
.args(&["-n", "+3"])
@ -802,6 +809,7 @@ once
/// Test for reading all but the first NUM lines: `tail -3`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_obsolete_syntax_positive_lines() {
new_ucmd!()
.args(&["-3"])
@ -812,6 +820,7 @@ fn test_obsolete_syntax_positive_lines() {
/// Test for reading all but the first NUM lines: `tail -n -10`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_small_file() {
new_ucmd!()
.args(&["-n -10"])
@ -822,6 +831,7 @@ fn test_small_file() {
/// Test for reading all but the first NUM lines: `tail -10`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_obsolete_syntax_small_file() {
new_ucmd!()
.args(&["-10"])
@ -832,6 +842,7 @@ fn test_obsolete_syntax_small_file() {
/// Test for reading all lines, specified by `tail -n +0`.
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_positive_zero_lines() {
new_ucmd!()
.args(&["-n", "+0"])
@ -841,6 +852,7 @@ fn test_positive_zero_lines() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_invalid_num() {
new_ucmd!()
.args(&["-c", "1024R", "emptyfile.txt"])
@ -878,6 +890,7 @@ fn test_invalid_num() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_num_with_undocumented_sign_bytes() {
// tail: '-' is not documented (8.32 man pages)
// head: '+' is not documented (8.32 man pages)
@ -1425,15 +1438,21 @@ fn test_follow_name_remove() {
at.copy(source, source_copy);
let expected_stdout = at.read(FOLLOW_NAME_SHORT_EXP);
let expected_stderr = format!(
"{}: {}: No such file or directory\n{0}: no files remaining\n",
ts.util_name, source_copy
);
let expected_stderr = [
format!(
"{}: {}: No such file or directory\n{0}: no files remaining\n",
ts.util_name, source_copy
),
format!(
"{}: {}: No such file or directory\n",
ts.util_name, source_copy
),
];
let delay = 2000;
let mut args = vec!["--follow=name", source_copy, "--use-polling"];
for _ in 0..2 {
for i in 0..2 {
at.copy(source, source_copy);
let mut p = ts.ucmd().set_stdin(Stdio::null()).args(&args).run_no_wait();
@ -1445,7 +1464,7 @@ fn test_follow_name_remove() {
let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
assert_eq!(buf_stdout, expected_stdout);
assert_eq!(buf_stderr, expected_stderr);
assert_eq!(buf_stderr, expected_stderr[i]);
args.pop();
}
@ -1597,7 +1616,7 @@ fn test_follow_name_move_create() {
}
#[test]
#[cfg(unix)]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_follow_name_move() {
// This test triggers a move event while `tail --follow=name logfile` is running.
// ((sleep 2 && mv logfile backup &)>/dev/null 2>&1 &) ; tail --follow=name logfile
@ -1638,6 +1657,59 @@ fn test_follow_name_move() {
}
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_follow_name_move_retry() {
// Similar to test_follow_name_move but with `--retry` (`-F`)
// This test triggers two move/rename events while `tail --follow=name --retry logfile` is running.
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;
let source = FOLLOW_NAME_TXT;
let backup = "backup";
let expected_stderr = format!(
"{0}: '{1}' has become inaccessible: No such file or directory\n\
{0}: '{1}' has appeared; following new file\n",
ts.util_name, source
);
let expected_stdout = "tailed\nnew content\n";
let mut args = vec!["--follow=name", "--retry", source, "--use-polling"];
#[allow(clippy::needless_range_loop)]
for _ in 0..2 {
at.touch(source);
let mut p = ts.ucmd().set_stdin(Stdio::null()).args(&args).run_no_wait();
sleep(Duration::from_millis(1000));
at.append(source, "tailed\n");
sleep(Duration::from_millis(2000));
// with --follow=name, tail should stop monitoring the renamed file
at.rename(source, backup);
sleep(Duration::from_millis(4000));
// overwrite backup while it's not monitored
at.truncate(backup, "new content\n");
sleep(Duration::from_millis(500));
// move back, tail should pick this up and print new content
at.rename(backup, source);
sleep(Duration::from_millis(4000));
p.kill().unwrap();
let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
dbg!(&buf_stdout, &buf_stderr);
assert_eq!(buf_stdout, expected_stdout);
assert_eq!(buf_stderr, expected_stderr);
at.remove(source);
args.pop();
}
}
#[test]
#[cfg(unix)]
fn test_follow_inotify_only_regular() {
@ -1685,11 +1757,13 @@ fn test_no_such_file() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_no_trailing_newline() {
new_ucmd!().pipe_in("x").succeeds().stdout_only("x");
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_lines_zero_terminated() {
new_ucmd!()
.args(&["-z", "-n", "2"])
@ -1704,6 +1778,7 @@ fn test_lines_zero_terminated() {
}
#[test]
#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
fn test_presume_input_pipe_default() {
new_ucmd!()
.arg("---presume-input-pipe")