On Wed, Sep 12, 2018 at 11:36 AM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Stefan Beller <sbeller@xxxxxxxxxx> 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? */) > > - 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.