gitpython-developers
diff --git a/‎git/objects/submodule/base.py‎
Lines changed: 69 additions & 71 deletions b/‎git/objects/submodule/base.py‎
Lines changed: 69 additions & 71 deletions
diff --git a/‎test/test_submodule.py‎
Lines changed: 73 additions & 0 deletions b/‎test/test_submodule.py‎
Lines changed: 73 additions & 0 deletions
@@ -779,82 +779,80 @@ def update(
                     return self
                 # END early abort if init is not allowed
 
-                # There is no git-repository yet - but delete empty paths.
                 checkout_module_abspath = self.abspath
-                if not dry_run and osp.isdir(checkout_module_abspath):
+                module_abspath = self._module_abspath(self.repo, self.path, self.name)
+
+                # ``git submodule deinit`` leaves the repository in
+                # ``.git/modules`` and empties the checkout. Reconnect that retained
+                # repository instead of trying to clone over it.
+                if not dry_run and osp.isdir(module_abspath):
                     try:
-                        os.rmdir(checkout_module_abspath)
-                    except OSError as e:
-                        raise OSError(
-                            "Module directory at %r does already exist and is non-empty" % checkout_module_abspath
-                        ) from e
-                    # END handle OSError
-                # END handle directory removal
-
-                # Don't check it out at first - nonetheless it will create a local
-                # branch according to the remote-HEAD if possible.
-                progress.update(
-                    BEGIN | CLONE,
-                    0,
-                    1,
-                    prefix
-                    + "Cloning url '%s' to '%s' in submodule %r" % (self.url, checkout_module_abspath, self.name),
-                )
-                if not dry_run:
-                    if self.url.startswith("."):
-                        url = urllib.parse.urljoin(self.repo.remotes.origin.url + "/", self.url)
+                        git.Repo(module_abspath)
+                    except InvalidGitRepositoryError:
+                        pass
                     else:
-                        url = self.url
-                    mrepo = self._clone_repo(
-                        self.repo,
-                        url,
-                        self.path,
-                        self.name,
-                        n=True,
-                        env=env,
-                        multi_options=clone_multi_options,
-                        allow_unsafe_options=allow_unsafe_options,
-                        allow_unsafe_protocols=allow_unsafe_protocols,
+                        os.makedirs(checkout_module_abspath, exist_ok=True)
+                        self._write_git_file_and_module_config(checkout_module_abspath, module_abspath)
+                        mrepo = git.Repo(checkout_module_abspath)
+                        mrepo.head.reset(mrepo.head.commit, index=True, working_tree=True)
+                        with self.repo.config_writer() as writer:
+                            writer.set_value(sm_section(self.name), "url", self.url)
+
+                if mrepo is None:
+                    # There is no git-repository yet - but delete empty paths.
+                    if not dry_run and osp.isdir(checkout_module_abspath):
+                        try:
+                            os.rmdir(checkout_module_abspath)
+                        except OSError as e:
+                            raise OSError(
+                                "Module directory at %r does already exist and is non-empty" % checkout_module_abspath
+                            ) from e
+                    # END handle directory removal
+
+                    # Don't check it out at first - nonetheless it will create a local
+                    # branch according to the remote-HEAD if possible.
+                    progress.update(
+                        BEGIN | CLONE,
+                        0,
+                        1,
+                        prefix
+                        + "Cloning url '%s' to '%s' in submodule %r" % (self.url, checkout_module_abspath, self.name),
                     )
-                # END handle dry-run
-                progress.update(
-                    END | CLONE,
-                    0,
-                    1,
-                    prefix + "Done cloning to %s" % checkout_module_abspath,
-                )
-
-                if not dry_run:
-                    # See whether we have a valid branch to check out.
-                    try:
-                        mrepo = cast("Repo", mrepo)
-                        # Find a remote which has our branch - we try to be flexible.
-                        remote_branch = find_first_remote_branch(mrepo.remotes, self.branch_name)
-                        local_branch = mkhead(mrepo, self.branch_path)
-
-                        # Have a valid branch, but no checkout - make sure we can figure
-                        # that out by marking the commit with a null_sha.
-                        local_branch.set_object(Object(mrepo, self.NULL_BIN_SHA))
-                        # END initial checkout + branch creation
-
-                        # Make sure HEAD is not detached.
-                        mrepo.head.set_reference(
-                            local_branch,
-                            logmsg="submodule: attaching head to %s" % local_branch,
+                    if not dry_run:
+                        if self.url.startswith("."):
+                            url = urllib.parse.urljoin(self.repo.remotes.origin.url + "/", self.url)
+                        else:
+                            url = self.url
+                        mrepo = self._clone_repo(
+                            self.repo,
+                            url,
+                            self.path,
+                            self.name,
+                            n=True,
+                            env=env,
+                            multi_options=clone_multi_options,
+                            allow_unsafe_options=allow_unsafe_options,
+                            allow_unsafe_protocols=allow_unsafe_protocols,
                         )
-                        mrepo.head.reference.set_tracking_branch(remote_branch)
-                    except (IndexError, InvalidGitRepositoryError):
-                        _logger.warning("Failed to checkout tracking branch %s", self.branch_path)
-                    # END handle tracking branch
-
-                    # NOTE: Have to write the repo config file as well, otherwise the
-                    # default implementation will be offended and not update the
-                    # repository. Maybe this is a good way to ensure it doesn't get into
-                    # our way, but we want to stay backwards compatible too... It's so
-                    # redundant!
-                    with self.repo.config_writer() as writer:
-                        writer.set_value(sm_section(self.name), "url", self.url)
-                # END handle dry_run
+                    progress.update(END | CLONE, 0, 1, prefix + "Done cloning to %s" % checkout_module_abspath)
+
+                    if not dry_run:
+                        # See whether we have a valid branch to check out.
+                        try:
+                            mrepo = cast("Repo", mrepo)
+                            remote_branch = find_first_remote_branch(mrepo.remotes, self.branch_name)
+                            local_branch = mkhead(mrepo, self.branch_path)
+                            local_branch.set_object(Object(mrepo, self.NULL_BIN_SHA))
+                            mrepo.head.set_reference(
+                                local_branch,
+                                logmsg="submodule: attaching head to %s" % local_branch,
+                            )
+                            mrepo.head.reference.set_tracking_branch(remote_branch)
+                        except (IndexError, InvalidGitRepositoryError):
+                            _logger.warning("Failed to checkout tracking branch %s", self.branch_path)
+
+                        with self.repo.config_writer() as writer:
+                            writer.set_value(sm_section(self.name), "url", self.url)
             # END handle initialization
 
             # DETERMINE SHAS TO CHECK OUT
 
@@ -728,6 +728,79 @@ def test_deinit_calls_git_submodule(self, rwdir):
 
             git_submodule.assert_called_once_with("deinit", "--force", "--", submodule.path)
 
+    @with_rw_directory
+    def test_update_after_deinit(self, rwdir):
+        source_path = osp.join(rwdir, "source")
+        source_repo = git.Repo.init(source_path)
+        touch(osp.join(source_path, "file"))
+        source_repo.index.add(["file"])
+        source_repo.index.commit("initial commit")
+
+        parent_path = osp.join(rwdir, "parent")
+        parent_repo = git.Repo.init(parent_path)
+        submodule = parent_repo.create_submodule("module", "module", source_path)
+        parent_repo.index.commit("add submodule")
+
+        submodule.deinit()
+        assert not submodule.module_exists()
+        assert osp.isdir(osp.join(parent_repo.git_dir, "modules", submodule.name))
+
+        submodule.update()
+
+        assert submodule.module_exists()
+        assert submodule.module().head.commit == source_repo.head.commit
+        assert osp.isfile(osp.join(submodule.abspath, "file"))
+
+    @with_rw_directory
+    def test_update_after_deinit_without_init_does_not_restore_checkout(self, rwdir):
+        source_path = osp.join(rwdir, "source")
+        source_repo = git.Repo.init(source_path)
+        source_repo.git.commit(m="initial commit", allow_empty=True)
+
+        parent_repo = git.Repo.init(osp.join(rwdir, "parent"))
+        submodule = parent_repo.create_submodule("module", "module", source_path)
+        parent_repo.index.commit("add submodule")
+        submodule.deinit()
+
+        assert submodule.update(init=False) is submodule
+        assert not submodule.module_exists()
+        assert not osp.exists(osp.join(submodule.abspath, ".git"))
+
+    @with_rw_directory
+    def test_dry_run_update_after_deinit_does_not_restore_checkout(self, rwdir):
+        source_path = osp.join(rwdir, "source")
+        source_repo = git.Repo.init(source_path)
+        source_repo.git.commit(m="initial commit", allow_empty=True)
+
+        parent_repo = git.Repo.init(osp.join(rwdir, "parent"))
+        submodule = parent_repo.create_submodule("module", "module", source_path)
+        parent_repo.index.commit("add submodule")
+        submodule.deinit()
+
+        assert submodule.update(dry_run=True) is submodule
+        assert not submodule.module_exists()
+        assert not osp.exists(osp.join(submodule.abspath, ".git"))
+
+    @with_rw_directory
+    def test_update_after_deinit_restores_nested_checkout(self, rwdir):
+        source_path = osp.join(rwdir, "source")
+        source_repo = git.Repo.init(source_path)
+        touch(osp.join(source_path, "file"))
+        source_repo.index.add(["file"])
+        source_repo.index.commit("initial commit")
+
+        parent_repo = git.Repo.init(osp.join(rwdir, "parent"))
+        submodule = parent_repo.create_submodule("nested/module", "deps/module", source_path)
+        parent_repo.index.commit("add nested submodule")
+        submodule.deinit()
+
+        submodule.update()
+
+        module_repo = submodule.module()
+        assert module_repo.head.commit == source_repo.head.commit
+        assert osp.isfile(osp.join(submodule.abspath, "file"))
+        assert osp.samefile(module_repo.git_dir, osp.join(parent_repo.git_dir, "modules", submodule.name))
+
     @with_rw_repo(k_no_subm_tag, bare=False)
     def test_first_submodule(self, rwrepo):
         assert len(list(rwrepo.iter_submodules())) == 0