From ffa6d9d3d0c7950878387739907a36f21b3b6c1b Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Fri, 17 Jan 2020 17:19:43 +0800 Subject: [PATCH 1/2] Delay TempDirectory.delete resolution to cleanup --- src/pip/_internal/utils/temp_dir.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/pip/_internal/utils/temp_dir.py b/src/pip/_internal/utils/temp_dir.py index 2d342236567..65e41bc70e2 100644 --- a/src/pip/_internal/utils/temp_dir.py +++ b/src/pip/_internal/utils/temp_dir.py @@ -107,13 +107,10 @@ def __init__( ): super(TempDirectory, self).__init__() - if path is None and delete is None: - # If we were not given an explicit directory, and we were not given - # an explicit delete option, then we'll default to deleting unless - # the tempdir_registry says otherwise. - delete = True - if _tempdir_registry: - delete = _tempdir_registry.get_delete(kind) + # If we were given an explicit directory, resolve delete option now. + # Otherwise we wait until cleanup and see what tempdir_registry says. + if path is not None and delete is None: + delete = False if path is None: path = self._create(kind) @@ -145,7 +142,14 @@ def __enter__(self): def __exit__(self, exc, value, tb): # type: (Any, Any, Any) -> None - if self.delete: + if self.delete is not None: + delete = self.delete + elif _tempdir_registry: + delete = _tempdir_registry.get_delete(self.kind) + else: + delete = True + + if delete: self.cleanup() def _create(self, kind): From 35377c995c1f56193031a8b8540b92e261b26771 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Sun, 19 Jan 2020 18:22:14 +0700 Subject: [PATCH 2/2] Add test for tempdir registry lazy eval --- tests/unit/test_utils_temp_dir.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/unit/test_utils_temp_dir.py b/tests/unit/test_utils_temp_dir.py index c395a7861c0..06055084e0c 100644 --- a/tests/unit/test_utils_temp_dir.py +++ b/tests/unit/test_utils_temp_dir.py @@ -234,3 +234,17 @@ def test_tempdir_registry(kind, delete, exists): path = d.path assert os.path.exists(path) assert os.path.exists(path) == exists + + +@pytest.mark.parametrize("should_delete", [True, False]) +def test_tempdir_registry_lazy(should_delete): + """ + Test the registry entry can be updated after a temp dir is created, + to change whether a kind should be deleted or not. + """ + with tempdir_registry() as registry: + with TempDirectory(delete=None, kind="test-for-lazy") as d: + path = d.path + registry.set_delete("test-for-lazy", should_delete) + assert os.path.exists(path) + assert os.path.exists(path) == (not should_delete)