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

A partial rewrite of "git submodule" in the 2.7 timeframe changed
the way the gitdir: pointer in the submodules point at the real
repository location to use absolute paths by accident.  This has
been corrected.

* 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

2 files changed, 59 insertions, 23 deletions

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 5295b727d4..3bea3aaa50 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -147,11 +147,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;
 
@@ -188,8 +188,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)
@@ -206,45 +216,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 e1abd19230..a41be3142e 100755
--- 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 &&