Skip to content

Commit 976b680

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 c6134ce commit 976b680

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

@@ -203,6 +203,162 @@ def test_movable_submodule_copies_are_isolated(movable_submodule, movable_submod
203203
assert "copy-only" not in original.heads
204204

205205

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

303458

304459
@pytest.mark.parametrize("operation", ["add", "clone", "update"])
305-
def test_submodule_rejects_nested_metadata_before_mutation(movable_submodule, operation):
306-
sm = movable_submodule
307-
other = sm.repo.create_submodule("other", "other", sm.url)
308-
other.module().close()
460+
def test_submodule_rejects_nested_metadata_before_mutation(nested_metadata_submodule, operation):
461+
sm, other = nested_metadata_submodule
309462
root = Path(sm.repo.working_tree_dir)
310463
before = _move_snapshot(sm), _move_snapshot(other)
311464
paths = set(root.rglob("*"))
@@ -322,16 +475,9 @@ def test_submodule_rejects_nested_metadata_before_mutation(movable_submodule, op
322475

323476

324477
@pytest.mark.parametrize("state", ["retained", "checked-out"])
325-
def test_update_rejects_existing_nested_metadata(movable_submodule, state):
326-
sm = movable_submodule
478+
def test_update_rejects_existing_nested_metadata(existing_nested_metadata, state):
479+
sm = existing_nested_metadata
327480
name = f"{sm.name}/child"
328-
checkout = Path(sm.repo.working_tree_dir, "new")
329-
with git.Repo.clone_from(
330-
sm.url, checkout, separate_git_dir=str(Path(sm.repo.git_dir, "modules", name)), allow_unsafe_options=True
331-
):
332-
pass
333-
if state == "retained":
334-
shutil.rmtree(checkout)
335481
before = _move_snapshot(sm)
336482
paths = set(Path(sm.repo.working_tree_dir).rglob("*"))
337483
with pytest.raises(ValueError, match="inside.*Git directory"):
@@ -440,10 +586,9 @@ def test_readd_retained_submodule_without_checkout(movable_submodule, ref_format
440586

441587

442588
@pytest.mark.parametrize("change", ["url", "clone-options", "branch", "depth"])
443-
def test_readd_retained_submodule_rejects_incompatible_options(movable_submodule, change):
444-
sm = movable_submodule
589+
def test_readd_retained_submodule_rejects_incompatible_options(retained_metadata_submodule, change):
590+
sm = retained_metadata_submodule
445591
name, path, url = sm.name, sm.path, sm.url
446-
sm.remove(force=True)
447592
root = Path(sm.repo.working_tree_dir)
448593

449594
def snapshot():
@@ -469,26 +614,16 @@ def snapshot():
469614
@pytest.mark.parametrize("configuration,module", [(True, True), (False, True), (True, False)])
470615
@pytest.mark.parametrize("absolute_path", [False, True])
471616
def test_move_rejects_intermediate_symlink(
472-
movable_submodule, tmp_path, target_kind, configuration, module, absolute_path
617+
intermediate_symlink_submodule, target_kind, configuration, module, absolute_path
473618
):
474619
"""Reject intermediate symlinks before changing repository state or their targets.
475620
476621
Cover relative and absolute destinations in every move mode, including links
477622
within the repository and dangling links, which must also be rejected.
478623
"""
479-
submodule = movable_submodule
624+
submodule, target = intermediate_symlink_submodule
480625
parent = submodule.repo
481626
root = Path(parent.working_tree_dir)
482-
target = root / "target" if target_kind == "internal" else tmp_path / "outside"
483-
if target_kind != "dangling":
484-
target.mkdir()
485-
(root / "nested").mkdir()
486-
link = root / "nested" / "link"
487-
link.symlink_to(
488-
target if target_kind == "absolute" else os.path.relpath(target, link.parent), target_is_directory=True
489-
)
490-
parent.index.add(["nested/link"])
491-
parent.index.commit("Record layout")
492627
tree = parent.git.write_tree()
493628
before = _move_snapshot(submodule)
494629
destination = root / "nested/link/new/moved" if absolute_path else "nested/link/new/moved"
@@ -522,25 +657,23 @@ def test_move_normal_destination(movable_submodule, absolute_path):
522657

523658

524659
@pytest.mark.parametrize("metadata_dir", ["git_dir", "common_dir"])
525-
def test_move_rejects_leaf_symlink_to_metadata(movable_submodule, tmp_path, metadata_dir):
526-
root = tmp_path / "worktree"
527-
movable_submodule.repo.git.worktree("add", "--detach", str(root))
528-
with git.Repo(root) as parent:
529-
assert not osp.samefile(parent.git_dir, parent.common_dir)
530-
submodule = parent.submodules[0]
531-
submodule.update(init=True)
532-
target = Path(getattr(parent, metadata_dir))
533-
destination = root / "destination"
534-
destination.symlink_to(target, target_is_directory=True)
535-
before = _move_snapshot(submodule)
536-
537-
with pytest.raises(ValueError, match="Git metadata"):
538-
submodule.move("destination", module=False)
660+
def test_move_rejects_leaf_symlink_to_metadata(leaf_metadata_submodule, metadata_dir):
661+
submodule = leaf_metadata_submodule
662+
parent = submodule.repo
663+
root = Path(parent.working_tree_dir)
664+
assert not osp.samefile(parent.git_dir, parent.common_dir)
665+
target = Path(getattr(parent, metadata_dir))
666+
destination = root / "destination"
667+
destination.symlink_to(target, target_is_directory=True)
668+
before = _move_snapshot(submodule)
539669

540-
assert _move_snapshot(submodule) == before
541-
assert destination.is_symlink()
542-
assert destination.samefile(target)
543-
assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content"
670+
with pytest.raises(ValueError, match="Git metadata"):
671+
submodule.move("destination", module=False)
672+
673+
assert _move_snapshot(submodule) == before
674+
assert destination.is_symlink()
675+
assert destination.samefile(target)
676+
assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content"
544677

545678

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

0 commit comments

Comments
 (0)