diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 20bfaeae6..4803790b9 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,19 @@ Changelog ========= +3.1.63 +====== + +Security fixes for + +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-gq48-pqfc-9p58 + +If you can, also try and provide feedback on the upcoming v4 branch +https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.63 + 3.1.62 ====== diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 563b20a18..8308e4459 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -9,6 +9,7 @@ import ntpath import os import os.path as osp +from pathlib import Path import shlex import stat import sys @@ -255,7 +256,7 @@ def _config_parser( # END handle parent_commit fp_module: Union[str, BytesIO] if not repo.bare and parent_matches_head and repo.working_tree_dir: - fp_module = osp.join(repo.working_tree_dir, cls.k_modules_file) + fp_module = cls._checked_abspath(repo.working_tree_dir, cls.k_modules_file) else: assert parent_commit is not None, "need valid parent_commit in bare repositories" try: @@ -322,7 +323,7 @@ def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> Path if cls._need_gitfile_submodules(parent_repo.git): return osp.join(parent_repo.git_dir, "modules", name) if parent_repo.working_tree_dir: - return osp.join(parent_repo.working_tree_dir, path) + return cls._checked_abspath(parent_repo.working_tree_dir, cls._to_relative_path(parent_repo, path)) raise NotADirectoryError() @classmethod @@ -361,8 +362,11 @@ def _clone_repo( :param kwargs: Additional arguments given to :manpage:`git-clone(1)`. """ + path = cls._to_relative_path(repo, path) + if repo.working_tree_dir is None: + raise NotADirectoryError("Submodules require a working tree") + module_checkout_path = cls._checked_abspath(repo.working_tree_dir, path) module_abspath = cls._module_abspath(repo, path, name) - module_checkout_path = module_abspath if cls._need_gitfile_submodules(repo.git): if not allow_unsafe_options: Git.check_unsafe_options(Git._option_candidates([], kwargs), repo.unsafe_git_clone_options) @@ -373,11 +377,22 @@ def _clone_repo( repo.unsafe_git_clone_options, ) allow_unsafe_options = True + if osp.islink(module_abspath): + # Clone into the target while retaining the metadata alias. Git for + # Windows cannot initialize through a dangling directory symlink. + # Read the link explicitly for Python 3.7, and remove the Windows + # namespace prefix returned by newer Python versions for Git. + target = os.readlink(module_abspath) + if sys.platform == "win32": + if target.startswith("\\\\?\\UNC\\"): + target = "\\\\" + target[8:] + elif target.startswith("\\\\?\\"): + target = target[4:] + module_abspath = to_native_path_linux(osp.join(osp.dirname(module_abspath), target)) kwargs["separate_git_dir"] = module_abspath module_abspath_dir = osp.dirname(module_abspath) if not osp.isdir(module_abspath_dir): os.makedirs(module_abspath_dir) - module_checkout_path = osp.join(repo.working_tree_dir, path) # type: ignore[arg-type] if url.startswith("../"): remote_name = cast("RemoteReference", repo.active_branch.tracking_branch()).remote_name @@ -419,13 +434,43 @@ def abspath(self) -> PathLike: root = self.repo.working_tree_dir if root is None: return super().abspath - path = root - for component in os.fspath(self._to_relative_path(self.repo, self.path)).split("/"): - path = join_path_native(path, component) + return self._checkout_abspath(self._to_relative_path(self.repo, self.path)) + + def _checkout_abspath(self, relative_path: PathLike, allow_final_symlink: bool = False) -> PathLike: + """Check a checkout path already normalized by :meth:`_to_relative_path`.""" + return self._checked_abspath(self.repo.working_tree_dir, relative_path, allow_final_symlink) + + @classmethod + def _checked_abspath( + cls, root: Union[PathLike, None], relative_path: PathLike, allow_final_symlink: bool = False + ) -> str: + """Reject symlinks below a trusted root before accessing submodule paths.""" + if root is None: + raise NotADirectoryError("Submodules require a working tree") + path = os.fspath(root) + components = to_native_path_linux(relative_path).split("/") + for index, component in enumerate(components): + path = os.fspath(join_path_native(path, component)) + if allow_final_symlink and index == len(components) - 1: + break if osp.islink(path): - raise ValueError("Submodule checkout path %r contains a symbolic link" % self.path) + 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 @@ -449,14 +494,19 @@ def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_ab :param module_abspath: Absolute path to the bare repository. """ + # Git resolves metadata symlinks before interpreting core.worktree. + # Path.resolve() also handles Windows symlinks on Python 3.7. + module_abspath = str(Path(module_abspath).resolve()) + working_tree_dir = str(Path(working_tree_dir).resolve()) git_file = osp.join(working_tree_dir, ".git") + module_config = osp.join(module_abspath, "config") rela_path = osp.relpath(module_abspath, start=working_tree_dir) if sys.platform == "win32" and osp.isfile(git_file): os.remove(git_file) with open(git_file, "wb") as fp: fp.write(("gitdir: %s" % rela_path).encode(defenc)) - with GitConfigParser(osp.join(module_abspath, "config"), read_only=False, merge_includes=False) as writer: + with GitConfigParser(module_config, read_only=False, merge_includes=False) as writer: writer.set_value( "core", "worktree", @@ -567,6 +617,8 @@ def add( name, url="invalid-temporary", ) + cls._checked_abspath(repo.working_tree_dir, cls.k_modules_file) + sm._checkout_abspath(path) if sm.exists(): # Reretrieve submodule from tree. try: @@ -661,7 +713,9 @@ def add( # We deliberately assume that our head matches our index! if mrepo: - sm.binsha = mrepo.head.commit.binsha + # Release cat-file processes before callers move the checkout on Windows. + with mrepo: + sm.binsha = mrepo.head.commit.binsha index.add([sm], write=True) return sm @@ -1039,7 +1093,8 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = self :raise ValueError: - If the module path existed and was not empty, or was a file. + If the module path existed and was not empty, was a file, or had a + symbolic link in an intermediate component. :note: Currently the method is not atomic, and it could leave the repository in an @@ -1057,7 +1112,11 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = return self # END handle no change - module_checkout_abspath = join_path_native(str(self.repo.working_tree_dir), module_checkout_path) + if configuration: + self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) + # Validate the source before removing the destination. + cur_path = self.abspath + module_checkout_abspath = self._checkout_abspath(module_checkout_path, allow_final_symlink=True) if osp.isfile(module_checkout_abspath): raise ValueError("Cannot move repository onto a file: %s" % module_checkout_abspath) # END handle target files @@ -1089,10 +1148,9 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = # END handle module # Move the module into place if possible. - cur_path = self.abspath 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")): @@ -1123,7 +1181,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 @@ -1180,6 +1238,12 @@ def remove( Doesn't work atomically, as failure to remove any part of the submodule will leave an inconsistent state. + :note: + Metadata-directory aliases under ``.git/modules`` are retained. A link + directly to the deleted repository becomes dangling; adding or initializing + the submodule again recreates its target. Linked parent directories remain + available to sibling submodules. + :raise git.exc.InvalidGitRepositoryError: Thrown if the repository cannot be deleted. @@ -1191,6 +1255,8 @@ def remove( # END handle parameters self._validated_name(self.name) + if configuration: + self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) # Recursively remove children of this submodule. nc = 0 for csm in self.children(): @@ -1209,7 +1275,7 @@ def remove( ################################ if module and self.module_exists(): mod = self.module() - git_dir = mod.git_dir + git_dir = str(Path(mod.git_dir).resolve()) if force: # Take the fast lane and just delete everything in our module path. # TODO: If we run into permission problems, we have a highly @@ -1450,6 +1516,9 @@ def rename(self, new_name: str) -> "Submodule": self._validated_name(self.name) self._validated_name(new_name) + destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) + mod = self.module() + self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) # .git/config with self.repo.config_writer() as pw: @@ -1466,17 +1535,15 @@ def rename(self, new_name: str) -> "Submodule": self._name = new_name # .git/modules - mod = self.module() if mod.has_separate_working_tree(): - destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) source_dir = mod.git_dir # 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 diff --git a/test/test_remote.py b/test/test_remote.py index e1793214c..0212ffab8 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -1090,14 +1090,16 @@ def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo): class TestTimeouts(TestBase): @with_rw_repo("HEAD", bare=False) def test_timeout_funcs(self, repo): + # Maintenance may outlive a timed-out fetch and race with fixture cleanup. + with repo.config_writer() as config: + config.set_value("maintenance", "auto", False) + config.set_value("gc", "auto", 0) # Older Git versions use auto-gc. + # Force error code to prevent a race condition if the python thread is slow. - default = Git.AutoInterrupt._status_code_if_terminate - Git.AutoInterrupt._status_code_if_terminate = -15 - for function in ["pull", "fetch"]: # Can't get push to time out. - f = getattr(repo.remotes.origin, function) - assert f is not None # Make sure these functions exist. - _ = f() # Make sure the function runs. - with pytest.raises(GitCommandError, match="kill_after_timeout=0 s"): - f(kill_after_timeout=0) - - Git.AutoInterrupt._status_code_if_terminate = default + with mock.patch.object(Git.AutoInterrupt, "_status_code_if_terminate", -15): + for function in ["pull", "fetch"]: # Can't get push to time out. + f = getattr(repo.remotes.origin, function) + assert f is not None # Make sure these functions exist. + _ = f() # Make sure the function runs. + with pytest.raises(GitCommandError, match="kill_after_timeout=0 s"): + f(kill_after_timeout=0) diff --git a/test/test_submodule.py b/test/test_submodule.py index ca9078aac..f5070c665 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -9,6 +9,7 @@ import shutil import sys import tempfile +from types import SimpleNamespace from unittest import mock, skipUnless import pytest @@ -51,6 +52,424 @@ def _patch_git_config(name, value): yield +@pytest.fixture +def movable_submodule(tmp_path): + """Create a committed local submodule whose logical name stays fixed when moved.""" + with git.Repo.init(tmp_path / "source") as source, git.Repo.init(tmp_path / "parent") as parent: + (tmp_path / "source" / "file").write_text("content", encoding="utf-8") + source.index.add(["file"]) + source.index.commit("Create source") + with _patch_git_config("protocol.file.allow", "always"): + submodule = parent.create_submodule("logical-name", "module", source.working_tree_dir) + parent.index.commit("Create submodule") + # Release clone handles before Windows moves the checkout. + submodule.module().close() + yield submodule + + +def _move_snapshot(submodule): + """Capture index, configuration, and path state to detect side effects of rejected moves.""" + parent = submodule.repo + with submodule.module() as module: + config = Path(module.git_dir, "config").read_bytes() + return ( + Path(parent.index.path).read_bytes(), + Path(parent.working_tree_dir, ".gitmodules").read_bytes(), + Path(submodule.abspath, ".git").read_bytes(), + config, + submodule.path, + ) + + +@pytest.mark.parametrize("target_kind", ["relative", "absolute", "internal", "dangling"]) +@pytest.mark.parametrize("configuration,module", [(True, True), (False, True), (True, False)]) +@pytest.mark.parametrize("absolute_path", [False, True]) +def test_move_rejects_intermediate_symlink( + movable_submodule, tmp_path, target_kind, configuration, module, absolute_path +): + """Reject intermediate symlinks before changing repository state or their targets. + + Cover relative and absolute destinations in every move mode, including links + within the repository and dangling links, which must also be rejected. + """ + submodule = movable_submodule + parent = submodule.repo + root = Path(parent.working_tree_dir) + target = root / "target" if target_kind == "internal" else tmp_path / "outside" + if target_kind != "dangling": + target.mkdir() + (root / "nested").mkdir() + link = root / "nested" / "link" + link.symlink_to( + target if target_kind == "absolute" else os.path.relpath(target, link.parent), target_is_directory=True + ) + parent.index.add(["nested/link"]) + parent.index.commit("Record layout") + tree = parent.git.write_tree() + before = _move_snapshot(submodule) + destination = root / "nested/link/new/moved" if absolute_path else "nested/link/new/moved" + + with pytest.raises(ValueError, match="contains a symbolic link"): + submodule.move(destination, configuration=configuration, module=module) + + assert _move_snapshot(submodule) == before + assert parent.git.write_tree() == tree + assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content" + if target_kind == "dangling": + assert not target.exists() + else: + assert list(target.iterdir()) == [] + + +@pytest.mark.parametrize("absolute_path", [False, True]) +def test_move_normal_destination(movable_submodule, absolute_path): + """Allow ordinary relative and absolute moves, and make a repeated move a no-op.""" + submodule = movable_submodule + root = Path(submodule.repo.working_tree_dir) + destination = root / "nested/moved" if absolute_path else "nested/moved" + assert submodule.move(destination) is submodule + assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content" + assert not (root / "module").exists() + assert submodule.path == "nested/moved" + submodule.repo.git.write_tree() + before = _move_snapshot(submodule) + assert submodule.move(destination) is submodule + assert _move_snapshot(submodule) == before + + +@pytest.mark.parametrize("kind", ["empty", "nonempty", "file", "dangling"]) +def test_move_leaf_symlink_compatibility(movable_submodule, tmp_path, kind): + """Preserve leaf-symlink replacement without modifying the external target. + + Moving onto a link to an empty directory replaces the link; nonempty, file, + and dangling targets fail without changing repository state. Direct checkout + path access must still reject every leaf symlink. + """ + submodule = movable_submodule + root = Path(submodule.repo.working_tree_dir) + target = tmp_path / "outside" + if kind in ("empty", "nonempty"): + target.mkdir() + if kind == "nonempty": + (target / "keep").write_text("keep", encoding="utf-8") + if kind == "file": + target.write_text("keep", encoding="utf-8") + destination = root / "destination" + destination.symlink_to(target, target_is_directory=kind != "file") + with pytest.raises(ValueError, match="contains a symbolic link"): + Submodule(submodule.repo, Submodule.NULL_BIN_SHA, name="unused", path="destination").abspath + before = _move_snapshot(submodule) + if kind == "empty": + assert submodule.move("destination") is submodule + assert not destination.is_symlink() + assert (destination / "file").is_file() + assert list(target.iterdir()) == [] + else: + with pytest.raises(OSError if kind == "dangling" else ValueError): + submodule.move("destination") + assert _move_snapshot(submodule) == before + assert destination.is_symlink() + if kind == "nonempty": + assert (target / "keep").read_text(encoding="utf-8") == "keep" + elif kind == "file": + assert target.read_text(encoding="utf-8") == "keep" + else: + assert not target.exists() + + +@pytest.mark.parametrize("leaf", [False, True]) +@pytest.mark.parametrize("dangling", [False, True]) +@pytest.mark.parametrize("operation", ["add", "clone"]) +@pytest.mark.parametrize("gitfile", [False, True]) +def test_clone_rejects_checkout_symlinks(movable_submodule, tmp_path, leaf, dangling, operation, gitfile): + """Reject checkout symlinks before add or clone creates metadata or touches the target. + + Cover leaf and intermediate links, including dangling targets, with both + embedded and separate Git directories. + """ + sm = movable_submodule + root = Path(sm.repo.working_tree_dir) + target = tmp_path / "outside" + if not dangling: + target.mkdir() + (root / "link").symlink_to(target, target_is_directory=True) + path = "link" if leaf else "link/new/module" + before = _move_snapshot(sm) + with mock.patch.object(Submodule, "_need_gitfile_submodules", return_value=gitfile): + with pytest.raises(ValueError, match="contains a symbolic link"): + if operation == "add": + Submodule.add(sm.repo, "new", path, sm.url) + else: + Submodule._clone_repo(sm.repo, sm.url, path, "new") + assert _move_snapshot(sm) == before + assert not (Path(sm.repo.git_dir) / "modules/new").exists() + assert not target.exists() if dangling else list(target.iterdir()) == [] + + +@pytest.mark.parametrize("link_kind", ["gitmodules", "checkout"]) +@pytest.mark.parametrize("operation", ["update", "move", "rename", "remove"]) +def test_submodule_rejects_checkout_and_gitmodules_symlinks(movable_submodule, tmp_path, link_kind, operation): + """Reject operations on symlinked checkouts or .gitmodules without side effects. + + Update, move, rename, and forced removal must preserve the external target, + repository configuration, index, checkout, and any existing move destination. + """ + sm = movable_submodule + sm.rename("nested/module") + root = Path(sm.repo.working_tree_dir) + path = root / (".gitmodules" if link_kind == "gitmodules" else "module") + target = tmp_path / "outside" + path.rename(target) + path.symlink_to(target, target_is_directory=target.is_dir()) + before = ( + {p.relative_to(target): p.read_bytes() for p in target.rglob("*") if p.is_file()} + if target.is_dir() + else target.read_bytes() + ) + config = Path(sm.repo.git_dir, "config").read_bytes() + index = Path(sm.repo.index.path).read_bytes() + gitmodules = (root / ".gitmodules").read_bytes() + (root / "moved").mkdir() + with pytest.raises(ValueError, match="contains a symbolic link"): + if operation == "update": + sm.update() + elif operation == "move": + sm.move("moved") + elif operation == "rename": + sm.rename("renamed") + else: + sm.remove(force=True) + after = ( + {p.relative_to(target): p.read_bytes() for p in target.rglob("*") if p.is_file()} + if target.is_dir() + else target.read_bytes() + ) + assert after == before + assert Path(sm.repo.git_dir, "config").read_bytes() == config + assert Path(sm.repo.index.path).read_bytes() == index + assert (root / ".gitmodules").read_bytes() == gitmodules + assert (root / "module/file").read_text() == "content" + assert path.is_symlink() + assert (root / "moved").is_dir() + + +def test_add_closes_checkout_processes(movable_submodule, monkeypatch): + """Adding a submodule must not leave a child process holding its checkout open.""" + sm = movable_submodule + checkout = Path(sm.repo.working_tree_dir, "new") + execute = Git.execute + processes = [] + + def capture_process(self, command, *args, **kwargs): + result = execute(self, command, *args, **kwargs) + if ( + kwargs.get("as_process") + and "cat-file" in command + and Path(self.working_dir).resolve() == checkout.resolve() + ): + processes.append(result.proc) + return result + + monkeypatch.setattr(Git, "execute", capture_process) + try: + added = Submodule.add(sm.repo, "new", "new", sm.url) + assert processes, "The HEAD read must exercise a persistent cat-file process" + assert all(process.poll() is not None for process in processes) + added.move("moved") + finally: + for process in processes: + if process.poll() is None: + process.terminate() + process.wait() + + +@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, metadata_realpath +): + """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" + + +@pytest.fixture(params=[False, True], ids=["native-realpath", "windows37-realpath"]) +def metadata_realpath(request): + """Model Python 3.7 on Windows without altering pathlib's own resolver.""" + if request.param: + paths = SimpleNamespace(**vars(osp)) + paths.realpath = osp.abspath + with mock.patch("git.objects.submodule.base.osp", paths): + yield + else: + yield + + +@pytest.mark.parametrize("operation", ["add", "reconnect", "rename"]) +def test_submodule_allows_metadata_destination_symlinks(movable_submodule, tmp_path, operation, metadata_realpath): + """Allow linked metadata destinations while keeping the checkout correctly connected. + + Adding, reconnecting after deinit, and renaming may store metadata outside the + parent repository through a symlink under .git/modules, preserving that link. + """ + sm = movable_submodule + root = Path(sm.repo.working_tree_dir) + outside = tmp_path / "outside" + outside.mkdir() + link = Path(sm.repo.git_dir) / "modules/link" + link.symlink_to(outside, target_is_directory=True) + if operation == "rename": + sm.rename("link/new") + else: + sm = Submodule.add(sm.repo, "link/new", "new", sm.url) + if operation == "reconnect": + sm.repo.index.commit("Add linked metadata submodule") + sm.repo.git.submodule("deinit", "--force", "new") + sm.update(init=True) + assert link.is_symlink() + assert (outside / "new/HEAD").is_file() + 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" + assert (root / ".gitmodules").is_file() + + +@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, windows_directory_symlink_removal, metadata_realpath +): + """Keep submodule operations compatible with existing symlinks in Git metadata. + + Cover linked metadata directories, gitfiles, configs, and internal aliases. + Update, move, and rename must retain a usable checkout; forced removal must + still remove both the checkout and the resolved metadata directory. + """ + sm = movable_submodule + sm.rename("nested/module") + root = Path(sm.repo.working_tree_dir) + modules = Path(sm.repo.git_dir) / "modules" + paths = { + "modules": modules, + "intermediate": modules / "nested", + "leaf": modules / "nested/module", + "gitfile": root / "module/.git", + "config": modules / "nested/module/config", + "alias": modules / "alias", + } + link = paths[kind] + target = tmp_path / "outside" + if kind == "alias": + target = modules / "nested" + (root / "module/.git").write_text("gitdir: ../.git/modules/alias/module") + else: + link.rename(target) + link.symlink_to(target, target_is_directory=target.is_dir()) + # Relocating metadata changes the base of a relative core.worktree setting. + sm.repo.git.config("--file", str(modules / "nested/module/config"), "core.worktree", str(root / "module")) + assert sm.module_exists() + if operation == "remove": + metadata_dir = (modules / "nested/module").resolve() + assert metadata_dir.is_dir() + url = sm.url + sm.remove(force=True) + assert not (root / "module").exists() + assert not metadata_dir.exists() + if kind in ("modules", "intermediate", "alias"): + assert link.is_symlink() and link.is_dir() + replacement = Submodule.add(sm.repo, "nested/module", "module", url) + if kind == "leaf": + assert link.is_symlink() and link.is_dir() + assert target.is_dir() + with replacement.module() as module: + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == (root / "module").resolve() + return + if operation == "update": + sm.update() + elif operation == "move": + 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" + + +@pytest.mark.parametrize("kind", ["modules", "intermediate", "leaf"]) +def test_remove_linked_metadata_keeps_siblings_and_can_reinitialize( + movable_submodule, tmp_path, kind, metadata_realpath +): + sm = movable_submodule + sm.rename("nested/module") + sibling = Submodule.add(sm.repo, "nested/sibling", "sibling", sm.url) + sm.repo.index.commit("Add sibling") + modules = Path(sm.repo.git_dir) / "modules" + link = {"modules": modules, "intermediate": modules / "nested", "leaf": modules / "nested/module"}[kind] + target = tmp_path / "outside" + link.rename(target) + link.symlink_to(target, target_is_directory=True) + for child in (sm, sibling): + sm.repo.git.config("--file", str(modules / child.name / "config"), "core.worktree", str(child.abspath)) + + sm.remove(force=True, configuration=False) + + assert link.is_symlink() + assert link.exists() == (kind != "leaf") + with sibling.module() as module: + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sibling.abspath).resolve() + assert Path(sibling.abspath, "file").read_text() == "content" + sm.update(init=True) + assert link.is_symlink() and link.is_dir() + assert Path(sm.abspath, "file").read_text() == "content" + with sm.module() as module: + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sm.abspath).resolve() + + +@pytest.mark.parametrize("relative_target", [False, True], ids=["absolute-target", "relative-target"]) +def test_add_to_dangling_metadata_symlink(movable_submodule, tmp_path, metadata_realpath, relative_target): + sm = movable_submodule + link = Path(sm.repo.git_dir) / "modules/new" + target = tmp_path / "missing" / "metadata" + link.symlink_to(osp.relpath(target, link.parent) if relative_target else target, target_is_directory=True) + link_target = os.readlink(str(link)) + + added = Submodule.add(sm.repo, "new", "new", sm.url) + + assert link.is_symlink() and link.is_dir() + assert os.readlink(str(link)) == link_target + assert (target / "HEAD").is_file() + with added.module() as module: + assert Path(module.git_dir).resolve() == target.resolve() + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(added.abspath).resolve() + assert Path(added.abspath, "file").read_text() == "content" + + class TestRootProgress(RootUpdateProgress): """Just prints messages, for now without checking the correctness of the states"""