From: Danh Doan <congdanhqx@gmail.com>
Date: Fri, 20 Mar 2020 01:29:56 GMT
Subject: Re: [PATCH 2/6] test-lib-functions: test_cmp: eval $GIT_TEST_CMP
Message-ID: <20200320012956.GD1858@danh.dev>
In-Reply-To: <20200319160211.GC3513282@coredump.intra.peff.net>

On 2020-03-19 12:02:11-0400, Jeff King <peff@peff.net> wrote:
> On Thu, Mar 19, 2020 at 09:00:03PM +0700, Đoàn Trần Công Danh wrote:
> 
> > Shell recognises first non-assignment token as command name.
> > Thus, ` cd t/perf; ./p0000-perf-lib-sanity.sh -d -i -v` reports:
> > 
> > > test_cmp:1: command not found: diff -u
> > 
> > Using `eval` to unquote $GIT_TEST_CMP as same as precedence in `git_editor`.
> 
> Adding an "eval" here will subtly change the behavior for other shells.
> Right now they'd just do whitespace splitting (which presumably busybox
> is not), but with this we'd expand metachars, etc.
> 
> I suspect that's fine (and maybe even preferable, because if you really
> do have a space in the path you can actually escape it). So I don't mind
> this change.
> 
> I do worry that this whitespace splitting behavior could bite us in
> other scripts. Curiously, my version of busybox (1.30.1) doesn't seem to
> have any problem with it, though.

Ah, this patch wasn't meant for busybox.
It run into failure with both bash and dash in VoidLinux, and bash in
ArchLinux, I couldn't start p0000 in Ubuntu docker image due to
missing /usr/bin/time (even if I have /usr/bin/time installed).

-- 
Danh