submodule.c: submodule_move_head works with broken submodules

Early on in submodule_move_head just after the check if the submodule is
initialized, we need to check if the submodule is populated correctly.

If the submodule is initialized but doesn't look like it is populated,
this is a red flag and can indicate multiple sorts of failures:
(1) The submodule may be recorded at an object name, that is missing.
(2) The submodule '.git' file link may be broken and it is not pointing
at a repository.

In both cases we want to complain to the user in the non-forced mode,
and in the forced mode ignoring the old state and just moving the
submodule into its new state with a fixed '.git' file link.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/submodule.c b/submodule.c
index ccf89327315bde39bb02ad71bb8b529c4fdbfacb..20ed5b5681d3217246acf80bb0cc57a39d395c8f 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -1332,10 +1332,24 @@ int submodule_move_head(const char *path,
        int ret = 0;
        struct child_process cp = CHILD_PROCESS_INIT;
        const struct submodule *sub;
        int ret = 0;
        struct child_process cp = CHILD_PROCESS_INIT;
        const struct submodule *sub;

+       int *error_code_ptr, error_code;

 
        if (!is_submodule_initialized(path))
                return 0;
 
 
        if (!is_submodule_initialized(path))
                return 0;
 

+       if (flags & SUBMODULE_MOVE_HEAD_FORCE)
+               /*
+                * Pass non NULL pointer to is_submodule_populated_gently
+                * to prevent die()-ing. We'll use connect_work_tree_and_git_dir
+                * to fixup the submodule in the force case later.
+                */
+               error_code_ptr = &error_code;
+       else
+               error_code_ptr = NULL;
+
+       if (old && !is_submodule_populated_gently(path, error_code_ptr))
+               return 0;
+

        sub = submodule_from_path(null_sha1, path);
 
        if (!sub)
        sub = submodule_from_path(null_sha1, path);
 
        if (!sub)


@@ -1353,15 +1367,21 @@ int submodule_move_head(const char *path,
                                absorb_git_dir_into_superproject("", path,
                                        ABSORB_GITDIR_RECURSE_SUBMODULES);
                } else {
                                absorb_git_dir_into_superproject("", path,
                                        ABSORB_GITDIR_RECURSE_SUBMODULES);
                } else {

-                       struct strbuf sb = STRBUF_INIT;
-                       strbuf_addf(&sb, "%s/modules/%s",
+                       char *gitdir = xstrfmt("%s/modules/%s",

                                    get_git_common_dir(), sub->name);
                                    get_git_common_dir(), sub->name);

-                       connect_work_tree_and_git_dir(path, sb.buf);
-                       strbuf_release(&sb);
+                       connect_work_tree_and_git_dir(path, gitdir);
+                       free(gitdir);

 
                        /* make sure the index is clean as well */
                        submodule_reset_index(path);
                }
 
                        /* make sure the index is clean as well */
                        submodule_reset_index(path);
                }

+
+               if (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {
+                       char *gitdir = xstrfmt("%s/modules/%s",
+                                   get_git_common_dir(), sub->name);
+                       connect_work_tree_and_git_dir(path, gitdir);
+                       free(gitdir);
+               }

        }
 
        prepare_submodule_repo_env_no_git_dir(&cp.env_array);
        }
 
        prepare_submodule_repo_env_no_git_dir(&cp.env_array);

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index 22dd9e060cffae345ff90e21d0ebf5af45a6e879..f0b1b1820607249929f9d505de3d8c25b5ae92b4 100755 (executable)
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -1213,14 +1213,31 @@ test_submodule_forced_switch_recursing () {
                )
        '
        # Updating a submodule from an invalid sha1 updates
                )
        '
        # Updating a submodule from an invalid sha1 updates

-       test_expect_success "$command: modified submodule does not update submodule work tree from invalid commit" '
+       test_expect_success "$command: modified submodule does update submodule work tree from invalid commit" '

                prolog &&
                reset_work_tree_to_interested invalid_sub1 &&
                (
                        cd submodule_update &&
                        git branch -t valid_sub1 origin/valid_sub1 &&
                prolog &&
                reset_work_tree_to_interested invalid_sub1 &&
                (
                        cd submodule_update &&
                        git branch -t valid_sub1 origin/valid_sub1 &&

-                       test_must_fail $command valid_sub1 &&
-                       test_superproject_content origin/invalid_sub1
+                       $command valid_sub1 &&
+                       test_superproject_content origin/valid_sub1 &&
+                       test_submodule_content sub1 origin/valid_sub1
+               )
+       '
+
+       # Old versions of Git were buggy writing the .git link file
+       # (e.g. before f8eaa0ba98b and then moving the superproject repo
+       # whose submodules contained absolute paths)
+       test_expect_success "$command: updating submodules fixes .git links" '
+               prolog &&
+               reset_work_tree_to_interested add_sub1 &&
+               (
+                       cd submodule_update &&
+                       git branch -t modify_sub1 origin/modify_sub1 &&
+                       echo "gitdir: bogus/path" >sub1/.git &&
+                       $command modify_sub1 &&
+                       test_superproject_content origin/modify_sub1 &&
+                       test_submodule_content sub1 origin/modify_sub1

                )
        '
 }
                )
        '
 }