`tail` overhaul (--follow=name, etc.) by jhscheer · Pull Request #2695 · uutils/coreutils

jhscheer · 2021-09-28T19:39:54Z

I finally consider this ready for review!

TLDR:

Implement missing flags: --follow=name, --retry, -F, ---disable-inotify
Add 44 new test cases
Make 19 more GNU test suite tests pass that were previously failing

Long version:

use the notify crate for cross-platform filesystem notifications in order to implement -F i.e. --follow=name --retry with backend support from: inotify (Linux), kqueue (macOS/BSD) and ReadDirectoryChanges (Windows)
Note: this uses a release candidate of the notify crate v5 because v4 is very old and doesn't have all the required features.
add support for polling in case a notification backend is not available or the resources are exhausted (e.g inotify.max_user_instances limit is reached), it can be forced with --use-polling
add support for stdin redirects, e.g. $ tail -f < file, $ tail < DIR, $ tail -F - < file, etc.
verify that -[nc]0 without -f, exit without reading
fix handling of files if permission is denied
fix handling of files if the file type is unsupported
add support for character devices and fifos

Ensure that:

--follow=name does not imply --retry
--follow={descriptor,name} (without --retry) does not wait for the files to appear
--retry without --follow results in a warning
tail --retry --follow={name,descriptor} waits for the file to appear
truncation is detected
tail --follow=descriptor --retry exits when the file appears untailable
--follow=descriptor (without --retry) does not try to open a file after an initial fail, even when there are other tailable files
tail -F retries when the file is initially untailable
inotify will switch to polling mode if directory of the watched file was initially missing and later created
inotify will switch to polling mode if directory of the watched file was removed and recreated
tail -f works when renaming the tailed files
the headers are correct for --verbose
tail --follow=name handles remove, move, truncation, rename, create events correctly
truncating a file with the same content it already has does not trigger a truncation event

Summary

Ensure that the following GNU test-suite tests pass:

Note: there are further details in tail/README.md

Edit:
Now theoverlay-headers.sh test passes in the CI, although the tests rerun without changes. I suspect scheduling/load on the test VM is the reason.

* treat input filenames as PathBuf instead of String

This implements `--follow=name` for create/move/delete events. Under the hood crate `notify` provides a cross-platform notification library.

* add stub for `--max-unchanged-stats`

On macOS only `kqueue` is suitable for our use case because `FSEvents` waits until file close to delivers modify events.

* Change data structure from Vec to HashMap in order to better keep track of files while watching them with `--follow=name`. E.g. file paths that were removed while watching them and exit if no files are remaining, etc. * Move all logic related to file handling into a FileHandling trait * Simplify handling of the verbose flag.

This makes uu_tail pass the "gnu/tests/tail-2/descriptor-vs-rename" test. * add tests for descriptor-vs-rename (with/without verbose) * fix some minor error messages

* fix test_retry7 * fix test_follow_descriptor_vs_rename2 * set permissions with set_permissions instead of a call to chmod * improve documentation

* add fixes to pass: - tail-2/F-vs-rename.sh - tail-2/follow-name.sh - tail-2/inotify-hash-abuse.sh - tail-2/inotify-only-regular.sh - tail-2/retry.sh * add/improve documentation

sylvestre · 2022-06-03T06:20:43Z

The coverage job on linux fails with:


---- test_tail::test_follow_name_move_create2 stdout ----
current_directory_resolved: 
touch: /tmp/.tmprXSPHB/1
touch: /tmp/.tmprXSPHB/2
touch: /tmp/.tmprXSPHB/3
touch: /tmp/.tmprXSPHB/4
touch: /tmp/.tmprXSPHB/5
touch: /tmp/.tmprXSPHB/6
touch: /tmp/.tmprXSPHB/7
touch: /tmp/.tmprXSPHB/8
touch: /tmp/.tmprXSPHB/9
run: /home/runner/work/coreutils/coreutils/target/debug/coreutils tail -s.1 --max-unchanged-stats=1 -q -F 1 2 3 4 5 6 7 8 9
write(truncate): /tmp/.tmprXSPHB/9
rename: "/tmp/.tmprXSPHB/1" "/tmp/.tmprXSPHB/f"
write(truncate): /tmp/.tmprXSPHB/1
remove: "/tmp/.tmprXSPHB/f"
run: /home/runner/work/coreutils/coreutils/target/debug/coreutils tail -s.1 --max-unchanged-stats=1 -q -F 1 2 3 4 5 6 7 8 9 ---disable-inotify
write(truncate): /tmp/.tmprXSPHB/9
rename: "/tmp/.tmprXSPHB/1" "/tmp/.tmprXSPHB/f"
write(truncate): /tmp/.tmprXSPHB/1
---- test_tail::test_follow_name_move_create2 stderr ----
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `"tail: 9: file truncated\ntail: '1' has become inaccessible: No such file or directory\ntail: '1' has appeared;  following new file\n"`,
 right: `"tail: '1' has become inaccessible: No such file or directory\ntail: '1' has appeared;  following new file\n"`', /home/runner/work/coreutils/coreutils/tests/by-util/test_tail.rs:1891:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

* add clippy fixes * add cicd fixes

tertsdiepraam

This is excellent! Bravo for sticking with this, the complexity of this feature is wild. I have left some comments for further cleanup, but it looks very good.

src/uu/tail/Cargo.toml

src/uu/tail/README.md

src/uu/tail/src/tail.rs

sylvestre · 2022-06-05T07:34:42Z

And the CI is now green, bravo :)

src/uu/tail/src/tail.rs

* minor code clean-up * remove test-suite summary from README

jhscheer · 2022-06-06T12:46:06Z

@tertsdiepraam and @sylvestre thank you very much for your review and comments. I addressed all your concerns.

tertsdiepraam

Looks good to me! Thanks for addressing those comments and, again, this is a great PR!

src/uu/tail/src/tail.rs

sylvestre

LGTM, bravo!

sylvestre · 2022-06-09T10:12:11Z

I see an intermittent here:
https://github.com/uutils/coreutils/runs/6808598985?check_suite_focus=true


---- test_tail::test_follow_name_move_create2 stdout ----
current_directory_resolved: 
touch: /tmp/.tmpVYRqaL/1
touch: /tmp/.tmpVYRqaL/2
touch: /tmp/.tmpVYRqaL/3
touch: /tmp/.tmpVYRqaL/4
touch: /tmp/.tmpVYRqaL/5
touch: /tmp/.tmpVYRqaL/6
touch: /tmp/.tmpVYRqaL/7
touch: /tmp/.tmpVYRqaL/8
touch: /tmp/.tmpVYRqaL/9
run: /target/x86_64-unknown-linux-gnu/debug/coreutils tail -s.1 --max-unchanged-stats=1 -q -F 1 2 3 4 5 6 7 8 9
write(truncate): /tmp/.tmpVYRqaL/9
rename: "/tmp/.tmpVYRqaL/1" "/tmp/.tmpVYRqaL/f"
write(truncate): /tmp/.tmpVYRqaL/1
remove: "/tmp/.tmpVYRqaL/f"
run: /target/x86_64-unknown-linux-gnu/debug/coreutils tail -s.1 --max-unchanged-stats=1 -q -F 1 2 3 4 5 6 7 8 9 ---disable-inotify
write(truncate): /tmp/.tmpVYRqaL/9
rename: "/tmp/.tmpVYRqaL/1" "/tmp/.tmpVYRqaL/f"
write(truncate): /tmp/.tmpVYRqaL/1
thread 'test_tail::test_follow_name_move_create2' panicked at 'assertion failed: `(left == right)`
  left: `"tail: 9: file truncated\ntail: '1' has become inaccessible: No such file or directory\ntail: '1' has appeared;  following new file\n"`,
 right: `"tail: '1' has become inaccessible: No such file or directory\ntail: '1' has appeared;  following new file\n"`', /project/tests/by-util/test_tail.rs:1898:9

jhscheer added 9 commits September 16, 2021 17:14

tail: implement sub-second sleep interval e.g. -s.1

1f24b1f

tail: handle file NotFound error correctly

a727b2e

tail: use crate notify for polling (implement --disable-inotify)

fe3d020

tail: switch from Notify 4.0.17 to 5.0.0-pre.13

a9066e2

* treat input filenames as PathBuf instead of String

tail: implement --follow=name

c70b7a0

This implements `--follow=name` for create/move/delete events. Under the hood crate `notify` provides a cross-platform notification library.

test_tail: add tests for --follow=name

5615ba9

test_tail: add tests for --follow=name --disable-inotify (polling)

d9cd28f

tail: implement handling of truncate event for --follow=name

e935d40

tail: update README

22f78b1

* add stub for `--max-unchanged-stats`

jhscheer force-pushed the tail_notify branch from bfb9f62 to 5f4f569 Compare October 1, 2021 12:40

tail: change notify backend on macOS from FSEvents to kqueue

94cc966

On macOS only `kqueue` is suitable for our use case because `FSEvents` waits until file close to delivers modify events.

jhscheer force-pushed the tail_notify branch from 5f4f569 to 94cc966 Compare October 1, 2021 19:33

Merge branch 'master' into tail_notify

22b5928

jhscheer force-pushed the tail_notify branch 3 times, most recently from 32e12a7 to ccd8890 Compare October 7, 2021 10:18

jhscheer added 2 commits October 7, 2021 14:37

tail: fix the behavior for -f and rename events

a120615

This makes uu_tail pass the "gnu/tests/tail-2/descriptor-vs-rename" test. * add tests for descriptor-vs-rename (with/without verbose) * fix some minor error messages

jhscheer force-pushed the tail_notify branch from ccd8890 to a120615 Compare October 8, 2021 21:44

jhscheer added 11 commits October 10, 2021 00:07

test_tail: clean up tests for --follow=name

e3b3586

test_tail: add test_retry1-2

9338b3f

test_tail: add test_retry3

8c5c528

test_tail: add test_retry4

5770921

test_tail: add test_retry5

4bfb462

test_tail: add test_retry6

c7c5deb

test_tail: add test_retr7

016291b

test_tail: add test_retry8-9

07eb502

test_tail: add test_follow_descriptor_vs_rename1-2

3f4b014

test_tail: add test_follow_name_truncate1-3

1d2940b

test_tail: add more tests for --follow=name

a9c34ef

jhscheer added 3 commits June 2, 2022 17:01

test_tail: add test_follow_name_move_create2

2e91881

test_tail: a lot of minor improvements

94fe426

* fix test_retry7 * fix test_follow_descriptor_vs_rename2 * set permissions with set_permissions instead of a call to chmod * improve documentation

tail: refactor and fixes to pass more GNU test-suite checks

70fed83

* add fixes to pass: - tail-2/F-vs-rename.sh - tail-2/follow-name.sh - tail-2/inotify-hash-abuse.sh - tail-2/inotify-only-regular.sh - tail-2/retry.sh * add/improve documentation

jhscheer force-pushed the tail_notify branch 4 times, most recently from 80b70da to b71b0da Compare June 2, 2022 23:22

jhscheer marked this pull request as ready for review June 2, 2022 23:22

sylvestre requested a review from tertsdiepraam June 3, 2022 06:21

sylvestre removed the awaiting response label Jun 3, 2022

sylvestre requested a review from jfinkels June 3, 2022 06:22

jhscheer force-pushed the tail_notify branch from b71b0da to a6fb719 Compare June 3, 2022 08:13

tail: update readme

e0efd6c

* add clippy fixes * add cicd fixes

jhscheer force-pushed the tail_notify branch from a6fb719 to e0efd6c Compare June 3, 2022 11:36

tertsdiepraam reviewed Jun 4, 2022

View reviewed changes

sylvestre requested a review from tertsdiepraam June 5, 2022 07:34

sylvestre reviewed Jun 5, 2022

View reviewed changes

src/uu/tail/src/tail.rs Show resolved Hide resolved

sylvestre reviewed Jun 5, 2022

View reviewed changes

src/uu/tail/src/tail.rs Show resolved Hide resolved

tail: use functionality from uucore::error where applicable

beb2b7c

* minor code clean-up * remove test-suite summary from README

Merge branch 'main' into tail_notify

e883459

jhscheer force-pushed the tail_notify branch from e9076f0 to e883459 Compare June 6, 2022 14:01

tertsdiepraam approved these changes Jun 6, 2022

View reviewed changes

src/uu/tail/src/tail.rs Show resolved Hide resolved

src/uu/tail/src/tail.rs Show resolved Hide resolved

sylvestre approved these changes Jun 7, 2022

View reviewed changes

sylvestre merged commit 0532c74 into uutils:main Jun 7, 2022

jhscheer mentioned this pull request Jun 9, 2022

test_tail: increase delay for test_follow_name_move_create2 #3606

Merged

tertsdiepraam mentioned this pull request Sep 5, 2022

Refactor tail #3905

Merged

Uh oh!

Comments

Conversation

jhscheer commented Sep 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

sylvestre commented Jun 3, 2022

Uh oh!

tertsdiepraam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sylvestre commented Jun 5, 2022

Uh oh!

Uh oh!

Uh oh!

jhscheer commented Jun 6, 2022

Uh oh!

tertsdiepraam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sylvestre left a comment

Choose a reason for hiding this comment

Uh oh!

sylvestre commented Jun 9, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jhscheer commented Sep 28, 2021 •

edited

Loading