Re: syntax error in git-rebase while running t34* tests

From: Jeff King <peff@peff.net>
Date: May 10, 2016, 20:47 UTC
Message-ID: <20160510204758.GB19958@sigill.intra.peff.net>
In-Reply-To: <xmqq60ulpw1s.fsf@gitster.mtv.corp.google.com>

On Tue, May 10, 2016 at 12:11:59PM -0700, Junio C Hamano wrote:

Show 20 quoted lines

> Armin Kunaschik <megabreit@googlemail.com> writes:
> 
> > I fail to see why eval is really necessary here.
> 
> It is necessary to work correctly with any strategy option with $IFS
> in it, I would think.  The calling script "git-rebase" accumulates
> --strategy-option values after passing each of them through
> "rev-parse --sq-quote" for that reason.
> 
> which means that eval'ed string here:
> 
> > My quick and dirty hotfix is to place a
> > test -n "$strategy_opts" &&
> > in front of the eval.
> > The tests run fine after this change.
> >
> > What do you think?
> 
> I do not see why "test -n &&" is necessary here, and would be very
> hesitant to accept a change that nobody understands why it works.

I think it is clear why it works. If $strategy_opts is empty, then the code we generate looks like:

  for strategy_opt in
  do
          ...
  done

and some shells (apparently) are not happy with no content after the "in". If the variable is empty, there is no need to run the loop at all, so it is OK to skip it the eval entirely.

-Peff