Refactor tail continued V2 by tertsdiepraam · Pull Request #5173 · uutils/coreutils

tertsdiepraam · 2023-08-19T23:13:20Z

Resurrection of #3952, which I'll close in favor of this PR.

Still a draft because I'll need to look at the commit about tests still. The rest should be properly rebased and cleaned up (even though there was little cleaning up to do).

I've opted to make a separate PR, because the changes I made required some intensive surgery (e.g. removing commits).

github-actions · 2023-08-19T23:59:21Z

GNU testsuite comparison:

Skip an intermittent issue tests/tail-2/inotify-dir-recreate

tertsdiepraam · 2023-08-26T15:22:15Z

This is how I think we should approach this PR:

I roughly agree with the contents now.
Everything that reviewers don't agree with just goes away.
Small changes we can still fix though.

…with error propagation ... Return io::Result from methods in chunks.rs to be able to replace unwraps with error propagation and return the bytes read from the source so far. Store how many bytes are read in LinesChunkBuffer. Fixes: tail does not output anything when there's no newline in the source and the `--follow` option is given. Flush the buffered writer after a write explicitly. Simplify the BytesChunkBuffer::fill and LinesChunkBuffer::fill methods and make use of some new helper methods in these structs. Add more unit tests for structs in chunks.rs

tail.rs: * Change and simplify the control flow to try to open the input or file first and then react appropriately on errors. Note this also fixes some misbehavior on macos when a fifo pointed to a directory and other minor related issues. * Differentiate between read and write errors to exit on write errors. Read errors in contrast are not fatal and are handled like the other `TailError` types in the `TailErrorHandler` in the newly created error.rs module. * Return `Result<u64, TailError>` from unbounded_tail and bounded_tail. The Ok result (bytes read and printed) is currently not used but can be used later in the follow module. * Fix printing of Fifos multiple times. For example `tail - - < file` printed the content of the file twice). Fifos should behave like other standard input in such cases. follow/files.rs: Add "untailable" files separately in an own method to the set of watched files. paths.rs: * Remove the resolve method and replace the workaround with the more reliable open method which also differentiates between a fifo and a pipe in case of stdin. It uses an improved version of `FileExtTail::is_seekable` to be able to do so. * Cleanup the redundant `stdin_is_bad_fd` function

sylvestre · 2023-12-25T10:58:12Z

no activity for a while, closing

tertsdiepraam · 2023-12-25T11:16:42Z

Well, it was ready for review the whole time 😄 But yeah even though I tried to save some changes I don't think it was worth it. Good call!

tertsdiepraam changed the title ~~Refactor tail continued (Reviewer~~ Refactor tail continued V2 Aug 19, 2023

tertsdiepraam mentioned this pull request Aug 19, 2023

tail: Refactor tail #3952

Closed

tertsdiepraam force-pushed the refactor-tail-continued branch 2 times, most recently from 5a63696 to 259eac8 Compare August 20, 2023 15:54

tertsdiepraam marked this pull request as ready for review August 26, 2023 15:20