Skip to content

Commit 93bc2fb

Browse files
codexByron
authored andcommitted
test: reuse prepared submodule rejection layouts
Repeated rejection cases recreated nested metadata, separate Git directories, symlink layouts, and retained modules before every operation. Prepare each layout once and restore a complete filesystem copy at its original path. Keeping that path preserves absolute gitfiles, linked-worktree registration, and symlink targets without patching Git metadata. Remove each active copy in fixture cleanup, including after a failed assertion. Move preparation out of the six test bodies while preserving their security snapshots and destination assertions. A deliberate-mutation check verifies that restoration removes extra files and restores content through an absolute symlink. The 51 variants passed on GixPython first in 12.34 seconds, then CLI in 26.23 seconds wall time; the restoration check passed on both. Ruff passed. The `test-cygwin` fast job also exposed `PermissionError` when Python 3.9 copied symlink metadata. Copy ordinary files with `shutil.copytree` and recreate directory links afterward, retaining link targets without metadata operations. The restoration regression simulates this denial. All 52 affected cases passed with GixPython first (13.98 s), then CLI (27.38 s); Ruff passed.
1 parent 7e0d142 commit 93bc2fb

1 file changed

Lines changed: 201 additions & 68 deletions

File tree

‎test/test_submodule.py‎

Lines changed: 201 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
from git.objects.submodule.base import Submodule
2929
from git.objects.submodule.root import RootModule, RootUpdateProgress
3030
from git.repo.fun import find_submodule_git_dir, touch
31-
from git.util import HIDE_WINDOWS_KNOWN_ERRORS, cwd, join_path_native, to_native_path_linux
31+
from git.util import HIDE_WINDOWS_KNOWN_ERRORS, cwd, join_path_native, to_native_path_linux, rmtree
3232

3333
from test.lib import TestBase, with_rw_directory, with_rw_repo, PathLikeMock
3434

@@ -191,6 +191,162 @@ def test_movable_submodule_copies_are_isolated(movable_submodule, movable_submod
191191
assert "copy-only" not in original.heads
192192

193193

194+
def _copy_rejection_layout(source, destination):
195+
"""Copy files normally, then recreate directory links without symlink metadata."""
196+
links = {}
197+
198+
def ignore_links(directory, names):
199+
ignored = []
200+
for name in names:
201+
path = Path(directory, name)
202+
if path.is_symlink():
203+
links[path.relative_to(source)] = os.readlink(path)
204+
ignored.append(name)
205+
return ignored
206+
207+
shutil.copytree(source, destination, ignore=ignore_links)
208+
for relative, target in links.items():
209+
Path(destination, relative).symlink_to(target, target_is_directory=True)
210+
211+
212+
@pytest.fixture(scope="module")
213+
def prepared_rejection_layout(tmp_path_factory, movable_submodule_baseline):
214+
"""Restore private copies at a stable path, preserving absolute Git links."""
215+
layouts = {}
216+
217+
@contextlib.contextmanager
218+
def restore(key, prepare):
219+
if key not in layouts:
220+
directory = tmp_path_factory.mktemp("rejection-layout")
221+
root = directory / "active"
222+
shutil.copytree(movable_submodule_baseline("logical-name"), root / "parent", symlinks=True)
223+
with git.Repo(root / "parent") as parent:
224+
prepare(root, parent.submodules[0])
225+
baseline = directory / "baseline"
226+
_copy_rejection_layout(root, baseline)
227+
rmtree(root)
228+
layouts[key] = root, baseline
229+
root, baseline = layouts[key]
230+
_copy_rejection_layout(baseline, root)
231+
try:
232+
yield root
233+
finally:
234+
rmtree(root)
235+
236+
return restore
237+
238+
239+
def test_prepared_rejection_layout_restores_private_state(prepared_rejection_layout, monkeypatch):
240+
copystat = shutil.copystat
241+
242+
def reject_symlink_metadata(source, destination, **kwargs):
243+
if Path(source).is_symlink():
244+
raise PermissionError("Cygwin cannot copy this symlink's metadata")
245+
return copystat(source, destination, **kwargs)
246+
247+
monkeypatch.setattr(shutil, "copystat", reject_symlink_metadata)
248+
249+
def prepare(root, sm):
250+
(root / "link").symlink_to(root / "parent/module", target_is_directory=True)
251+
252+
with prepared_rejection_layout("restore-check", prepare) as root:
253+
(root / "parent/module/file").write_text("changed", encoding="utf-8")
254+
(root / "extra").touch()
255+
assert not root.exists()
256+
with prepared_rejection_layout("restore-check", prepare) as restored:
257+
assert restored == root
258+
assert (root / "link/file").read_text(encoding="utf-8") == "content"
259+
assert not (root / "extra").exists()
260+
261+
262+
@pytest.fixture
263+
def nested_metadata_submodule(prepared_rejection_layout):
264+
def prepare(root, sm):
265+
sm.repo.create_submodule("other", "other", sm.url).module().close()
266+
267+
with prepared_rejection_layout("nested", prepare) as root, git.Repo(root / "parent") as parent:
268+
yield parent.submodules[0], parent.submodules[1]
269+
270+
271+
@pytest.fixture
272+
def existing_nested_metadata(prepared_rejection_layout, state):
273+
def prepare(root, sm):
274+
checkout = Path(sm.repo.working_tree_dir, "new")
275+
with git.Repo.clone_from(
276+
sm.url,
277+
checkout,
278+
separate_git_dir=str(Path(sm.repo.git_dir, "modules", sm.name, "child")),
279+
allow_unsafe_options=True,
280+
):
281+
pass
282+
if state == "retained":
283+
shutil.rmtree(checkout)
284+
285+
with prepared_rejection_layout(("existing-nested", state), prepare) as root, git.Repo(root / "parent") as parent:
286+
yield parent.submodules[0]
287+
288+
289+
@pytest.fixture
290+
def separate_metadata_submodule(prepared_rejection_layout):
291+
def prepare(root, sm):
292+
checkout = root / "separate"
293+
with git.Repo.init(checkout, separate_git_dir=str(checkout / "metadata"), allow_unsafe_options=True) as parent:
294+
module = parent.create_submodule("module", "module", sm.url)
295+
parent.index.commit("Add submodule")
296+
module.module().close()
297+
298+
with prepared_rejection_layout("separate", prepare) as root, git.Repo(root / "separate") as parent:
299+
yield parent.submodules[0]
300+
301+
302+
@pytest.fixture
303+
def intermediate_symlink_submodule(prepared_rejection_layout, target_kind):
304+
def prepare(root, sm):
305+
checkout = Path(sm.repo.working_tree_dir)
306+
target = checkout / "target" if target_kind == "internal" else root / "outside"
307+
if target_kind != "dangling":
308+
target.mkdir()
309+
(checkout / "nested").mkdir()
310+
link = checkout / "nested/link"
311+
link.symlink_to(
312+
target if target_kind == "absolute" else os.path.relpath(target, link.parent), target_is_directory=True
313+
)
314+
sm.repo.index.add(["nested/link"])
315+
sm.repo.index.commit("Record layout")
316+
317+
with prepared_rejection_layout(("intermediate-link", target_kind), prepare) as root, git.Repo(
318+
root / "parent"
319+
) as parent:
320+
target = root / "parent/target" if target_kind == "internal" else root / "outside"
321+
yield parent.submodules[0], target
322+
323+
324+
@pytest.fixture
325+
def leaf_metadata_submodule(prepared_rejection_layout):
326+
def prepare(root, sm):
327+
sm.repo.git.worktree("add", "--detach", str(root / "worktree"))
328+
with git.Repo(root / "worktree") as parent:
329+
module = parent.submodules[0]
330+
module.update(init=True)
331+
module.module().close()
332+
333+
with prepared_rejection_layout("leaf-metadata", prepare) as root, git.Repo(root / "worktree") as parent:
334+
yield parent.submodules[0]
335+
336+
337+
@pytest.fixture
338+
def retained_metadata_submodule(prepared_rejection_layout):
339+
def prepare(root, sm):
340+
sm.remove(force=True)
341+
342+
with prepared_rejection_layout("retained", prepare) as root, git.Repo(root / "parent") as parent:
343+
# The submodule is no longer in the index; retain its original identity.
344+
with GitConfigParser(Path(parent.git_dir, "modules/logical-name/config")) as reader:
345+
url = reader.get_value('remote "origin"', "url")
346+
sm = parent.head.commit.tree["module"]
347+
yield Submodule(parent, sm.binsha, name="logical-name", path="module", url=url)
348+
349+
194350
def _move_snapshot(submodule):
195351
"""Capture index, configuration, and path state to detect side effects of rejected moves."""
196352
parent = submodule.repo
@@ -265,35 +421,32 @@ def test_add_rejects_metadata_checkout_without_writing_files(movable_submodule):
265421
@pytest.mark.parametrize("metadata_name", ["metadata", "MeTaDaTa"])
266422
@pytest.mark.parametrize("operation", ["add", "clone", "update", "move"])
267423
def test_submodule_rejects_checkout_in_separate_metadata(
268-
movable_submodule, tmp_path, absolute_path, metadata_name, operation
424+
separate_metadata_submodule, absolute_path, metadata_name, operation
269425
):
270-
root = tmp_path / "separate"
271-
with git.Repo.init(root, separate_git_dir=str(root / "metadata"), allow_unsafe_options=True) as parent:
272-
if not (root / metadata_name).is_dir():
273-
pytest.skip("Requires a case-insensitive filesystem")
274-
sm = parent.create_submodule("module", "module", movable_submodule.url)
275-
parent.index.commit("Add submodule")
276-
before = _move_snapshot(sm)
277-
paths = set(root.rglob("*"))
278-
path = root / metadata_name / "new" if absolute_path else f"{metadata_name}/new"
279-
with pytest.raises(ValueError, match="Git metadata"):
280-
if operation == "add":
281-
parent.create_submodule("new", path, sm.url)
282-
elif operation == "clone":
283-
Submodule._clone_repo(parent, sm.url, path, "new")
284-
elif operation == "update":
285-
Submodule(parent, sm.binsha, name="new", path=path, url=sm.url).update(init=True)
286-
else:
287-
sm.move(path)
288-
assert _move_snapshot(sm) == before
289-
assert set(root.rglob("*")) == paths
426+
sm = separate_metadata_submodule
427+
parent = sm.repo
428+
root = Path(parent.working_tree_dir)
429+
if not (root / metadata_name).is_dir():
430+
pytest.skip("Requires a case-insensitive filesystem")
431+
before = _move_snapshot(sm)
432+
paths = set(root.rglob("*"))
433+
path = root / metadata_name / "new" if absolute_path else f"{metadata_name}/new"
434+
with pytest.raises(ValueError, match="Git metadata"):
435+
if operation == "add":
436+
parent.create_submodule("new", path, sm.url)
437+
elif operation == "clone":
438+
Submodule._clone_repo(parent, sm.url, path, "new")
439+
elif operation == "update":
440+
Submodule(parent, sm.binsha, name="new", path=path, url=sm.url).update(init=True)
441+
else:
442+
sm.move(path)
443+
assert _move_snapshot(sm) == before
444+
assert set(root.rglob("*")) == paths
290445

291446

292447
@pytest.mark.parametrize("operation", ["add", "clone", "update"])
293-
def test_submodule_rejects_nested_metadata_before_mutation(movable_submodule, operation):
294-
sm = movable_submodule
295-
other = sm.repo.create_submodule("other", "other", sm.url)
296-
other.module().close()
448+
def test_submodule_rejects_nested_metadata_before_mutation(nested_metadata_submodule, operation):
449+
sm, other = nested_metadata_submodule
297450
root = Path(sm.repo.working_tree_dir)
298451
before = _move_snapshot(sm), _move_snapshot(other)
299452
paths = set(root.rglob("*"))
@@ -310,16 +463,9 @@ def test_submodule_rejects_nested_metadata_before_mutation(movable_submodule, op
310463

311464

312465
@pytest.mark.parametrize("state", ["retained", "checked-out"])
313-
def test_update_rejects_existing_nested_metadata(movable_submodule, state):
314-
sm = movable_submodule
466+
def test_update_rejects_existing_nested_metadata(existing_nested_metadata, state):
467+
sm = existing_nested_metadata
315468
name = f"{sm.name}/child"
316-
checkout = Path(sm.repo.working_tree_dir, "new")
317-
with git.Repo.clone_from(
318-
sm.url, checkout, separate_git_dir=str(Path(sm.repo.git_dir, "modules", name)), allow_unsafe_options=True
319-
):
320-
pass
321-
if state == "retained":
322-
shutil.rmtree(checkout)
323469
before = _move_snapshot(sm)
324470
paths = set(Path(sm.repo.working_tree_dir).rglob("*"))
325471
with pytest.raises(ValueError, match="inside.*Git directory"):
@@ -428,10 +574,9 @@ def test_readd_retained_submodule_without_checkout(movable_submodule, ref_format
428574

429575

430576
@pytest.mark.parametrize("change", ["url", "clone-options", "branch", "depth"])
431-
def test_readd_retained_submodule_rejects_incompatible_options(movable_submodule, change):
432-
sm = movable_submodule
577+
def test_readd_retained_submodule_rejects_incompatible_options(retained_metadata_submodule, change):
578+
sm = retained_metadata_submodule
433579
name, path, url = sm.name, sm.path, sm.url
434-
sm.remove(force=True)
435580
root = Path(sm.repo.working_tree_dir)
436581

437582
def snapshot():
@@ -457,26 +602,16 @@ def snapshot():
457602
@pytest.mark.parametrize("configuration,module", [(True, True), (False, True), (True, False)])
458603
@pytest.mark.parametrize("absolute_path", [False, True])
459604
def test_move_rejects_intermediate_symlink(
460-
movable_submodule, tmp_path, target_kind, configuration, module, absolute_path
605+
intermediate_symlink_submodule, target_kind, configuration, module, absolute_path
461606
):
462607
"""Reject intermediate symlinks before changing repository state or their targets.
463608
464609
Cover relative and absolute destinations in every move mode, including links
465610
within the repository and dangling links, which must also be rejected.
466611
"""
467-
submodule = movable_submodule
612+
submodule, target = intermediate_symlink_submodule
468613
parent = submodule.repo
469614
root = Path(parent.working_tree_dir)
470-
target = root / "target" if target_kind == "internal" else tmp_path / "outside"
471-
if target_kind != "dangling":
472-
target.mkdir()
473-
(root / "nested").mkdir()
474-
link = root / "nested" / "link"
475-
link.symlink_to(
476-
target if target_kind == "absolute" else os.path.relpath(target, link.parent), target_is_directory=True
477-
)
478-
parent.index.add(["nested/link"])
479-
parent.index.commit("Record layout")
480615
tree = parent.git.write_tree()
481616
before = _move_snapshot(submodule)
482617
destination = root / "nested/link/new/moved" if absolute_path else "nested/link/new/moved"
@@ -510,25 +645,23 @@ def test_move_normal_destination(movable_submodule, absolute_path):
510645

511646

512647
@pytest.mark.parametrize("metadata_dir", ["git_dir", "common_dir"])
513-
def test_move_rejects_leaf_symlink_to_metadata(movable_submodule, tmp_path, metadata_dir):
514-
root = tmp_path / "worktree"
515-
movable_submodule.repo.git.worktree("add", "--detach", str(root))
516-
with git.Repo(root) as parent:
517-
assert not osp.samefile(parent.git_dir, parent.common_dir)
518-
submodule = parent.submodules[0]
519-
submodule.update(init=True)
520-
target = Path(getattr(parent, metadata_dir))
521-
destination = root / "destination"
522-
destination.symlink_to(target, target_is_directory=True)
523-
before = _move_snapshot(submodule)
524-
525-
with pytest.raises(ValueError, match="Git metadata"):
526-
submodule.move("destination", module=False)
648+
def test_move_rejects_leaf_symlink_to_metadata(leaf_metadata_submodule, metadata_dir):
649+
submodule = leaf_metadata_submodule
650+
parent = submodule.repo
651+
root = Path(parent.working_tree_dir)
652+
assert not osp.samefile(parent.git_dir, parent.common_dir)
653+
target = Path(getattr(parent, metadata_dir))
654+
destination = root / "destination"
655+
destination.symlink_to(target, target_is_directory=True)
656+
before = _move_snapshot(submodule)
527657

528-
assert _move_snapshot(submodule) == before
529-
assert destination.is_symlink()
530-
assert destination.samefile(target)
531-
assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content"
658+
with pytest.raises(ValueError, match="Git metadata"):
659+
submodule.move("destination", module=False)
660+
661+
assert _move_snapshot(submodule) == before
662+
assert destination.is_symlink()
663+
assert destination.samefile(target)
664+
assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content"
532665

533666

534667
@pytest.mark.parametrize("kind", ["empty", "nonempty", "file", "dangling"])

0 commit comments

Comments
 (0)