Re: [PATCH v2 6/8] grep: add repository to OID grep sources

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 <redacted>
---
 builtin/grep.c | 15 ++++++---------
 grep.c         |  7 +++++--
 grep.h         | 13 ++++++++++++-
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 69d8ea0808..d27e95e092 100644
--- 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
index 8a8105c2eb..79598f245f 100644
--- 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
index 480b3f5bba..24c6b64cea 100644
--- a/grep.h
+++ b/grep.h

@@ -120,7 +120,16 @@ 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, not per-repo.

            [...] 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/ (local)
+        */
        struct repository *repo;
+
        const char *prefix;
        int prefix_length;
        regex_t regexp;

@@ -187,6 +196,7 @@ struct grep_source {
                GREP_SOURCE_BUF,
        } type;
        void *identifier;
+       struct repository *repo; /* if GREP_SOURCE_OID */

        char *buf;
        unsigned long size;

@@ -198,7 +208,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,
--

2.33.0.rc1.237.g0d66db33f3-goog