git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3] grep: support the --pathspec-from-file option

From
Emily Shaffer <emilyshaffer@google.com>
Date
Dec 13, 2019, 04:12 UTC
Message-ID
<20191213041254.13202-1-emilyshaffer@google.com>
In-Reply-To
<20191204203911.237056-1-emilyshaffer@google.com>

Teach 'git grep' to use OPT_PATHSPEC_FROM_FILE and update the documentation accordingly.

This changes enables 'git grep' to receive the pathspec from a file by specifying the path, or from stdin by specifying '-' as the path. This matches the previous functionality of '-f', so the documentation of '-f' has been expanded to describe this functionality. To let '-f' match the new '--pathspec-from-file' option, also teach a '--patterns-from-file' long name to '-f'.

Since there are now two arguments which can attempt to read from stdin, add a safeguard to check whether the user specified '-' for both of them. It is still possible for a user to pass '/dev/stdin' to one or both arguments at present; we do not explicitly check.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
I built this on 'next' locally, but it does require 'am/pathspec-from-file'
(which was merged to 'next').

Thanks all who caught the OSX/GfW CI failures; the test suite passed for me in Linux and so I completely missed the uninitialized variables. Not a great look for me...

Since v2, I got rid of the callback for -f arg and am now running it directly; this means we can check for stdin in only one place, instead of two, and get rid of some of the globals which were added elsewhere. The test suite still passes for me (but still on Linux) so here's hoping for a more favorable result with the rest of the CI.

 - Emily
 Documentation/git-grep.txt | 22 ++++++++++++++++--
 builtin/grep.c             | 43 ++++++++++++++++++++++++++--------
 t/t7810-grep.sh            | 47 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 98 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index c89fb569e3..56b1c5a302 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -24,7 +24,8 @@ SYNOPSIS
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-W | --function-context]
 	   [--threads <num>]
-	   [-f <file>] [-e] <pattern>
+	   [-f | --patterns-from-file <file>] [-e] <pattern>
+	   [--pathspec-from-file=<file> [--pathspec-file-nul]]
 	   [--and|--or|--not|(|)|-e <pattern>...]
 	   [--recurse-submodules] [--parent-basename <basename>]
 	   [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
@@ -270,7 +271,10 @@ providing this option will cause it to die.
 	See `grep.threads` in 'CONFIGURATION' for more information.
 
 -f <file>::
-	Read patterns from <file>, one per line.
+--patterns-from-file <file>::
+	Read patterns from <file>, one per line. If `<file>` is exactly `-` then
+	standard input is used; standard input cannot be used for both
+	--patterns-from-file and --pathspec-from-file.
 +
 Passing the pattern via <file> allows for providing a search pattern
 containing a \0.
@@ -289,6 +293,20 @@ In future versions we may learn to support patterns containing \0 for
 more search backends, until then we'll die when the pattern type in
 question doesn't support them.
 
+--pathspec-from-file <file>::
+	Read pathspec from <file> instead of the command line. If `<file>` is
+	exactly `-` then standard input is used; standard input cannot be used
+	for both --patterns-from-file and --pathspec-from-file. Pathspec elements
+	are separated by LF or CR/LF. Pathspec elements can be quoted as
+	explained for the configuration variable `core.quotePath` (see
+	linkgit:git-config[1]). See also `--pathspec-file-nul` and global
+	`--literal-pathspecs`.
+
+--pathspec-file-nul::
+	Only meaningful with `--pathspec-from-file`. Pathspec elements are
+	separated with NUL character and all other characters are taken
+	literally (including newlines and quotes).
+
 -e::
 	The next parameter is the pattern. This option has to be
 	used for patterns starting with `-` and should be used in
diff --git a/builtin/grep.c b/builtin/grep.c
index 50ce8d9461..28b9f99d4c 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -720,26 +720,23 @@ static int context_callback(const struct option *opt, const char *arg,
 	return 0;
 }
 
-static int file_callback(const struct option *opt, const char *arg, int unset)
+static int read_pattern_file(struct grep_opt *grep_opt, const char *path)
 {
-	struct grep_opt *grep_opt = opt->value;
 	int from_stdin;
 	FILE *patterns;
 	int lno = 0;
 	struct strbuf sb = STRBUF_INIT;
 
-	BUG_ON_OPT_NEG(unset);
-
-	from_stdin = !strcmp(arg, "-");
-	patterns = from_stdin ? stdin : fopen(arg, "r");
+	from_stdin = !strcmp(path, "-");
+	patterns = from_stdin ? stdin : fopen(path, "r");
 	if (!patterns)
-		die_errno(_("cannot open '%s'"), arg);
+		die_errno(_("cannot open '%s'"), path);
 	while (strbuf_getline(&sb, patterns) == 0) {
 		/* ignore empty line like grep does */
 		if (sb.len == 0)
 			continue;
 
-		append_grep_pat(grep_opt, sb.buf, sb.len, arg, ++lno,
+		append_grep_pat(grep_opt, sb.buf, sb.len, path, ++lno,
 				GREP_PATTERN);
 	}
 	if (!from_stdin)
@@ -809,6 +806,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int use_index = 1;
 	int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
 	int allow_revs;
+	char *patterns_from_file = NULL;
+	char *pathspec_from_file = NULL;
+	int pathspec_file_nul = 0;
 
 	struct option options[] = {
 		OPT_BOOL(0, "cached", &cached,
@@ -896,8 +896,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('W', "function-context", &opt.funcbody,
 			N_("show the surrounding function")),
 		OPT_GROUP(""),
-		OPT_CALLBACK('f', NULL, &opt, N_("file"),
-			N_("read patterns from file"), file_callback),
+		OPT_STRING('f', "patterns-from-file", &patterns_from_file, N_("file"),
+			N_("read patterns from file")),
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 		{ OPTION_CALLBACK, 'e', NULL, &opt, N_("pattern"),
 			N_("match <pattern>"), PARSE_OPT_NONEG, pattern_callback },
 		{ OPTION_CALLBACK, 0, "and", &opt, NULL,
@@ -949,6 +951,15 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	grep_commit_pattern_type(pattern_type_arg, &opt);
 
+	if (patterns_from_file && pathspec_from_file &&
+	    !strcmp(pathspec_from_file, "-") &&
+	    !strcmp(patterns_from_file, "-"))
+		die(_("cannot specify both patterns and pathspec via stdin"));
+
+	if (patterns_from_file)
+		read_pattern_file(&opt, patterns_from_file);
+
+
 	if (use_index && !startup_info->have_repository) {
 		int fallback = 0;
 		git_config_get_bool("grep.fallbacktonoindex", &fallback);
@@ -1062,6 +1073,18 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	pathspec.recursive = 1;
 	pathspec.recurse_submodules = !!recurse_submodules;
 
+	if (pathspec_from_file) {
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+
+		parse_pathspec_file(&pathspec, 0, PATHSPEC_PREFER_CWD |
+				    (opt.max_depth != -1 ? PATHSPEC_MAXDEPTH_VALID : 0),
+				    prefix, pathspec_from_file,
+				    pathspec_file_nul);
+	} else if (pathspec_file_nul) {
+		die(_("--pathspec-file-nul requires --pathspec-from-file"));
+	}
+
 	if (list.nr || cached || show_in_pager) {
 		if (num_threads > 1)
 			warning(_("invalid option combination, ignoring --threads"));
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 7d7b396c23..3ace2dfdac 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -549,6 +549,10 @@ test_expect_success 'grep -f, non-existent file' '
 	test_must_fail git grep -f patterns
 '
 
+text_expect_success 'grep --pathspec-from-file, non-existent file' '
+	test_must_fail git grep --pathspec-from-file pathspecs
+'
+
 cat >expected <<EOF
 file:foo mmap bar
 file:foo_mmap bar
@@ -582,8 +586,8 @@ mmap
 vvv
 EOF
 
-test_expect_success 'grep -f, multiple patterns' '
-	git grep -f patterns >actual &&
+test_expect_success 'grep --patterns-from-file, multiple patterns' '
+	git grep --patterns-from-file patterns >actual &&
 	test_cmp expected actual
 '
 
@@ -1125,6 +1129,45 @@ test_expect_success 'grep --no-index descends into repos, but not .git' '
 	)
 '
 
+test_expect_success 'setup pathspecs-file tests' '
+cat >excluded-file <<EOF &&
+bar
+EOF
+cat >pathspec-file <<EOF &&
+foo
+bar
+baz
+EOF
+cat >unrelated-file <<EOF &&
+xyz
+EOF
+git add excluded-file pathspec-file unrelated-file
+'
+
+cat >pathspecs <<EOF
+pathspec-file
+unrelated-file
+EOF
+
+cat >expected <<EOF
+pathspec-file:bar
+EOF
+
+test_expect_success 'grep --pathspec-from-file with file' '
+	git grep --pathspec-from-file pathspecs "bar" >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'grep --pathspec-file with stdin' '
+	git grep --pathspec-from-file - "bar" <pathspecs >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'grep with two stdin inputs fails' '
+	test_must_fail git grep --pathspec-from-file - --patterns-from-file - <pathspecs 2>err &&
+	test_i18ngrep "cannot specify both patterns and pathspec via stdin" err
+'
+
 test_expect_success 'setup double-dash tests' '
 cat >double-dash <<EOF &&
 --

-- 
2.24.1.735.g03f4e72817-goog
Previous: SZEDER GáborNext: Alexandr Miloslavskiy
Message 19 of 23 in “grep: provide pathspecs/patterns via file or stdin”
  1. grep: provide pathspecs/patterns via file or stdinEmily Shaffer, Nov 22, 2019
  2. Denton LiuNov 22, 2019
  3. Junio C HamanoNov 22, 2019
  4. Junio C HamanoNov 22, 2019
  5. Denton LiuNov 22, 2019
  6. Emily ShafferNov 22, 2019
  7. Emily ShafferNov 22, 2019
  8. Junio C HamanoNov 23, 2019
  9. Junio C HamanoNov 22, 2019
  10. grep: support the --pathspec-from-file optionEmily Shaffer, Dec 4, 2019
  11. Denton LiuDec 4, 2019
  12. Junio C HamanoDec 4, 2019
  13. Junio C HamanoDec 4, 2019
  14. Emily ShafferDec 13, 2019
  15. Alexandr MiloslavskiyDec 5, 2019
  16. Emily ShafferDec 13, 2019
  17. Johannes SchindelinDec 6, 2019
  18. SZEDER GáborDec 6, 2019
  19. grep: support the --pathspec-from-file optionEmily Shaffer, Dec 13, 2019
  20. Alexandr MiloslavskiyDec 13, 2019
  21. Junio C HamanoDec 13, 2019
  22. Alexandr MiloslavskiyDec 13, 2019
  23. Emily ShafferDec 17, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.