add_submodule_odb() is a hack - it adds a submodule's odb as an
alternate, allowing the submodule's objects to be read via
the_repository. Its last caller is submodule_has_commits(), which calls
add_submodule_odb() to prepare for check_has_commit(). This used to be
necessary because check_has_commit() used the_repository's odb, but this
is longer true as of 13a2f620b2 (submodule: pass repo to
check_has_commit(), 2021-10-08).
Removing add_submodule_odb() reveals a bug in check_has_commit(), where
check_has_commit() will segfault if the submodule is missing (e.g. the
user has not init-ed the submodule). This happens because the
submodule's struct repository cannot be initialized, but
check_has_commit() tries to cleanup the uninitialized struct anyway.
This was masked by add_submodule_odb(), because add_submodule_odb()
fails when the submodule is missing, causing the caller to return early
and avoid calling check_has_commit().
Fix the bug and remove the call to add_submodule_odb(). Since
add_submodule_odb() has no more callers, remove it too.
Note that submodule odbs can still by added as alternates via
add_submodule_odb_by_path().
Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx>
---
This bug only exists because we can't call repo_clear() twice on the
same struct repository. So instead of just fixing this site, an
alternative (and maybe better) fix would be to fix repo_clear(). If
others think that's a good idea, I'll do that instead.
submodule.c | 35 ++---------------------------------
submodule.h | 9 ++++-----
2 files changed, 6 insertions(+), 38 deletions(-)
diff --git a/submodule.c b/submodule.c
index 0c02bbc9c3..fdfddd3aac 100644
--- a/submodule.c
+++ b/submodule.c
@@ -168,26 +168,6 @@ void stage_updated_gitmodules(struct index_state *istate)
static struct string_list added_submodule_odb_paths = STRING_LIST_INIT_NODUP;
-/* TODO: remove this function, use repo_submodule_init instead. */
-int add_submodule_odb(const char *path)
-{
- struct strbuf objects_directory = STRBUF_INIT;
- int ret = 0;
-
- ret = strbuf_git_path_submodule(&objects_directory, path, "objects/");
- if (ret)
- goto done;
- if (!is_directory(objects_directory.buf)) {
- ret = -1;
- goto done;
- }
- string_list_insert(&added_submodule_odb_paths,
- strbuf_detach(&objects_directory, NULL));
-done:
- strbuf_release(&objects_directory);
- return ret;
-}
-
void add_submodule_odb_by_path(const char *path)
{
string_list_insert(&added_submodule_odb_paths, xstrdup(path));
@@ -972,7 +952,8 @@ static int check_has_commit(const struct object_id *oid, void *data)
if (repo_submodule_init(&subrepo, cb->repo, cb->path, cb->super_oid)) {
cb->result = 0;
- goto cleanup;
+ /* subrepo failed to init, so don't clean it up. */
+ return 0;
}
type = oid_object_info(&subrepo, oid, NULL);
@@ -1003,18 +984,6 @@ static int submodule_has_commits(struct repository *r,
{
struct has_commit_data has_commit = { r, 1, path, super_oid };
- /*
- * Perform a cheap, but incorrect check for the existence of 'commits'.
- * This is done by adding the submodule's object store to the in-core
- * object store, and then querying for each commit's existence. If we
- * do not have the commit object anywhere, there is no chance we have
- * it in the object store of the correct submodule and have it
- * reachable from a ref, so we can fail early without spawning rev-list
- * which is expensive.
- */
- if (add_submodule_odb(path))
- return 0;
-
oid_array_for_each_unique(commits, check_has_commit, &has_commit);
if (has_commit.result) {
diff --git a/submodule.h b/submodule.h
index 784ceffc0e..ca1f12b78b 100644
--- a/submodule.h
+++ b/submodule.h
@@ -103,12 +103,11 @@ int submodule_uses_gitfile(const char *path);
int bad_to_remove_submodule(const char *path, unsigned flags);
/*
- * Call add_submodule_odb() 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.
+ * 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.
*/
-int add_submodule_odb(const char *path);
void add_submodule_odb_by_path(const char *path);
int register_all_submodule_odb_as_alternates(void);
--
2.33.GIT
