All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Jeff King <peff@peff.net>, Luke Diamand <luke@diamand.org>,
	Lars Schneider <larsxschneider@gmail.com>
Subject: Re: [PATCH 01/11] test-lib: fix interrupt handling with 'dash' and '--verbose-log -x'
Date: Thu, 14 Mar 2019 15:50:26 +0100	[thread overview]
Message-ID: <20190314145026.GF28939@szeder.dev> (raw)
In-Reply-To: <xmqq1s3aci66.fsf@gitster-ct.c.googlers.com>

On Thu, Mar 14, 2019 at 11:18:41AM +0900, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
> 
> > Putting these together, when a test script run with 'dash' and
> > '--verbose-log -x' is interrupted, then 'dash' tries to write the
> > trace output from the EXIT trap to the script's original standard
> > error, but it very likely can't, because the 'tee' downstream of the
> > pipe is interrupted as well.  This causes the shell running the test
> > script to die because of SIGPIPE, without running any of the commands
> > in the EXIT trap.
> 
> So... the clean-up actions do not get a chance to run because the
> shell that is going to execute would be killed by SIGPIPE because
> tee that is reading from it goes away?

Yes.

> While I like the patch very much, I also wonder if it is possible to
> tell tee to not to die upon seeing INT, TERM, etc.

'tee -i' ignores only INT, but not TERM and HUP.

What could work is something like:

  <re-executing the test script> | (
        # Ignore common interrupt signals to prevent 'tee' from dying
        # while the upstream test process still produces output.
        trap '' INT TERM HUP
        tee logfile
  )

but I'm not sure what should happen with a process ignoring HUP when
it still produces output to the terminal after that terminal has been
closed, i.e. this 'tee' process.  FWIW, In my quick test such a
process continued happily without receiving SIGPIPE or other
unpleasantness.

>  When the shell
> upstream of tee exits (after performing clean-up actions), tee that
> is reading from it will exit, too, and will not be left behind (I do
> not mean to say it that is a better alternative solution---I am just
> trying to see if I read the problem correctly from the description
> given above).
> 
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 8665b0a9b6..db3875d1e4 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -631,7 +631,10 @@ die () {
> >  
> >  GIT_EXIT_OK=
> >  trap 'die' EXIT
> > -trap 'exit $?' INT TERM HUP
> > +# Disable '-x' tracing, because with some shells, notably dash, it
> > +# prevents running the cleanup commands when a test script run with
> > +# '--verbose-log -x' is interrupted.
> > +trap '{ code=$?; set +x; } 2>/dev/null; exit $code' INT TERM HUP
> >  
> >  # The user-facing functions are loaded from a separate file so that
> >  # test_perf subshells can have them too
> 
> Thanks.

  reply	other threads:[~2019-03-14 14:50 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-13 12:24 [PATCH 00/11] tests: introduce 'test_atexit' SZEDER Gábor
2019-03-13 12:24 ` [PATCH 01/11] test-lib: fix interrupt handling with 'dash' and '--verbose-log -x' SZEDER Gábor
2019-03-14  2:18   ` Junio C Hamano
2019-03-14 14:50     ` SZEDER Gábor [this message]
2019-03-13 12:24 ` [PATCH 02/11] t/lib-git-daemon: make sure to kill the 'git-daemon' process SZEDER Gábor
2019-03-14  2:35   ` Junio C Hamano
2019-03-13 12:24 ` [PATCH 03/11] test-lib: introduce 'test_atexit' SZEDER Gábor
2019-03-14  3:21   ` Junio C Hamano
2019-03-14 17:40     ` SZEDER Gábor
2019-03-18  1:50       ` Junio C Hamano
2019-03-13 12:24 ` [PATCH 04/11] git-daemon: use 'test_atexit` to stop 'git-daemon' SZEDER Gábor
2019-03-13 12:24 ` [PATCH 05/11] tests: use 'test_atexit' to stop httpd SZEDER Gábor
2019-03-14  3:28   ` Junio C Hamano
2019-03-14  4:34     ` Junio C Hamano
2019-03-14 15:19     ` SZEDER Gábor
2019-03-13 12:24 ` [PATCH 06/11] t0301-credential-cache: use 'test_atexit' to stop the credentials helper SZEDER Gábor
2019-03-13 12:24 ` [PATCH 07/11] git p4 test: use 'test_atexit' to kill p4d and the watchdog process SZEDER Gábor
2019-03-13 12:24 ` [PATCH 08/11] git p4 test: clean up the p4d cleanup functions SZEDER Gábor
2019-03-13 12:24 ` [PATCH 09/11] git p4 test: simplify timeout handling SZEDER Gábor
2019-03-13 12:24 ` [PATCH 10/11] git p4 test: disable '-x' tracing in the p4d watchdog loop SZEDER Gábor
2019-03-13 12:24 ` [PATCH 11/11] t9811-git-p4-label-import: fix pipeline negation SZEDER Gábor

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=20190314145026.GF28939@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=larsxschneider@gmail.com \
    --cc=luke@diamand.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.