Merge branch 'sb/submodule-helper-clone-regression-fix' into sb/submodule-init

* sb/submodule-helper-clone-regression-fix:
submodule--helper, module_clone: catch fprintf failure
submodule--helper: do not borrow absolute_path() result for too long
submodule--helper, module_clone: always operate on absolute paths
submodule--helper clone: create the submodule path just once
submodule--helper: fix potential NULL-dereference
recursive submodules: test for relative paths

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a484945d37911828554769a38affddaf3067dc54..a39ad1be995da7e46be8db3db2173d323c6beae9 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -153,11 +153,11 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
 
 static int module_clone(int argc, const char **argv, const char *prefix)
 {
-       const char *path = NULL, *name = NULL, *url = NULL;
+       const char *name = NULL, *url = NULL;
        const char *reference = NULL, *depth = NULL;
        int quiet = 0;
        FILE *submodule_dot_git;
-       char *sm_gitdir, *cwd, *p;
+       char *p, *path = NULL, *sm_gitdir;
        struct strbuf rel_path = STRBUF_INIT;
        struct strbuf sb = STRBUF_INIT;
 
@@ -194,8 +194,18 @@ static int module_clone(int argc, const char **argv, const char *prefix)
        argc = parse_options(argc, argv, prefix, module_clone_options,
                             git_submodule_helper_usage, 0);
 
+       if (!path || !*path)
+               die(_("submodule--helper: unspecified or empty --path"));
+
        strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
-       sm_gitdir = strbuf_detach(&sb, NULL);
+       sm_gitdir = xstrdup(absolute_path(sb.buf));
+       strbuf_reset(&sb);
+
+       if (!is_absolute_path(path)) {
+               strbuf_addf(&sb, "%s/%s", get_git_work_tree(), path);
+               path = strbuf_detach(&sb, NULL);
+       } else
+               path = xstrdup(path);
 
        if (!file_exists(sm_gitdir)) {
                if (safe_create_leading_directories_const(sm_gitdir) < 0)
@@ -212,45 +222,30 @@ static int module_clone(int argc, const char **argv, const char *prefix)
        }
 
        /* Write a .git file in the submodule to redirect to the superproject. */
-       if (safe_create_leading_directories_const(path) < 0)
-               die(_("could not create directory '%s'"), path);
-
-       if (path && *path)
-               strbuf_addf(&sb, "%s/.git", path);
-       else
-               strbuf_addstr(&sb, ".git");
-
+       strbuf_addf(&sb, "%s/.git", path);
        if (safe_create_leading_directories_const(sb.buf) < 0)
                die(_("could not create leading directories of '%s'"), sb.buf);
        submodule_dot_git = fopen(sb.buf, "w");
        if (!submodule_dot_git)
                die_errno(_("cannot open file '%s'"), sb.buf);
 
-       fprintf(submodule_dot_git, "gitdir: %s\n",
-               relative_path(sm_gitdir, path, &rel_path));
+       fprintf_or_die(submodule_dot_git, "gitdir: %s\n",
+                      relative_path(sm_gitdir, path, &rel_path));
        if (fclose(submodule_dot_git))
                die(_("could not close file %s"), sb.buf);
        strbuf_reset(&sb);
        strbuf_reset(&rel_path);
 
-       cwd = xgetcwd();
        /* Redirect the worktree of the submodule in the superproject's config */
-       if (!is_absolute_path(sm_gitdir)) {
-               strbuf_addf(&sb, "%s/%s", cwd, sm_gitdir);
-               free(sm_gitdir);
-               sm_gitdir = strbuf_detach(&sb, NULL);
-       }
-
-       strbuf_addf(&sb, "%s/%s", cwd, path);
        p = git_pathdup_submodule(path, "config");
        if (!p)
                die(_("could not get submodule directory for '%s'"), path);
        git_config_set_in_file(p, "core.worktree",
-                              relative_path(sb.buf, sm_gitdir, &rel_path));
+                              relative_path(path, sm_gitdir, &rel_path));
        strbuf_release(&sb);
        strbuf_release(&rel_path);
        free(sm_gitdir);
-       free(cwd);
+       free(path);
        free(p);
        return 0;
 }

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 5991e3c015dc8e21c483ab74e858b508ef37a061..033d034f5d69cb9fb7c70f1330a2e6cc7343ca92 100755 (executable)
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -818,6 +818,47 @@ test_expect_success 'submodule add --name allows to replace a submodule with ano
        )
 '
 
+test_expect_success 'recursive relative submodules stay relative' '
+       test_when_finished "rm -rf super clone2 subsub sub3" &&
+       mkdir subsub &&
+       (
+               cd subsub &&
+               git init &&
+               >t &&
+               git add t &&
+               git commit -m "initial commit"
+       ) &&
+       mkdir sub3 &&
+       (
+               cd sub3 &&
+               git init &&
+               >t &&
+               git add t &&
+               git commit -m "initial commit" &&
+               git submodule add ../subsub dirdir/subsub &&
+               git commit -m "add submodule subsub"
+       ) &&
+       mkdir super &&
+       (
+               cd super &&
+               git init &&
+               >t &&
+               git add t &&
+               git commit -m "initial commit" &&
+               git submodule add ../sub3 &&
+               git commit -m "add submodule sub"
+       ) &&
+       git clone super clone2 &&
+       (
+               cd clone2 &&
+               git submodule update --init --recursive &&
+               echo "gitdir: ../.git/modules/sub3" >./sub3/.git_expect &&
+               echo "gitdir: ../../../.git/modules/sub3/modules/dirdir/subsub" >./sub3/dirdir/subsub/.git_expect
+       ) &&
+       test_cmp clone2/sub3/.git_expect clone2/sub3/.git &&
+       test_cmp clone2/sub3/dirdir/subsub/.git_expect clone2/sub3/dirdir/subsub/.git
+'
+
 test_expect_success 'submodule add with an existing name fails unless forced' '
        (
                cd addtest2 &&