Merge pull request #2064 from TheButlah/master

Keep empty in-project venv dir when recreating venv to allow for docker volumes

poetry/utils/env.py

@@ -15,6 +15,7 @@ from typing import Dict
 from typing import List
 from typing import Optional
 from typing import Tuple
+from typing import Union
 
 import tomlkit
 
@@ -403,7 +404,7 @@ class EnvManager(object):
                 if venv.path.name == python:
                     # Exact virtualenv name
                     if not envs_file.exists():
-                        self.remove_venv(str(venv.path))
+                        self.remove_venv(venv.path)
 
                         return venv
 
@@ -413,7 +414,7 @@ class EnvManager(object):
 
                     current_env = envs.get(base_env_name)
                     if not current_env:
-                        self.remove_venv(str(venv.path))
+                        self.remove_venv(venv.path)
 
                         return venv
 
@@ -421,7 +422,7 @@ class EnvManager(object):
                         del envs[base_env_name]
                         envs_file.write(envs)
 
-                    self.remove_venv(str(venv.path))
+                    self.remove_venv(venv.path)
 
                     return venv
 
@@ -475,7 +476,7 @@ class EnvManager(object):
                     del envs[base_env_name]
                     envs_file.write(envs)
 
-        self.remove_venv(str(venv))
+        self.remove_venv(venv)
 
         return VirtualEnv(venv)
 
@@ -621,7 +622,7 @@ class EnvManager(object):
                 "Creating virtualenv <c1>{}</> in {}".format(name, str(venv_path))
             )
 
-            self.build_venv(str(venv), executable=executable)
+            self.build_venv(venv, executable=executable)
         else:
             if force:
                 if not env.is_sane():
@@ -633,8 +634,8 @@ class EnvManager(object):
                 io.write_line(
                     "Recreating virtualenv <c1>{}</> in {}".format(name, str(venv))
                 )
-                self.remove_venv(str(venv))
-                self.build_venv(str(venv), executable=executable)
+                self.remove_venv(venv)
+                self.build_venv(venv, executable=executable)
             elif io.is_very_verbose():
                 io.write_line("Virtualenv <c1>{}</> already exists.".format(name))
 
@@ -657,7 +658,9 @@ class EnvManager(object):
         return VirtualEnv(venv)
 
     @classmethod
-    def build_venv(cls, path, executable=None):
+    def build_venv(
+        cls, path, executable=None
+    ):  # type: (Union[Path,str], Optional[str]) -> ()
         if executable is not None:
             # Create virtualenv by using an external executable
             try:
@@ -682,21 +685,41 @@ class EnvManager(object):
                 use_symlinks = True
 
             builder = EnvBuilder(with_pip=True, symlinks=use_symlinks)
-            builder.create(path)
+            builder.create(str(path))
         except ImportError:
             try:
                 # We fallback on virtualenv for Python 2.7
                 from virtualenv import create_environment
 
-                create_environment(path)
+                create_environment(str(path))
             except ImportError:
                 # since virtualenv>20 we have to use cli_run
                 from virtualenv import cli_run
 
-                cli_run([path])
+                cli_run([str(path)])
 
-    def remove_venv(self, path):  # type: (str) -> None
-        shutil.rmtree(path)
+    @classmethod
+    def remove_venv(cls, path):  # type: (Union[Path,str]) -> None
+        if isinstance(path, str):
+            path = Path(path)
+        assert path.is_dir()
+        try:
+            shutil.rmtree(str(path))
+            return
+        except OSError as e:
+            # Continue only if e.errno == 16
+            if e.errno != 16:  # ERRNO 16: Device or resource busy
+                raise e
+
+        # Delete all files and folders but the toplevel one. This is because sometimes
+        # the venv folder is mounted by the OS, such as in a docker volume. In such
+        # cases, an attempt to delete the folder itself will result in an `OSError`.
+        # See https://github.com/python-poetry/poetry/pull/2064
+        for file_path in path.iterdir():
+            if file_path.is_file() or file_path.is_symlink():
+                file_path.unlink()
+            elif file_path.is_dir():
+                shutil.rmtree(str(file_path))
 
     def get_base_prefix(self):  # type: () -> Path
         if hasattr(sys, "real_prefix"):

tests/console/commands/env/test_use.py

 import os
-import shutil
 import sys
 
+from typing import Optional
+from typing import Union
+
 import tomlkit
 
 from cleo.testers import CommandTester
@@ -16,12 +18,8 @@ from poetry.utils.toml_file import TomlFile
 CWD = Path(__file__).parent.parent / "fixtures" / "simple_project"
 
 
-def build_venv(path, executable=None):
-    os.mkdir(path)
-
-
-def remove_venv(path):
-    shutil.rmtree(path)
+def build_venv(path, executable=None):  # type: (Union[Path,str], Optional[str]) -> ()
+    os.mkdir(str(path))
 
 
 def check_output_wrapper(version=Version.parse("3.7.1")):
@@ -62,7 +60,7 @@ def test_activate_activates_non_existing_virtualenv_no_envs_file(app, tmp_dir, m
     )
 
     m.assert_called_with(
-        os.path.join(tmp_dir, "{}-py3.7".format(venv_name)), executable="python3.7"
+        Path(tmp_dir) / "{}-py3.7".format(venv_name), executable="python3.7"
     )
 
     envs_file = TomlFile(Path(tmp_dir) / "envs.toml")

tests/utils/test_env.py

@@ -2,6 +2,9 @@ import os
 import shutil
 import sys
 
+from typing import Optional
+from typing import Union
+
 import pytest
 import tomlkit
 
@@ -83,12 +86,8 @@ def test_env_get_in_project_venv(manager, poetry):
     shutil.rmtree(str(venv.path))
 
 
-def build_venv(path, executable=None):
-    os.mkdir(path)
-
-
-def remove_venv(path):
-    shutil.rmtree(path)
+def build_venv(path, executable=None):  # type: (Union[Path,str], Optional[str]) -> ()
+    os.mkdir(str(path))
 
 
 def check_output_wrapper(version=Version.parse("3.7.1")):
@@ -125,7 +124,7 @@ def test_activate_activates_non_existing_virtualenv_no_envs_file(
     venv_name = EnvManager.generate_env_name("simple-project", str(poetry.file.parent))
 
     m.assert_called_with(
-        os.path.join(tmp_dir, "{}-py3.7".format(venv_name)), executable="python3.7"
+        Path(tmp_dir) / "{}-py3.7".format(venv_name), executable="python3.7"
     )
 
     envs_file = TomlFile(Path(tmp_dir) / "envs.toml")
@@ -243,7 +242,7 @@ def test_activate_activates_different_virtualenv_with_envs_file(
     env = manager.activate("python3.6", NullIO())
 
     m.assert_called_with(
-        os.path.join(tmp_dir, "{}-py3.6".format(venv_name)), executable="python3.6"
+        Path(tmp_dir) / "{}-py3.6".format(venv_name), executable="python3.6"
     )
 
     assert envs_file.exists()
@@ -289,17 +288,15 @@ def test_activate_activates_recreates_for_different_patch(
         "poetry.utils.env.EnvManager.build_venv", side_effect=build_venv
     )
     remove_venv_m = mocker.patch(
-        "poetry.utils.env.EnvManager.remove_venv", side_effect=remove_venv
+        "poetry.utils.env.EnvManager.remove_venv", side_effect=EnvManager.remove_venv
     )
 
     env = manager.activate("python3.7", NullIO())
 
     build_venv_m.assert_called_with(
-        os.path.join(tmp_dir, "{}-py3.7".format(venv_name)), executable="python3.7"
-    )
-    remove_venv_m.assert_called_with(
-        os.path.join(tmp_dir, "{}-py3.7".format(venv_name))
+        Path(tmp_dir) / "{}-py3.7".format(venv_name), executable="python3.7"
     )
+    remove_venv_m.assert_called_with(Path(tmp_dir) / "{}-py3.7".format(venv_name))
 
     assert envs_file.exists()
     envs = envs_file.read()
@@ -340,7 +337,7 @@ def test_activate_does_not_recreate_when_switching_minor(
         "poetry.utils.env.EnvManager.build_venv", side_effect=build_venv
     )
     remove_venv_m = mocker.patch(
-        "poetry.utils.env.EnvManager.remove_venv", side_effect=remove_venv
+        "poetry.utils.env.EnvManager.remove_venv", side_effect=EnvManager.remove_venv
     )
 
     env = manager.activate("python3.6", NullIO())
@@ -535,6 +532,54 @@ def test_remove_also_deactivates(tmp_dir, manager, poetry, config, mocker):
     assert venv_name not in envs
 
 
+def test_remove_keeps_dir_if_not_deleteable(tmp_dir, manager, poetry, config, mocker):
+    # Ensure we empty rather than delete folder if its is an active mount point.
+    # See https://github.com/python-poetry/poetry/pull/2064
+    config.merge({"virtualenvs": {"path": str(tmp_dir)}})
+
+    venv_name = manager.generate_env_name("simple-project", str(poetry.file.parent))
+    venv_path = Path(tmp_dir) / "{}-py3.6".format(venv_name)
+    venv_path.mkdir()
+
+    folder1_path = venv_path / "folder1"
+    folder1_path.mkdir()
+
+    file1_path = folder1_path / "file1"
+    file1_path.touch(exist_ok=False)
+
+    file2_path = venv_path / "file2"
+    file2_path.touch(exist_ok=False)
+
+    mocker.patch(
+        "poetry.utils._compat.subprocess.check_output",
+        side_effect=check_output_wrapper(Version.parse("3.6.6")),
+    )
+
+    original_rmtree = shutil.rmtree
+
+    def err_on_rm_venv_only(path, *args, **kwargs):
+        print(path)
+        if path == str(venv_path):
+            raise OSError(16, "Test error")  # ERRNO 16: Device or resource busy
+        else:
+            original_rmtree(path)
+
+    m = mocker.patch("shutil.rmtree", side_effect=err_on_rm_venv_only)
+
+    venv = manager.remove("{}-py3.6".format(venv_name))
+
+    m.assert_any_call(str(venv_path))
+
+    assert venv_path == venv.path
+    assert venv_path.exists()
+
+    assert not folder1_path.exists()
+    assert not file1_path.exists()
+    assert not file2_path.exists()
+
+    m.side_effect = original_rmtree  # Avoid teardown using `err_on_rm_venv_only`
+
+
 def test_env_has_symlinks_on_nix(tmp_dir, tmp_venv):
     venv_available = False
     try:
@@ -584,7 +629,7 @@ def test_create_venv_tries_to_find_a_compatible_python_executable_using_generic_
     manager.create_venv(NullIO())
 
     m.assert_called_with(
-        str(Path("/foo/virtualenvs/{}-py3.7".format(venv_name))), executable="python3"
+        Path("/foo/virtualenvs/{}-py3.7".format(venv_name)), executable="python3"
     )
 
 
@@ -608,7 +653,7 @@ def test_create_venv_tries_to_find_a_compatible_python_executable_using_specific
     manager.create_venv(NullIO())
 
     m.assert_called_with(
-        str(Path("/foo/virtualenvs/{}-py3.8".format(venv_name))), executable="python3.8"
+        Path("/foo/virtualenvs/{}-py3.8".format(venv_name)), executable="python3.8"
     )
 
 
@@ -691,12 +736,10 @@ def test_create_venv_uses_patch_version_to_detect_compatibility(
 
     assert not check_output.called
     m.assert_called_with(
-        str(
         Path(
             "/foo/virtualenvs/{}-py{}.{}".format(
                 venv_name, version.major, version.minor
             )
-            )
         ),
         executable=None,
     )
@@ -730,12 +773,10 @@ def test_create_venv_uses_patch_version_to_detect_compatibility_with_executable(
 
     assert check_output.called
     m.assert_called_with(
-        str(
         Path(
             "/foo/virtualenvs/{}-py{}.{}".format(
                 venv_name, version.major, version.minor - 1
             )
-            )
         ),
         executable="python{}.{}".format(version.major, version.minor - 1),
     )
@@ -768,9 +809,7 @@ def test_activate_with_in_project_setting_does_not_fail_if_no_venvs_dir(
 
     manager.activate("python3.7", NullIO())
 
-    m.assert_called_with(
-        os.path.join(str(poetry.file.parent), ".venv"), executable="python3.7"
-    )
+    m.assert_called_with(poetry.file.parent / ".venv", executable="python3.7")
 
     envs_file = TomlFile(Path(tmp_dir) / "virtualenvs" / "envs.toml")
     assert not envs_file.exists()