Re: [PATCH 06/10] repository: repo_submodule_init to take a submodule struct
From: Jonathan Tan <hidden>
Date: 2018-10-26 19:16:00
quoted hunk ↗ jump to hunk
diff --git a/builtin/grep.c b/builtin/grep.c index 7da8fef31a..ba7634258a 100644 --- a/builtin/grep.c +++ b/builtin/grep.c@@ -418,7 +418,10 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject, const struct object_id *oid, const char *filename, const char *path) { - struct repository submodule; + struct repository subrepo; + const struct submodule *sub = submodule_from_path(superproject, + &null_oid, path);
[snip]
- if (repo_submodule_init(&submodule, superproject, path)) {
+ if (repo_submodule_init(&subrepo, superproject, sub)) {The last argument to repo_submodule_init is now "submodule_from_path(superproject, &null_oid, path)" instead of "path", and looking forward into the patch, we do not need a NULL check because repo_submodule_init() tolerates NULL in that argument. Let's see if the rest of the code follows the pattern - a call to submodule_from_path() with the 3 expected arguments (repo, null OID, path).
quoted hunk ↗ jump to hunk
diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 7f9919a362..4d1649c1b3 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c@@ -206,17 +206,19 @@ static void show_files(struct repository *repo, struct dir_struct *dir); static void show_submodule(struct repository *superproject, struct dir_struct *dir, const char *path) { - struct repository submodule; + struct repository subrepo; + const struct submodule *sub = submodule_from_path(superproject, + &null_oid, path); - if (repo_submodule_init(&submodule, superproject, path)) + if (repo_submodule_init(&subrepo, superproject, sub))
So far so good.
quoted hunk ↗ jump to hunk
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 5f8a804a6e..015aa1471f 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c@@ -2037,7 +2037,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix) if (!sub) BUG("We could get the submodule handle before?"); - if (repo_submodule_init(&subrepo, the_repository, path)) + if (repo_submodule_init(&subrepo, the_repository, sub))
The definition of "sub" is not quoted here in this e-mail, but it is
indeed "submodule_from_path(the_repository, &null_oid, path)".
("the_repository" in the invocation to submodule_from_path() is correct
because the 2nd argument to the invocation of repo_submodule_init() is
"the_repository".)
-int repo_submodule_init(struct repository *submodule,
+int repo_submodule_init(struct repository *subrepo,
struct repository *superproject,
- const char *path)
+ const struct submodule *sub)
{
- const struct submodule *sub;
struct strbuf gitdir = STRBUF_INIT;
struct strbuf worktree = STRBUF_INIT;
int ret = 0;
- sub = submodule_from_path(superproject, &null_oid, path);As expected, this line is removed.
if (!sub) {
ret = -1;
goto out;
}
- strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", path);
- strbuf_repo_worktree_path(&worktree, superproject, "%s", path);
+ strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", sub->path);
+ strbuf_repo_worktree_path(&worktree, superproject, "%s", sub->path);path and sub->path are the same, so this is fine. (This can be seen from cache_put_path() and cache_lookup_path() in submodule-config.c.)
- submodule->submodule_prefix = xstrfmt("%s%s/",
- superproject->submodule_prefix ?
- superproject->submodule_prefix :
- "", path);
+ subrepo->submodule_prefix = xstrfmt("%s%s/",
+ superproject->submodule_prefix ?
+ superproject->submodule_prefix :
+ "", sub->path);Likewise.
+/* + * Initialize the repository 'subrepo' as the submodule given by the + * struct submodule 'sub' in parent repository 'superproject'. + * Return 0 upon success and a non-zero value upon failure, which may happen + * if the submodule is not found, or 'sub' is NULL. + */ +struct submodule; +int repo_submodule_init(struct repository *subrepo, struct repository *superproject, - const char *path); + const struct submodule *sub);
Here is where it says that the last argument can be NULL.
quoted hunk ↗ jump to hunk
diff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c index a31e2a9bea..bc97929bbc 100644 --- a/t/helper/test-submodule-nested-repo-config.c +++ b/t/helper/test-submodule-nested-repo-config.c@@ -10,19 +10,21 @@ static void die_usage(int argc, const char **argv, const char *msg) int cmd__submodule_nested_repo_config(int argc, const char **argv) { - struct repository submodule; + struct repository subrepo; + const struct submodule *sub; if (argc < 3) die_usage(argc, argv, "Wrong number of arguments."); setup_git_directory(); - if (repo_submodule_init(&submodule, the_repository, argv[1])) { + sub = submodule_from_path(the_repository, &null_oid, argv[1]); + if (repo_submodule_init(&subrepo, the_repository, sub)) {
The expected pattern. This patch looks good to me.