@matheusbsbs / matheus-bagliano / commits / b5b81136da

grep: fix "--" rev/pathspec disambiguation

If we see "git grep pattern rev -- file" then we apply the usual rev/pathspec disambiguation rules: any "rev" before the "--" must be a revision, and we do not need to apply the verify_non_filename() check. But there are two bugs here: 1. We keep a seen_dashdash flag to handle this case, but we set it in the same left-to-right pass over the arguments in which we parse "rev". So when we see "rev", we do not yet know that there is a "--", and we mistakenly complain if there is a matching file. We can fix this by making a preliminary pass over the arguments to find the "--", and only then checking the rev arguments. 2. If we can't resolve "rev" but there isn't a dashdash, that's OK. We treat it like a path, and complain later if it doesn't exist. But if there _is_ a dashdash, then we know it must be a rev, and should treat it as such, complaining if it does not resolve. The current code instead ignores it and tries to treat it like a path. This patch fixes both bugs, and tries to comment the parsing flow a bit better. It adds tests that cover the two bugs, but also some related situations (which already worked, but this confirms that our fixes did not break anything). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 14, 2017 at 01:05 UTC b5b81136da11638bdf1e336d9ec371b820fbf412

2 files changed +57 -5

builtin/grep.c

+24 -5

@@ -1149,7 +1149,22 @@ int cmd_grep(int argc, const char **argv, const char *prefix)

1149

1150 compile_grep_patterns(&opt);

1151

1152 - /* Check revs and then paths */

1152 + /*

1153 + * We have to find "--" in a separate pass, because its presence

1154 + * influences how we will parse arguments that come before it.

1155 + */

1156 + for (i = 0; i < argc; i++) {

1157 + if (!strcmp(argv[i], "--")) {

1158 + seen_dashdash = 1;

1159 + break;

1160 + }

1161 + }

1162 +

1163 + /*

1164 + * Resolve any rev arguments. If we have a dashdash, then everything up

1165 + * to it must resolve as a rev. If not, then we stop at the first

1166 + * non-rev and assume everything else is a path.

1167 + */

1168 for (i = 0; i < argc; i++) {

1169 const char *arg = argv[i];

1170 unsigned char sha1[20];

@@ -1158,13 +1173,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)

1173

1174 if (!strcmp(arg, "--")) {

1175 i++;

1161 - seen_dashdash = 1;

1176 break;

1177 }

1178

1165 - /* Stop at the first non-rev */

1166 - if (get_sha1_with_context(arg, 0, sha1, &oc))

1179 + if (get_sha1_with_context(arg, 0, sha1, &oc)) {

1180 + if (seen_dashdash)

1181 + die(_("unable to resolve revision: %s"), arg);

1182 break;

1183 + }

1184

1185 object = parse_object_or_die(sha1, arg);

1186 if (!seen_dashdash)

@@ -1172,7 +1188,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)

1188 add_object_array_with_path(object, arg, &list, oc.mode, oc.path);

1189 }

1190

1175 - /* The rest are paths */

1191 + /*

1192 + * Anything left over is presumed to be a path. But in the non-dashdash

1193 + * "do what I mean" case, we verify and complain when that isn't true.

1194 + */

1195 if (!seen_dashdash) {

1196 int j;

1197 for (j = i; j < argc; j++)

t/t7810-grep.sh

+33

@@ -982,6 +982,39 @@ test_expect_success 'grep -e -- -- path' '

982 test_cmp expected actual

983 '

984

985 +test_expect_success 'dashdash disambiguates rev as rev' '

986 + test_when_finished "rm -f master" &&

987 + echo content >master &&

988 + echo master:hello.c >expect &&

989 + git grep -l o master -- hello.c >actual &&

990 + test_cmp expect actual

991 +'

992 +

993 +test_expect_success 'dashdash disambiguates pathspec as pathspec' '

994 + test_when_finished "git rm -f master" &&

995 + echo content >master &&

996 + git add master &&

997 + echo master:content >expect &&

998 + git grep o -- master >actual &&

999 + test_cmp expect actual

1000 +'

1001 +

1002 +test_expect_success 'report bogus arg without dashdash' '

1003 + test_must_fail git grep o does-not-exist

1004 +'

1005 +

1006 +test_expect_success 'report bogus rev with dashdash' '

1007 + test_must_fail git grep o hello.c --

1008 +'

1009 +

1010 +test_expect_success 'allow non-existent path with dashdash' '

1011 + # We need a real match so grep exits with success.

1012 + tree=$(git ls-tree HEAD |

1013 + sed s/hello.c/not-in-working-tree/ |

1014 + git mktree) &&

1015 + git grep o "$tree" -- not-in-working-tree

1016 +'

1017 +

1018 test_expect_success 'grep --no-index pattern -- path' '

1019 rm -fr non &&

1020 mkdir -p non/git &&