fix: stabilize Windows submodules and temporary test cleanup

On Windows, `git init --separate-git-dir` can try to rename a submodule's
metadata directory onto itself. An open file inside that directory makes
the rename fail with `Directory not empty`, as seen in the CI tutorial.
Close the cloned `Repo` before reconnecting it and ask
`git rev-parse --resolve-git-dir` whether its gitfile already points to the
destination. When it does, ordinary `git init` preserves the connection
without the redundant rename. Missing or stale connections still use
`--separate-git-dir`, with Git retaining ownership of initialization and
storage formats.

Add `test.cleanup.cleanup_directory` and `TemporaryDirectory` for disposal
of isolated test directories. Remove whatever is possible, log filesystem
errors, and leave locked files behind without changing the test result.
Retry read-only Windows files and directories without changing symlink or
junction targets. Support both cleanup callback APIs and Python 3.8+.
Migrate test contexts and fixture teardown to these helpers while keeping
`git.util.rmtree`, operations under test, and fixture reuse strict.

Release repository handles before deletion, run the missing performance
test teardown, and wait for the killed Git daemon to exit. Invalidate
cached fixture layouts when disposal fails so subsequent tests rebuild
at fresh paths. Give `test/run-local.py` a private pytest temporary root
to avoid shared-root ownership failures. Remove the diff test's cleanup
xfail and document the test cleanup convention.

Regression coverage includes real Windows file locks, metadata reconnects
through ordinary and 8.3 paths, read-only directories, unchanged symlink
targets, cleanup retries, preserved test-body exceptions, decorator
teardown, and recovery from a locked cached fixture.

Validation:

- Windows with Git 2.55.0.windows.5: 164 passed and 1 skipped in the CLI
  regression selection; final focused checks passed with 25 passed and
  1 skipped for each of the CLI and Gix backends.
- Linux under WSL: 27 passed and 4 skipped in the portable regression
  selection.
- Pinned pre-commit hooks, `mypy`, and `basedpyright` passed.

Author: codex

CONTRIBUTING.md

@@ -31,6 +31,19 @@ Attributing AI assistance in commit metadata, for example with a `Co-authored-by
 trailer, is welcome but not required. Code is reviewed the same way regardless of its
 origin.
 
+## Temporary test directories
+
+Use `test.cleanup.TemporaryDirectory` for isolated temporary directories and
+`test.cleanup.cleanup_directory` when disposing of directories created by test
+fixtures. Close repository handles before removing their files. Cleanup is
+best-effort: it logs filesystem errors, removes whatever it can, and leaves
+locked files behind without failing or skipping a test. The shared writable
+repository decorators still keep failed tests' directories for debugging.
+
+Deletions and renames that exercise library behavior or prepare a fixture for
+reuse must remain strict. If a cached fixture cannot be cleaned up, rebuild it
+at a fresh location before handing it to another test.
+
 ## Fuzzing Test Specific Documentation
 
 For details related to contributing to the fuzzing test suite and OSS-Fuzz integration, please 

doc/gix-backend.md

@@ -54,9 +54,11 @@ The prepared environments in this checkout are `.venv` (CLI) and `.tox/gix`
 ```
 
 The runner uses local version tags, creates an isolated Git configuration,
-prepares the historical test fixture inside a temporary shared clone, and
-disables package-index access. Tests use local repositories, including the
-tutorial example. The suite needs loopback sockets for its Git daemon and
+prepares the historical test fixture inside a temporary shared clone, gives
+pytest a separate temporary root for each run, and disables package-index
+access. Cleanup of these isolated directories is best-effort, so a locked
+leftover cannot change pytest's exit status. Tests use local repositories,
+including the tutorial example. The suite needs loopback sockets for its Git daemon and
 permission to inspect its own child processes. It does not need a remote Git
 server. Missing local tags or packages are errors, not invitations to download.
 
@@ -581,7 +583,7 @@ candidates, not proof that a shared fixture is safe in every execution order.
 | --- | --- | --- |
 | 166 | `TExc` (157) and `TestActor` (9) inherit repository-building `TestBase`. | Use the existing `TestCase` base without repository setup. |
 | 182 | Three submodule rejection bodies repeatedly build `movable_submodule`, then check snapshots for no mutation. | Prepare logical-name baselines once and copy the parent per case, retaining fresh wrappers and independent writable files. |
-| 51 | Six submodule rejection bodies prepare nested metadata, separate metadata, intermediate/leaf symlinks, or retained metadata before checking rejection. | Cache ten prepared layouts and restore complete copies at their original paths, preserving absolute Git links and symlinks. Cleanup removes the active copy even after failure. |
+| 51 | Six submodule rejection bodies prepare nested metadata, separate metadata, intermediate/leaf symlinks, or retained metadata before checking rejection. | Cache ten prepared layouts and restore complete copies at their original paths, preserving absolute Git links and symlinks. Cleanup is best-effort; a locked active copy invalidates the layout so the next case rebuilds at a fresh path. |
 | 15 | Eight revision-query bodies rebuild the same four-commit graph, refs, index, and reflogs through `rev_parse_repo`. | Prepare the graph once and copy it for every consumer, including mutating cases; recreate repository, branch and commit wrappers. |
 | 8 | Tree lookup bodies clone and check out `0.3.2.1` through `with_rw_repo`. | Read the historical tree directly through the existing class repository, removing clones and checkouts. |
 

git/objects/submodule/base.py

@@ -460,8 +460,8 @@ def _clone_repo(
                 except FileNotFoundError:
                     pass
                 raise
-            cls._connect_module(module_checkout_path, module_abspath)
             clone.close()
+            cls._connect_module(module_checkout_path, module_abspath)
             clone = git.Repo(module_checkout_path)
 
         return clone

test/cleanup.py

@@ -0,0 +1,81 @@
+# This module is part of GitPython and is released under the
+# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
+
+"""Best-effort disposal of isolated test directories, including on Python 3.8.
+
+Use these helpers only when discarding a directory owned by a test. Deleting or
+moving files to exercise Git behavior, or to prepare a reusable fixture, must
+still report errors.
+
+Keep this module independent of GitPython and pytest so the local test runner
+can use it before configuring the test environment.
+"""
+
+import logging
+import os
+import shutil
+import stat
+import sys
+import tempfile
+import weakref
+
+_logger = logging.getLogger(__name__)
+
+
+def cleanup_directory(path):
+    """Try to remove an owned test directory; return False and log on filesystem errors."""
+    errors = []
+
+    def onerror(function, filename, exception):
+        if isinstance(exception, FileNotFoundError):
+            return
+        if sys.platform == "win32" and function in (os.unlink, os.rmdir) and isinstance(exception, PermissionError):
+            try:
+                # Git files and test directories may be read-only. Never chmod a
+                # symlink or junction target, which may be outside the owned tree.
+                if not os.lstat(filename).st_file_attributes & stat.FILE_ATTRIBUTE_REPARSE_POINT:
+                    os.chmod(filename, stat.S_IWUSR)
+                    function(filename)
+                    return
+            except FileNotFoundError:
+                return
+            except OSError as retry_error:
+                exception = retry_error
+        errors.append(exception)
+
+    try:
+        if sys.version_info >= (3, 12):
+            shutil.rmtree(path, onexc=onerror)
+        else:
+            shutil.rmtree(path, onerror=lambda function, filename, excinfo: onerror(function, filename, excinfo[1]))
+    except FileNotFoundError:
+        pass
+    except OSError as error:
+        errors.append(error)
+
+    if errors:
+        _logger.warning("Could not fully remove temporary test directory %r: %s", os.fspath(path), errors[0])
+    return not errors
+
+
+class TemporaryDirectory:
+    """A test-owned temporary directory whose cleanup cannot fail on file locks.
+
+    Supports context management, ``name``, and explicit ``cleanup()``. A finalizer
+    also attempts cleanup if a test drops the object without closing it. Explicit
+    cleanup detaches that finalizer, but may be called again to retry later.
+    """
+
+    def __init__(self, suffix=None, prefix=None, dir=None):
+        self.name = tempfile.mkdtemp(suffix=suffix, prefix=prefix, dir=dir)
+        self._finalizer = weakref.finalize(self, cleanup_directory, self.name)
+
+    def __enter__(self):
+        return self.name
+
+    def __exit__(self, *args):
+        self.cleanup()
+
+    def cleanup(self):
+        self._finalizer.detach()
+        cleanup_directory(self.name)

test/lib/helper.py

@@ -42,10 +42,10 @@
 import venv
 from typing import Union, Type, Tuple
 
-import gitdb
 import pytest
 
-from git.util import rmtree, cwd
+from git.util import cwd
+from test.cleanup import TemporaryDirectory, cleanup_directory
 
 TestCase = unittest.TestCase
 SkipTest = unittest.SkipTest
@@ -107,7 +107,10 @@ def wait(self, stderr=None):
 
 def with_rw_directory(func):
     """Create a temporary directory which can be written to, remove it if the
-    test succeeds, but leave it otherwise to aid additional debugging."""
+    test succeeds, but leave it otherwise to aid additional debugging.
+
+    Cleanup is best-effort: a locked file must not change the test result.
+    """
 
     @wraps(func)
     def wrapper(self, *args, **kwargs):
@@ -132,7 +135,7 @@ def wrapper(self, *args, **kwargs):
             # though this is not the case here unless we collect explicitly.
             gc.collect()
             if not keep:
-                rmtree(path)
+                cleanup_directory(path)
 
     return wrapper
 
@@ -179,13 +182,10 @@ def repo_creator(self):
                 raise
             finally:
                 os.chdir(prev_cwd)
-                rw_repo.git.clear_cache()
+                rw_repo.close()
                 rw_repo = None
                 if repo_dir is not None:
-                    gc.collect()
-                    gitdb.util.mman.collect()
-                    gc.collect()
-                    rmtree(repo_dir)
+                    cleanup_directory(repo_dir)
                 # END rm test repo if possible
             # END cleanup
 
@@ -257,6 +257,7 @@ def git_daemon_launched(base_path, ip, port):
             try:
                 _logger.debug("Killing git-daemon...")
                 gd.proc.kill()
+                gd.proc.wait(timeout=5)
             except Exception as ex:
                 # Either it has died (and we're here), or it won't die, again here...
                 _logger.debug("Hidden error while Killing git-daemon: %s", ex, exc_info=1)
@@ -346,17 +347,14 @@ def remote_repo_creator(self):
                             raise
 
             finally:
-                rw_repo.git.clear_cache()
-                rw_daemon_repo.git.clear_cache()
+                rw_repo.close()
+                rw_daemon_repo.close()
                 del rw_repo
                 del rw_daemon_repo
-                gc.collect()
-                gitdb.util.mman.collect()
-                gc.collect()
                 if rw_repo_dir:
-                    rmtree(rw_repo_dir)
+                    cleanup_directory(rw_repo_dir)
                 if rw_daemon_repo_dir:
-                    rmtree(rw_daemon_repo_dir)
+                    cleanup_directory(rw_daemon_repo_dir)
             # END cleanup
 
         # END bare repo creator
@@ -416,7 +414,7 @@ def setUpClass(cls):
 
     @classmethod
     def tearDownClass(cls):
-        cls.rorepo.git.clear_cache()
+        cls.rorepo.close()
         cls.rorepo.git = None
 
     def _make_file(self, rela_path, data, repo=None):
@@ -496,7 +494,7 @@ def symlinks_supported() -> bool:
     Developer Mode or SeCreateSymbolicLinkPrivilege, and an unprivileged process gets
     OSError (WinError 1314) instead.
     """
-    with tempfile.TemporaryDirectory(prefix="gitpython-symlink-check-") as temp_dir:
+    with TemporaryDirectory(prefix="gitpython-symlink-check-") as temp_dir:
         link_path = osp.join(temp_dir, "link")
         try:
             os.symlink("missing-target", link_path)

test/performance/lib.py

@@ -10,8 +10,8 @@
 
 from git import Repo
 from git.db import GitCmdObjectDB, GitDB
-from git.util import rmtree
 
+from test.cleanup import cleanup_directory
 from test.lib import TestBase
 
 # { Invariants
@@ -51,9 +51,9 @@ def setUp(self):
         self.puregitrorepo = Repo(repo_path, odbt=GitDB, search_parent_directories=True)
 
     def tearDown(self):
-        self.gitrorepo.git.clear_cache()
+        self.gitrorepo.close()
         self.gitrorepo = None
-        self.puregitrorepo.git.clear_cache()
+        self.puregitrorepo.close()
         self.puregitrorepo = None
 
 
@@ -72,12 +72,15 @@ def setUp(self):
 
     def tearDown(self):
         super().tearDown()
+        dirname = None
         if self.gitrwrepo is not None:
-            rmtree(self.gitrwrepo.working_dir)
-            self.gitrwrepo.git.clear_cache()
+            dirname = self.gitrwrepo.working_dir
+            self.gitrwrepo.close()
         self.gitrwrepo = None
-        self.puregitrwrepo.git.clear_cache()
+        self.puregitrwrepo.close()
         self.puregitrwrepo = None
+        if dirname is not None:
+            cleanup_directory(dirname)
 
 
 # } END base classes

test/performance/test_commit.py

@@ -21,6 +21,7 @@
 class TestPerformance(TestBigRepoRW, TestCommitSerialization):
     def tearDown(self):
         gc.collect()
+        super().tearDown()
 
     # ref with about 100 commits in its history.
     ref_100 = "0.1.6"

test/run-local.py

@@ -9,12 +9,13 @@
 import socket
 import subprocess
 import sys
-import tempfile
+
+from cleanup import TemporaryDirectory
 
 
 def main():
     root = Path(__file__).resolve().parent.parent
-    with tempfile.TemporaryDirectory(prefix="gitpython-local-tests-") as directory:
+    with TemporaryDirectory(prefix="gitpython-local-tests-") as directory:
         temporary = Path(directory)
         config = temporary / "gitconfig"
         config.write_text("[user]\nname = GitPython Tests\nemail = tests@example.invalid\n", encoding="utf-8")
@@ -51,7 +52,11 @@ def git(*args, cwd=root):
         with socket.socket() as listener:
             listener.bind(("127.0.0.1", 0))
             env["GIT_PYTHON_TEST_GIT_DAEMON_PORT"] = str(listener.getsockname()[1])
-        return subprocess.call([sys.executable, "-m", "pytest", *sys.argv[1:]], cwd=root, env=env)
+        return subprocess.call(
+            [sys.executable, "-m", "pytest", "--basetemp", str(temporary / "pytest"), *sys.argv[1:]],
+            cwd=root,
+            env=env,
+        )
 
 
 if __name__ == "__main__":