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

[PATCH v3] grep: stop looking at random places for .gitattributes

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Oct 12, 2012, 10:49 UTC
Message-ID
<1350038978-26153-1-git-send-email-pclouds@gmail.com>
In-Reply-To
<1349877544-17648-3-git-send-email-pclouds@gmail.com>

grep searches for .gitattributes using "name" field in struct grep_source but that field is not real on-disk path name. For example, "grep pattern rev" fills the field with "rev:path", and Git looks for .gitattributes in the (non-existent but exploitable) path "rev:path" instead of "path".

This patch passes real paths down to grep_source_load_driver() when:
 - grep on work tree
 - grep on the index
 - grep a commit (or a tag if it points to a commit)

so that these cases look up .gitattributes at proper paths. .gitattributes lookup is disabled in all other cases.

Initial-work-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 This fixes t7008 as Johannes commented and makes "git grep foo HEAD:t"
 not look up worktree's .gitattributes (in fact it does not look up
 anywhere).
 The quote_path_relative() patch is not required, it's an independent
 design issue.
 builtin/grep.c         | 31 ++++++++++++++++++-------------
 grep.c                 | 11 ++++++++---
 grep.h                 |  4 +++-
 t/t7008-grep-binary.sh | 22 ++++++++++++++++++++++
 4 files changed, 51 insertions(+), 17 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 82530a6..38a17eb 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -86,7 +86,7 @@ static pthread_cond_t cond_result;
 static int skip_first_line;
 
 static void add_work(struct grep_opt *opt, enum grep_source_type type,
-		     const char *name, const void *id)
+		     const char *name, const char *path, const void *id)
 {
 	grep_lock();
 
@@ -94,7 +94,7 @@ static void add_work(struct grep_opt *opt, enum grep_source_type type,
 		pthread_cond_wait(&cond_write, &grep_mutex);
 	}
 
-	grep_source_init(&todo[todo_end].source, type, name, id);
+	grep_source_init(&todo[todo_end].source, type, name, path, id);
 	if (opt->binary != GREP_BINARY_TEXT)
 		grep_source_load_driver(&todo[todo_end].source);
 	todo[todo_end].done = 0;
@@ -371,7 +371,8 @@ static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type
 }
 
 static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
-		     const char *filename, int tree_name_len)
+		     const char *filename, int tree_name_len,
+		     const char *path)
 {
 	struct strbuf pathbuf = STRBUF_INIT;
 
@@ -385,7 +386,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 
 #ifndef NO_PTHREADS
 	if (use_threads) {
-		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, sha1);
+		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
 		strbuf_release(&pathbuf);
 		return 0;
 	} else
@@ -394,7 +395,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 		struct grep_source gs;
 		int hit;
 
-		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, sha1);
+		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
 		strbuf_release(&pathbuf);
 		hit = grep_source(opt, &gs);
 
@@ -414,7 +415,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 
 #ifndef NO_PTHREADS
 	if (use_threads) {
-		add_work(opt, GREP_SOURCE_FILE, buf.buf, filename);
+		add_work(opt, GREP_SOURCE_FILE, buf.buf, filename, filename);
 		strbuf_release(&buf);
 		return 0;
 	} else
@@ -423,7 +424,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 		struct grep_source gs;
 		int hit;
 
-		grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename);
+		grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);
 		strbuf_release(&buf);
 		hit = grep_source(opt, &gs);
 
@@ -479,7 +480,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
 		if (cached || (ce->ce_flags & CE_VALID) || ce_skip_worktree(ce)) {
 			if (ce_stage(ce))
 				continue;
-			hit |= grep_sha1(opt, ce->sha1, ce->name, 0);
+			hit |= grep_sha1(opt, ce->sha1, ce->name, 0, ce->name);
 		}
 		else
 			hit |= grep_file(opt, ce->name);
@@ -497,7 +498,8 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
 }
 
 static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
-		     struct tree_desc *tree, struct strbuf *base, int tn_len)
+		     struct tree_desc *tree, struct strbuf *base, int tn_len,
+		     int check_attr)
 {
 	int hit = 0;
 	enum interesting match = entry_not_interesting;
@@ -518,7 +520,8 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 		strbuf_add(base, entry.path, te_len);
 
 		if (S_ISREG(entry.mode)) {
-			hit |= grep_sha1(opt, entry.sha1, base->buf, tn_len);
+			hit |= grep_sha1(opt, entry.sha1, base->buf, tn_len,
+					 check_attr ? base->buf + tn_len : NULL);
 		}
 		else if (S_ISDIR(entry.mode)) {
 			enum object_type type;
@@ -533,7 +536,8 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 
 			strbuf_addch(base, '/');
 			init_tree_desc(&sub, data, size);
-			hit |= grep_tree(opt, pathspec, &sub, base, tn_len);
+			hit |= grep_tree(opt, pathspec, &sub, base, tn_len,
+					 check_attr);
 			free(data);
 		}
 		strbuf_setlen(base, old_baselen);
@@ -548,7 +552,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 		       struct object *obj, const char *name)
 {
 	if (obj->type == OBJ_BLOB)
-		return grep_sha1(opt, obj->sha1, name, 0);
+		return grep_sha1(opt, obj->sha1, name, 0, NULL);
 	if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
 		struct tree_desc tree;
 		void *data;
@@ -571,7 +575,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 			strbuf_addch(&base, ':');
 		}
 		init_tree_desc(&tree, data, size);
-		hit = grep_tree(opt, pathspec, &tree, &base, base.len);
+		hit = grep_tree(opt, pathspec, &tree, &base, base.len,
+				obj->type == OBJ_COMMIT);
 		strbuf_release(&base);
 		free(data);
 		return hit;
diff --git a/grep.c b/grep.c
index edc7776..e36c01b 100644
--- a/grep.c
+++ b/grep.c
@@ -1373,7 +1373,7 @@ int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
 	struct grep_source gs;
 	int r;
 
-	grep_source_init(&gs, GREP_SOURCE_BUF, NULL, NULL);
+	grep_source_init(&gs, GREP_SOURCE_BUF, NULL, NULL, NULL);
 	gs.buf = buf;
 	gs.size = size;
 
@@ -1384,10 +1384,12 @@ int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
 }
 
 void grep_source_init(struct grep_source *gs, enum grep_source_type type,
-		      const char *name, const void *identifier)
+		      const char *name, const char *path,
+		      const void *identifier)
 {
 	gs->type = type;
 	gs->name = name ? xstrdup(name) : NULL;
+	gs->path = path ? xstrdup(path) : NULL;
 	gs->buf = NULL;
 	gs->size = 0;
 	gs->driver = NULL;
@@ -1409,6 +1411,8 @@ void grep_source_clear(struct grep_source *gs)
 {
 	free(gs->name);
 	gs->name = NULL;
+	free(gs->path);
+	gs->path = NULL;
 	free(gs->identifier);
 	gs->identifier = NULL;
 	grep_source_clear_data(gs);
@@ -1501,7 +1505,8 @@ void grep_source_load_driver(struct grep_source *gs)
 		return;
 
 	grep_attr_lock();
-	gs->driver = userdiff_find_by_path(gs->name);
+	if (gs->path)
+		gs->driver = userdiff_find_by_path(gs->path);
 	if (!gs->driver)
 		gs->driver = userdiff_find_by_name("default");
 	grep_attr_unlock();
diff --git a/grep.h b/grep.h
index c256ac6..c2cf57b 100644
--- a/grep.h
+++ b/grep.h
@@ -158,11 +158,13 @@ struct grep_source {
 	char *buf;
 	unsigned long size;
 
+	char *path; /* for attribute lookups */
 	struct userdiff_driver *driver;
 };
 
 void grep_source_init(struct grep_source *gs, enum grep_source_type type,
-		      const char *name, const void *identifier);
+		      const char *name, const char *path,
+		      const void *identifier);
 void grep_source_clear_data(struct grep_source *gs);
 void grep_source_clear(struct grep_source *gs);
 void grep_source_load_driver(struct grep_source *gs);
diff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh
index fd6410f..26f8319 100755
--- a/t/t7008-grep-binary.sh
+++ b/t/t7008-grep-binary.sh
@@ -111,6 +111,28 @@ test_expect_success 'grep respects binary diff attribute' '
 	test_cmp expect actual
 '
 
+test_expect_success 'grep --cached respects binary diff attribute' '
+	git grep --cached text t >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'grep --cached respects binary diff attribute (2)' '
+	git add .gitattributes &&
+	rm .gitattributes &&
+	git grep --cached text t >actual &&
+	test_when_finished "git rm --cached .gitattributes" &&
+	test_when_finished "git checkout .gitattributes" &&
+	test_cmp expect actual
+'
+
+test_expect_success 'grep revision respects binary diff attribute' '
+	git commit -m new &&
+	echo "Binary file HEAD:t matches" >expect &&
+	git grep text HEAD -- t >actual &&
+	test_when_finished "git reset HEAD^" &&
+	test_cmp expect actual
+'
+
 test_expect_success 'grep respects not-binary diff attribute' '
 	echo binQary | q_to_nul >b &&
 	git add b &&

-- 
1.7.12.1.406.g6ab07c4
Previous: Junio C HamanoNext: Junio C Hamano
Message 42 of 43 in “'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktree”
  1. Johannes SixtOct 9, 2012
  2. Nguyen Thai Ngoc DuyOct 9, 2012
  3. Nguyen Thai Ngoc DuyOct 9, 2012
  4. Jeff KingOct 9, 2012
  5. Junio C HamanoOct 9, 2012
  6. Nguyen Thai Ngoc DuyOct 10, 2012
  7. Junio C HamanoOct 10, 2012
  8. Nguyen Thai Ngoc DuyOct 10, 2012
  9. Nguyễn Thái Ngọc DuyOct 10, 2012
  10. 1/3 quote: let caller reset buffer for quote_path_relative()Nguyễn Thái Ngọc Duy, Oct 10, 2012
  11. Junio C HamanoOct 10, 2012
  12. Nguyen Thai Ngoc DuyOct 11, 2012
  13. Junio C HamanoOct 11, 2012
  14. 2/3 grep: pass true path name to grep machineryNguyễn Thái Ngọc Duy, Oct 10, 2012
  15. 3/3 grep: stop looking at random places for .gitattributesNguyễn Thái Ngọc Duy, Oct 10, 2012
  16. Johannes SixtOct 10, 2012
  17. Nguyen Thai Ngoc DuyOct 10, 2012
  18. Johannes SixtOct 10, 2012
  19. Nguyen Thai Ngoc DuyOct 10, 2012
  20. Johannes SixtOct 10, 2012
  21. Nguyen Thai Ngoc DuyOct 10, 2012
  22. Junio C HamanoOct 10, 2012
  23. Johannes SixtOct 11, 2012
  24. Michael HaggertyOct 11, 2012
  25. Nguyen Thai Ngoc DuyOct 11, 2012
  26. 0/2 Re: 'git grep needle rev' attempts to access 'rev:.../.gitattributes' in the worktreeNguyễn Thái Ngọc Duy, Oct 10, 2012
  27. 1/2 quote: let caller reset buffer for quote_path_relative()Nguyễn Thái Ngọc Duy, Oct 10, 2012
  28. 2/2 grep: stop looking at random places for .gitattributesNguyễn Thái Ngọc Duy, Oct 10, 2012
  29. Johannes SixtOct 10, 2012
  30. Junio C HamanoOct 10, 2012
  31. Johannes SixtOct 11, 2012
  32. Junio C HamanoOct 11, 2012
  33. Johannes SixtOct 12, 2012
  34. Junio C HamanoOct 14, 2012
  35. Johannes SixtOct 15, 2012
  36. Junio C HamanoOct 15, 2012
  37. Johannes SixtOct 16, 2012
  38. Johannes SixtOct 17, 2012
  39. Junio C HamanoOct 17, 2012
  40. Nguyen Thai Ngoc DuyOct 11, 2012
  41. Junio C HamanoOct 11, 2012
  42. grep: stop looking at random places for .gitattributesNguyễn Thái Ngọc Duy, Oct 12, 2012
  43. Junio C HamanoOct 12, 2012

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.