Re: [PATCH] tail: display file headers correctly with inotify

[Pádraig Brady]

On 09/06/15 07:20, Bernhard Voelker wrote:
> Nice one, but I think the fix is not sufficient ... see below.
> 
> On 06/09/2015 02:54 AM, Pádraig Brady wrote:
>> diff --git a/src/tail.c b/src/tail.c
>> index c9736ca..047d93c 100644
>> --- a/src/tail.c
>> +++ b/src/tail.c
>> @@ -1046,7 +1046,9 @@ recheck (struct File_spec *f, bool blocking)
>>          {
>>            /* This happens when one iteration finds the file missing,
>>               then the preceding <dev,inode> pair is reused as the
>> -             file is recreated.  */
>> +             file is recreated.  XXX: This means the file is redisplayed
>> +             in --follow=name mode if renamed away from and back to
>> +             a monitored name.  */
>>            new_file = true;
>>          }
>>        else
> 
> s/XXX: // ?
> 
>> diff --git a/tests/tail-2/F-headers.sh b/tests/tail-2/F-headers.sh
>> new file mode 100755
>> index 0000000..9355c0c
>> --- /dev/null
>> +++ b/tests/tail-2/F-headers.sh
>> @@ -0,0 +1,59 @@
>> +#!/bin/sh
>> +# Ensure tail -F distinguishes output with the correct headers
>> +# Between coreutils 7.5 and 8.23 inclusive, 'tail -F ...' would
>> +# not output headers for cor created/renamed files in certain cases.
> 
> s/ cor / /
> 
>> +for mode in '' '---disable-inotify'; do
>> +  rm -f a b out
>> +
>> +  tail $mode -F $fastpoll a b > out 2>&1 & pid=$!
>> +
>> +  # Wait up to 12.7s for tail to start.
>> +  tail_re="cannot open 'b'" retry_delay_ check_tail_output .1 7 ||
>> +    { cat out; fail=1; }
>> +
>> +  echo x > a
>> +  # Wait up to 12.7s for a's header to appear in the output:
>> +  tail_re='==> a <==' retry_delay_ check_tail_output .1 7 ||
>> +    { echo "$0: a: unexpected delay?"; cat out; fail=1; }
>> +
>> +  echo y > b
>> +  # Wait up to 12.7s for a's header to appear in the output:
>> +  tail_re='==> b <==' retry_delay_ check_tail_output .1 7 ||
>> +    { echo "$0: a: unexpected delay?"; cat out; fail=1; }
> 
> s/ a: / b: /

and s/a's/b's/


> While this patch fixes the outputting of the header for multiple
> files, it does not fix the same issue for the same file, i.e. the
> "==> file <==" header is missing when a file re-appears the 2nd
> time.  Reproducer:
> 
>   rm -f a b out
>   src/tail -F a b > out 2>&1 &
> 
>   sleep 2; echo a > a
> 
>   sleep 2; rm a
> 
>   sleep 2; echo a > a
> 
>   sleep 2; kill $!
> 
> Output:
> 
>   src/tail: cannot open ‘a’ for reading: No such file or directory
>   src/tail: cannot open ‘b’ for reading: No such file or directory
>   src/tail: ‘a’ has appeared;  following new file
>   ==> a <==
>   a
>   src/tail: ‘a’ has been replaced;  following new file
>   a
> 
> Shouldn't tail(1) output the "==> a <==" header before the last
> line here, too?

Well the data being output by tail is logically connected,
even though the files are being rotated underneath.
So while we could add a redisplay flag to the fspec struct
to enable showing a new header upon truncation or creation,
it's probably best not to.

Now we should always output diagnostics to stderr in these cases,
and I see we don't when inodes are reused (which is common).
So I've a attached a further patch to improve that as well.

thanks for the careful review!
Pádraig.

tail-F-header.patch
Description: Text Data