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$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=john@keeping$(echo .)me.uk \
--cc=jrnieder@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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