Improve detection of installed packages (#1786)

Author: sdispater

poetry/installation/pip_installer.py

@@ -96,7 +96,12 @@ def install(self, package, update=False):
 
             self.run(*args)
 
-    def update(self, _, target):
+    def update(self, package, target):
+        if package.source_type != target.source_type:
+            # If the source type has changed, we remove the current
+            # package to avoid perpetual updates in some cases
+            self.remove(package)
+
         self.install(target, update=True)
 
     def remove(self, package):

poetry/repositories/installed_repository.py

@@ -15,45 +15,52 @@ def load(cls, env):  # type: (Env) -> InstalledRepository
         For now, it uses the pip "freeze" command.
         """
         repo = cls()
+        seen = set()
 
-        for distribution in sorted(
-            metadata.distributions(path=env.sys_path), key=lambda d: str(d._path),
-        ):
-            name = distribution.metadata["name"]
-            version = distribution.metadata["version"]
-            package = Package(name, version, version)
-            package.description = distribution.metadata.get("summary", "")
-
-            repo.add_package(package)
-
-            path = Path(str(distribution._path))
-            is_standard_package = True
-            try:
-                path.relative_to(env.site_packages)
-            except ValueError:
-                is_standard_package = False
-
-            if is_standard_package:
-                continue
-
-            src_path = env.path / "src"
-
-            # A VCS dependency should have been installed
-            # in the src directory. If not, it's a path dependency
-            try:
-                path.relative_to(src_path)
-
-                from poetry.vcs.git import Git
-
-                git = Git()
-                revision = git.rev_parse("HEAD", src_path / package.name).strip()
-                url = git.remote_url(src_path / package.name)
-
-                package.source_type = "git"
-                package.source_url = url
-                package.source_reference = revision
-            except ValueError:
-                package.source_type = "directory"
-                package.source_url = str(path.parent)
+        for entry in env.sys_path:
+            for distribution in sorted(
+                metadata.distributions(path=[entry]), key=lambda d: str(d._path),
+            ):
+                name = distribution.metadata["name"]
+                version = distribution.metadata["version"]
+                package = Package(name, version, version)
+                package.description = distribution.metadata.get("summary", "")
+
+                if package.name in seen:
+                    continue
+
+                seen.add(package.name)
+
+                repo.add_package(package)
+
+                path = Path(str(distribution._path))
+                is_standard_package = True
+                try:
+                    path.relative_to(env.site_packages)
+                except ValueError:
+                    is_standard_package = False
+
+                if is_standard_package:
+                    continue
+
+                src_path = env.path / "src"
+
+                # A VCS dependency should have been installed
+                # in the src directory. If not, it's a path dependency
+                try:
+                    path.relative_to(src_path)
+
+                    from poetry.vcs.git import Git
+
+                    git = Git()
+                    revision = git.rev_parse("HEAD", src_path / package.name).strip()
+                    url = git.remote_url(src_path / package.name)
+
+                    package.source_type = "git"
+                    package.source_url = url
+                    package.source_reference = revision
+                except ValueError:
+                    package.source_type = "directory"
+                    package.source_url = str(path.parent)
 
         return repo

poetry/utils/env.py

@@ -705,6 +705,7 @@ def __init__(self, path, base=None):  # type: (Path, Optional[Path]) -> None
 
         self._marker_env = None
         self._pip_version = None
+        self._site_packages = None
 
     @property
     def path(self):  # type: () -> Path
@@ -760,20 +761,25 @@ def pip_version(self):
 
     @property
     def site_packages(self):  # type: () -> Path
-        # It seems that PyPy3 virtual environments
-        # have their site-packages directory at the root
-        if self._path.joinpath("site-packages").exists():
-            return self._path.joinpath("site-packages")
-
-        if self._is_windows:
-            return self._path / "Lib" / "site-packages"
-
-        return (
-            self._path
-            / "lib"
-            / "python{}.{}".format(*self.version_info[:2])
-            / "site-packages"
-        )
+        if self._site_packages is None:
+            site_packages = []
+            dist_packages = []
+            for entry in self.sys_path:
+                entry = Path(entry)
+                if entry.name == "site-packages":
+                    site_packages.append(entry)
+                elif entry.name == "dist-packages":
+                    dist_packages.append(entry)
+
+            if not site_packages and not dist_packages:
+                raise RuntimeError("Unable to find the site-packages directory")
+
+            if site_packages:
+                self._site_packages = site_packages[0]
+            else:
+                self._site_packages = dist_packages[0]
+
+        return self._site_packages
 
     @property
     def sys_path(self):  # type: () -> List[str]
@@ -1116,6 +1122,7 @@ def __init__(
         os_name="posix",
         is_venv=False,
         pip_version="19.1",
+        sys_path=None,
         **kwargs
     ):
         super(MockEnv, self).__init__(**kwargs)
@@ -1126,6 +1133,7 @@ def __init__(
         self._os_name = os_name
         self._is_venv = is_venv
         self._pip_version = Version.parse(pip_version)
+        self._sys_path = sys_path
 
     @property
     def version_info(self):  # type: () -> Tuple[int]
@@ -1147,5 +1155,12 @@ def os(self):  # type: () -> str
     def pip_version(self):
         return self._pip_version
 
+    @property
+    def sys_path(self):
+        if self._sys_path is None:
+            return super(MockEnv, self).sys_path
+
+        return self._sys_path
+
     def is_venv(self):  # type: () -> bool
         return self._is_venv

tests/masonry/builders/test_editable.py

@@ -17,8 +17,7 @@
 def test_build_should_delegate_to_pip_for_non_pure_python_packages(tmp_dir, mocker):
     move = mocker.patch("shutil.move")
     tmp_dir = Path(tmp_dir)
-    env = MockEnv(path=tmp_dir, pip_version="18.1", execute=False)
-    env.site_packages.mkdir(parents=True)
+    env = MockEnv(path=tmp_dir, pip_version="18.1", execute=False, sys_path=[])
     module_path = fixtures_dir / "extended"
 
     builder = EditableBuilder(Factory().create_poetry(module_path), env, NullIO())
@@ -33,8 +32,7 @@ def test_build_should_delegate_to_pip_for_non_pure_python_packages(tmp_dir, mock
 def test_build_should_temporarily_remove_the_pyproject_file(tmp_dir, mocker):
     move = mocker.patch("shutil.move")
     tmp_dir = Path(tmp_dir)
-    env = MockEnv(path=tmp_dir, pip_version="19.1", execute=False)
-    env.site_packages.mkdir(parents=True)
+    env = MockEnv(path=tmp_dir, pip_version="19.1", execute=False, sys_path=[])
     module_path = fixtures_dir / "extended"
 
     builder = EditableBuilder(Factory().create_poetry(module_path), env, NullIO())

tests/utils/test_env.py

@@ -31,6 +31,20 @@
 """
 
 
+class MockVirtualEnv(VirtualEnv):
+    def __init__(self, path, base=None, sys_path=None):
+        super(MockVirtualEnv, self).__init__(path, base=base)
+
+        self._sys_path = sys_path
+
+    @property
+    def sys_path(self):
+        if self._sys_path is not None:
+            return self._sys_path
+
+        return super(MockVirtualEnv, self).sys_path
+
+
 @pytest.fixture()
 def poetry(config):
     poetry = Factory().create_poetry(
@@ -786,7 +800,7 @@ def test_env_site_packages_should_find_the_site_packages_directory_if_standard(t
 
     site_packages.mkdir(parents=True)
 
-    env = VirtualEnv(Path(tmp_dir), Path(tmp_dir))
+    env = MockVirtualEnv(Path(tmp_dir), Path(tmp_dir), sys_path=[str(site_packages)])
 
     assert site_packages == env.site_packages
 
@@ -795,6 +809,37 @@ def test_env_site_packages_should_find_the_site_packages_directory_if_root(tmp_d
     site_packages = Path(tmp_dir).joinpath("site-packages")
     site_packages.mkdir(parents=True)
 
-    env = VirtualEnv(Path(tmp_dir), Path(tmp_dir))
+    env = MockVirtualEnv(Path(tmp_dir), Path(tmp_dir), sys_path=[str(site_packages)])
+
+    assert site_packages == env.site_packages
+
+
+def test_env_site_packages_should_find_the_dist_packages_directory_if_necessary(
+    tmp_dir,
+):
+    site_packages = Path(tmp_dir).joinpath("dist-packages")
+    site_packages.mkdir(parents=True)
+
+    env = MockVirtualEnv(Path(tmp_dir), Path(tmp_dir), sys_path=[str(site_packages)])
 
     assert site_packages == env.site_packages
+
+
+def test_env_site_packages_should_prefer_site_packages_over_dist_packages(tmp_dir):
+    dist_packages = Path(tmp_dir).joinpath("dist-packages")
+    dist_packages.mkdir(parents=True)
+    site_packages = Path(tmp_dir).joinpath("site-packages")
+    site_packages.mkdir(parents=True)
+
+    env = MockVirtualEnv(
+        Path(tmp_dir), Path(tmp_dir), sys_path=[str(dist_packages), str(site_packages)]
+    )
+
+    assert site_packages == env.site_packages
+
+
+def test_env_site_packages_should_raise_an_error_if_no_site_packages(tmp_dir):
+    env = MockVirtualEnv(Path(tmp_dir), Path(tmp_dir), sys_path=[])
+
+    with pytest.raises(RuntimeError):
+        env.site_packages