git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: [PATCH] t5400: avoid concurrent writes into a trace file
Date: Thu, 18 May 2017 01:02:09 -0400	[thread overview]
Message-ID: <20170518050209.dezna5tillvlbwgw@sigill.intra.peff.net> (raw)
In-Reply-To: <83b6c8d3-2787-995a-a4e5-e6a437c53e89@kdbg.org>

On Wed, May 17, 2017 at 08:41:01PM +0200, Johannes Sixt wrote:

> 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.

Thanks for testing. I actually wrote one in preparation, but I like
yours a little better.

I'll repost here with my signoff and a cc to Junio, since the earlier
subject line was less likely to grab attention. This goes on top of
jk/alternate-ref-optim.

-- >8 --
Subject: 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.

Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jeff King <peff@peff.net>
---
 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 3331e0f53..d375d7110 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
 	EOF
 
-	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
 '
-- 
2.13.0.219.g63f6bc368


      reply	other threads:[~2017-05-18  5:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-15 20:05 t5400 failure on Windows Johannes Sixt
2017-05-15 22:24 ` Jeff King
2017-05-16 18:35   ` Johannes Sixt
2017-05-17  5:44     ` Jeff King
2017-05-17 18:41       ` Johannes Sixt
2017-05-18  5:02         ` Jeff King [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170518050209.dezna5tillvlbwgw@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).