Re: [PATCH v9 2/3] introduce submodule.hasSuperproject record

From: Glen Choo <chooglen@google.com>
Date: Mar 15, 2022, 21:19 UTC
Message-ID: <kl6lbky6ykuo.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <YjD9bgmy9ArubMoG@google.com>

Emily Shaffer <emilyshaffer@google.com> writes:

Show 18 quoted lines

> On Tue, Mar 15, 2022 at 01:48:25PM -0700, Emily Shaffer wrote:
>> > +static void set_hassuperproject(const char *sm_path)
>> > +{
>> > +	struct repository subrepo;
>> > +	char *cfg_file;
>> > +
>> > +	if (repo_submodule_init(&subrepo, the_repository, sm_path, null_oid()))
>> > +		die(_("could not get a repository handle for submodule '%s'"), sm_path);
>> 
>> Isn't the repo_submodule_init() fairly expensive? I think this is doing
>> a whole repo_init() call we would not otherwise be doing.... Is it good
>> enough to generate the config from sm_path, by using
>> strbuf_repo_worktree_path(), and simply be tolerant of the failure if
>> <sm-gitdir>/config doesn't exist?
>
> Ah, I was misreading the implementation of repo_submodule_init() and I
> see now that won't work. I guess it is fine to just invoke
> repo_submodule_init() then, unless someone has another idea.

Yes, it's difficult to avoid calling repo_submodule_init() because it's hard to get the gitdir using just the path to the submodule in the working tree (sm_path).

Are we particular about avoiding calls to repo_submodule_init()? I don't recall hearing this as an objection before. If so, I'll keep this in mind as I work on more submodule things.

As an aside, ensure_core_worktree() already calls repo_submodule_init(), so this wouldn't be the first time "submodule update" calls repo_submodule_init(), and a potential optimization might be to cache the result in between invocations of repo_submodule_init().