Re: [PATCH v3 19/20] repository: enable initialization of submodules

[Brandon Williams]

On 06/21, Jonathan Tan wrote:
> On Tue, 20 Jun 2017 12:19:50 -0700
> Brandon Williams  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.-1-sbel...@google.com/
> 
> >   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 
> 
> [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(_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(_path);
> > +   strbuf_repo_git_path(_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(_path);
> > +   return ret;
> > +}

-- 
Brandon Williams

Re: [PATCH v3 19/20] repository: enable initialization of submodules

[Jonathan Tan]

On Tue, 20 Jun 2017 12:19:50 -0700
Brandon Williams  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.

[1] https://public-inbox.org/git/20170621173756.-1-sbel...@google.com/

>   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 

[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(_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.

> + strbuf_reset(_path);
> + strbuf_repo_git_path(_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(_path);
> + return ret;
> +}

[PATCH v3 19/20] repository: enable initialization of submodules

[Brandon Williams]

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.

  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 
---
 repository.c | 44 
 repository.h | 10 ++
 2 files changed, 54 insertions(+)

diff --git a/repository.c b/repository.c
index 317041a4a..673b35ca5 100644
--- a/repository.c
+++ b/repository.c
@@ -155,6 +155,48 @@ int repo_init(struct repository *repo, const char *gitdir, 
const char *worktree)
return -1;
 }
 
+/*
+ * 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(_path, superproject, "%s", path);
+
+   if (repo_init(submodule, submodule_path.buf, submodule_path.buf)) {
+   strbuf_reset(_path);
+   strbuf_repo_git_path(_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(_path);
+   return ret;
+}
+
 void repo_clear(struct repository *repo)
 {
free(repo->gitdir);
@@ -169,6 +211,8 @@ void repo_clear(struct repository *repo)
repo->index_file = NULL;
free(repo->worktree);
repo->worktree = NULL;
+   free(repo->submodule_prefix);
+   repo->submodule_prefix = NULL;
 
if (repo->config) {
git_configset_clear(repo->config);
diff --git a/repository.h b/repository.h
index 4bc70ebc5..7b352accb 100644
--- a/repository.h
+++ b/repository.h
@@ -25,6 +25,13 @@ struct repository {
/* Path to the working directory */
char *worktree;
 
+   /*
+* Path from the root of the top-level superproject down to this
+* repository.  This is only non-NULL if the repository is initialized
+* as a submodule of another repository.
+*/
+   char *submodule_prefix;
+
/* Subsystems */
/*
 * Repository's config which contains key-value pairs from the usual
@@ -59,6 +66,9 @@ extern struct repository *the_repository;
 extern void repo_set_gitdir(struct repository *repo, const char *path);
 extern void repo_set_worktree(struct repository *repo, const char *path);
 extern int repo_init(struct repository *repo, const char *gitdir, const char 
*worktree);
+extern int repo_submodule_init(struct repository *submodule,
+  struct repository *superproject,
+  const char *path);
 extern void repo_clear(struct repository *repo);
 
 extern int repo_read_index(struct repository *repo);
-- 
2.13.1.611.g7e3b11ae1-goog