Re: [PATCH] pathspec: give better message for submodule related pathspec error

From: Jeff King <peff@peff.net>
Date: Jan 4, 2017, 01:23 UTC
Message-ID: <20170104012318.262o5743ne2u7rwc@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kZ5F46UzeNeVYQV1EKEEa6+az-Pe_jrT+wZF9X6b34CGA@mail.gmail.com>

On Tue, Jan 03, 2017 at 10:15:38AM -0800, Stefan Beller wrote:

Show 9 quoted lines

> > I take that the new "BUG" thing tells the Git developers that no
> > sane codepath should throw an pathspec_item that satisfies the
> > condition of the if() statement for non-submodules?
> 
> If we want to keep the semantics of the assert around, then we
> have to have a blank statement if the submodule error message
> is not triggered.
> 
> I assume if we print this BUG, then there is an actual bug.

Right. I think this isn't a new "BUG", but rather a loosening of an existing one. IOW, two things are going on here:

  1. Switch assert() to die("BUG") to give a more readable message.

  2. Take one of the cases where we hit a BUG and turn it into a normal
     "there was something wrong with the input" message.

If I understand the submodule case correctly, then (2) is reasonable. The user gave a bogus pathspec that crosses the submodule boundary.

I've no idea if there are other cases that could ever hit the remaining BUG, but it seems like a good defensive measure to leave it in. If somebody hits it, then we can investigate their case.

-Peff