Hi Eff
Thanks for reporting this
On 09/08/2023 18:17, Jeff King wrote:
On Wed, Jul 05, 2023 at 08:49:30PM +0100, Phillip Wood wrote:
+test_expect_success PIPE 'diff --no-index refuses to diff a named pipe and a directory' '
+ test_when_finished "rm -f pipe" &&
+ mkfifo pipe &&
+ {
+ (>pipe) &
+ } &&
+ test_when_finished "kill $!" &&
+ test_must_fail git diff --no-index -- pipe a 2>err &&
+ grep "fatal: cannot compare a named pipe to a directory" err
+'
+
+test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' '
+ test_when_finished "rm -f old new new-link" &&
+ mkfifo old &&
+ mkfifo new &&
+ ln -s new new-link &&
+ {
+ (test_write_lines a b c >old) &
+ } &&
+ test_when_finished "! kill $!" &&
+ {
+ (test_write_lines a x c >new) &
+ } &&
+ test_when_finished "! kill $!" &&
+
+ cat >expect <<-EOF &&
+ diff --git a/old b/new-link
+ --- a/old
+ +++ b/new-link
+ @@ -1,3 +1,3 @@
+ a
+ -b
+ +x
+ c
+ EOF
+
+ test_expect_code 1 git diff --no-index old new-link >actual &&
+ test_cmp expect actual
+'
I've had t4053 hang for me once or twice in the last few days. I haven't
managed to pinpoint the problem, but I did notice that running it with
--stress seems to occasionally fail in thie "reads from pipes" test.
The problem is that the "kill" is racy. Even after we've read all of the
input from those sub-processes, they might still be hanging around
waiting to exit when our test_when_finished runs. And then kill will
return "0". So I think we need to either:
1. Soften the when_finished to "kill $! || true" so that we are OK if
they are still there.
I think this is the easiest option, I'll send a patch later today
2. If the diff command completed as expected, it should be safe to
"wait" for each of them to confirm that they successfully wrote
everything. I'm not sure it buys us much over testing the output
from "diff", though.
If the diff output is OK that's I think that's all we really care about.
I still don't see where the hang comes from, though. It might be
unrelated. I'll try to examine more next time I catch it in the act.
I had a look at the tests again and nothing jumped out at me. If you do
manage to catch it hanging we should at least we should be able to tell
which test is causing the problem.
Thanks again,
Phillip
