Re: [PATCH 7/9] submodule: fetch in submodules git directory instead of in worktree
- From
Stefan Beller <sbeller@google.com>
- Date
- Sep 13, 2018, 19:29 UTC
- Message-ID
- <CAGZ79kZKKf9N8yx9EuCRZhrZS_mA2218PouEG7aHDhK2bJGEdA@mail.gmail.com>
- In-Reply-To
- <xmqqlg86lg06.fsf@gitster-ct.c.googlers.com>
On Wed, Sep 12, 2018 at 11:36 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 49 quoted lines
>
> Stefan Beller <sbeller@google.com> writes:
>
> > This patch started as a refactoring to make 'get_next_submodule' more
> > readable, but upon doing so, I realized that git-fetch actually doesn't
> > need to be run in the worktree. So let's run it in the git dir instead.
>
> It may be clear to the author but not clear to the reader of the
> above paragraph that "worktree", "fetch" and "git dir" all refer to
> the recursively invoked operation that updates the submodules
> repository. s/git-fetch/"git fetch" for the submodule/ should be
> sufficient to help the readers.
>
> > That should pave the way towards fetching submodules that are currently
> > not checked out.
>
> Very good.
>
> > +static void prepare_submodule_repo_env_in_gitdir(struct argv_array *out)
> > +{
> > + prepare_submodule_repo_env_no_git_dir(out);
> > + argv_array_pushf(out, "%s=.", GIT_DIR_ENVIRONMENT);
> > +}
> > +
> > /* Helper function to display the submodule header line prior to the full
> > * summary output. If it can locate the submodule objects directory it will
> > * attempt to lookup both the left and right commits and put them into the
> > @@ -1227,6 +1233,27 @@ static int get_fetch_recurse_config(const struct submodule *submodule,
> > return spf->default_option;
> > }
> >
> > +static const char *get_submodule_git_dir(struct repository *r, const char *path)
> > +{
> > + struct repository subrepo;
> > + const char *ret;
> > +
> > + if (repo_submodule_init(&subrepo, r, path)) {
> > + /* no entry in .gitmodules? */
> > + struct strbuf gitdir = STRBUF_INIT;
> > + strbuf_repo_worktree_path(&gitdir, r, "%s/.git", path);
> > + if (repo_init(&subrepo, gitdir.buf, NULL)) {
> > + strbuf_release(&gitdir);
> > + return NULL;
> > + }
>
> This is for the modern "absorbed" layout? Do we get a notice and
> encouragement to migrate from the historical layout, or there is no
> need to (e.g. the migration happens automatically in some other
> codepaths)?No, the absorbed would also be handled by repo_submodule_init. I wrote a patch once to migrate repo_submodule_init to take a "struct *submodule" instead of a path as the third argument, which would fall in line with this patch as well, I'll dig it up.
Historically git-fetch supported repositories that are not submodules (but have a gitlink and a working tree in place) as well. That is covered here. (see comment /* no entry in .gitmodules? */)
Show 5 quoted lines
> > - strbuf_release(&submodule_path); > > - strbuf_release(&submodule_git_dir); > > But if it is a leak, it is easily plugged by freeing git_dir here, I > think.
Thanks.