Skip to content

Commit 415c211

Browse files
Byroncodex
andcommitted
test: Simplify submodule path simulation and verify metadata removal
rubber stamp <!-- agent --> The Python 3.7 Windows path simulation wrapped the entire os.path module in Mock. On Windows, all six simulated removal cases hit sharing violations that the default error handling converted into skipped tests. Use a SimpleNamespace copy of the path module and replace only realpath with abspath. Patch only the submodule module's osp binding so pathlib keeps its own resolver and other path operations remain ordinary calls. This retains Python 3.7 compatibility and allows the removal cases to run successfully without Windows permission-error suppression. Before removing a submodule, resolve and verify its metadata directory, then assert that removal deletes it as well as the checkout. Checking only the checkout could miss metadata left behind through a directory symlink. Validation: all 57 focused submodule compatibility and process-cleanup tests passed on Windows with Python 3.10 and HIDE_WINDOWS_KNOWN_ERRORS=0. Ruff 0.16.5 lint and formatting checks and git diff --check passed. Native Python 3.7 was unavailable; its realpath behavior is simulated. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
1 parent 2bfd829 commit 415c211

2 files changed

Lines changed: 53 additions & 3 deletions

File tree

git/objects/submodule/base.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,6 +1226,12 @@ def remove(
12261226
Doesn't work atomically, as failure to remove any part of the submodule will
12271227
leave an inconsistent state.
12281228
1229+
:note:
1230+
Metadata-directory aliases under ``.git/modules`` are retained. A link
1231+
directly to the deleted repository becomes dangling; adding or initializing
1232+
the submodule again recreates its target. Linked parent directories remain
1233+
available to sibling submodules.
1234+
12291235
:raise git.exc.InvalidGitRepositoryError:
12301236
Thrown if the repository cannot be deleted.
12311237

test/test_submodule.py

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import shutil
1010
import sys
1111
import tempfile
12+
from types import SimpleNamespace
1213
from unittest import mock, skipUnless
1314

1415
import pytest
@@ -316,8 +317,9 @@ def test_submodule_allows_symlink_above_worktree(
316317
def metadata_realpath(request):
317318
"""Model Python 3.7 on Windows without altering pathlib's own resolver."""
318319
if request.param:
319-
with mock.patch("git.objects.submodule.base.osp", wraps=osp) as paths:
320-
paths.realpath.side_effect = osp.abspath
320+
paths = SimpleNamespace(**vars(osp))
321+
paths.realpath = osp.abspath
322+
with mock.patch("git.objects.submodule.base.osp", paths):
321323
yield
322324
else:
323325
yield
@@ -361,7 +363,7 @@ def test_submodule_allows_existing_metadata_symlinks(
361363
362364
Cover linked metadata directories, gitfiles, configs, and internal aliases.
363365
Update, move, and rename must retain a usable checkout; forced removal must
364-
still remove it.
366+
still remove both the checkout and the resolved metadata directory.
365367
"""
366368
sm = movable_submodule
367369
sm.rename("nested/module")
@@ -387,8 +389,20 @@ def test_submodule_allows_existing_metadata_symlinks(
387389
sm.repo.git.config("--file", str(modules / "nested/module/config"), "core.worktree", str(root / "module"))
388390
assert sm.module_exists()
389391
if operation == "remove":
392+
metadata_dir = (modules / "nested/module").resolve()
393+
assert metadata_dir.is_dir()
394+
url = sm.url
390395
sm.remove(force=True)
391396
assert not (root / "module").exists()
397+
assert not metadata_dir.exists()
398+
if kind in ("modules", "intermediate", "alias"):
399+
assert link.is_symlink() and link.is_dir()
400+
replacement = Submodule.add(sm.repo, "nested/module", "module", url)
401+
if kind == "leaf":
402+
assert link.is_symlink() and link.is_dir()
403+
assert target.is_dir()
404+
with replacement.module() as module:
405+
assert Path(module.git.rev_parse("--show-toplevel")).resolve() == (root / "module").resolve()
392406
return
393407
if operation == "update":
394408
sm.update()
@@ -407,6 +421,36 @@ def test_submodule_allows_existing_metadata_symlinks(
407421
assert Path(sm.abspath, "file").read_text() == "content"
408422

409423

424+
@pytest.mark.parametrize("kind", ["modules", "intermediate", "leaf"])
425+
def test_remove_linked_metadata_keeps_siblings_and_can_reinitialize(
426+
movable_submodule, tmp_path, kind, metadata_realpath
427+
):
428+
sm = movable_submodule
429+
sm.rename("nested/module")
430+
sibling = Submodule.add(sm.repo, "nested/sibling", "sibling", sm.url)
431+
sm.repo.index.commit("Add sibling")
432+
modules = Path(sm.repo.git_dir) / "modules"
433+
link = {"modules": modules, "intermediate": modules / "nested", "leaf": modules / "nested/module"}[kind]
434+
target = tmp_path / "outside"
435+
link.rename(target)
436+
link.symlink_to(target, target_is_directory=True)
437+
for child in (sm, sibling):
438+
sm.repo.git.config("--file", str(modules / child.name / "config"), "core.worktree", str(child.abspath))
439+
440+
sm.remove(force=True, configuration=False)
441+
442+
assert link.is_symlink()
443+
assert link.exists() == (kind != "leaf")
444+
with sibling.module() as module:
445+
assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sibling.abspath).resolve()
446+
assert Path(sibling.abspath, "file").read_text() == "content"
447+
sm.update(init=True)
448+
assert link.is_symlink() and link.is_dir()
449+
assert Path(sm.abspath, "file").read_text() == "content"
450+
with sm.module() as module:
451+
assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sm.abspath).resolve()
452+
453+
410454
class TestRootProgress(RootUpdateProgress):
411455
"""Just prints messages, for now without checking the correctness of the states"""
412456

0 commit comments

Comments
 (0)