Re: [PATCH] t/test-lib.sh: do not trust $SHELL

[Junio C Hamano <gitster@pobox.com>]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Here's a patch.
>
> -- 8< --
> From: Ramkumar Ramachandra <artagnon@gmail.com>
> Date: Sat, 22 Sep 2012 10:25:10 +0530
> Subject: [PATCH] test-lib: do not trust $SHELL
>
> Do not trust $SHELL to be a bourne-compatible shell.  Instead, use the
> Makefile variable $SHELL_PATH.  This fixes a bug: when a test was run
> with --tee and $SHELL was set to ZSH, $PATH on line 479 was not
> getting split due to ZSH not respecting $IFS.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---

The part that this starts letting run, which the original "Re-run
the command under tee as early as possible" wanted to avoid running,
does not affect anything that would affect how we run that tee magic
(e.g. "mkdir -p test-results" will still create it directly inside
the directory the test script was started in), so I think this patch
is safe _for now_.

However, it forces people who need to update earlier parts of this
script to be extra careful; it has been true before the patch, and
the patch makes it even more so.

I am not opposed to queuing this as an interim solution, but I
wonder if we can get rid of that double-launch altogether.

Instead of re-launching the script with its output piped to "tee",
can't we do the same by redirecting our standard output to the file
in the file, and spawn a "tail -f" that reads from the file and
outputs to our original output?  Something along the lines of:

        mkdir -p test-results
        tee_base=test-results/$(basename "$0" .sh)

        # empty the file and start "tail -f" on it ...
        : >"$tee_base.out"
        ( tail -f "$tee_base.out" ) &
        tee_pid=$!
	trap 'kill $tee_pid; exit' 0 1 2 3
	# ... and then redirect our output to it
        exec >"$tee_base.out"

and wrap it in a shell helper function that is called from where the
parsing of the command line arguments for "--tee" happens, and don't
forget to kill $tee_pid when we exit.

Hrm?