Now keeping empty venv dir when recreating venv

poetry/puzzle/provider.py

@@ -327,7 +327,7 @@ def get_package_from_directory(
 
             try:
                 with temporary_directory() as tmp_dir:
-                    EnvManager.build_venv(tmp_dir)
+                    EnvManager.build_venv(Path(tmp_dir))
                     venv = VirtualEnv(Path(tmp_dir), Path(tmp_dir))
                     venv.run("python", "setup.py", "egg_info")
             except EnvCommandError:

poetry/utils/env.py

@@ -403,7 +403,7 @@ def remove(self, python):  # type: (str) -> Env
                 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,15 +413,15 @@ def remove(self, python):  # type: (str) -> Env
 
                     current_env = envs.get(base_env_name)
                     if not current_env:
-                        self.remove_venv(str(venv.path))
+                        self.remove_venv(venv.path)
 
                         return venv
 
                     if current_env["minor"] == venv_minor:
                         del envs[base_env_name]
                         envs_file.write(envs)
 
-                    self.remove_venv(str(venv.path))
+                    self.remove_venv(venv.path)
 
                     return venv
 
@@ -475,7 +475,7 @@ def remove(self, python):  # type: (str) -> Env
                     del envs[base_env_name]
                     envs_file.write(envs)
 
-        self.remove_venv(str(venv))
+        self.remove_venv(venv)
 
         return VirtualEnv(venv)
 
@@ -621,7 +621,7 @@ def create_venv(
                 "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 +633,9 @@ def create_venv(
                 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,7 @@ def create_venv(
         return VirtualEnv(venv)
 
     @classmethod
-    def build_venv(cls, path, executable=None):
+    def build_venv(cls, path, executable=None):  # type: (Path, Optional[str]) -> None
         if executable is not None:
             # Create virtualenv by using an external executable
             try:
@@ -666,7 +667,7 @@ def build_venv(cls, path, executable=None):
                     stdin=subprocess.PIPE,
                     shell=True,
                 )
-                p.communicate(encode(CREATE_VENV_COMMAND.format(path)))
+                p.communicate(encode(CREATE_VENV_COMMAND.format(str(path))))
             except CalledProcessError as e:
                 raise EnvCommandError(e)
 
@@ -682,21 +683,33 @@ def build_venv(cls, path, executable=None):
                 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: (Path) -> None
+        assert path.is_dir()
+
+        # 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.isfile() 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_remove.py

@@ -7,6 +7,14 @@
 from .test_use import check_output_wrapper
 
 
+def assert_dir_exists_and_is_empty(path):  # type: (Path) -> None
+    # Ensure the venv folder exists but is empty.
+    # See https://github.com/python-poetry/poetry/pull/2064
+    assert path.exists()
+    assert path.is_dir()
+    assert len(list(path.iterdir())) == 0
+
+
 def test_remove_by_python_version(app, tmp_dir, mocker):
     app.poetry.config.merge({"virtualenvs": {"path": str(tmp_dir)}})
 
@@ -26,7 +34,7 @@ def test_remove_by_python_version(app, tmp_dir, mocker):
     tester.execute("3.6")
 
     assert check_output.called
-    assert not (Path(tmp_dir) / "{}-py3.6".format(venv_name)).exists()
+    assert_dir_exists_and_is_empty(Path(tmp_dir) / "{}-py3.6".format(venv_name))
 
     expected = "Deleted virtualenv: {}\n".format(
         (Path(tmp_dir) / "{}-py3.6".format(venv_name))
@@ -48,7 +56,7 @@ def test_remove_by_name(app, tmp_dir):
     tester = CommandTester(command)
     tester.execute("{}-py3.6".format(venv_name))
 
-    assert not (Path(tmp_dir) / "{}-py3.6".format(venv_name)).exists()
+    assert_dir_exists_and_is_empty(Path(tmp_dir) / "{}-py3.6".format(venv_name))
 
     expected = "Deleted virtualenv: {}\n".format(
         (Path(tmp_dir) / "{}-py3.6".format(venv_name))

tests/console/commands/env/test_use.py

@@ -1,7 +1,8 @@
 import os
-import shutil
 import sys
 
+from typing import Optional
+
 import tomlkit
 
 from cleo.testers import CommandTester
@@ -16,12 +17,8 @@
 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: (Path, Optional[str]) -> None
+    path.mkdir(exist_ok=True)
 
 
 def check_output_wrapper(version=Version.parse("3.7.1")):
@@ -62,7 +59,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,8 @@
 import shutil
 import sys
 
+from typing import Optional
+
 import pytest
 import tomlkit
 
@@ -64,18 +66,26 @@ def manager(poetry):
 def tmp_venv(tmp_dir, manager):
     venv_path = Path(tmp_dir) / "venv"
 
-    manager.build_venv(str(venv_path))
+    manager.build_venv(venv_path)
 
     venv = VirtualEnv(venv_path)
     yield venv
 
     shutil.rmtree(str(venv.path))
 
 
+def assert_dir_exists_and_is_empty(path):  # type: (Path) -> None
+    # Ensure the venv folder exists but is empty.
+    # See https://github.com/python-poetry/poetry/pull/2064
+    assert path.exists()
+    assert path.is_dir()
+    assert len(list(path.iterdir())) == 0
+
+
 def test_virtualenvs_with_spaces_in_their_path_work_as_expected(tmp_dir, manager):
     venv_path = Path(tmp_dir) / "Virtual Env"
 
-    manager.build_venv(str(venv_path))
+    manager.build_venv(venv_path)
 
     venv = VirtualEnv(venv_path)
 
@@ -95,12 +105,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: (Path, Optional[str]) -> None
+    path.mkdir(exist_ok=True)
 
 
 def check_output_wrapper(version=Version.parse("3.7.1")):
@@ -121,6 +127,7 @@ def test_activate_activates_non_existing_virtualenv_no_envs_file(
     if "VIRTUAL_ENV" in os.environ:
         del os.environ["VIRTUAL_ENV"]
 
+    print("tmp dir:", tmp_dir)
     config.merge({"virtualenvs": {"path": str(tmp_dir)}})
 
     mocker.patch(
@@ -137,7 +144,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")
@@ -255,7 +262,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()
@@ -301,17 +308,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()
@@ -352,7 +357,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())
@@ -500,7 +505,7 @@ def test_remove_by_python_version(tmp_dir, manager, poetry, config, mocker):
     venv = manager.remove("3.6")
 
     assert (Path(tmp_dir) / "{}-py3.6".format(venv_name)) == venv.path
-    assert not (Path(tmp_dir) / "{}-py3.6".format(venv_name)).exists()
+    assert_dir_exists_and_is_empty(Path(tmp_dir) / "{}-py3.6".format(venv_name))
 
 
 def test_remove_by_name(tmp_dir, manager, poetry, config, mocker):
@@ -518,7 +523,7 @@ def test_remove_by_name(tmp_dir, manager, poetry, config, mocker):
     venv = manager.remove("{}-py3.6".format(venv_name))
 
     assert (Path(tmp_dir) / "{}-py3.6".format(venv_name)) == venv.path
-    assert not (Path(tmp_dir) / "{}-py3.6".format(venv_name)).exists()
+    assert_dir_exists_and_is_empty(Path(tmp_dir) / "{}-py3.6".format(venv_name))
 
 
 def test_remove_also_deactivates(tmp_dir, manager, poetry, config, mocker):
@@ -541,7 +546,7 @@ def test_remove_also_deactivates(tmp_dir, manager, poetry, config, mocker):
     venv = manager.remove("python3.6")
 
     assert (Path(tmp_dir) / "{}-py3.6".format(venv_name)) == venv.path
-    assert not (Path(tmp_dir) / "{}-py3.6".format(venv_name)).exists()
+    assert_dir_exists_and_is_empty(Path(tmp_dir) / "{}-py3.6".format(venv_name))
 
     envs = envs_file.read()
     assert venv_name not in envs
@@ -596,7 +601,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"
     )
 
 
@@ -620,7 +625,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"
     )
 
 
@@ -703,11 +708,9 @@ 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
-                )
+        Path(
+            "/foo/virtualenvs/{}-py{}.{}".format(
+                venv_name, version.major, version.minor
             )
         ),
         executable=None,
@@ -742,11 +745,9 @@ 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
-                )
+        Path(
+            "/foo/virtualenvs/{}-py{}.{}".format(
+                venv_name, version.major, version.minor - 1
             )
         ),
         executable="python{}.{}".format(version.major, version.minor - 1),
@@ -780,9 +781,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()