From 5df5106e1e8b52ff54c0726ba6919afa4b745980 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 9 Sep 2021 11:47:27 -0700 Subject: submodule: remove unnecessary unabsorbed fallback In get_submodule_repo_for(), there is a fallback code path for the case in which a submodule has an unabsorbed gitdir. (See the documentation for "git submodule absorbgitdirs" for more information about absorbed and unabsorbed gitdirs.) However, this code path is unnecessary, because such submodules are already handled: when the fetch_task is created in fetch_task_create(), it will create its own struct submodule with a path and name, and repo_submodule_init() can handle such a struct. This fallback was introduced in 26f80ccfc1 ("submodule: migrate get_next_submodule to use repository structs", 2018-12-05). It was unnecessary even then, but perhaps it escaped notice because its parent commit d5498e0871 ("repository: repo_submodule_init to take a submodule struct", 2018-12-05) was the one that taught repo_submodule_init() to handle such created structs. Before, it took a path and always checked .gitmodules, so it truly would have failed if there were no entry in .gitmodules. (Note to reviewers: in 26f80ccfc1, the "own struct submodule" I mentioned is in get_next_submodule(), not fetch_task_create().) Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- submodule.c | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) (limited to 'submodule.c') diff --git a/submodule.c b/submodule.c index 8de1aecaeb..3af3da5b5e 100644 --- a/submodule.c +++ b/submodule.c @@ -1409,19 +1409,8 @@ static struct repository *get_submodule_repo_for(struct repository *r, struct repository *ret = xmalloc(sizeof(*ret)); if (repo_submodule_init(ret, r, sub)) { - /* - * No entry in .gitmodules? Technically not a submodule, - * but historically we supported repositories that happen to be - * in-place where a gitlink is. Keep supporting them. - */ - struct strbuf gitdir = STRBUF_INIT; - strbuf_repo_worktree_path(&gitdir, r, "%s/.git", sub->path); - if (repo_init(ret, gitdir.buf, NULL)) { - strbuf_release(&gitdir); - free(ret); - return NULL; - } - strbuf_release(&gitdir); + free(ret); + return NULL; } return ret; -- cgit v1.3 From 8eb8dcf94643ca6e7c3f040f3e0bf96e11c7ae47 Mon Sep 17 00:00:00 2001 From: Jonathan Tan Date: Thu, 9 Sep 2021 11:47:28 -0700 Subject: repository: support unabsorbed in repo_submodule_init In preparation for a subsequent commit that migrates code using add_submodule_odb() to repo_submodule_init(), teach repo_submodule_init() to support submodules with unabsorbed gitdirs. (See the documentation for "git submodule absorbgitdirs" for more information about absorbed and unabsorbed gitdirs.) Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- builtin/grep.c | 5 +---- builtin/ls-files.c | 4 +--- builtin/submodule--helper.c | 7 +------ repository.c | 21 ++++++++++++--------- repository.h | 15 +++++++++------ submodule.c | 9 +++------ t/helper/test-submodule-nested-repo-config.c | 4 +--- 7 files changed, 28 insertions(+), 37 deletions(-) (limited to 'submodule.c') diff --git a/builtin/grep.c b/builtin/grep.c index 51278b01fa..8af5249a7b 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -433,17 +433,14 @@ static int grep_submodule(struct grep_opt *opt, { struct repository *subrepo; struct repository *superproject = opt->repo; - const struct submodule *sub; struct grep_opt subopt; int hit = 0; - sub = submodule_from_path(superproject, null_oid(), path); - if (!is_submodule_active(superproject, path)) return 0; subrepo = xmalloc(sizeof(*subrepo)); - if (repo_submodule_init(subrepo, superproject, sub)) { + if (repo_submodule_init(subrepo, superproject, path, null_oid())) { free(subrepo); return 0; } diff --git a/builtin/ls-files.c b/builtin/ls-files.c index 29a26ad8ae..ec19bf54b2 100644 --- a/builtin/ls-files.c +++ b/builtin/ls-files.c @@ -209,10 +209,8 @@ static void show_submodule(struct repository *superproject, struct dir_struct *dir, const char *path) { struct repository subrepo; - const struct submodule *sub = submodule_from_path(superproject, - null_oid(), path); - if (repo_submodule_init(&subrepo, superproject, sub)) + if (repo_submodule_init(&subrepo, superproject, path, null_oid())) return; if (repo_read_index(&subrepo) < 0) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index ef2776a9e4..6718f202db 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2540,7 +2540,6 @@ static int push_check(int argc, const char **argv, const char *prefix) static int ensure_core_worktree(int argc, const char **argv, const char *prefix) { - const struct submodule *sub; const char *path; const char *cw; struct repository subrepo; @@ -2550,11 +2549,7 @@ static int ensure_core_worktree(int argc, const char **argv, const char *prefix) path = argv[1]; - sub = submodule_from_path(the_repository, null_oid(), path); - if (!sub) - BUG("We could get the submodule handle before?"); - - if (repo_submodule_init(&subrepo, the_repository, sub)) + if (repo_submodule_init(&subrepo, the_repository, path, null_oid())) die(_("could not get a repository handle for submodule '%s'"), path); if (!repo_config_get_string_tmp(&subrepo, "core.worktree", &cw)) { diff --git a/repository.c b/repository.c index b2bf44c6fa..e4a1afb0ac 100644 --- a/repository.c +++ b/repository.c @@ -190,19 +190,15 @@ error: int repo_submodule_init(struct repository *subrepo, struct repository *superproject, - const struct submodule *sub) + const char *path, + const struct object_id *treeish_name) { struct strbuf gitdir = STRBUF_INIT; struct strbuf worktree = STRBUF_INIT; int ret = 0; - if (!sub) { - ret = -1; - goto out; - } - - strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", sub->path); - strbuf_repo_worktree_path(&worktree, superproject, "%s", sub->path); + strbuf_repo_worktree_path(&gitdir, superproject, "%s/.git", path); + strbuf_repo_worktree_path(&worktree, superproject, "%s", path); if (repo_init(subrepo, gitdir.buf, worktree.buf)) { /* @@ -212,6 +208,13 @@ int repo_submodule_init(struct repository *subrepo, * in the superproject's 'modules' directory. In this case the * submodule would not have a worktree. */ + const struct submodule *sub = + submodule_from_path(superproject, treeish_name, path); + if (!sub) { + ret = -1; + goto out; + } + strbuf_reset(&gitdir); strbuf_repo_git_path(&gitdir, superproject, "modules/%s", sub->name); @@ -225,7 +228,7 @@ int repo_submodule_init(struct repository *subrepo, subrepo->submodule_prefix = xstrfmt("%s%s/", superproject->submodule_prefix ? superproject->submodule_prefix : - "", sub->path); + "", path); out: strbuf_release(&gitdir); diff --git a/repository.h b/repository.h index 3740c93bc0..c24e177c7e 100644 --- a/repository.h +++ b/repository.h @@ -172,15 +172,18 @@ void initialize_the_repository(void); int repo_init(struct repository *r, const char *gitdir, const char *worktree); /* - * 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. + * Initialize the repository 'subrepo' as the submodule at the given path. If + * the submodule's gitdir cannot be found at /.git, this function calls + * submodule_from_path() to try to find it. treeish_name is only used if + * submodule_from_path() needs to be called; see its documentation for more + * information. + * Return 0 upon success and a non-zero value upon failure. */ -struct submodule; +struct object_id; int repo_submodule_init(struct repository *subrepo, struct repository *superproject, - const struct submodule *sub); + const char *path, + const struct object_id *treeish_name); void repo_clear(struct repository *repo); /* diff --git a/submodule.c b/submodule.c index 3af3da5b5e..ecda0229af 100644 --- a/submodule.c +++ b/submodule.c @@ -520,9 +520,6 @@ static void prepare_submodule_repo_env_in_gitdir(struct strvec *out) /* * Initialize a repository struct for a submodule based on the provided 'path'. * - * Unlike repo_submodule_init, this tolerates submodules not present - * in .gitmodules. This function exists only to preserve historical behavior, - * * Returns the repository struct on success, * NULL when the submodule is not present. */ @@ -1404,11 +1401,11 @@ static void fetch_task_release(struct fetch_task *p) } static struct repository *get_submodule_repo_for(struct repository *r, - const struct submodule *sub) + const char *path) { struct repository *ret = xmalloc(sizeof(*ret)); - if (repo_submodule_init(ret, r, sub)) { + if (repo_submodule_init(ret, r, path, null_oid())) { free(ret); return NULL; } @@ -1452,7 +1449,7 @@ static int get_next_submodule(struct child_process *cp, continue; } - task->repo = get_submodule_repo_for(spf->r, task->sub); + task->repo = get_submodule_repo_for(spf->r, task->sub->path); if (task->repo) { struct strbuf submodule_prefix = STRBUF_INIT; child_process_init(cp); diff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c index e3f11ff5a7..dc1c14bde3 100644 --- a/t/helper/test-submodule-nested-repo-config.c +++ b/t/helper/test-submodule-nested-repo-config.c @@ -11,15 +11,13 @@ static void die_usage(const char **argv, const char *msg) int cmd__submodule_nested_repo_config(int argc, const char **argv) { struct repository subrepo; - const struct submodule *sub; if (argc < 3) die_usage(argv, "Wrong number of arguments."); setup_git_directory(); - sub = submodule_from_path(the_repository, null_oid(), argv[1]); - if (repo_submodule_init(&subrepo, the_repository, sub)) { + if (repo_submodule_init(&subrepo, the_repository, argv[1], null_oid())) { die_usage(argv, "Submodule not found."); } -- cgit v1.3