1
Fork 0
mirror of https://github.com/RGBCube/uutils-coreutils synced 2025-07-28 11:37:44 +00:00

tail: fix handling of stdin redirects for macOS

On macOS path.is_dir() can be false for directories
if it was a redirect, e.g. ` tail < DIR`

* fix some tests for macOS

Cleanup:
* fix clippy/spell-checker
* fix build for windows by refactoring stdin_is_pipe_or_fifo()
This commit is contained in:
Jan Scheer 2022-05-18 14:22:53 +02:00
parent 75a6641ced
commit 07231e6c6c
No known key found for this signature in database
GPG key ID: C62AD4C29E2B9828
4 changed files with 82 additions and 32 deletions

4
Cargo.lock generated
View file

@ -190,9 +190,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610"
[[package]] [[package]]
name = "cc" name = "cc"
version = "1.0.72" version = "1.0.73"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "22a9137b95ea06864e018375b72adfb7db6e6f68cfc8df5a04d00288050485ee" checksum = "2fff2a6927b3bb87f9595d67196a70493f627687a71d87a0d692242c33f58c11"
[[package]] [[package]]
name = "cexpr" name = "cexpr"

View file

@ -9,6 +9,7 @@
*/ */
// spell-checker:ignore (ToDO) errno EPERM ENOSYS // spell-checker:ignore (ToDO) errno EPERM ENOSYS
// spell-checker:ignore (options) GETFD
use std::io::{stdin, Error}; use std::io::{stdin, Error};

View file

@ -7,7 +7,7 @@
// * For the full copyright and license information, please view the LICENSE // * For the full copyright and license information, please view the LICENSE
// * file that was distributed with this source code. // * file that was distributed with this source code.
// spell-checker:ignore (ToDO) seekable seek'd tail'ing ringbuffer ringbuf unwatch // spell-checker:ignore (ToDO) seekable seek'd tail'ing ringbuffer ringbuf unwatch Uncategorized
// spell-checker:ignore (libs) kqueue // spell-checker:ignore (libs) kqueue
// spell-checker:ignore (acronyms) // spell-checker:ignore (acronyms)
// spell-checker:ignore (env/flags) // spell-checker:ignore (env/flags)
@ -46,8 +46,6 @@ use uucore::lines::lines;
use uucore::parse_size::{parse_size, ParseSizeError}; use uucore::parse_size::{parse_size, ParseSizeError};
use uucore::ringbuffer::RingBuffer; use uucore::ringbuffer::RingBuffer;
#[cfg(unix)]
use crate::platform::{stdin_is_bad_fd, stdin_is_pipe_or_fifo};
#[cfg(unix)] #[cfg(unix)]
use std::fs::metadata; use std::fs::metadata;
#[cfg(unix)] #[cfg(unix)]
@ -70,7 +68,6 @@ pub mod text {
pub static STDIN_HEADER: &str = "standard input"; pub static STDIN_HEADER: &str = "standard input";
pub static NO_FILES_REMAINING: &str = "no files remaining"; pub static NO_FILES_REMAINING: &str = "no files remaining";
pub static NO_SUCH_FILE: &str = "No such file or directory"; pub static NO_SUCH_FILE: &str = "No such file or directory";
#[cfg(unix)]
pub static BAD_FD: &str = "Bad file descriptor"; pub static BAD_FD: &str = "Bad file descriptor";
#[cfg(target_os = "linux")] #[cfg(target_os = "linux")]
pub static BACKEND: &str = "inotify"; pub static BACKEND: &str = "inotify";
@ -250,13 +247,11 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
// skip expansive fstat check if PRESUME_INPUT_PIPE is selected // skip expansive fstat check if PRESUME_INPUT_PIPE is selected
if !args.stdin_is_pipe_or_fifo { if !args.stdin_is_pipe_or_fifo {
// FIXME windows has GetFileType which can determine if the file is a pipe/FIFO args.stdin_is_pipe_or_fifo = stdin_is_pipe_or_fifo();
// so this check can also be performed
if cfg!(unix) {
args.stdin_is_pipe_or_fifo = stdin_is_pipe_or_fifo();
}
} }
// dbg!(args.stdin_is_pipe_or_fifo);
uu_tail(args) uu_tail(args)
} }
@ -298,6 +293,9 @@ fn uu_tail(mut settings: Settings) -> UResult<()> {
} }
} }
let mut buf = [0; 0]; // empty buffer to check if stdin().read().is_err()
let stdin_read_possible = settings.stdin_is_pipe_or_fifo && stdin().read(&mut buf).is_ok();
if path.is_stdin() && settings.follow == Some(FollowMode::Name) { if path.is_stdin() && settings.follow == Some(FollowMode::Name) {
// TODO: still needed? // TODO: still needed?
// Mimic GNU's tail; Exit immediately even though there might be other valid files. // Mimic GNU's tail; Exit immediately even though there might be other valid files.
@ -306,16 +304,30 @@ fn uu_tail(mut settings: Settings) -> UResult<()> {
if settings.follow == Some(FollowMode::Descriptor) && settings.retry { if settings.follow == Some(FollowMode::Descriptor) && settings.retry {
show_warning!("--retry only effective for the initial open"); show_warning!("--retry only effective for the initial open");
} }
if !path.exists() { if !path.exists() && !settings.stdin_is_pipe_or_fifo {
settings.return_code = 1; settings.return_code = 1;
show_error!( show_error!(
"cannot open {} for reading: {}", "cannot open {} for reading: {}",
display_name.quote(), display_name.quote(),
text::NO_SUCH_FILE text::NO_SUCH_FILE
); );
} else if path.is_dir() { } else if path.is_dir() || display_name.is_stdin() && !stdin_read_possible {
let err_msg = "Is a directory".to_string();
// NOTE: On macOS path.is_dir() can be false for directories
// if it was a redirect, e.g. ` tail < DIR`
if !path.is_dir() {
// TODO: match against ErrorKind
// if unstable library feature "io_error_more" becomes stable
// if let Err(e) = stdin().read(&mut buf) {
// if e.kind() != std::io::ErrorKind::IsADirectory {
// err_msg = e.message.to_string();
// }
// }
}
settings.return_code = 1; settings.return_code = 1;
show_error!("error reading {}: Is a directory", display_name.quote()); show_error!("error reading {}: {}", display_name.quote(), err_msg);
if settings.follow.is_some() { if settings.follow.is_some() {
let msg = if !settings.retry { let msg = if !settings.retry {
"; giving up on this name" "; giving up on this name"
@ -333,14 +345,25 @@ fn uu_tail(mut settings: Settings) -> UResult<()> {
continue; continue;
} }
} else { } else {
// TODO: [2021-10; jhscheer] how to handle block device, socket, fifo? // TODO: [2021-10; jhscheer] how to handle block device or socket?
// dbg!(path.is_tailable());
// dbg!(path.is_stdin());
// dbg!(path.is_dir());
// dbg!(path.exists());
// if let Ok(md) = path.metadata() {
// let ft = md.file_type();
// dbg!(ft.is_fifo());
// dbg!(ft.is_socket());
// dbg!(ft.is_block_device());
// dbg!(ft.is_char_device());
// }
todo!(); todo!();
} }
} }
let md = path.metadata().ok(); let md = path.metadata().ok();
if display_name.is_stdin() { if display_name.is_stdin() && path.is_tailable() {
if settings.verbose { if settings.verbose {
if !first_header { if !first_header {
println!(); println!();
@ -725,10 +748,10 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
paths.push(path.to_path_buf()); paths.push(path.to_path_buf());
} }
} }
for path in paths.iter_mut() { for path in &mut paths {
if let Ok(new_md) = path.metadata() { if let Ok(new_md) = path.metadata() {
if let Some(old_md) = &files.map.get(path).unwrap().metadata { if let Some(old_md) = &files.map.get(path).unwrap().metadata {
// TODO: [2021-10; jhscheer] reduce dublicate code // TODO: [2021-10; jhscheer] reduce duplicate code
let display_name = files.map.get(path).unwrap().display_name.to_path_buf(); let display_name = files.map.get(path).unwrap().display_name.to_path_buf();
if new_md.len() <= old_md.len() if new_md.len() <= old_md.len()
&& new_md.modified().unwrap() != old_md.modified().unwrap() && new_md.modified().unwrap() != old_md.modified().unwrap()
@ -1407,6 +1430,26 @@ fn get_block_size(md: &Metadata) -> u64 {
} }
} }
pub fn stdin_is_pipe_or_fifo() -> bool {
#[cfg(unix)]
{
platform::stdin_is_pipe_or_fifo()
}
// FIXME windows has GetFileType which can determine if the file is a pipe/FIFO
// so this check can also be performed
#[cfg(not(unix))]
false
}
pub fn stdin_is_bad_fd() -> bool {
#[cfg(unix)]
{
platform::stdin_is_bad_fd()
}
#[cfg(not(unix))]
false
}
trait PathExt { trait PathExt {
fn is_stdin(&self) -> bool; fn is_stdin(&self) -> bool;
fn print_header(&self); fn print_header(&self);
@ -1416,9 +1459,9 @@ trait PathExt {
impl PathExt for Path { impl PathExt for Path {
fn is_stdin(&self) -> bool { fn is_stdin(&self) -> bool {
self.eq(Path::new(text::DASH)) self.eq(Self::new(text::DASH))
|| self.eq(Path::new(text::DEV_STDIN)) || self.eq(Self::new(text::DEV_STDIN))
|| self.eq(Path::new(text::STDIN_HEADER)) || self.eq(Self::new(text::STDIN_HEADER))
} }
fn print_header(&self) { fn print_header(&self) {
println!("==> {} <==", self.display()); println!("==> {} <==", self.display());
@ -1431,7 +1474,10 @@ impl PathExt for Path {
{ {
// TODO: [2021-10; jhscheer] what about fifos? // TODO: [2021-10; jhscheer] what about fifos?
self.is_file() self.is_file()
|| (self.exists() && metadata(self).unwrap().file_type().is_char_device()) || self.exists() && {
let ft = metadata(self).unwrap().file_type();
ft.is_char_device() || ft.is_fifo()
}
} }
#[cfg(not(unix))] #[cfg(not(unix))]
{ {

View file

@ -3,7 +3,7 @@
// * For the full copyright and license information, please view the LICENSE // * For the full copyright and license information, please view the LICENSE
// * file that was distributed with this source code. // * file that was distributed with this source code.
// spell-checker:ignore (ToDO) abcdefghijklmnopqrstuvwxyz efghijklmnopqrstuvwxyz vwxyz emptyfile logfile bogusfile siette ocho nueve diez // spell-checker:ignore (ToDO) abcdefghijklmnopqrstuvwxyz efghijklmnopqrstuvwxyz vwxyz emptyfile logfile file siette ocho nueve diez
// spell-checker:ignore (libs) kqueue // spell-checker:ignore (libs) kqueue
// spell-checker:ignore (jargon) tailable untailable // spell-checker:ignore (jargon) tailable untailable
@ -89,7 +89,6 @@ fn test_stdin_redirect_file() {
p.kill().unwrap(); p.kill().unwrap();
let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p); let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
dbg!(&buf_stdout);
assert!(buf_stdout.eq("foo")); assert!(buf_stdout.eq("foo"));
assert!(buf_stderr.is_empty()); assert!(buf_stderr.is_empty());
} }
@ -200,7 +199,6 @@ fn test_follow_stdin_explicit_indefinitely() {
p.kill().unwrap(); p.kill().unwrap();
let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p); let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
dbg!(&buf_stdout, &buf_stderr);
assert!(buf_stdout.eq("==> standard input <==")); assert!(buf_stdout.eq("==> standard input <=="));
assert!(buf_stderr.eq("tail: warning: following standard input indefinitely is ineffective")); assert!(buf_stderr.eq("tail: warning: following standard input indefinitely is ineffective"));
@ -495,9 +493,9 @@ fn test_bytes_single() {
#[test] #[test]
fn test_bytes_stdin() { fn test_bytes_stdin() {
new_ucmd!() new_ucmd!()
.pipe_in_fixture(FOOBAR_TXT)
.arg("-c") .arg("-c")
.arg("13") .arg("13")
.pipe_in_fixture(FOOBAR_TXT)
.run() .run()
.stdout_is_fixture("foobar_bytes_stdin.expected") .stdout_is_fixture("foobar_bytes_stdin.expected")
.no_stderr(); .no_stderr();
@ -945,7 +943,12 @@ fn test_retry2() {
let ts = TestScenario::new(util_name!()); let ts = TestScenario::new(util_name!());
let missing = "missing"; let missing = "missing";
let result = ts.ucmd().arg(missing).arg("--retry").run(); let result = ts
.ucmd()
.set_stdin(Stdio::null())
.arg(missing)
.arg("--retry")
.run();
result result
.stderr_is( .stderr_is(
"tail: warning: --retry ignored; --retry is useful only when following\n\ "tail: warning: --retry ignored; --retry is useful only when following\n\
@ -1300,7 +1303,6 @@ fn test_retry9() {
p.kill().unwrap(); p.kill().unwrap();
let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p); let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
// println!("stdout:\n{}\nstderr:\n{}", buf_stdout, buf_stderr); // dbg
assert_eq!(buf_stdout, expected_stdout); assert_eq!(buf_stdout, expected_stdout);
assert_eq!(buf_stderr, expected_stderr); assert_eq!(buf_stderr, expected_stderr);
} }
@ -1672,10 +1674,12 @@ fn take_stdout_stderr(p: &mut std::process::Child) -> (String, String) {
#[test] #[test]
fn test_no_such_file() { fn test_no_such_file() {
new_ucmd!() new_ucmd!()
.arg("bogusfile") .set_stdin(Stdio::null())
.arg("missing")
.fails() .fails()
.stderr_is("tail: cannot open 'missing' for reading: No such file or directory")
.no_stdout() .no_stdout()
.stderr_contains("cannot open 'bogusfile' for reading: No such file or directory"); .code_is(1);
} }
#[test] #[test]
@ -1708,8 +1712,7 @@ fn test_presume_input_pipe_default() {
} }
#[test] #[test]
#[cfg(target_os = "linux")] #[cfg(unix)]
#[cfg(disable_until_fixed)]
fn test_fifo() { fn test_fifo() {
let ts = TestScenario::new(util_name!()); let ts = TestScenario::new(util_name!());
let at = &ts.fixtures; let at = &ts.fixtures;