submodule: detect conflicts with existing gitdir configs

Credit goes to Emily and Josh for testing and noticing a corner-case
which caused conflicts with existing gitdir configs to silently pass
validation, then fail later in add_submodule() with a cryptic error:

fatal: A git directory for 'nested%2fsub' is found locally with remote(s):
  origin	/.../trash directory.t7425-submodule-gitdir-path-extension/sub

This change ensures the validation step checks existing gitdirs for
conflicts. We only have to do this for submodules having gitdirs,
because those without submodule.%s.gitdir need to be migrated and
will throw an error earlier in the submodule codepath.

Quoting Josh:
 My testing setup has been as follows:
 * Using our locally-built Git with our downstream patch of [1] included:
   * create a repo "sub"
   * create a repo "super"
   * In "super":
     * mkdir nested
     * git submodule add ../sub nested/sub
     * Verify that the submodule's gitdir is .git/modules/nested%2fsub
 * Using a build of git from upstream `next` plus this series:
   * git config set --global extensions.submodulepathconfig true
   * git clone --recurse-submodules super super2
   * create a repo "nested%2fsub"
   * In "super2":
     * git submodule add ../nested%2fsub

At this point I'd expect the collision detection / encoding to take
effect, but instead I get the error listed above.
End quote

Suggested-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/submodule.c b/submodule.c
index 8b7f6ed4e56b39f9555aa4cae7f2cc12ac5ca4fb..51dbbe18634ec9dc7979cde893be4ed201ec05b5 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -2295,6 +2295,61 @@ cleanup:
        return ret;
 }
 
+struct submodule_from_gitdir_cb {
+       const char *gitdir;
+       const char *submodule_name;
+       bool conflict_found;
+};
+
+static int find_conflict_by_gitdir_cb(const char *var, const char *value,
+                                     const struct config_context *ctx UNUSED, void *data)
+{
+       struct submodule_from_gitdir_cb *cb = data;
+       const char *submodule_name_start;
+       size_t submodule_name_len;
+       const char *suffix = ".gitdir";
+       size_t suffix_len = strlen(suffix);
+
+       if (!skip_prefix(var, "submodule.", &submodule_name_start))
+               return 0;
+
+       /* Check if submodule_name_start ends with ".gitdir" */
+       submodule_name_len = strlen(submodule_name_start);
+       if (submodule_name_len < suffix_len ||
+           strcmp(submodule_name_start + submodule_name_len - suffix_len, suffix) != 0)
+               return 0; /* Does not end with ".gitdir" */
+
+       submodule_name_len -= suffix_len;
+
+       /*
+        * A conflict happens if:
+        * 1. The submodule names are different and
+        * 2. The gitdir paths resolve to the same absolute path
+        */
+       if (value && strncmp(cb->submodule_name, submodule_name_start, submodule_name_len)) {
+               char *abs_path_cb = absolute_pathdup(cb->gitdir);
+               char *abs_path_value = absolute_pathdup(value);
+
+               cb->conflict_found = !strcmp(abs_path_cb, abs_path_value);
+
+               free(abs_path_cb);
+               free(abs_path_value);
+       }
+
+       return cb->conflict_found;
+}
+static bool submodule_conflicts_with_existing(const char *gitdir, const char *submodule_name)
+{
+       struct submodule_from_gitdir_cb cb = { 0 };
+       cb.submodule_name = submodule_name;
+       cb.gitdir = gitdir;
+
+       /* Find conflicts with existing repo gitdir configs */
+       repo_config(the_repository, find_conflict_by_gitdir_cb, &cb);
+
+       return cb.conflict_found;
+}
+
 /*
  * Encoded gitdir validation, only used when extensions.submodulePathConfig is enabled.
  * This does not print errors like the non-encoded version, because encoding is supposed
@@ -2320,6 +2375,11 @@ static int validate_submodule_encoded_git_dir(char *git_dir, const char *submodu
        if (!last_submodule_name || strchr(last_submodule_name, '/'))
                return -1;
 
+       /* Prevent conflicts with existing submodule gitdirs */
+       if (is_git_directory(git_dir) &&
+           submodule_conflicts_with_existing(git_dir, submodule_name))
+                       return -1;
+
        /* Prevent conflicts on case-folding filesystems */
        repo_config_get_bool(the_repository, "core.ignorecase", &config_ignorecase);
        if (ignore_case || config_ignorecase) {

diff --git a/t/t7425-submodule-gitdir-path-extension.sh b/t/t7425-submodule-gitdir-path-extension.sh
index 5fcfc29363bcc331c6598ba399c280b585e8fb3b..5f221c507afe7cc3bc9fb20c3ab2395b57da5dd5 100755 (executable)
--- a/t/t7425-submodule-gitdir-path-extension.sh
+++ b/t/t7425-submodule-gitdir-path-extension.sh
@@ -478,4 +478,31 @@ test_expect_success CASE_INSENSITIVE_FS 'verify hashing conflict resolution as a
        verify_submodule_gitdir_path cloned-hash "Foo" "modules/${hash}"
 '
 
+test_expect_success 'submodule gitdir conflicts with previously encoded name (local config)' '
+       git init -b main super_with_encoded &&
+       (
+               cd super_with_encoded &&
+
+               git config extensions.submodulePathConfig true &&
+
+               # Add a submodule with a nested path
+               git submodule add --name "nested/sub" ../sub nested/sub &&
+               test_commit add-encoded-gitdir &&
+
+               verify_submodule_gitdir_path . "nested/sub" "modules/nested%2fsub" &&
+               test_path_is_dir ".git/modules/nested%2fsub"
+       ) &&
+
+       # create a submodule that will conflict with the encoded gitdir name:
+       # the existing gitdir is ".git/modules/nested%2fsub", which is used
+       # by "nested/sub", so the new submod will get another (non-conflicting)
+       # name: "nested%252fsub".
+       (
+               cd super_with_encoded &&
+               git submodule add ../sub "nested%2fsub" &&
+               verify_submodule_gitdir_path . "nested%2fsub" "modules/nested%252fsub" &&
+               test_path_is_dir ".git/modules/nested%252fsub"
+       )
+'
+
 test_done