fix: Preserve directory symlinks when pruning submodule move sources

A quick rubber-stamp, admittedly. V4 will probably review all tests and
make it more proper, if there can be such a thing in python anyway.
<!-- agent -->
Both Windows failures came from os.renames() pruning a directory symlink
above the source after the rename succeeded. Unlike POSIX, Windows rmdir()
can remove a directory symlink even when its target is nonempty. Moving a
checkout through a worktree alias therefore deleted the alias and broke
configuration updates and rollback. Renaming metadata through a linked
.git/modules directory deleted that link and broke config.lock creation.

Route checkout moves, rollback, and metadata renames through one helper.
It creates destination parents and renames the source, then prunes empty
source parents only until it reaches a symlink or a directory it cannot
remove. This keeps ordinary empty-directory cleanup while preserving
parent links and their targets, including targets that become empty.
Leaf symlinks continue to move as links rather than moving their targets.

Exercise Windows directory-symlink removal semantics on POSIX in the
existing compatibility tests, and use native behavior on Windows. Both
reported failures reproduced locally before the fix. Strengthen assertions
that worktree and metadata parent aliases survive, their targets remain
directories, and leaf metadata symlinks move without moving their targets.

Assisted-by: GPT 6.0
Co-authored-by: GPT 6.0 <codex@openai.com>

Author: Byron

git/objects/submodule/base.py

@@ -444,6 +444,20 @@ def _checked_abspath(
                 raise ValueError("Submodule path %r contains a symbolic link" % relative_path)
         return path
 
+    @staticmethod
+    def _renames(source: PathLike, destination: PathLike) -> None:
+        os.makedirs(osp.dirname(destination), exist_ok=True)
+        os.rename(source, destination)
+        # Match renames() cleanup, but stop before directory symlinks: Windows
+        # rmdir() removes the link even when its target is nonempty.
+        parent = osp.dirname(source)
+        while parent and not osp.islink(parent):
+            try:
+                os.rmdir(parent)
+            except OSError:
+                break
+            parent = osp.dirname(parent)
+
     @classmethod
     def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_abspath: PathLike) -> None:
         """Write a ``.git`` file containing a (preferably) relative path to the actual
@@ -1120,7 +1134,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool =
         # Move the module into place if possible.
         renamed_module = False
         if module and osp.exists(cur_path):
-            os.renames(cur_path, module_checkout_abspath)
+            self._renames(cur_path, module_checkout_abspath)
             renamed_module = True
 
             if osp.isfile(osp.join(module_checkout_abspath, ".git")):
@@ -1151,7 +1165,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool =
             # END handle configuration flag
         except Exception:
             if renamed_module:
-                os.renames(module_checkout_abspath, cur_path)
+                self._renames(module_checkout_abspath, cur_path)
             # END undo module renaming
             raise
         # END handle undo rename
@@ -1504,10 +1518,10 @@ def rename(self, new_name: str) -> "Submodule":
             # Let's be sure the submodule name is not so obviously tied to a directory.
             if str(destination_module_abspath).startswith(str(mod.git_dir)):
                 tmp_dir = self._module_abspath(self.repo, self.path, str(uuid.uuid4()))
-                os.renames(source_dir, tmp_dir)
+                self._renames(source_dir, tmp_dir)
                 source_dir = tmp_dir
             # END handle self-containment
-            os.renames(source_dir, destination_module_abspath)
+            self._renames(source_dir, destination_module_abspath)
             if mod.working_tree_dir:
                 self._write_git_file_and_module_config(mod.working_tree_dir, destination_module_abspath)
         # END move separate git repository

test/test_submodule.py

@@ -252,14 +252,29 @@ def test_submodule_rejects_checkout_and_gitmodules_symlinks(movable_submodule, t
     assert (root / "moved").is_dir()
 
 
-def test_submodule_allows_symlink_above_worktree(movable_submodule, tmp_path):
+@pytest.fixture
+def windows_directory_symlink_removal(monkeypatch):
+    """Exercise Windows rmdir semantics on POSIX, where rmdir rejects symlinks."""
+    if sys.platform != "win32":
+        original_rmdir = os.rmdir
+
+        def rmdir(path, *args, **kwargs):
+            if osp.islink(path):
+                return os.unlink(path, *args, **kwargs)
+            return original_rmdir(path, *args, **kwargs)
+
+        monkeypatch.setattr(os, "rmdir", rmdir)
+
+
+def test_submodule_allows_symlink_above_worktree(movable_submodule, tmp_path, windows_directory_symlink_removal):
     """Allow adding and moving submodules when the parent is opened through a symlink."""
     sm = movable_submodule
     alias = tmp_path / "alias"
     alias.symlink_to(sm.repo.working_tree_dir, target_is_directory=True)
     with git.Repo(alias) as parent:
         added = Submodule.add(parent, "new", "new", sm.url)
         added.move("moved")
+        assert alias.is_symlink()
         with added.module() as module:
             assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(added.abspath).resolve()
         assert Path(added.abspath, "file").read_text() == "content"
@@ -296,7 +311,9 @@ def test_submodule_allows_metadata_destination_symlinks(movable_submodule, tmp_p
 
 @pytest.mark.parametrize("kind", ["modules", "intermediate", "leaf", "gitfile", "config", "alias"])
 @pytest.mark.parametrize("operation", ["update", "move", "rename", "remove"])
-def test_submodule_allows_existing_metadata_symlinks(movable_submodule, tmp_path, kind, operation):
+def test_submodule_allows_existing_metadata_symlinks(
+    movable_submodule, tmp_path, kind, operation, windows_directory_symlink_removal
+):
     """Keep submodule operations compatible with existing symlinks in Git metadata.
 
     Cover linked metadata directories, gitfiles, configs, and internal aliases.
@@ -336,6 +353,12 @@ def test_submodule_allows_existing_metadata_symlinks(movable_submodule, tmp_path
         sm.move("moved")
     else:
         sm.rename("renamed")
+    if kind == "modules" or (operation == "rename" and kind in ("intermediate", "alias")):
+        assert link.is_symlink()
+        assert link.is_dir()
+    if operation == "rename" and kind == "leaf":
+        assert (modules / "renamed").is_symlink()
+        assert target.is_dir()
     with sm.module() as module:
         assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sm.abspath).resolve()
     assert Path(sm.abspath, "file").read_text() == "content"