threads / bug / 60351

Bug: Git grep -f reads the filename relative to the repository root

Subject: Bug: Git grep -f reads the filename relative to the repository root

## tl;dr

4 messages between Oct 12, 2023 and Oct 31, 2023.

replies: 3people: 3as markdown or json

Erik Cervin Edin· Oct 12, 2023, 12:38 UTC · lore

Thank you for filling out a Git bug report! Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
In the Git repository, I ran
    echo tig > pattern-file &&
        echo git > xdiff/pattern-file &&
        cd xdfiff &&
        git grep -f pattern-file
What did you expect to happen? (Expected behavior)

Git grep -f to read the pattern-file, in the xdiff directory and search for lines matching `git` in the xdiff directory.

What happened instead? (Actual behavior)

Git grep -f reads the filename, relative to the Git root and searches for lines matching `tig` in the xdiff directory.

What's different between what you expected and what actually happened?

The file that Git grep uses for patterns is read relative to the root of the Git repository, and not the current directory.

Anything else you want to add:

Please review the rest of the bug report below. You can delete any lines you don't wish to share.

[System Info] git version: git version 2.42.0 cpu: x86_64 no commit associated with this build sizeof-long: 8 sizeof-size_t: 8 shell-path: /bin/sh uname: Linux 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 compiler info: gnuc: 11.4 libc info: glibc: 2.35 $SHELL (typically, interactive shell): /bin/bash

Junio C Hamano· Oct 12, 2023, 17:28 UTC · re: Erik Cervin Edin · lore

Re: Bug: Git grep -f reads the filename relative to the repository root

Erik Cervin Edin <erik@cervined.in> writes:
Show 11 quoted lines
> In the Git repository, I ran
>
>     echo tig > pattern-file &&
>         echo git > xdiff/pattern-file &&
>         cd xdfiff &&
>         git grep -f pattern-file
>
> What did you expect to happen? (Expected behavior)
>
> Git grep -f to read the pattern-file, in the xdiff directory and
> search for lines matching `git` in the xdiff directory.

That does sound like a bug. It should use the original directory as the base of the relative path computation, similar to the way how OPT_FILENAME() options are handled.

Perhaps something along this line, but this is not even compile tested yet.

----- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH] grep: -f <path> is relative to $cwd

Just like OPT_FILENAME() does, "git grep -f <path>" should treat the <path> relative to the original $cwd by paying attention to the prefix the command is given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/grep.c  | 13 +++++++++++--
 t/t7810-grep.sh | 13 +++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index b71222330a..fe78d4c98b 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2006 Junio C Hamano
  */
 #include "builtin.h"
+#include "abspath.h"
 #include "gettext.h"
 #include "hex.h"
 #include "repository.h"
@@ -812,14 +813,20 @@ static int file_callback(const struct option *opt, const char *arg, int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	int from_stdin;
+	const char *filename = arg;
 	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");
+	if (!*filename)
+		; /* leave it as-is */
+	else
+		filename = prefix_filename_except_for_dash(grep_prefix, filename);
+
+	from_stdin = !strcmp(filename, "-");
+	patterns = from_stdin ? stdin : fopen(filename, "r");
 	if (!patterns)
 		die_errno(_("cannot open '%s'"), arg);
 	while (strbuf_getline(&sb, patterns) == 0) {
@@ -833,6 +840,8 @@ static int file_callback(const struct option *opt, const char *arg, int unset)
 	if (!from_stdin)
 		fclose(patterns);
 	strbuf_release(&sb);
+	if (filename != arg)
+		free((void *)filename);
 	return 0;
 }
 
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 39d6d713ec..91ac66935f 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -808,6 +808,19 @@ test_expect_success 'grep -f, ignore empty lines, read patterns from stdin' '
 	test_cmp expected actual
 '
 
+test_expect_success 'grep -f, use cwd relative file' '
+	test_when_finished "git rm -f sub/dir/file" &&
+	mkdir -p sub/dir &&
+	echo hit >sub/dir/file &&
+	git add sub/dir/file &&
+	echo hit >sub/dir/pattern &&
+	echo miss >pattern &&
+	(
+		cd sub/dir && git grep -f pattern file
+	) &&
+	git -C sub/dir grep -f pattern file
+'
+
 cat >expected <<EOF
 y:y yy
 --
-- 
2.42.0-345-gaab89be2eb
Taylor Blau· Oct 30, 2023, 21:58 UTC · re: Junio C Hamano · lore

Re: Bug: Git grep -f reads the filename relative to the repository root

On Thu, Oct 12, 2023 at 10:28:16AM -0700, Junio C Hamano wrote:
Show 20 quoted lines
> Erik Cervin Edin <erik@cervined.in> writes:
>
> > In the Git repository, I ran
> >
> >     echo tig > pattern-file &&
> >         echo git > xdiff/pattern-file &&
> >         cd xdfiff &&
> >         git grep -f pattern-file
> >
> > What did you expect to happen? (Expected behavior)
> >
> > Git grep -f to read the pattern-file, in the xdiff directory and
> > search for lines matching `git` in the xdiff directory.
>
> That does sound like a bug.  It should use the original directory as
> the base of the relative path computation, similar to the way how
> OPT_FILENAME() options are handled.
>
> Perhaps something along this line, but this is not even compile
> tested yet.

Just going through old mail that I didn't have a chance to respond to, the proposed patch that you included here does compile and pass t7810 for me, and the fix looks reasonable as-is. I don't think I see this patch on master, but would have no objections to you merging it down.

Thanks, Taylor

Junio C Hamano· Oct 31, 2023, 00:49 UTC · re: Taylor Blau · lore

Re: Bug: Git grep -f reads the filename relative to the repository root

Taylor Blau <me@ttaylorr.com> writes:
> Just going through old mail that I didn't have a chance to respond to,
> the proposed patch that you included here does compile and pass t7810
> for me, and the fix looks reasonable as-is. I don't think I see this
> patch on master, but would have no objections to you merging it down.
Thanks.

← back to recent threads