Web lists-archives.com

Re: t5400 failure on Windows

Am 17.05.2017 um 07:44 schrieb Jeff King:
> I wonder if there's a way we could convince the trace for the two
> programs to go to separate locations. We don't care about receive-pack's
> trace at all. So maybe:

This works. Below it is with a commit message. I'm unsure about the
sign-off procedure, though.

>> * refs/files-backend.c: There are uses in functions open_or_create_logfile()
>> and log_ref_setup(). Sounds like it is in reflogs. Sounds like this is about
>> reflogs. If there are concurrent accesses, there is a danger that a reflog
>> is not recorded correctly. Then reflogs may be missing and objects may be
>> pruned earlier than expected. That's something to worry about, but I would
>> not count it as mission critical.

Of course, the reflog can also be corrupted, but:

> We should always hold the matching ref lock already when modifying a
> reflog. I seem to recall there was a case that missed this (I think
> maybe modifying the HEAD reflog without holding HEAD.lock), but it was
> fixed in the last few versions.

we should be fairly safe then.

---- 8< ----
From: Jeff King <peff@xxxxxxxx>
Subject: [PATCH jk/alternate-ref-optim] t5400: avoid concurrent writes into a trace file

One test in t5400 examines the packet exchange between git-push and
git-receive-pack. The latter inherits the GIT_TRACE_PACKET environment
variable, so that both processes dump trace data into the same file
concurrently. This should not be a problem because the trace file is
opened with O_APPEND.

On Windows, however, O_APPEND is not atomic as it should be: it is
emulated as lseek(SEEK_END) followed by write(). For this reason, the
test is unreliable: it can happen that one process overwrites a line
that was just written by the other process. As a consequence, the test
sometimes does not find one or another line that is expected (and it is
also successful occasionally).

The test case is actually only interested in the output of git-push.
To ensure that only git-push writes to the trace file, override the
receive-pack command such that it does not even open the trace file.

Signed-off-by: Johannes Sixt <j6t@xxxxxxxx>
 t/t5400-send-pack.sh | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index 3331e0f534..d375d7110d 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -288,7 +288,10 @@ test_expect_success 'receive-pack de-dupes .have lines' '
 	$shared .have
-	GIT_TRACE_PACKET=$(pwd)/trace git push fork HEAD:foo &&
+	GIT_TRACE_PACKET=$(pwd)/trace \
+	    git push \
+		--receive-pack="unset GIT_TRACE_PACKET; git-receive-pack" \
+		fork HEAD:foo &&
 	extract_ref_advertisement <trace >refs &&
 	test_cmp expect refs