[PATCH for NEXT] grep: do not unnecessarily query repo for "--"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When running a command of the form

  git grep --no-index pattern -- path

in the absence of a Git repository, an error message will be printed:

  fatal: BUG: setup_git_env called without repository

This is because "git grep" tries to interpret "--" as a rev. "git grep"
has always tried to first interpret "--" as a rev for at least a few
years, but this issue was upgraded from a pessimization to a bug in
commit 59332d1 ("Resurrect "git grep --no-index"", 2010-02-06), which
calls get_sha1 regardless of whether --no-index was specified. This bug
appeared to be benign until commit b1ef400 ("setup_git_env: avoid blind
fall-back to ".git"", 2016-10-20) when Git was taught to die in this
situation.  (This "git grep" bug appears to be one of the bugs that
commit b1ef400 is meant to flush out.)

Therefore, always interpret "--" as signaling the end of options,
instead of trying to interpret it as a rev first.

Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
---

There is probably a similar bug for commands of the form:

  git grep --no-index pattern foo

If there is a repo and "foo" is a rev, the "--no-index or --untracked
cannot be used with revs." error would occur. If there is a repo and
"foo" is not a rev, this command would proceed as usual. If there is no
repo, the "setup_git_env called without repository" error would occur.
(This is my understanding from reading the code - I haven't tested it
out.)

This patch does not fix this similar bug, but I decided to send it out
anyway because it still fixes a bug and unlocks the ability to
specify paths with "git grep --no-index".

 builtin/grep.c  |  9 +++++----
 t/t7810-grep.sh | 12 ++++++++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef49..1b68d1638 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1154,6 +1154,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		const char *arg = argv[i];
 		unsigned char sha1[20];
 		struct object_context oc;
+		if (!strcmp(arg, "--")) {
+			i++;
+			seen_dashdash = 1;
+			break;
+		}
 		/* Is it a rev? */
 		if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
 			struct object *object = parse_object_or_die(sha1, arg);
@@ -1162,10 +1167,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
 			continue;
 		}
-		if (!strcmp(arg, "--")) {
-			i++;
-			seen_dashdash = 1;
-		}
 		break;
 	}
 
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 19f0108f8..29202f0e7 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -982,6 +982,18 @@ test_expect_success 'grep -e -- -- path' '
 	test_cmp expected actual
 '
 
+test_expect_success 'grep --no-index pattern -- path' '
+	rm -fr non &&
+	mkdir -p non/git &&
+	(
+		GIT_CEILING_DIRECTORIES="$(pwd)/non" &&
+		export GIT_CEILING_DIRECTORIES &&
+		cd non/git &&
+		echo hello >hello &&
+		git grep --no-index o -- .
+	)
+'
+
 cat >expected <<EOF
 hello.c:int main(int argc, const char **argv)
 hello.c:	printf("Hello world.\n");
-- 
2.11.0.483.g087da7b7c-goog




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]