Re: [PATCH v4 2/3] diff: Let "git diff -O" read orderfile from any file, fail properly

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2013, 21:32 UTC
Message-ID: <xmqqwqj4mqhe.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1387224586-10169-3-git-send-email-naesten@gmail.com>

Samuel Bronson <naesten@gmail.com> writes:

Show 6 quoted lines

>  for i in 1 2
>  do
>  	test_expect_success "orderfile using option ($i)" '
>  	git diff -Oorder_file_$i --name-only HEAD^..HEAD >actual &&
>  	test_cmp expect_$i actual
>  '

This funny indentation in the previous step needs to be fixed, and the added block below should match.

> +
> +	test_expect_success PIPE "orderfile is fifo ($i)" '
> +	rm -f order_fifo &&

> +	mkfifo order_fifo &&
> +	cat order_file_$i >order_fifo &
> +	git diff -O order_fifo --name-only HEAD^..HEAD >actual &&

I think this part can be racy depending on which between cat and "git diff" are scheduled first, no? Try running this test under load and I think you will see it deadlocked.

Besides, the above breaks && chain; even if mkfifo breaks (hence not allowing cat to run), "git diff" will go ahead and run, no?

Show 5 quoted lines

> +	test_cmp expect_$i actual
> +'
>  done
>  
>  test_done