On Fri, Jan 19, 2018 at 12:34:04PM -0500, randall.s.bec...@rogers.com wrote:

> From: "Randall S. Becker" <rsbec...@nexbridge.com>
> 
> * t/lib-git-daemon.sh: fix incompatibilities with ksh traps not being
>   cleared automatically on platform. This caused tests to seem to fail
>   while actually succeeding.
> 
> Signed-off-by: Randall S. Becker <rsbec...@nexbridge.com>
> ---
>  t/lib-git-daemon.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
> index 987d40680..955beecd9 100644
> --- a/t/lib-git-daemon.sh
> +++ b/t/lib-git-daemon.sh
> @@ -68,6 +68,7 @@ start_git_daemon() {
>               test_skip_or_die $GIT_TEST_GIT_DAEMON \
>                       "git daemon failed to start"
>       fi
> +     trap '' EXIT
>  }

I don't think this can be right. The way these traps are supposed to
work is:

  - as soon as test-lib.sh is loaded, we "trap die EXIT" to catch an
    accidental death/exit from one of the scripts

  - when test_done is called, we clear the trap (i.e., it is OK to exit
    now because the script has given us a positive indication that all
    tests have been run)

  - while the child git-daemon is running, we'd want to clean up after
    ourselves. So during start_git_daemon() we add our cleanup (followed
    by the original "die", because shell traps don't push onto a stack).
    And then at stop_git_daemon(), we restore the original die trap.

But this patch means that while git-daemon is running, we have no trap
at all! That means that a failed test which causes us to exit would
leave a child daemon running.

Furthermore, both of these functions now drop the 'die' trap entirely,
meaning the script would fail to notice premature exit from one of the
test snippets.

So while this may be papering over a problem on NonStop, I think it's
breaking the intent of the traps.

I'm not sure what the root of the problem is, but it sounds like ksh may
be triggering EXIT traps when we don't expect (during function returns?
Subshell exits? Other?)

-Peff

Reply via email to