Re: [PATCH v2 05/13] submodule-config: stop registering submodule sources

[Justin Tobler <jltobler@gmail.com>]

On 26/09/02 03:34PM, Patrick Steinhardt wrote:
> When reading the ".gitmodules" file from a blob in a repository other
> than `the_repository`, we register the repository's object database as
> an in-memory source of `the_repository`'s object database. This call has
> its origins in d9b8b8f896 (submodule-config.c: use repo_get_oid for
> reading .gitmodules, 2019-04-16): back then, `config_with_options()` was
> not able to read a blob from an arbitrary repository, but would always
> read it via `the_repository`. So even though the blob could be resolved
> in the submodule repository via `repo_get_oid()`, the submodule's object
> database had to be registered as an in-memory source of `the_repository`
> so that the subsequent object read was able to find the blob at all.

Ok, so IIUC the problem was that a gitmodule blob from a repository that
is not "the_repository" would fail to be read by `config_with_options()`
because it would only read objects from "the_repository". The workaround
was to add the other repositories as another source to ensure the
gitmodule blob could be read. I had to reread the commit message a
couple of times to follow, but I think it makes sense now.

> That need went away with e3e8bf046e (submodule-config: pass repo
> upon blob config read, 2021-08-16), which taught the config machinery
> to read the blob from the repository we pass to it. The same series
> converted the eager submodule source registration into a lazy mechanism
> that only registers submodule sources with the object database when an
> object lookup failed. The intent though was that we don't ever have to
> fall back to this mechanism in the first place, and to verify that this
> is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set,
> then any such lazy registration would cause us to BUG.
> 
> At the beginning of this series, we still triggered this bug in t1092.
> But now that we have converted the "cache-tree" subsystem to not depend
> on `the_repository` anymore it also knows to properly access objects via
> the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB
> does not cause any failures anymore.

Ok, now we can properly read objects from their respective repository
source and no longer neeed the workaround.

> Remove the call to `odb_add_submodule_source_by_path()`. This removes
> the last user of `the_repository`, so at the same time we can also get
> rid of `USE_THE_REPOSITORY_VARIABLE`.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  submodule-config.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/submodule-config.c b/submodule-config.c
> index 7c73fa108b..37c3be377b 100644
> --- a/submodule-config.c
> +++ b/submodule-config.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
>  #define DISABLE_SIGN_COMPARE_WARNINGS
>  
>  #include "git-compat-util.h"
> @@ -803,9 +802,6 @@ static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void
>  		} else if (repo_get_oid(repo, GITMODULES_INDEX, &oid) >= 0 ||
>  			   repo_get_oid(repo, GITMODULES_HEAD, &oid) >= 0) {
>  			config_source.blob = oidstr = xstrdup(oid_to_hex(&oid));
> -			if (repo != the_repository)
> -				odb_add_submodule_source_by_path(the_repository->objects,
> -								 repo->objects->sources->path);

Nice to have one less workaround. :)

-Justin