On 06/21, Jonathan Tan wrote: > On Tue, 20 Jun 2017 12:19:50 -0700 > Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > > Introduce 'repo_submodule_init()' which performs initialization of a > > 'struct repository' as a submodule of another 'struct repository'. > > > > The resulting submodule can be in one of three states: > > > > 1. The submodule is initialized and has a worktree. > > > > 2. The submodule is initialized but does not have a worktree. This > > would occur when the submodule's gitdir is present in the > > superproject's 'gitdir/modules/' directory yet the submodule has not > > been checked out in superproject's worktree. > > In a recent proposal [1] to update the submodule documentation, an > "initialized submodule" is one that has a working directory, which seems > to have a different meaning of "initialized" (to the paragraphs above). > > Or did you mean the "struct repository" is initialized etc.? In which > case, it does not seem strange to me that a repository is initialized > but does not have a worktree, since bare repositories are like that too. Yes "initialization" only refers to the state of the 'struct repository'. > > [1] https://public-inbox.org/git/20170621173756.4444-1-sbeller@xxxxxxxxxx/ > > > 3. The submodule remains uninitialized due to an error in the > > initialization process or there is no matching submodule at the > > provided path in the superproject. > > > > Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx> > > [snip] > > > +/* > > + * Initialize 'submodule' as the submodule given by 'path' in parent repository > > + * 'superproject'. > > + * Return 0 upon success and a non-zero value upon failure. > > + */ > > +int repo_submodule_init(struct repository *submodule, > > + struct repository *superproject, > > + const char *path) > > +{ > > + const struct submodule *sub; > > + struct strbuf submodule_path = STRBUF_INIT; > > + int ret = 0; > > + > > + sub = submodule_from_cache(superproject, null_sha1, path); > > + if (!sub) { > > + ret = -1; > > + goto out; > > + } > > + > > + strbuf_repo_worktree_path(&submodule_path, superproject, "%s", path); > > + > > + if (repo_init(submodule, submodule_path.buf, submodule_path.buf)) { > > This works because the 2nd parameter (git_dir) can take in either the > Git directory itself or its parent, but it does make the call site look > strange. Would it be a good idea to make it mandatory to specify the Git > directory? That would make call sites clearer but require more code > there. Correct, The idea was to make it easy for callers to initialize repositories...but you may have convinced me to change that and require and exact path to the gitdir. That would actually make the repo_init code cleaner too. Originally I was worried that extra boiler plate code would be needed everytime we wanted to init a submodule, but then I introduced this funciton so the extra logic would be contained in this function. > > > + strbuf_reset(&submodule_path); > > + strbuf_repo_git_path(&submodule_path, superproject, > > + "modules/%s", sub->name); > > + > > + if (repo_init(submodule, submodule_path.buf, NULL)) { > > + ret = -1; > > + goto out; > > + } > > + } > > + > > + submodule->submodule_prefix = xstrfmt("%s%s/", > > + superproject->submodule_prefix ? > > + superproject->submodule_prefix : > > + "", path); > > + > > +out: > > + strbuf_release(&submodule_path); > > + return ret; > > +} -- Brandon Williams
