odb: get rid of `the_repository` when handling submodule sources

The "--recursive" flag for git-grep(1) allows users to grep for a string
across submodule boundaries. To make this work we add each submodule's
object sources to our own object database so that the objects can be
accessed directly.

The infrastructure for this depends on a global string list of submodule
paths. The caller is expected to call `add_submodule_odb_by_path()` for
each source and the object database will then eventually register all
submodule sources via `do_oid_object_info_extended()` in case it isn't
able to look up a specific object.

This reliance on global state is of course suboptimal with regards to
our libification efforts.

Refactor the logic so that the list of submodule sources is instead
tracked in the object database itself. This allows us to lose the
condition of `r == the_repository` before registering submodule sources
as we only ever add submodule sources to `the_repository` anyway. As
such, behaviour before and after this refactoring should always be the
same.

Rename the functions accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/grep.c b/builtin/grep.c
index 336cfcab6fbce8bc77c52b1674de07c6c39b2ce4..cfcf916bce1e3e1e025d97fa03aca6888f8390d3 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -505,7 +505,8 @@ static int grep_submodule(struct grep_opt *opt,
         * 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->sources->path);
+       odb_add_submodule_source_by_path(the_repository->objects,
+                                        subrepo->objects->sources->path);
        obj_read_unlock();
 
        memcpy(&subopt, opt, sizeof(subopt));

diff --git a/odb.c b/odb.c
index b154e91953d4a0c49a260b52e881aa5c6f38db8f..decf261dec6625e23c55235262186b177ff3d2f5 100644 (file)
--- a/odb.c
+++ b/odb.c
@@ -24,6 +24,7 @@
 #include "strbuf.h"
 #include "strvec.h"
 #include "submodule.h"
+#include "trace2.h"
 #include "write-or-die.h"
 
 KHASH_INIT(odb_path_map, const char * /* key: odb_path */,
@@ -469,6 +470,12 @@ struct odb_source *odb_find_source(struct object_database *odb, const char *obj_
        return source;
 }
 
+void odb_add_submodule_source_by_path(struct object_database *odb,
+                                     const char *path)
+{
+       string_list_insert(&odb->submodule_source_paths, path);
+}
+
 static void fill_alternate_refs_command(struct child_process *cmd,
                                        const char *repo_path)
 {
@@ -623,6 +630,23 @@ void disable_obj_read_lock(void)
 
 int fetch_if_missing = 1;
 
+static int register_all_submodule_sources(struct object_database *odb)
+{
+       int ret = odb->submodule_source_paths.nr;
+
+       for (size_t i = 0; i < odb->submodule_source_paths.nr; i++)
+               odb_add_to_alternates_memory(odb,
+                                            odb->submodule_source_paths.items[i].string);
+       if (ret) {
+               string_list_clear(&odb->submodule_source_paths, 0);
+               trace2_data_intmax("submodule", odb->repo,
+                                  "register_all_submodule_sources/registered", ret);
+               if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0))
+                       BUG("register_all_submodule_sources() called");
+       }
+       return ret;
+}
+
 static int do_oid_object_info_extended(struct repository *r,
                                       const struct object_id *oid,
                                       struct object_info *oi, unsigned flags)
@@ -676,13 +700,12 @@ static int do_oid_object_info_extended(struct repository *r,
                }
 
                /*
-                * If r is the_repository, this might be an attempt at
-                * accessing a submodule object as if it were in the_repository
-                * (having called add_submodule_odb() on that submodule's ODB).
-                * If any such ODBs exist, register them and try again.
+                * This might be an attempt at accessing a submodule object as
+                * if it were in main object store (having called
+                * `odb_add_submodule_source_by_path()` on that submodule's
+                * ODB). If any such ODBs exist, register them and try again.
                 */
-               if (r == the_repository &&
-                   register_all_submodule_odb_as_alternates())
+               if (register_all_submodule_sources(r->objects))
                        /* We added some alternates; retry */
                        continue;
 
@@ -968,6 +991,7 @@ struct object_database *odb_new(struct repository *repo)
        INIT_LIST_HEAD(&o->packed_git_mru);
        hashmap_init(&o->pack_map, pack_map_entry_cmp, NULL, 0);
        pthread_mutex_init(&o->replace_mutex, NULL);
+       string_list_init_dup(&o->submodule_source_paths);
        return o;
 }
 
@@ -1017,4 +1041,5 @@ void odb_clear(struct object_database *o)
        o->packed_git = NULL;
 
        hashmap_clear(&o->pack_map);
+       string_list_clear(&o->submodule_source_paths, 0);
 }

diff --git a/odb.h b/odb.h
index 4e2d1004f8a5a1e21151dafc07f6f53e9c431768..0ea9d4faa702250992e6a70aac5286cbf326e936 100644 (file)
--- a/odb.h
+++ b/odb.h
@@ -6,6 +6,7 @@
 #include "list.h"
 #include "oidset.h"
 #include "oidmap.h"
+#include "string-list.h"
 #include "thread-utils.h"
 
 struct oidmap;
@@ -165,6 +166,12 @@ struct object_database {
         * packs.
         */
        unsigned packed_git_initialized : 1;
+
+       /*
+        * Submodule source paths that will be added as additional sources to
+        * allow lookup of submodule objects via the main object database.
+        */
+       struct string_list submodule_source_paths;
 };
 
 struct object_database *odb_new(struct repository *repo);
@@ -191,6 +198,14 @@ void odb_restore_primary_source(struct object_database *odb,
                                struct odb_source *restore_source,
                                const char *old_path);
 
+/*
+ * Call odb_add_submodule_source_by_path() to add the submodule at the given
+ * path to a list. The object stores of all submodules in that list will be
+ * added as additional sources in the object store when looking up objects.
+ */
+void odb_add_submodule_source_by_path(struct object_database *odb,
+                                     const char *path);
+
 /*
  * Iterate through all alternates of the database and execute the provided
  * callback function for each of them. Stop iterating once the callback

diff --git a/submodule-config.c b/submodule-config.c
index 9c80f9f7b666086c2e20b208932f38158811d6c8..a9f721078889ebe2a950ab6605028fa3ada9f0c6 100644 (file)
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -810,7 +810,8 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void
                           repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) {
                        config_source.blob = oidstr = xstrdup(oid_to_hex(&oid));
                        if (repo != the_repository)
-                               add_submodule_odb_by_path(repo->objects->sources->path);
+                               odb_add_submodule_source_by_path(the_repository->objects,
+                                                                repo->objects->sources->path);
                } else {
                        goto out;
                }

diff --git a/submodule.c b/submodule.c
index 386be2342307be1073818d21c2e988d8ef3b9554..788c9e55ed36061dbe525d3dd92d106d774074fd 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -31,7 +31,6 @@
 #include "commit-reach.h"
 #include "read-cache-ll.h"
 #include "setup.h"
-#include "trace2.h"
 
 static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
 static int initialized_fetch_ref_tips;
@@ -176,31 +175,6 @@ void stage_updated_gitmodules(struct index_state *istate)
                die(_("staging updated .gitmodules failed"));
 }
 
-static struct string_list added_submodule_odb_paths = STRING_LIST_INIT_DUP;
-
-void add_submodule_odb_by_path(const char *path)
-{
-       string_list_insert(&added_submodule_odb_paths, path);
-}
-
-int register_all_submodule_odb_as_alternates(void)
-{
-       int i;
-       int ret = added_submodule_odb_paths.nr;
-
-       for (i = 0; i < added_submodule_odb_paths.nr; i++)
-               odb_add_to_alternates_memory(the_repository->objects,
-                                            added_submodule_odb_paths.items[i].string);
-       if (ret) {
-               string_list_clear(&added_submodule_odb_paths, 0);
-               trace2_data_intmax("submodule", the_repository,
-                                  "register_all_submodule_odb_as_alternates/registered", ret);
-               if (git_env_bool("GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB", 0))
-                       BUG("register_all_submodule_odb_as_alternates() called");
-       }
-       return ret;
-}
-
 void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
                                             const char *path)
 {

diff --git a/submodule.h b/submodule.h
index db980c1d083bc0e4c3bd3c94eb3bf19ae4d711ff..b10e16e6c063d2ada14094ba6a6d271455a814e7 100644 (file)
--- a/submodule.h
+++ b/submodule.h
@@ -104,15 +104,6 @@ int submodule_uses_gitfile(const char *path);
 #define SUBMODULE_REMOVAL_IGNORE_IGNORED_UNTRACKED (1<<2)
 int bad_to_remove_submodule(const char *path, unsigned flags);
 
-/*
- * Call add_submodule_odb_by_path() to add the submodule at the given
- * path to a list. When register_all_submodule_odb_as_alternates() is
- * called, the object stores of all submodules in that list will be
- * added as alternates in the_repository.
- */
-void add_submodule_odb_by_path(const char *path);
-int register_all_submodule_odb_as_alternates(void);
-
 /*
  * Checks if there are submodule changes in a..b. If a is the null OID,
  * checks b and all its ancestors instead.