From: Junio C Hamano <gitster@pobox.com>
To: John Keeping <john@keeping.me.uk>
Cc: git@vger.kernel.org, Eric Sunshine <sunshine@sunshineco.com>,
Jonathan Nieder <jrnieder@gmail.com>
Subject: Re: [RFC] test_when_finished in subshells
Date: Fri, 04 Sep 2015 11:43:15 -0700 [thread overview]
Message-ID: <xmqqfv2uf2kc.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <54923cf9cc5a66bf9034051b3c2f930fa7ef88a4.1441388803.git.john@keeping.me.uk> (John Keeping's message of "Fri, 4 Sep 2015 18:58:45 +0100")
John Keeping <john@keeping.me.uk> writes:
> All are harmless at the moment and t7610 and t5801 can be fixed by
> moving the test_when_finished call out of the subshell relatively
> easily.
>
> t7800 (in its final test) calls test_config in a subshell which has cd'd
> into a submodule.
>
> Is this something worth worrying about, or is it sufficiently rare that
> we can live with the current behaviour?
Fixing the instances you found is good, obviously ;-). Thanks for
working on this.
Even though the proposed detection is BASH-ism, I think it would not
hurt other shells (they obviously do not help you catch bugs, but
they would not misbehave as long as you make sure BASH_SUBSHELL is
either unset or set to 0 at the beginning of the test), and the only
impact to them would be a invocation of (often built-in) 'test'
utility, whose performance impact should be miniscule.
I'll wait for opinion from others, of course.
>
> [0] http://article.gmane.org/gmane.comp.version-control.git/277199
>
> -- >8 --
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index e8d3c0f..d29cd7b 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -722,6 +722,8 @@ test_seq () {
> # what went wrong.
>
> test_when_finished () {
> + test "${BASH_SUBSHELL-0}" = 0 ||
> + error "bug in test script: test_when_finished does nothing in a subshell"
> test_cleanup="{ $*
> } && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
> }
next prev parent reply other threads:[~2015-09-04 18:43 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-04 17:58 [RFC] test_when_finished in subshells John Keeping
2015-09-04 18:43 ` Junio C Hamano [this message]
2015-09-05 8:54 ` Jeff King
2015-09-05 13:12 ` [PATCH 0/5] disallow " John Keeping
2015-09-05 13:12 ` [PATCH 1/5] t7610: don't use test_config in a subshell John Keeping
2015-09-05 13:12 ` [PATCH 2/5] t5801: don't use test_when_finished " John Keeping
2015-09-05 13:12 ` [PATCH 3/5] test-lib-functions: support "test_config -C <dir> ..." John Keeping
2015-09-05 13:12 ` [PATCH 4/5] t7800: don't use test_config in a subshell John Keeping
2015-09-05 13:12 ` [PATCH 5/5] test-lib-functions: detect test_when_finished in subshell John Keeping
2015-09-06 9:51 ` Eric Sunshine
2015-09-06 11:46 ` John Keeping
2015-09-06 16:22 ` Eric Sunshine
2015-09-05 17:36 ` [PATCH 0/5] disallow test_when_finished in subshells Junio C Hamano
2015-09-05 17:57 ` John Keeping
2015-09-05 20:17 ` Junio C Hamano
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=xmqqfv2uf2kc.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=john@keeping.me.uk \
--cc=jrnieder@gmail.com \
--cc=sunshine@sunshineco.com \
/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.