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

From: Jeff King <peff@peff.net>
Date: May 11, 2016, 13:36 UTC
Message-ID: <20160511133615.GA19356@sigill.intra.peff.net>
In-Reply-To: <alpine.DEB.2.20.1605111518470.4092@virtualbox>

On Wed, May 11, 2016 at 03:28:35PM +0200, Johannes Schindelin wrote:

> Looks obviously correct to me.

Thanks.

Show 9 quoted lines

> I had a look at our other shell scripts and it looks as if there is only
> one more candidate for this issue: git-bisect.sh has a couple of 'for arg
> in "$@"' constructs. But from a cursory look, it appears that none of
> these "$@" can be empty lists because at least one parameter is passed to
> those functions (check_expected_revs() is only called from bisect_state()
> with 1 or 2 parameters, bisect_skip() makes no sense without parameters,
> and bisect_state() has another for loop if it got 2 parameters).
> 
> So I think we're fine.

I'm not even sure that:

  for arg in "$@"

is a problem if "$@" is empty. The issue here is the eval, in which we generate syntactically funny code and expect the shell to interpret it. It's possible a shell could get the more mundane case wrong, but I'd expect most to get it right.

I did some brief grepping around myself, but didn't find any other interesting cases. That doesn't mean much, though; tracking down what content actually makes it into some of our "eval" calls would be pretty time-consuming. So I'd rely on people like Armin to report failures in the test suite.

-Peff