| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 43a43cd commit 2bfd829
2 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -9,6 +9,7 @@ | |||
| 9 | 9 | import ntpath | |
| 10 | 10 | import os | |
| 11 | 11 | import os.path as osp | |
| 12 | + from pathlib import Path | ||
| 12 | 13 | import shlex | |
| 13 | 14 | import stat | |
| 14 | 15 | import sys | |
@@ -444,6 +445,20 @@ def _checked_abspath( | |||
| 444 | 445 | raise ValueError("Submodule path %r contains a symbolic link" % relative_path) | |
| 445 | 446 | return path | |
| 446 | 447 | ||
| 448 | + @staticmethod | ||
| 449 | + def _renames(source: PathLike, destination: PathLike) -> None: | ||
| 450 | + os.makedirs(osp.dirname(destination), exist_ok=True) | ||
| 451 | + os.rename(source, destination) | ||
| 452 | + # Match renames() cleanup, but stop before directory symlinks: Windows | ||
| 453 | + # rmdir() removes the link even when its target is nonempty. | ||
| 454 | + parent = osp.dirname(source) | ||
| 455 | + while parent and not osp.islink(parent): | ||
| 456 | + try: | ||
| 457 | + os.rmdir(parent) | ||
| 458 | + except OSError: | ||
| 459 | + break | ||
| 460 | + parent = osp.dirname(parent) | ||
| 461 | + | ||
| 447 | 462 | @classmethod | |
| 448 | 463 | def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_abspath: PathLike) -> None: | |
| 449 | 464 | """Write a ``.git`` file containing a (preferably) relative path to the actual | |
@@ -468,8 +483,9 @@ def _write_git_file_and_module_config(cls, working_tree_dir: PathLike, module_ab | |||
| 468 | 483 | Absolute path to the bare repository. | |
| 469 | 484 | """ | |
| 470 | 485 | # Git resolves metadata symlinks before interpreting core.worktree. | |
| 471 | - module_abspath = osp.realpath(module_abspath) | ||
| 472 | - working_tree_dir = osp.realpath(working_tree_dir) | ||
| 486 | + # Path.resolve() also handles Windows symlinks on Python 3.7. | ||
| 487 | + module_abspath = str(Path(module_abspath).resolve()) | ||
| 488 | + working_tree_dir = str(Path(working_tree_dir).resolve()) | ||
| 473 | 489 | git_file = osp.join(working_tree_dir, ".git") | |
| 474 | 490 | module_config = osp.join(module_abspath, "config") | |
| 475 | 491 | rela_path = osp.relpath(module_abspath, start=working_tree_dir) | |
@@ -685,7 +701,9 @@ def add( | |||
| 685 | 701 | ||
| 686 | 702 | # We deliberately assume that our head matches our index! | |
| 687 | 703 | if mrepo: | |
| 688 | - sm.binsha = mrepo.head.commit.binsha | ||
| 704 | + # Release cat-file processes before callers move the checkout on Windows. | ||
| 705 | + with mrepo: | ||
| 706 | + sm.binsha = mrepo.head.commit.binsha | ||
| 689 | 707 | index.add([sm], write=True) | |
| 690 | 708 | ||
| 691 | 709 | return sm | |
@@ -1120,7 +1138,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = | |||
| 1120 | 1138 | # Move the module into place if possible. | |
| 1121 | 1139 | renamed_module = False | |
| 1122 | 1140 | if module and osp.exists(cur_path): | |
| 1123 | - os.renames(cur_path, module_checkout_abspath) | ||
| 1141 | + self._renames(cur_path, module_checkout_abspath) | ||
| 1124 | 1142 | renamed_module = True | |
| 1125 | 1143 | ||
| 1126 | 1144 | if osp.isfile(osp.join(module_checkout_abspath, ".git")): | |
@@ -1151,7 +1169,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = | |||
| 1151 | 1169 | # END handle configuration flag | |
| 1152 | 1170 | except Exception: | |
| 1153 | 1171 | if renamed_module: | |
| 1154 | - os.renames(module_checkout_abspath, cur_path) | ||
| 1172 | + self._renames(module_checkout_abspath, cur_path) | ||
| 1155 | 1173 | # END undo module renaming | |
| 1156 | 1174 | raise | |
| 1157 | 1175 | # END handle undo rename | |
@@ -1239,7 +1257,7 @@ def remove( | |||
| 1239 | 1257 | ################################ | |
| 1240 | 1258 | if module and self.module_exists(): | |
| 1241 | 1259 | mod = self.module() | |
| 1242 | - git_dir = osp.realpath(mod.git_dir) | ||
| 1260 | + git_dir = str(Path(mod.git_dir).resolve()) | ||
| 1243 | 1261 | if force: | |
| 1244 | 1262 | # Take the fast lane and just delete everything in our module path. | |
| 1245 | 1263 | # TODO: If we run into permission problems, we have a highly | |
@@ -1504,10 +1522,10 @@ def rename(self, new_name: str) -> "Submodule": | |||
| 1504 | 1522 | # Let's be sure the submodule name is not so obviously tied to a directory. | |
| 1505 | 1523 | if str(destination_module_abspath).startswith(str(mod.git_dir)): | |
| 1506 | 1524 | tmp_dir = self._module_abspath(self.repo, self.path, str(uuid.uuid4())) | |
| 1507 | - os.renames(source_dir, tmp_dir) | ||
| 1525 | + self._renames(source_dir, tmp_dir) | ||
| 1508 | 1526 | source_dir = tmp_dir | |
| 1509 | 1527 | # END handle self-containment | |
| 1510 | - os.renames(source_dir, destination_module_abspath) | ||
| 1528 | + self._renames(source_dir, destination_module_abspath) | ||
| 1511 | 1529 | if mod.working_tree_dir: | |
| 1512 | 1530 | self._write_git_file_and_module_config(mod.working_tree_dir, destination_module_abspath) | |
| 1513 | 1531 | # END move separate git repository | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -252,21 +252,79 @@ def test_submodule_rejects_checkout_and_gitmodules_symlinks(movable_submodule, t | |||
| 252 | 252 | assert (root / "moved").is_dir() | |
| 253 | 253 | ||
| 254 | 254 | ||
| 255 | - def test_submodule_allows_symlink_above_worktree(movable_submodule, tmp_path): | ||
| 255 | + def test_add_closes_checkout_processes(movable_submodule, monkeypatch): | ||
| 256 | + """Adding a submodule must not leave a child process holding its checkout open.""" | ||
| 257 | + sm = movable_submodule | ||
| 258 | + checkout = Path(sm.repo.working_tree_dir, "new") | ||
| 259 | + execute = Git.execute | ||
| 260 | + processes = [] | ||
| 261 | + | ||
| 262 | + def capture_process(self, command, *args, **kwargs): | ||
| 263 | + result = execute(self, command, *args, **kwargs) | ||
| 264 | + if ( | ||
| 265 | + kwargs.get("as_process") | ||
| 266 | + and "cat-file" in command | ||
| 267 | + and Path(self.working_dir).resolve() == checkout.resolve() | ||
| 268 | + ): | ||
| 269 | + processes.append(result.proc) | ||
| 270 | + return result | ||
| 271 | + | ||
| 272 | + monkeypatch.setattr(Git, "execute", capture_process) | ||
| 273 | + try: | ||
| 274 | + added = Submodule.add(sm.repo, "new", "new", sm.url) | ||
| 275 | + assert processes, "The HEAD read must exercise a persistent cat-file process" | ||
| 276 | + assert all(process.poll() is not None for process in processes) | ||
| 277 | + added.move("moved") | ||
| 278 | + finally: | ||
| 279 | + for process in processes: | ||
| 280 | + if process.poll() is None: | ||
| 281 | + process.terminate() | ||
| 282 | + process.wait() | ||
| 283 | + | ||
| 284 | + | ||
| 285 | + @pytest.fixture | ||
| 286 | + def windows_directory_symlink_removal(monkeypatch): | ||
| 287 | + """Exercise Windows rmdir semantics on POSIX, where rmdir rejects symlinks.""" | ||
| 288 | + if sys.platform != "win32": | ||
| 289 | + original_rmdir = os.rmdir | ||
| 290 | + | ||
| 291 | + def rmdir(path, *args, **kwargs): | ||
| 292 | + if osp.islink(path): | ||
| 293 | + return os.unlink(path, *args, **kwargs) | ||
| 294 | + return original_rmdir(path, *args, **kwargs) | ||
| 295 | + | ||
| 296 | + monkeypatch.setattr(os, "rmdir", rmdir) | ||
| 297 | + | ||
| 298 | + | ||
| 299 | + def test_submodule_allows_symlink_above_worktree( | ||
| 300 | + movable_submodule, tmp_path, windows_directory_symlink_removal, metadata_realpath | ||
| 301 | + ): | ||
| 256 | 302 | """Allow adding and moving submodules when the parent is opened through a symlink.""" | |
| 257 | 303 | sm = movable_submodule | |
| 258 | 304 | alias = tmp_path / "alias" | |
| 259 | 305 | alias.symlink_to(sm.repo.working_tree_dir, target_is_directory=True) | |
| 260 | 306 | with git.Repo(alias) as parent: | |
| 261 | 307 | added = Submodule.add(parent, "new", "new", sm.url) | |
| 262 | 308 | added.move("moved") | |
| 309 | + assert alias.is_symlink() | ||
| 263 | 310 | with added.module() as module: | |
| 264 | 311 | assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(added.abspath).resolve() | |
| 265 | 312 | assert Path(added.abspath, "file").read_text() == "content" | |
| 266 | 313 | ||
| 267 | 314 | ||
| 315 | + @pytest.fixture(params=[False, True], ids=["native-realpath", "windows37-realpath"]) | ||
| 316 | + def metadata_realpath(request): | ||
| 317 | + """Model Python 3.7 on Windows without altering pathlib's own resolver.""" | ||
| 318 | + if request.param: | ||
| 319 | + with mock.patch("git.objects.submodule.base.osp", wraps=osp) as paths: | ||
| 320 | + paths.realpath.side_effect = osp.abspath | ||
| 321 | + yield | ||
| 322 | + else: | ||
| 323 | + yield | ||
| 324 | + | ||
| 325 | + | ||
| 268 | 326 | @pytest.mark.parametrize("operation", ["add", "reconnect", "rename"]) | |
| 269 | - def test_submodule_allows_metadata_destination_symlinks(movable_submodule, tmp_path, operation): | ||
| 327 | + def test_submodule_allows_metadata_destination_symlinks(movable_submodule, tmp_path, operation, metadata_realpath): | ||
| 270 | 328 | """Allow linked metadata destinations while keeping the checkout correctly connected. | |
| 271 | 329 | ||
| 272 | 330 | Adding, reconnecting after deinit, and renaming may store metadata outside the | |
@@ -296,7 +354,9 @@ def test_submodule_allows_metadata_destination_symlinks(movable_submodule, tmp_p | |||
| 296 | 354 | ||
| 297 | 355 | @pytest.mark.parametrize("kind", ["modules", "intermediate", "leaf", "gitfile", "config", "alias"]) | |
| 298 | 356 | @pytest.mark.parametrize("operation", ["update", "move", "rename", "remove"]) | |
| 299 | - def test_submodule_allows_existing_metadata_symlinks(movable_submodule, tmp_path, kind, operation): | ||
| 357 | + def test_submodule_allows_existing_metadata_symlinks( | ||
| 358 | + movable_submodule, tmp_path, kind, operation, windows_directory_symlink_removal, metadata_realpath | ||
| 359 | + ): | ||
| 300 | 360 | """Keep submodule operations compatible with existing symlinks in Git metadata. | |
| 301 | 361 | ||
| 302 | 362 | Cover linked metadata directories, gitfiles, configs, and internal aliases. | |
@@ -336,6 +396,12 @@ def test_submodule_allows_existing_metadata_symlinks(movable_submodule, tmp_path | |||
| 336 | 396 | sm.move("moved") | |
| 337 | 397 | else: | |
| 338 | 398 | sm.rename("renamed") | |
| 399 | + if kind == "modules" or (operation == "rename" and kind in ("intermediate", "alias")): | ||
| 400 | + assert link.is_symlink() | ||
| 401 | + assert link.is_dir() | ||
| 402 | + if operation == "rename" and kind == "leaf": | ||
| 403 | + assert (modules / "renamed").is_symlink() | ||
| 404 | + assert target.is_dir() | ||
| 339 | 405 | with sm.module() as module: | |
| 340 | 406 | assert Path(module.git.rev_parse("--show-toplevel")).resolve() == Path(sm.abspath).resolve() | |
| 341 | 407 | assert Path(sm.abspath, "file").read_text() == "content" | |
| Back | FazBrowse Home | New Git URL |
0 commit comments