Re: [PATCH] t4014: shell portability fix

From: Jeff King <peff@peff.net>
Date: Jun 1, 2016, 16:07 UTC
Message-ID: <20160601160735.GB9219@sigill.intra.peff.net>
In-Reply-To: <xmqqfusx0wyj.fsf@gitster.mtv.corp.google.com>

On Wed, Jun 01, 2016 at 08:07:16AM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> > Here's the patch I wrote up earlier (but was too timid to send out after
> > my barrage of emails :) ).
> 
> Looks very sensible.  I'll drop all these "Attempt to test with
> ksh93 found these breakages" patches and queue this one.

Curious based on our previous discussion, I applied your patches and did a "make SHELL_PATH=/bin/ksh93 test". There were only a handful of failures remaining. Some were definitely the "../.git" thing (which Andreas earlier reported as fixed, though the fix still has not made it into the version in Debian), but some were just puzzling. I shrugged and gave up.

Show 11 quoted lines

> > We sometimes get around this by using env, like:
> >
> >   test_must_fail env FOO=BAR some-program
> >
> > But that only works because test_must_fail's arguments are
> > themselves a command which can be run. You can't run:
> 
> We can do "test_must_fail test_commit ...", but "test_must_fail env
> FOO=BAR test_commit ..." would not work, right?
> 
> If so s/because/when/, perhaps?

Right. What I was trying to say is that it works in this case because test_must_fail further executes its arguments, which gives us an opportunity to put the "env" on the right-hand side of the function call. I'm not sure s/because/when/ makes that any better, though.

Maybe:

    We sometimes get around this by using env, like:

      test_must_fail env FOO=BAR some-program

    But that works for test_must_fail because it further runs its
    arguments via the shell, so we can stick the "env" on the right-hand
    side of the function. It would not work to do:

      env FOO=BAR test_must_fail some-program

    because env does not know about our shell functions...

is more clear?

-Peff