| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 24b6f95 commit 43a43cd
2 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -255,7 +255,7 @@ def _config_parser( | |||
| 255 | 255 | # END handle parent_commit | |
| 256 | 256 | fp_module: Union[str, BytesIO] | |
| 257 | 257 | if not repo.bare and parent_matches_head and repo.working_tree_dir: | |
| 258 | - fp_module = osp.join(repo.working_tree_dir, cls.k_modules_file) | ||
| 258 | + fp_module = cls._checked_abspath(repo.working_tree_dir, cls.k_modules_file) | ||
| 259 | 259 | else: | |
| 260 | 260 | assert parent_commit is not None, "need valid parent_commit in bare repositories" | |
| 261 | 261 | try: | |
@@ -322,7 +322,7 @@ def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> Path | |||
| 322 | 322 | if cls._need_gitfile_submodules(parent_repo.git): | |
| 323 | 323 | return osp.join(parent_repo.git_dir, "modules", name) | |
| 324 | 324 | if parent_repo.working_tree_dir: | |
| 325 | - return osp.join(parent_repo.working_tree_dir, path) | ||
| 325 | + return cls._checked_abspath(parent_repo.working_tree_dir, cls._to_relative_path(parent_repo, path)) | ||
| 326 | 326 | raise NotADirectoryError() | |
| 327 | 327 | ||
| 328 | 328 | @classmethod | |
@@ -361,8 +361,11 @@ def _clone_repo( | |||
| 361 | 361 | :param kwargs: | |
| 362 | 362 | Additional arguments given to :manpage:`git-clone(1)`. | |
| 363 | 363 | """ | |
| 364 | + path = cls._to_relative_path(repo, path) | ||
| 365 | + if repo.working_tree_dir is None: | ||
| 366 | + raise NotADirectoryError("Submodules require a working tree") | ||
| 367 | + module_checkout_path = cls._checked_abspath(repo.working_tree_dir, path) | ||
| 364 | 368 | module_abspath = cls._module_abspath(repo, path, name) | |
| 365 | - module_checkout_path = module_abspath | ||
| 366 | 369 | if cls._need_gitfile_submodules(repo.git): | |
| 367 | 370 | if not allow_unsafe_options: | |
| 368 | 371 | Git.check_unsafe_options(Git._option_candidates([], kwargs), repo.unsafe_git_clone_options) | |
@@ -377,7 +380,6 @@ def _clone_repo( | |||
| 377 | 380 | module_abspath_dir = osp.dirname(module_abspath) | |
| 378 | 381 | if not osp.isdir(module_abspath_dir): | |
| 379 | 382 | os.makedirs(module_abspath_dir) | |
| 380 | - module_checkout_path = osp.join(repo.working_tree_dir, path) # type: ignore[arg-type] | ||
| 381 | 383 | ||
| 382 | 384 | if url.startswith("../"): | |
| 383 | 385 | remote_name = cast("RemoteReference", repo.active_branch.tracking_branch()).remote_name | |
@@ -423,16 +425,23 @@ def abspath(self) -> PathLike: | |||
| 423 | 425 | ||
| 424 | 426 | def _checkout_abspath(self, relative_path: PathLike, allow_final_symlink: bool = False) -> PathLike: | |
| 425 | 427 | """Check a checkout path already normalized by :meth:`_to_relative_path`.""" | |
| 426 | - path = self.repo.working_tree_dir | ||
| 427 | - if path is None: | ||
| 428 | + return self._checked_abspath(self.repo.working_tree_dir, relative_path, allow_final_symlink) | ||
| 429 | + | ||
| 430 | + @classmethod | ||
| 431 | + def _checked_abspath( | ||
| 432 | + cls, root: Union[PathLike, None], relative_path: PathLike, allow_final_symlink: bool = False | ||
| 433 | + ) -> str: | ||
| 434 | + """Reject symlinks below a trusted root before accessing submodule paths.""" | ||
| 435 | + if root is None: | ||
| 428 | 436 | raise NotADirectoryError("Submodules require a working tree") | |
| 429 | - components = os.fspath(relative_path).split("/") | ||
| 437 | + path = os.fspath(root) | ||
| 438 | + components = to_native_path_linux(relative_path).split("/") | ||
| 430 | 439 | for index, component in enumerate(components): | |
| 431 | - path = join_path_native(path, component) | ||
| 440 | + path = os.fspath(join_path_native(path, component)) | ||
| 432 | 441 | if allow_final_symlink and index == len(components) - 1: | |
| 433 | 442 | break | |
| 434 | 443 | if osp.islink(path): | |
| 435 | - raise ValueError("Submodule checkout path %r contains a symbolic link" % relative_path) | ||
| 444 | + raise ValueError("Submodule path %r contains a symbolic link" % relative_path) | ||
| 436 | 445 | return path | |
| 437 | 446 | ||
| 438 | 447 | @classmethod | |
@@ -458,14 +467,18 @@ def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_ab | |||
| 458 | 467 | :param module_abspath: | |
| 459 | 468 | Absolute path to the bare repository. | |
| 460 | 469 | """ | |
| 470 | + # Git resolves metadata symlinks before interpreting core.worktree. | ||
| 471 | + module_abspath = osp.realpath(module_abspath) | ||
| 472 | + working_tree_dir = osp.realpath(working_tree_dir) | ||
| 461 | 473 | git_file = osp.join(working_tree_dir, ".git") | |
| 474 | + module_config = osp.join(module_abspath, "config") | ||
| 462 | 475 | rela_path = osp.relpath(module_abspath, start=working_tree_dir) | |
| 463 | 476 | if sys.platform == "win32" and osp.isfile(git_file): | |
| 464 | 477 | os.remove(git_file) | |
| 465 | 478 | with open(git_file, "wb") as fp: | |
| 466 | 479 | fp.write(("gitdir: %s" % rela_path).encode(defenc)) | |
| 467 | 480 | ||
| 468 | - with GitConfigParser(osp.join(module_abspath, "config"), read_only=False, merge_includes=False) as writer: | ||
| 481 | + with GitConfigParser(module_config, read_only=False, merge_includes=False) as writer: | ||
| 469 | 482 | writer.set_value( | |
| 470 | 483 | "core", | |
| 471 | 484 | "worktree", | |
@@ -576,6 +589,8 @@ def add( | |||
| 576 | 589 | name, | |
| 577 | 590 | url="invalid-temporary", | |
| 578 | 591 | ) | |
| 592 | + cls._checked_abspath(repo.working_tree_dir, cls.k_modules_file) | ||
| 593 | + sm._checkout_abspath(path) | ||
| 579 | 594 | if sm.exists(): | |
| 580 | 595 | # Reretrieve submodule from tree. | |
| 581 | 596 | try: | |
@@ -1067,6 +1082,10 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = | |||
| 1067 | 1082 | return self | |
| 1068 | 1083 | # END handle no change | |
| 1069 | 1084 | ||
| 1085 | + if configuration: | ||
| 1086 | + self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) | ||
| 1087 | + # Validate the source before removing the destination. | ||
| 1088 | + cur_path = self.abspath | ||
| 1070 | 1089 | module_checkout_abspath = self._checkout_abspath(module_checkout_path, allow_final_symlink=True) | |
| 1071 | 1090 | if osp.isfile(module_checkout_abspath): | |
| 1072 | 1091 | raise ValueError("Cannot move repository onto a file: %s" % module_checkout_abspath) | |
@@ -1099,7 +1118,6 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = | |||
| 1099 | 1118 | # END handle module | |
| 1100 | 1119 | ||
| 1101 | 1120 | # Move the module into place if possible. | |
| 1102 | - cur_path = self.abspath | ||
| 1103 | 1121 | renamed_module = False | |
| 1104 | 1122 | if module and osp.exists(cur_path): | |
| 1105 | 1123 | os.renames(cur_path, module_checkout_abspath) | |
@@ -1201,6 +1219,8 @@ def remove( | |||
| 1201 | 1219 | # END handle parameters | |
| 1202 | 1220 | ||
| 1203 | 1221 | self._validated_name(self.name) | |
| 1222 | + if configuration: | ||
| 1223 | + self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) | ||
| 1204 | 1224 | # Recursively remove children of this submodule. | |
| 1205 | 1225 | nc = 0 | |
| 1206 | 1226 | for csm in self.children(): | |
@@ -1219,7 +1239,7 @@ def remove( | |||
| 1219 | 1239 | ################################ | |
| 1220 | 1240 | if module and self.module_exists(): | |
| 1221 | 1241 | mod = self.module() | |
| 1222 | - git_dir = mod.git_dir | ||
| 1242 | + git_dir = osp.realpath(mod.git_dir) | ||
| 1223 | 1243 | if force: | |
| 1224 | 1244 | # Take the fast lane and just delete everything in our module path. | |
| 1225 | 1245 | # TODO: If we run into permission problems, we have a highly | |
@@ -1460,6 +1480,9 @@ def rename(self, new_name: str) -> "Submodule": | |||
| 1460 | 1480 | ||
| 1461 | 1481 | self._validated_name(self.name) | |
| 1462 | 1482 | self._validated_name(new_name) | |
| 1483 | + destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) | ||
| 1484 | + mod = self.module() | ||
| 1485 | + self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) | ||
| 1463 | 1486 | ||
| 1464 | 1487 | # .git/config | |
| 1465 | 1488 | with self.repo.config_writer() as pw: | |
@@ -1476,9 +1499,7 @@ def rename(self, new_name: str) -> "Submodule": | |||
| 1476 | 1499 | self._name = new_name | |
| 1477 | 1500 | ||
| 1478 | 1501 | # .git/modules | |
| 1479 | - mod = self.module() | ||
| 1480 | 1502 | if mod.has_separate_working_tree(): | |
| 1481 | - destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) | ||
| 1482 | 1503 | source_dir = mod.git_dir | |
| 1483 | 1504 | # Let's be sure the submodule name is not so obviously tied to a directory. | |
| 1484 | 1505 | if str(destination_module_abspath).startswith(str(mod.git_dir)): | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -176,6 +176,171 @@ def test_move_leaf_symlink_compatibility(movable_submodule, tmp_path, kind): | |||
| 176 | 176 | assert not target.exists() | |
| 177 | 177 | ||
| 178 | 178 | ||
| 179 | + @pytest.mark.parametrize("leaf", [False, True]) | ||
| 180 | + @pytest.mark.parametrize("dangling", [False, True]) | ||
| 181 | + @pytest.mark.parametrize("operation", ["add", "clone"]) | ||
| 182 | + @pytest.mark.parametrize("gitfile", [False, True]) | ||
| 183 | + def test_clone_rejects_checkout_symlinks(movable_submodule, tmp_path, leaf, dangling, operation, gitfile): | ||
| 184 | + """Reject checkout symlinks before add or clone creates metadata or touches the target. | ||
| 185 | + | ||
| 186 | + Cover leaf and intermediate links, including dangling targets, with both | ||
| 187 | + embedded and separate Git directories. | ||
| 188 | + """ | ||
| 189 | + sm = movable_submodule | ||
| 190 | + root = Path(sm.repo.working_tree_dir) | ||
| 191 | + target = tmp_path / "outside" | ||
| 192 | + if not dangling: | ||
| 193 | + target.mkdir() | ||
| 194 | + (root / "link").symlink_to(target, target_is_directory=True) | ||
| 195 | + path = "link" if leaf else "link/new/module" | ||
| 196 | + before = _move_snapshot(sm) | ||
| 197 | + with mock.patch.object(Submodule, "_need_gitfile_submodules", return_value=gitfile): | ||
| 198 | + with pytest.raises(ValueError, match="contains a symbolic link"): | ||
| 199 | + if operation == "add": | ||
| 200 | + Submodule.add(sm.repo, "new", path, sm.url) | ||
| 201 | + else: | ||
| 202 | + Submodule._clone_repo(sm.repo, sm.url, path, "new") | ||
| 203 | + assert _move_snapshot(sm) == before | ||
| 204 | + assert not (Path(sm.repo.git_dir) / "modules/new").exists() | ||
| 205 | + assert not target.exists() if dangling else list(target.iterdir()) == [] | ||
| 206 | + | ||
| 207 | + | ||
| 208 | + @pytest.mark.parametrize("link_kind", ["gitmodules", "checkout"]) | ||
| 209 | + @pytest.mark.parametrize("operation", ["update", "move", "rename", "remove"]) | ||
| 210 | + def test_submodule_rejects_checkout_and_gitmodules_symlinks(movable_submodule, tmp_path, link_kind, operation): | ||
| 211 | + """Reject operations on symlinked checkouts or .gitmodules without side effects. | ||
| 212 | + | ||
| 213 | + Update, move, rename, and forced removal must preserve the external target, | ||
| 214 | + repository configuration, index, checkout, and any existing move destination. | ||
| 215 | + """ | ||
| 216 | + sm = movable_submodule | ||
| 217 | + sm.rename("nested/module") | ||
| 218 | + root = Path(sm.repo.working_tree_dir) | ||
| 219 | + path = root / (".gitmodules" if link_kind == "gitmodules" else "module") | ||
| 220 | + target = tmp_path / "outside" | ||
| 221 | + path.rename(target) | ||
| 222 | + path.symlink_to(target, target_is_directory=target.is_dir()) | ||
| 223 | + before = ( | ||
| 224 | + {p.relative_to(target): p.read_bytes() for p in target.rglob("*") if p.is_file()} | ||
| 225 | + if target.is_dir() | ||
| 226 | + else target.read_bytes() | ||
| 227 | + ) | ||
| 228 | + config = Path(sm.repo.git_dir, "config").read_bytes() | ||
| 229 | + index = Path(sm.repo.index.path).read_bytes() | ||
| 230 | + gitmodules = (root / ".gitmodules").read_bytes() | ||
| 231 | + (root / "moved").mkdir() | ||
| 232 | + with pytest.raises(ValueError, match="contains a symbolic link"): | ||
| 233 | + if operation == "update": | ||
| 234 | + sm.update() | ||
| 235 | + elif operation == "move": | ||
| 236 | + sm.move("moved") | ||
| 237 | + elif operation == "rename": | ||
| 238 | + sm.rename("renamed") | ||
| 239 | + else: | ||
| 240 | + sm.remove(force=True) | ||
| 241 | + after = ( | ||
| 242 | + {p.relative_to(target): p.read_bytes() for p in target.rglob("*") if p.is_file()} | ||
| 243 | + if target.is_dir() | ||
| 244 | + else target.read_bytes() | ||
| 245 | + ) | ||
| 246 | + assert after == before | ||
| 247 | + assert Path(sm.repo.git_dir, "config").read_bytes() == config | ||
| 248 | + assert Path(sm.repo.index.path).read_bytes() == index | ||
| 249 | + assert (root / ".gitmodules").read_bytes() == gitmodules | ||
| 250 | + assert (root / "module/file").read_text() == "content" | ||
| 251 | + assert path.is_symlink() | ||
| 252 | + assert (root / "moved").is_dir() | ||
| 253 | + | ||
| 254 | + | ||
| 255 | + def test_submodule_allows_symlink_above_worktree(movable_submodule, tmp_path): | ||
| 256 | + """Allow adding and moving submodules when the parent is opened through a symlink.""" | ||
| 257 | + sm = movable_submodule | ||
| 258 | + alias = tmp_path / "alias" | ||
| 259 | + alias.symlink_to(sm.repo.working_tree_dir, target_is_directory=True) | ||
| 260 | + with git.Repo(alias) as parent: | ||
| 261 | + added = Submodule.add(parent, "new", "new", sm.url) | ||
| 262 | + added.move("moved") | ||
| 263 | + with added.module() as module: | ||
| 264 | + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(added.abspath).resolve() | ||
| 265 | + assert Path(added.abspath, "file").read_text() == "content" | ||
| 266 | + | ||
| 267 | + | ||
| 268 | + @pytest.mark.parametrize("operation", ["add", "reconnect", "rename"]) | ||
| 269 | + def test_submodule_allows_metadata_destination_symlinks(movable_submodule, tmp_path, operation): | ||
| 270 | + """Allow linked metadata destinations while keeping the checkout correctly connected. | ||
| 271 | + | ||
| 272 | + Adding, reconnecting after deinit, and renaming may store metadata outside the | ||
| 273 | + parent repository through a symlink under .git/modules, preserving that link. | ||
| 274 | + """ | ||
| 275 | + sm = movable_submodule | ||
| 276 | + root = Path(sm.repo.working_tree_dir) | ||
| 277 | + outside = tmp_path / "outside" | ||
| 278 | + outside.mkdir() | ||
| 279 | + link = Path(sm.repo.git_dir) / "modules/link" | ||
| 280 | + link.symlink_to(outside, target_is_directory=True) | ||
| 281 | + if operation == "rename": | ||
| 282 | + sm.rename("link/new") | ||
| 283 | + else: | ||
| 284 | + sm = Submodule.add(sm.repo, "link/new", "new", sm.url) | ||
| 285 | + if operation == "reconnect": | ||
| 286 | + sm.repo.index.commit("Add linked metadata submodule") | ||
| 287 | + sm.repo.git.submodule("deinit", "--force", "new") | ||
| 288 | + sm.update(init=True) | ||
| 289 | + assert link.is_symlink() | ||
| 290 | + assert (outside / "new/HEAD").is_file() | ||
| 291 | + with sm.module() as module: | ||
| 292 | + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sm.abspath).resolve() | ||
| 293 | + assert Path(sm.abspath, "file").read_text() == "content" | ||
| 294 | + assert (root / ".gitmodules").is_file() | ||
| 295 | + | ||
| 296 | + | ||
| 297 | + @pytest.mark.parametrize("kind", ["modules", "intermediate", "leaf", "gitfile", "config", "alias"]) | ||
| 298 | + @pytest.mark.parametrize("operation", ["update", "move", "rename", "remove"]) | ||
| 299 | + def test_submodule_allows_existing_metadata_symlinks(movable_submodule, tmp_path, kind, operation): | ||
| 300 | + """Keep submodule operations compatible with existing symlinks in Git metadata. | ||
| 301 | + | ||
| 302 | + Cover linked metadata directories, gitfiles, configs, and internal aliases. | ||
| 303 | + Update, move, and rename must retain a usable checkout; forced removal must | ||
| 304 | + still remove it. | ||
| 305 | + """ | ||
| 306 | + sm = movable_submodule | ||
| 307 | + sm.rename("nested/module") | ||
| 308 | + root = Path(sm.repo.working_tree_dir) | ||
| 309 | + modules = Path(sm.repo.git_dir) / "modules" | ||
| 310 | + paths = { | ||
| 311 | + "modules": modules, | ||
| 312 | + "intermediate": modules / "nested", | ||
| 313 | + "leaf": modules / "nested/module", | ||
| 314 | + "gitfile": root / "module/.git", | ||
| 315 | + "config": modules / "nested/module/config", | ||
| 316 | + "alias": modules / "alias", | ||
| 317 | + } | ||
| 318 | + link = paths[kind] | ||
| 319 | + target = tmp_path / "outside" | ||
| 320 | + if kind == "alias": | ||
| 321 | + target = modules / "nested" | ||
| 322 | + (root / "module/.git").write_text("gitdir: ../.git/modules/alias/module") | ||
| 323 | + else: | ||
| 324 | + link.rename(target) | ||
| 325 | + link.symlink_to(target, target_is_directory=target.is_dir()) | ||
| 326 | + # Relocating metadata changes the base of a relative core.worktree setting. | ||
| 327 | + sm.repo.git.config("--file", str(modules / "nested/module/config"), "core.worktree", str(root / "module")) | ||
| 328 | + assert sm.module_exists() | ||
| 329 | + if operation == "remove": | ||
| 330 | + sm.remove(force=True) | ||
| 331 | + assert not (root / "module").exists() | ||
| 332 | + return | ||
| 333 | + if operation == "update": | ||
| 334 | + sm.update() | ||
| 335 | + elif operation == "move": | ||
| 336 | + sm.move("moved") | ||
| 337 | + else: | ||
| 338 | + sm.rename("renamed") | ||
| 339 | + with sm.module() as module: | ||
| 340 | + assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sm.abspath).resolve() | ||
| 341 | + assert Path(sm.abspath, "file").read_text() == "content" | ||
| 342 | + | ||
| 343 | + | ||
| 179 | 344 | class TestRootProgress(RootUpdateProgress): | |
| 180 | 345 | """Just prints messages, for now without checking the correctness of the states""" | |
| 181 | 346 | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments