Re: Failed with output: Hydra job gnu:coreutils-master:build on x86_64-linux

[Pádraig Brady]

On 06/03/2013 12:14 PM, Bernhard Voelker wrote:
> Hi Padraig,
> 
> thanks again for working on this.
> 
> On 06/03/2013 12:13 PM, Pádraig Brady wrote:
>> From 12b6c3bf70dde23c2c1bf5db7650a959f327b04f Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?P=C3=A1draig=20Brady?= <address@hidden>
>> Date: Mon, 3 Jun 2013 01:29:17 +0100
>> Subject: [PATCH] tests: avoid a race in tail --retry testing
>>
>> Prompted by the continuous integration build failure at:
>> http://hydra.nixos.org/build/5221053
> 
> Can we rely on this URL to be available quite some time, or is
> there some cleanup on Hydra from time to time?
> 
>> * tests/tail-2/retry.sh: Ensure the 'out' file is not present
>> as it's used to arbitrate the run order of commands.
>> Relying on the truncation in the background tail command,
> 
> s/,//
> 
>> is racy because the truncation can occur after the fork
>> of the background shell and thus the 'missing' file could
>> be created by the time tail(1) looks for it.
> 
> I'm not sure I understand the above.
> Did you mean
>   s/missing/out/
>   s/tail/wait4lines_/
> ?

As a consequence of wait4lines waiting incorrectly on 'out',
the 'missing' file would be populated by the time tail(1)
gets to process it. I'll clarify the description.

> 
>> ---
>>  tests/tail-2/retry.sh |   11 ++++++-----
>>  1 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/tail-2/retry.sh b/tests/tail-2/retry.sh
>> index d56d4c1..070e4fd 100644
>> --- a/tests/tail-2/retry.sh
>> +++ b/tests/tail-2/retry.sh
>> @@ -44,9 +44,10 @@ grep -F 'tail: warning: --retry ignored' out || fail=1
>>  
>>  # === Test:
>>  # Ensure that "tail --retry --follow=name" waits for the file to appear.
>> +rm out                                       || framework_failure_
>>  timeout 10 tail -s.1 --follow=name --retry missing >out 2>&1 & pid=$!
> 
> It doesn't look obvious that rm(1) avoids a race, and someone could
> easily remove that 'redundant' line again in a future change.
> Would you mind to add a short comment there?

You also pointed out that 'out' must be present
or the current code in wait4lines would print confusion warnings.

So I'll make this additional change:

-rm out                          || framework_failure_
+# Clear 'out' so that we can check its contents without races
+:>out                           || framework_failure_

thanks,
Pádraig.