git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Larry D'Anna <larry@elder-gods.org>, Jeff King <peff@peff.net>,
	git@vger.kernel.org
Subject: Re: t5401-update-hooks test failure
Date: Tue, 9 Feb 2010 09:51:39 -0800	[thread overview]
Message-ID: <20100209175139.GA28936@spearce.org> (raw)
In-Reply-To: <7v4olqlva7.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> $ while sh t5401-*.sh -i; do :; done
> ... wait for a while ...
> * FAIL 12: send-pack stderr contains hook messages
> 
>                 grep ^remote: send.err | sed "s/ *\$//" >actual &&
>                 test_cmp - actual <expect
> 
> $ t/(364643e...); cat trash\ directory.t5401-update-hooks/actual 
> remote: STDOUT pre-receive
> remote: STDERR pre-receive
> remote: STDOUT update refs/heads/master
> remote: STDERR update refs/heads/master
> remote: STDOUT update refs/heads/tofail
> remote: STDOUT post-receive
> remote: STDERR post-receive
> remote: STDOUT post-update
> remote: STDERR post-update
> $ t/(364643e...); cat trash\ directory.t5401-update-hooks/expect 
> remote: STDOUT pre-receive
> remote: STDERR pre-receive
> remote: STDOUT update refs/heads/master
> remote: STDERR update refs/heads/master
> remote: STDOUT update refs/heads/tofail
> remote: STDERR update refs/heads/tofail
> remote: STDOUT post-receive
> remote: STDERR post-receive
> remote: STDOUT post-update
> remote: STDERR post-update

A quick visual inspection shows that only the STDERR tofail message
is missing here.  That sounds to me like a race condition in the
recv_sideband decoder.  Or, a race condition in the hook code in
builtin-receive-pack.c.

I doubt its in receive-pack. run_update_hook() directly calls the
copy_to_sideband() function, and that reads until EOF on the hook's
stderr stream before it returns and waits for the hook's exit status.
So we should be pulling everything and dumping it into the sideband.

builtin-send-pack.c clearly isn't stopping early while processing
the stream, since we see later messages from the post-receive and
post-update hooks just fine.

So I think the only code that is in question is the case 2 arm of
recv_sideband().  But to be honest, I can't find any fault with it.


I've been running this test in a while loop for a while now, and
I can't make it trigger this failure.  Maybe its possible that
its the update hook itself, not flushing its stderr buffer before
it terminates?

-- 
Shawn.

  parent reply	other threads:[~2010-02-09 17:51 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-05  0:41 [PATCH] fix an error message in git-push so it goes to stderr Larry D'Anna
2010-02-05 15:06 ` Jeff King
2010-02-05 19:34   ` [PATCH 1/3] " Larry D'Anna
2010-02-05 19:34   ` [PATCH 2/3] silence human readable info messages going to stderr from git push --porcelain Larry D'Anna
2010-02-05 20:20     ` Junio C Hamano
2010-02-05 20:30       ` Larry D'Anna
2010-02-05 20:49       ` [PATCH v2 0/3] Larry D'Anna
2010-02-05 20:49       ` [PATCH v2 1/3] fix an error message in git-push so it goes to stderr Larry D'Anna
2010-02-05 20:49       ` [PATCH v2 2/3] clean up some of the output from git push --porcelain Larry D'Anna
2010-02-05 21:07         ` Junio C Hamano
2010-02-05 20:49       ` [PATCH v2 3/3] make git push --dry-run --porcelain exit with status 0 even if updates will be rejected Larry D'Anna
2010-02-05 23:50         ` Tay Ray Chuan
2010-02-08 20:19           ` Larry D'Anna
2010-02-08 20:31             ` [PATCH v3 1/3] git-push: fix an error message so it goes to stderr Larry D'Anna
2010-02-08 20:45               ` Junio C Hamano
2010-02-08 20:31             ` [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain Larry D'Anna
2010-02-08 20:51               ` Junio C Hamano
2010-02-08 21:13                 ` Junio C Hamano
2010-02-08 21:32                   ` Jeff King
2010-02-08 22:15                     ` Larry D'Anna
2010-02-08 22:21                     ` Junio C Hamano
2010-02-08 22:31                       ` Larry D'Anna
2010-02-08 22:33                         ` [PATCH] git-push: clean up some of the output from git push Larry D'Anna
2010-02-08 22:48                         ` [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain Junio C Hamano
2010-02-08 23:10                           ` Larry D'Anna
2010-02-08 23:11                             ` Junio C Hamano
2010-02-08 23:44                               ` [PATCH] git-push: fix the documentation to explain all the status flags Larry D'Anna
2010-02-09  0:23                                 ` Junio C Hamano
2010-02-09  0:30                                   ` Junio C Hamano
2010-02-09  0:45                                     ` Junio C Hamano
2010-02-09  0:56                                       ` Larry D'Anna
2010-02-09  1:00                                         ` Junio C Hamano
2010-02-09  0:54                                   ` Larry D'Anna
2010-02-09  4:54                           ` [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain Larry D'Anna
2010-02-09  7:31                             ` Junio C Hamano
2010-02-09 16:21                               ` Larry D'Anna
2010-02-09 17:51                               ` Shawn O. Pearce [this message]
2010-02-09 19:20                                 ` t5401-update-hooks test failure Nicolas Pitre
2010-02-09 19:26                                   ` Shawn O. Pearce
2010-02-09 22:44                                     ` Junio C Hamano
2010-02-09 23:16                                       ` Junio C Hamano
2010-02-10  1:29                                       ` Shawn O. Pearce
2010-02-09  5:48                           ` [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain Larry D'Anna
2010-02-09  5:53                             ` [PATCH 1/4] git-push: fix an error message so it goes to stderr Larry D'Anna
2010-02-09  5:54                             ` [PATCH 2/4] git-push: squelch advice message if in --porcelain mode Larry D'Anna
2010-02-09  5:54                             ` [PATCH 3/4] git-push: send "To <remoteurl>" messages to the standard output " Larry D'Anna
2010-02-11 22:54                               ` Tay Ray Chuan
2010-02-11 23:19                                 ` Junio C Hamano
2010-02-09  5:54                             ` [PATCH 4/4] git-push: make git push --dry-run --porcelain exit with status 0 even if updates will be rejected Larry D'Anna
2010-02-10  5:39                       ` [PATCH v3 2/3] git-push: clean up some of the output from git push --porcelain Jeff King
2010-02-10  5:55                         ` Larry D'Anna
2010-02-10 10:43                           ` Tay Ray Chuan
2010-02-08 22:59                     ` Junio C Hamano
2010-02-10  5:49                       ` Jeff King
2010-02-11 23:23                         ` Junio C Hamano
2010-02-12  0:03                           ` Jeff King
2010-02-08 20:31             ` [PATCH v3 3/3] git-push: make git push --dry-run --porcelain exit with status 0 even if updates will be rejected Larry D'Anna
2010-02-08 20:59               ` Junio C Hamano
2010-02-08 21:49                 ` Larry D'Anna
2010-02-09 22:25               ` Junio C Hamano
2010-02-10  4:13                 ` Larry D'Anna
2010-02-10  4:51                   ` [PATCH 4/4] " Larry D'Anna
2010-02-15 17:40                 ` [PATCH v3 3/3] " Larry D'Anna
2010-02-15 20:42                   ` Junio C Hamano
2010-02-05 19:34   ` [PATCH 3/3] " Larry D'Anna
2010-02-05 19:56     ` Jeff King
2010-02-05 20:05       ` Larry D'Anna
2010-02-05 20:13         ` Jeff King
2010-02-05 19:39   ` [PATCH] fix an error message in git-push so it goes to stderr Larry D'Anna
2010-02-05 19:48     ` Jeff King
2010-02-05 19:50       ` Larry D'Anna
2010-02-05 19:50       ` Jeff King

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

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

  git send-email \
    --in-reply-to=20100209175139.GA28936@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=larry@elder-gods.org \
    --cc=peff@peff.net \
    /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.
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 NNTP newsgroup(s).