tail: update readme

* add clippy fixes
* add cicd fixes

src/uu/tail/README.md

@@ -1,91 +1,83 @@
-<!-- spell-checker:ignore markdownlint ; (libs) kqueue -->
+<!-- spell-checker:ignore markdownlint ; (misc) backends kqueue Testsuite ksyms stdlib -->
 
 # Notes / ToDO
 
 ## Missing features
 
 * `--max-unchanged-stats`
-* The current implementation doesn't follow stdin on non-unix platforms
 * check whether process p is alive at least every number of seconds (relevant for `--pid`)
 
 Note:
-There's a stub for `--max-unchanged-stats` so GNU tests using it can run, however this flag has no functionality yet.
+There's a stub for `--max-unchanged-stats` so GNU test-suite checks using it can run, however this flag has no functionality yet.
 
-### Platform support for `--follow`
+### Platform support for `--follow` and `--retry`
 The `--follow=descriptor`, `--follow=name` and `--retry` flags have very good support on Linux (inotify backend).
-They work good enough on macOS/BSD (kqueue backend) with some tests failing due to differences how kqueue works compared to inotify.
+They work good enough on macOS/BSD (kqueue backend) with some tests failing due to differences of how kqueue works compared to inotify.
-Windows support is there in theory due to support by the notify-crate, however it's completely untested.
+Windows support is there in theory due to ReadDirectoryChanges support by the notify-crate, however these flags are completely untested on Windows.
-
-### Flags with features
-
-- [x] fast poll := `-s.1 --max-unchanged-stats=1`
-    - [x] sub-second sleep interval e.g. `-s.1`
-    - [ ] `--max-unchanged-stats` (only meaningful with `--follow=name` `---disable-inotify`)
-- [x] `--follow=name`
-- [x] `--retry`
-- [x] `-F` (same as `--follow=name` `--retry`)
-- [x] `---disable-inotify` (three hyphens is correct)
-- [x] `---presume-input-pipe` (three hyphens is correct)
 
 Note:
-`---disable-inotify` means to use polling instead of inotify,
+The undocumented `---disable-inotify` flag is used to disable the inotify backend to test polling. 
-however inotify is a Linux only backend and polling is now supported also for the other backends.
+However inotify is a Linux only backend and polling is now supported also for the other backends.
 Because of this, `disable-inotify` is now an alias to the new and more versatile flag name: `--use-polling`.
 
 ## Possible optimizations
 
 * Don't read the whole file if not using `-f` and input is regular file. Read in chunks from the end going backwards, reading each individual chunk forward.
 * Reduce number of system calls to e.g. `fstat`
+* Improve resource management by adding more system calls to `inotify_rm_watch` when appropriate.
 
-# GNU tests results
+# GNU test-suite results
 
 The functionality for the test "gnu/tests/tail-2/follow-stdin.sh" is implemented.
 It fails because it is provoking closing a file descriptor with `tail -f <&-` and as part of a workaround, Rust's stdlib reopens closed FDs as `/dev/null` which means uu_tail cannot detect this.
 See also, e.g. the discussion at: https://github.com/uutils/coreutils/issues/2873
 
-The functionality for the test "gnu/tests/tail-2/inotify-rotate-resourced.sh" is implemented.
+The functionality for the test "gnu/tests/tail-2/inotify-rotate-resources.sh" is implemented.
-It fails with an error because it is using `strace` to look for calls to 'inotify_add_watch' and 'inotify_rm_watch',
+It fails with an error because it is using `strace` to look for calls to `inotify_add_watch` and `inotify_rm_watch`,
-however in uu_tail these system calls are invoked from a seperate thread. If the GNU test would use `strace -f` this issue could be resolved.
+however in uu_tail these system calls are invoked from a separate thread.
+If the GNU test would follow threads, i.e. use `strace -f`, this issue could be resolved.
 
 ## Testsuite summary for GNU coreutils 9.1.8-e08752:
 
 ### PASS:
-tail-2/F-headers.sh
+- [x] `tail-2/F-headers.sh`
-tail-2/F-vs-missing.sh
+- [x] `tail-2/F-vs-missing.sh`
-tail-2/append-only.sh # skipped test: must be run as root
+- [x] `tail-2/F-vs-rename.sh`
-tail-2/assert-2.sh
+- [x] `tail-2/append-only.sh # skipped test: must be run as root`
-tail-2/assert.sh
+- [x] `tail-2/assert-2.sh`
-tail-2/big-4gb.sh
+- [x] `tail-2/assert.sh`
-tail-2/descriptor-vs-rename.sh
+- [x] `tail-2/big-4gb.sh`
-tail-2/end-of-device.sh # skipped test: must be run as root
+- [x] `tail-2/descriptor-vs-rename.sh`
-tail-2/flush-initial.sh
+- [x] `tail-2/end-of-device.sh # skipped test: must be run as root`
-tail-2/follow-name.sh
+- [x] `tail-2/flush-initial.sh`
-tail-2/inotify-dir-recreate.sh
+- [x] `tail-2/follow-name.sh`
-tail-2/inotify-hash-abuse.sh
+- [x] `tail-2/inotify-dir-recreate.sh`
-tail-2/inotify-hash-abuse2.sh
+- [x] `tail-2/inotify-hash-abuse.sh`
-tail-2/inotify-only-regular.sh
+- [x] `tail-2/inotify-hash-abuse2.sh`
-tail-2/inotify-rotate.sh
+- [x] `tail-2/inotify-only-regular.sh`
-tail-2/overlay-headers.sh
+- [x] `tail-2/inotify-rotate.sh`
-tail-2/pid.sh
+- [x] `tail-2/overlay-headers.sh`
-tail-2/pipe-f2.sh
+- [x] `tail-2/pid.sh`
-tail-2/proc-ksyms.sh
+- [x] `tail-2/pipe-f2.sh`
-tail-2/start-middle.sh
+- [x] `tail-2/proc-ksyms.sh`
-tail-2/tail-c.sh
+- [x] `tail-2/retry.sh`
-tail-2/tail-n0f.sh
+- [x] `tail-2/start-middle.sh`
-tail-2/truncate.sh
+- [x] `tail-2/tail-c.sh`
+- [x] `tail-2/tail-n0f.sh`
+- [x] `tail-2/truncate.sh`
+
 
 ### SKIP:
-tail-2/inotify-race.sh # skipped test: breakpoint not hit
+- [ ] `tail-2/inotify-race.sh # skipped test: breakpoint not hit`
-tail-2/inotify-race2.sh # skipped test: breakpoint not hit
+- [ ] `tail-2/inotify-race2.sh # skipped test: breakpoint not hit`
-tail-2/pipe-f.sh # skipped test: trapping SIGPIPE is not supported
+- [ ] `tail-2/pipe-f.sh # skipped test: trapping SIGPIPE is not supported`
 
 ### FAIL:
-misc/tail.pl
+- [ ] `misc/tail.pl`
-tail-2/F-vs-rename.sh
+- [ ] `tail-2/follow-stdin.sh`
-tail-2/follow-stdin.sh
+- [ ] `tail-2/symlink.sh`
-tail-2/retry.sh
+- [ ] `tail-2/wait.sh`
-tail-2/symlink.sh
+
-tail-2/wait.sh
 
 ### ERROR:
-tail-2/inotify-rotate-resources.sh
+- [ ] `tail-2/inotify-rotate-resources.sh`

src/uu/tail/src/parse.rs

@@ -5,7 +5,7 @@
 
 use std::ffi::OsString;
 
-#[derive(PartialEq, Debug)]
+#[derive(PartialEq, Eq, Debug)]
 pub enum ParseError {
     Syntax,
     Overflow,

src/uu/tail/src/tail.rs

@@ -605,7 +605,7 @@ pub fn uu_app<'a>() -> Command<'a> {
         )
         .arg(
             Arg::new(options::USE_POLLING)
-                .visible_alias(options::DISABLE_INOTIFY_TERM) // NOTE: Used by GNU's test suite
+                .alias(options::DISABLE_INOTIFY_TERM) // NOTE: Used by GNU's test suite
                 .alias("dis") // NOTE: Used by GNU's test suite
                 .long(options::USE_POLLING)
                 .help(POLLING_HELP),
@@ -761,6 +761,7 @@ fn follow(files: &mut FileHandling, settings: &mut Settings) -> UResult<()> {
         // e.g. `echo "X1" > missing ; sleep 0.1 ; echo "X" > missing ;` should trigger a
         // truncation event, but PollWatcher doesn't recognize it.
         // This is relevant to pass, e.g.: "gnu/tests/tail-2/truncate.sh"
+        // TODO: [2022-06; jhscheer] maybe use `--max-unchanged-stats` here to reduce fstat calls?
         if settings.use_polling && settings.follow.is_some() {
             for path in &files
                 .map
@@ -1059,9 +1060,8 @@ impl FileHandling {
 
     /// Return true if there is only stdin remaining
     fn only_stdin_remaining(&self) -> bool {
-        self.map.len() == 1
+        self.map.len() == 1 && (self.map.contains_key(Path::new(text::DASH)))
-            && (self.map.contains_key(Path::new(text::DASH)))
+        // || self.map.contains_key(Path::new(text::DEV_STDIN))) // TODO: still needed?
-                // || self.map.contains_key(Path::new(text::DEV_STDIN))) // TODO: still needed?
     }
 
     /// Return true if there is at least one "tailable" path (or stdin) remaining
@@ -1436,6 +1436,8 @@ pub fn stdin_is_bad_fd() -> bool {
 }
 
 trait FileExtTail {
+    // clippy complains, but it looks like a false positive
+    #[allow(clippy::wrong_self_convention)]
     fn is_seekable(&mut self) -> bool;
 }
 

tests/by-util/test_tail.rs

@@ -195,7 +195,7 @@ fn test_permission_denied_multiple() {
 fn test_follow_redirect_stdin_name_retry() {
     // $ touch f && tail -F - < f
     // tail: cannot follow '-' by name
-    // NOTE: Note sure why GNU's tail doesn't just follow `f` in this case.
+    // NOTE: Not sure why GNU's tail doesn't just follow `f` in this case.
 
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
@@ -1276,9 +1276,6 @@ fn test_retry7() {
     let at = &ts.fixtures;
     let untailable = "untailable";
 
-    // tail: 'untailable' has appeared;  following new file\n\
-    // tail: 'untailable' has become inaccessible: No such file or directory\n\
-    // tail: 'untailable' has appeared;  following new file\n";
     let expected_stderr = "tail: error reading 'untailable': Is a directory\n\
                            tail: untailable: cannot follow end of this type of file\n\
                            tail: 'untailable' has become accessible\n\
@@ -1336,7 +1333,7 @@ fn test_retry7() {
 }
 
 #[test]
-#[cfg(all(unix, not(target_os = "android")))] // FIXME: fix this test for Android
+#[cfg(all(unix, not(any(target_os = "android", target_vendor = "apple"))))] // FIXME: make this work not just on Linux
 fn test_retry8() {
     // Ensure that inotify will switch to polling mode if directory
     // of the watched file was initially missing and later created.
@@ -1466,6 +1463,7 @@ fn test_retry9() {
     sleep(Duration::from_millis(delay));
 
     p.kill().unwrap();
+    sleep(Duration::from_millis(delay));
 
     let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
     assert_eq!(buf_stdout, expected_stdout);
@@ -1576,7 +1574,7 @@ fn test_follow_descriptor_vs_rename2() {
 }
 
 #[test]
-#[cfg(unix)]
+#[cfg(all(unix, not(target_os = "android")))] // FIXME: make this work not just on Linux
 fn test_follow_name_remove() {
     // This test triggers a remove event while `tail --follow=name file` is running.
     // ((sleep 2 && rm file &)>/dev/null 2>&1 &) ; tail --follow=name file
@@ -1696,7 +1694,8 @@ fn test_follow_name_truncate2() {
 #[test]
 #[cfg(target_os = "linux")] // FIXME: fix this test for BSD/macOS
 fn test_follow_name_truncate3() {
-    // Opening an empty file in truncate mode should not trigger a truncate event while.
+    // Opening an empty file in truncate mode should not trigger a truncate event while
+    // `tail --follow=name file` is running.
     // $ rm -f file && touch file
     // $ ((sleep 1 && echo -n "x\n" > file &)>/dev/null 2>&1 &) ; tail --follow=name file
 
@@ -1724,6 +1723,7 @@ fn test_follow_name_truncate3() {
 }
 
 #[test]
+#[cfg(unix)]
 fn test_follow_name_truncate4() {
     // Truncating a file with the same content it already has should not trigger a truncate event
 
@@ -1755,6 +1755,7 @@ fn test_follow_name_truncate4() {
 }
 
 #[test]
+#[cfg(unix)]
 fn test_follow_truncate_fast() {
     // inspired by: "gnu/tests/tail-2/truncate.sh"
     // Ensure all logs are output upon file truncation
@@ -1765,12 +1766,7 @@ fn test_follow_truncate_fast() {
     let ts = TestScenario::new(util_name!());
     let at = &ts.fixtures;
 
-    let mut args = vec![
+    let mut args = vec!["-s.1", "--max-unchanged-stats=1", "f", "---disable-inotify"];
-        "-s.1",
-        "--max-unchanged-stats=1",
-        "f",
-        "---disable-inotify",
-    ];
     let follow = vec!["-f", "-F"];
 
     let delay = 100;
@@ -1847,6 +1843,7 @@ fn test_follow_name_move_create() {
 }
 
 #[test]
+#[cfg(all(unix, not(any(target_os = "android", target_vendor = "apple"))))] // FIXME: make this work not just on Linux
 fn test_follow_name_move_create2() {
     // inspired by: "gnu/tests/tail-2/inotify-hash-abuse.sh"
     // Exercise an abort-inducing flaw in inotify-enabled tail -F
@@ -1859,12 +1856,22 @@ fn test_follow_name_move_create2() {
     }
 
     let mut args = vec![
-        "-s.1", "--max-unchanged-stats=1",
+        "-s.1",
-        "-q", "-F",
+        "--max-unchanged-stats=1",
-        "1", "2", "3", "4", "5", "6", "7", "8", "9",
+        "-q",
+        "-F",
+        "1",
+        "2",
+        "3",
+        "4",
+        "5",
+        "6",
+        "7",
+        "8",
+        "9",
     ];
 
-    let delay = 100;
+    let delay = 300;
     for _ in 0..2 {
         let mut p = ts.ucmd().set_stdin(Stdio::null()).args(&args).run_no_wait();
         sleep(Duration::from_millis(100));
@@ -1882,11 +1889,14 @@ fn test_follow_name_move_create2() {
         sleep(Duration::from_millis(delay));
 
         let (buf_stdout, buf_stderr) = take_stdout_stderr(&mut p);
-        assert_eq!(buf_stderr, "tail: '1' has become inaccessible: No such file or directory\n\
+        assert_eq!(
-                                tail: '1' has appeared;  following new file\n");
+            buf_stderr,
+            "tail: '1' has become inaccessible: No such file or directory\n\
+                                tail: '1' has appeared;  following new file\n"
+        );
 
-        // NOTE: Because "gnu/tests/tail-2/inotify-hash-abuse.sh" forgets to clear the files used
+        // NOTE: Because "gnu/tests/tail-2/inotify-hash-abuse.sh" 'forgets' to clear the files used
-        // during the first loop iteration, we also won't clear them to get the same side-effects.
+        // during the first loop iteration, we also don't clear them to get the same side-effects.
         // Side-effects are truncating a file with the same content, see: test_follow_name_truncate4
         // at.remove("1");
         // at.touch("1");