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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:
>
>> Here's a patch.
>>
>> -- 8< --
>> From: Ramkumar Ramachandra <artagnon@xxxxxxxxx>
>> 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@xxxxxxxxx>
>> ---
>
> 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.

I see you've queued it in `pu` after rewriting the commit message.

> 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?

Good idea.  I'll write a patch to do this once the interim solution
graduates to `master`.

Ram
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]