git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC] test_when_finished in subshells

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 4, 2015, 18:43 UTC
Message-ID
<xmqqfv2uf2kc.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<54923cf9cc5a66bf9034051b3c2f930fa7ef88a4.1441388803.git.john@keeping.me.uk>
John Keeping <john@keeping.me.uk> writes:
Show 9 quoted lines
> 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.
Show 17 quoted lines
>
> [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"
>  }
Previous: John KeepingNext: Jeff King
Message 2 of 15 in “[RFC] test_when_finished in subshells”
  1. John KeepingSep 4, 2015
  2. Junio C HamanoSep 4, 2015
  3. Jeff KingSep 5, 2015
  4. 0/5 disallow test_when_finished in subshellsJohn Keeping, Sep 5, 2015
  5. 1/5 t7610: don't use test_config in a subshellJohn Keeping, Sep 5, 2015
  6. 2/5 t5801: don't use test_when_finished in a subshellJohn Keeping, Sep 5, 2015
  7. 3/5 test-lib-functions: support "test_config -C <dir> ..."John Keeping, Sep 5, 2015
  8. 4/5 t7800: don't use test_config in a subshellJohn Keeping, Sep 5, 2015
  9. 5/5 test-lib-functions: detect test_when_finished in subshellJohn Keeping, Sep 5, 2015
  10. Eric SunshineSep 6, 2015
  11. John KeepingSep 6, 2015
  12. Eric SunshineSep 6, 2015
  13. Junio C HamanoSep 5, 2015
  14. John KeepingSep 5, 2015
  15. Junio C HamanoSep 5, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.