Skip to content

Commit

Permalink
get_sha1: don't die() on bogus search strings
Browse files Browse the repository at this point in the history
The get_sha1() function generally returns an error code
rather than dying, and we sometimes speculatively call it
with something that may be a revision or a pathspec, in
order to see which one it might be.

If it sees a bogus ":/" search string, though, it complains,
without giving the caller the opportunity to recover. We can
demonstrate this in t6133 by looking for ":/*.t", which
should mean "*.t at the root of the tree", but instead dies
because of the invalid regex (the "*" has nothing to operate
on).

We can fix this by returning an error rather than calling
die(). Unfortunately, the tradeoff is that the error message
is slightly worse in cases where we _do_ know we have a rev.
E.g., running "git log ':/*.t' --" before yielded:

  fatal: Invalid search pattern: *.t

and now we get only:

  fatal: bad revision ':/*.t'

There's not a simple way to fix this short of passing a
"quiet" flag all the way through the get_sha1() stack.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Feb 10, 2016
1 parent df714f8 commit aac4fac
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
4 changes: 2 additions & 2 deletions sha1_name.c
Original file line number Diff line number Diff line change
Expand Up @@ -858,12 +858,12 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,

if (prefix[0] == '!') {
if (prefix[1] != '!')
die ("Invalid search pattern: %s", prefix);
return -1;
prefix++;
}

if (regcomp(&regex, prefix, REG_EXTENDED))
die("Invalid search pattern: %s", prefix);
return -1;

for (l = list; l; l = l->next) {
l->item->object.flags |= ONELINE_SEEN;
Expand Down
10 changes: 10 additions & 0 deletions t/t6133-pathspec-rev-dwim.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,14 @@ test_expect_success '@{foo} with metacharacters dwims to rev' '
test_cmp expect actual
'

test_expect_success ':/*.t from a subdir dwims to a pathspec' '
mkdir subdir &&
(
cd subdir &&
git log -- ":/*.t" >expect &&
git log ":/*.t" >actual &&
test_cmp expect actual
)
'

test_done

0 comments on commit aac4fac

Please sign in to comment.