grep: add repository to OID grep sources

jonathantanmy · gitster · commit 0693806bf82f · 2021-09-08T11:48:05.000-07:00
Record the repository whenever an OID grep source is created, and teach
the worker threads to explicitly provide the repository when accessing
objects.

Signed-off-by: Jonathan Tan &lt;jonathantanmy@google.com&gt;
Reviewed-by: Matheus Tavares &lt;matheus.bernardino@usp.br&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/builtin/grep.c b/builtin/grep.c
@@ -349,7 +349,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
 	struct grep_source gs;
 
 	grep_source_name(opt, filename, tree_name_len, &pathbuf);
-	grep_source_init_oid(&gs, pathbuf.buf, path, oid);
+	grep_source_init_oid(&gs, pathbuf.buf, path, oid, opt->repo);
 	strbuf_release(&pathbuf);
 
 	if (num_threads > 1) {
@@ -462,14 +462,11 @@ static int grep_submodule(struct grep_opt *opt,
 	repo_read_gitmodules(subrepo, 0);
 
 	/*
-	 * NEEDSWORK: This adds the submodule's object directory to the list of
-	 * alternates for the single in-memory object store.  This has some bad
-	 * consequences for memory (processed objects will never be freed) and
-	 * performance (this increases the number of pack files git has to pay
-	 * attention to, to the sum of the number of pack files in all the
-	 * repositories processed so far).  This can be removed once the object
-	 * store is no longer global and instead is a member of the repository
-	 * object.
+	 * All code paths tested by test code no longer need submodule ODBs to
+	 * be added as alternates, but add it to the list just in case.
+	 * Submodule ODBs added through add_submodule_odb_by_path() will be
+	 * lazily registered as alternates when needed (and except in an
+	 * unexpected code interaction, it won't be needed).
 	 */
 	add_submodule_odb_by_path(subrepo->objects->odb->path);
 	obj_read_unlock();
diff --git a/grep.c b/grep.c
@@ -1863,7 +1863,8 @@ void grep_source_init_file(struct grep_source *gs, const char *name,
 }
 
 void grep_source_init_oid(struct grep_source *gs, const char *name,
-			  const char *path, const struct object_id *oid)
+			  const char *path, const struct object_id *oid,
+			  struct repository *repo)
 {
 	gs->type = GREP_SOURCE_OID;
 	gs->name = xstrdup_or_null(name);
@@ -1872,6 +1873,7 @@ void grep_source_init_oid(struct grep_source *gs, const char *name,
 	gs->size = 0;
 	gs->driver = NULL;
 	gs->identifier = oiddup(oid);
+	gs->repo = repo;
 }
 
 void grep_source_clear(struct grep_source *gs)
@@ -1900,7 +1902,8 @@ static int grep_source_load_oid(struct grep_source *gs)
 {
 	enum object_type type;
 
-	gs->buf = read_object_file(gs->identifier, &type, &gs->size);
+	gs->buf = repo_read_object_file(gs->repo, gs->identifier, &type,
+					&gs->size);
 	if (!gs->buf)
 		return error(_("'%s': unable to read %s"),
 			     gs->name,
diff --git a/grep.h b/grep.h
@@ -120,7 +120,20 @@ struct grep_opt {
 	struct grep_pat *header_list;
 	struct grep_pat **header_tail;
 	struct grep_expr *pattern_expression;
+
+	/*
+	 * NEEDSWORK: See if we can remove this field, because the repository
+	 * should probably be per-source. That is, grep.c functions using this
+	 * field should probably start using "repo" in "struct grep_source"
+	 * instead.
+	 *
+	 * This is potentially the cause of at least one bug - "git grep"
+	 * ignoring the textconv attributes from submodules. See [1] for more
+	 * information.
+	 * [1] https://lore.kernel.org/git/CAHd-oW5iEQarYVxEXoTG-ua2zdoybTrSjCBKtO0YT292fm0NQQ@mail.gmail.com/
+	 */
 	struct repository *repo;
+
 	const char *prefix;
 	int prefix_length;
 	regex_t regexp;
@@ -187,6 +200,7 @@ struct grep_source {
 		GREP_SOURCE_BUF,
 	} type;
 	void *identifier;
+	struct repository *repo; /* if GREP_SOURCE_OID */
 
 	char *buf;
 	unsigned long size;
@@ -198,7 +212,8 @@ struct grep_source {
 void grep_source_init_file(struct grep_source *gs, const char *name,
 			   const char *path);
 void grep_source_init_oid(struct grep_source *gs, const char *name,
-			  const char *path, const struct object_id *oid);
+			  const char *path, const struct object_id *oid,
+			  struct repository *repo);
 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,

Original file line number	Diff line number	Diff line change
`@@ -1863,7 +1863,8 @@ void grep_source_init_file(struct grep_source gs, const char name,`
`1863`	`1863`	`}`
`1864`	`1864`
`1865`	`1865`	`void grep_source_init_oid(struct grep_source gs, const char name,`
`1866`		`- const char path, const struct object_id oid)`
	`1866`	`+ const char path, const struct object_id oid,`
	`1867`	`+ struct repository *repo)`
`1867`	`1868`	`{`
`1868`	`1869`	`gs->type = GREP_SOURCE_OID;`
`1869`	`1870`	`gs->name = xstrdup_or_null(name);`
`@@ -1872,6 +1873,7 @@ void grep_source_init_oid(struct grep_source gs, const char name,`
`1872`	`1873`	`gs->size = 0;`
`1873`	`1874`	`gs->driver = NULL;`
`1874`	`1875`	`gs->identifier = oiddup(oid);`
	`1876`	`+ gs->repo = repo;`
`1875`	`1877`	`}`
`1876`	`1878`
`1877`	`1879`	`void grep_source_clear(struct grep_source *gs)`
`@@ -1900,7 +1902,8 @@ static int grep_source_load_oid(struct grep_source *gs)`
`1900`	`1902`	`{`
`1901`	`1903`	`enum object_type type;`
`1902`	`1904`
`1903`		`- gs->buf = read_object_file(gs->identifier, &type, &gs->size);`
	`1905`	`+ gs->buf = repo_read_object_file(gs->repo, gs->identifier, &type,`
	`1906`	`+ &gs->size);`
`1904`	`1907`	`if (!gs->buf)`
`1905`	`1908`	`return error(_("'%s': unable to read %s"),`
`1906`	`1909`	`gs->name,`