submodule: extract path to submodule gitdir func

We currently store each submodule gitdir in ".git/modules/<name>", but
this has problems with some submodule naming schemes, as described in a
comment in submodule_name_to_gitdir() in this patch.

Extract the determination of the location of a submodule's gitdir into
its own function submodule_name_to_gitdir(). For now, the problem
remains unsolved, but this puts us in a better position for finding a
solution.

This was motivated, at $DAYJOB, by a part of Android's repo hierarchy
[1]. In particular, there is a repo "build", and several repos of the
form "build/<name>".

This is based on earlier work by Brandon Williams [2].

[1] https://android.googlesource.com/platform/
[2] https://lore.kernel.org/git/20180808223323.79989-2-bmwill@google.com/

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4da9781b991286614bd7070b5fa76221513e8f44..29ca0bedf652e79643df40e48d7f8753f3a49e1d 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1704,18 +1704,24 @@ static int add_possible_reference_from_superproject(
         * standard layout with .git/(modules/<name>)+/objects
         */
        if (strip_suffix(odb->path, "/objects", &len)) {
+               struct repository alternate;
                char *sm_alternate;
                struct strbuf sb = STRBUF_INIT;
                struct strbuf err = STRBUF_INIT;
                strbuf_add(&sb, odb->path, len);
 
+               repo_init(&alternate, sb.buf, NULL);
+
                /*
                 * We need to end the new path with '/' to mark it as a dir,
                 * otherwise a submodule name containing '/' will be broken
                 * as the last part of a missing submodule reference would
                 * be taken as a file name.
                 */
-               strbuf_addf(&sb, "/modules/%s/", sas->submodule_name);
+               strbuf_reset(&sb);
+               submodule_name_to_gitdir(&sb, &alternate, sas->submodule_name);
+               strbuf_addch(&sb, '/');
+               repo_clear(&alternate);
 
                sm_alternate = compute_alternate_path(sb.buf, &err);
                if (sm_alternate) {
@@ -1785,7 +1791,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
        struct strbuf sb = STRBUF_INIT;
        struct child_process cp = CHILD_PROCESS_INIT;
 
-       strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), clone_data->name);
+       submodule_name_to_gitdir(&sb, the_repository, clone_data->name);
        sm_gitdir = absolute_pathdup(sb.buf);
        strbuf_reset(&sb);
 

diff --git a/dir.c b/dir.c
index 03c4d212672bc4b68bc45a514fe16180f57ee1f4..5fe5d3a42eaddbfcc6a82ffcc2d7dd65d1e2a324 100644 (file)
--- a/dir.c
+++ b/dir.c
@@ -3633,7 +3633,7 @@ static void connect_wt_gitdir_in_nested(const char *sub_worktree,
                strbuf_reset(&sub_wt);
                strbuf_reset(&sub_gd);
                strbuf_addf(&sub_wt, "%s/%s", sub_worktree, sub->path);
-               strbuf_addf(&sub_gd, "%s/modules/%s", sub_gitdir, sub->name);
+               submodule_name_to_gitdir(&sub_gd, &subrepo, sub->name);
 
                connect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 1);
        }

diff --git a/repository.c b/repository.c
index b2bf44c6fafec83b6360354ae77081759096be6f..710a3b4bf872fb0db64f859d5c49d085c198f089 100644 (file)
--- a/repository.c
+++ b/repository.c
@@ -213,8 +213,7 @@ int repo_submodule_init(struct repository *subrepo,
                 * submodule would not have a worktree.
                 */
                strbuf_reset(&gitdir);
-               strbuf_repo_git_path(&gitdir, superproject,
-                                    "modules/%s", sub->name);
+               submodule_name_to_gitdir(&gitdir, superproject, sub->name);
 
                if (repo_init(subrepo, gitdir.buf, NULL)) {
                        ret = -1;

diff --git a/submodule.c b/submodule.c
index 8e611fe1dbf1f7616040f8359ee5d9b9892ad191..9382ddf0cc3aac10bdc1cf951f777e28e75a9c69 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -1819,14 +1819,16 @@ out:
 
 void submodule_unset_core_worktree(const struct submodule *sub)
 {
-       char *config_path = xstrfmt("%s/modules/%s/config",
-                                   get_git_dir(), sub->name);
+       struct strbuf config_path = STRBUF_INIT;
 
-       if (git_config_set_in_file_gently(config_path, "core.worktree", NULL))
+       submodule_name_to_gitdir(&config_path, the_repository, sub->name);
+       strbuf_addstr(&config_path, "/config");
+
+       if (git_config_set_in_file_gently(config_path.buf, "core.worktree", NULL))
                warning(_("Could not unset core.worktree setting in submodule '%s'"),
                          sub->path);
 
-       free(config_path);
+       strbuf_release(&config_path);
 }
 
 static const char *get_super_prefix_or_empty(void)
@@ -1922,20 +1924,22 @@ int submodule_move_head(const char *path,
                                absorb_git_dir_into_superproject(path,
                                        ABSORB_GITDIR_RECURSE_SUBMODULES);
                } else {
-                       char *gitdir = xstrfmt("%s/modules/%s",
-                                   get_git_dir(), sub->name);
-                       connect_work_tree_and_git_dir(path, gitdir, 0);
-                       free(gitdir);
+                       struct strbuf gitdir = STRBUF_INIT;
+                       submodule_name_to_gitdir(&gitdir, the_repository,
+                                                sub->name);
+                       connect_work_tree_and_git_dir(path, gitdir.buf, 0);
+                       strbuf_release(&gitdir);
 
                        /* make sure the index is clean as well */
                        submodule_reset_index(path);
                }
 
                if (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
-                       char *gitdir = xstrfmt("%s/modules/%s",
-                                   get_git_dir(), sub->name);
-                       connect_work_tree_and_git_dir(path, gitdir, 1);
-                       free(gitdir);
+                       struct strbuf gitdir = STRBUF_INIT;
+                       submodule_name_to_gitdir(&gitdir, the_repository,
+                                                sub->name);
+                       connect_work_tree_and_git_dir(path, gitdir.buf, 1);
+                       strbuf_release(&gitdir);
                }
        }
 
@@ -2050,7 +2054,7 @@ int validate_submodule_git_dir(char *git_dir, const char *submodule_name)
 static void relocate_single_git_dir_into_superproject(const char *path)
 {
        char *old_git_dir = NULL, *real_old_git_dir = NULL, *real_new_git_dir = NULL;
-       char *new_git_dir;
+       struct strbuf new_gitdir = STRBUF_INIT;
        const struct submodule *sub;
 
        if (submodule_uses_worktrees(path))
@@ -2068,14 +2072,13 @@ static void relocate_single_git_dir_into_superproject(const char *path)
        if (!sub)
                die(_("could not lookup name for submodule '%s'"), path);
 
-       new_git_dir = git_pathdup("modules/%s", sub->name);
-       if (validate_submodule_git_dir(new_git_dir, sub->name) < 0)
+       submodule_name_to_gitdir(&new_gitdir, the_repository, sub->name);
+       if (validate_submodule_git_dir(new_gitdir.buf, sub->name) < 0)
                die(_("refusing to move '%s' into an existing git dir"),
                    real_old_git_dir);
-       if (safe_create_leading_directories_const(new_git_dir) < 0)
-               die(_("could not create directory '%s'"), new_git_dir);
-       real_new_git_dir = real_pathdup(new_git_dir, 1);
-       free(new_git_dir);
+       if (safe_create_leading_directories_const(new_gitdir.buf) < 0)
+               die(_("could not create directory '%s'"), new_gitdir.buf);
+       real_new_git_dir = real_pathdup(new_gitdir.buf, 1);
 
        fprintf(stderr, _("Migrating git directory of '%s%s' from\n'%s' to\n'%s'\n"),
                get_super_prefix_or_empty(), path,
@@ -2086,6 +2089,7 @@ static void relocate_single_git_dir_into_superproject(const char *path)
        free(old_git_dir);
        free(real_old_git_dir);
        free(real_new_git_dir);
+       strbuf_release(&new_gitdir);
 }
 
 /*
@@ -2105,6 +2109,7 @@ void absorb_git_dir_into_superproject(const char *path,
        /* Not populated? */
        if (!sub_git_dir) {
                const struct submodule *sub;
+               struct strbuf sub_gitdir = STRBUF_INIT;
 
                if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
                        /* unpopulated as expected */
@@ -2126,8 +2131,9 @@ void absorb_git_dir_into_superproject(const char *path,
                sub = submodule_from_path(the_repository, null_oid(), path);
                if (!sub)
                        die(_("could not lookup name for submodule '%s'"), path);
-               connect_work_tree_and_git_dir(path,
-                       git_path("modules/%s", sub->name), 0);
+               submodule_name_to_gitdir(&sub_gitdir, the_repository, sub->name);
+               connect_work_tree_and_git_dir(path, sub_gitdir.buf, 0);
+               strbuf_release(&sub_gitdir);
        } else {
                /* Is it already absorbed into the superprojects git dir? */
                char *real_sub_git_dir = real_pathdup(sub_git_dir, 1);
@@ -2278,9 +2284,36 @@ int submodule_to_gitdir(struct strbuf *buf, const char *submodule)
                        goto cleanup;
                }
                strbuf_reset(buf);
-               strbuf_git_path(buf, "%s/%s", "modules", sub->name);
+               submodule_name_to_gitdir(buf, the_repository, sub->name);
        }
 
 cleanup:
        return ret;
 }
+
+void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
+                             const char *submodule_name)
+{
+       /*
+        * NEEDSWORK: The current way of mapping a submodule's name to
+        * its location in .git/modules/ has problems with some naming
+        * schemes. For example, if a submodule is named "foo" and
+        * another is named "foo/bar" (whether present in the same
+        * superproject commit or not - the problem will arise if both
+        * superproject commits have been checked out at any point in
+        * time), or if two submodule names only have different cases in
+        * a case-insensitive filesystem.
+        *
+        * There are several solutions, including encoding the path in
+        * some way, introducing a submodule.<name>.gitdir config in
+        * .git/config (not .gitmodules) that allows overriding what the
+        * gitdir of a submodule would be (and teach Git, upon noticing
+        * a clash, to automatically determine a non-clashing name and
+        * to write such a config), or introducing a
+        * submodule.<name>.gitdir config in .gitmodules that repo
+        * administrators can explicitly set. Nothing has been decided,
+        * so for now, just append the name at the end of the path.
+        */
+       strbuf_repo_git_path(buf, r, "modules/");
+       strbuf_addstr(buf, submodule_name);
+}

diff --git a/submodule.h b/submodule.h
index 84640c49c1149d665f88b75c38405a40e4157a8c..c11e0807fcc2575fcbd3d0bf2b3b0fe099a0b269 100644 (file)
--- a/submodule.h
+++ b/submodule.h
@@ -124,6 +124,13 @@ int push_unpushed_submodules(struct repository *r,
  */
 int submodule_to_gitdir(struct strbuf *buf, const char *submodule);
 
+/*
+ * Given a submodule name, create a path to where the submodule's gitdir lives
+ * inside of the provided repository's 'modules' directory.
+ */
+void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
+                             const char *submodule_name);
+
 /*
  * Make sure that no submodule's git dir is nested in a sibling submodule's.
  */