LocalPackageResolver: Accept multiple pyenvs

This adds the core logic for FawltyDeps to become able to handle
multiple --pyenv arguments.

Instead of taking an optional 'pyenv_path', we now take a 'pyenv_paths'
set of paths; the new behavior is an extension of the old:

 - An empty set (corresponds to pyenv_path == None) implies that we use
   sys.path to look up packages.
 - A single-element set (corresponds to pyenv_path != None) will use the
   Python environment found at that path instead.
 - A multi-element set (new in this commit) causes packages to be looked
   up in _all_ the given Python environments. For packages that occur in
   more than one of these environments, the resulting Package object
   will _merge_ all the imports found among the various copies of this
   package (similar to how we deal with the same package occurring
   across multiple user-defined mappings).

Author: jherland

fawltydeps/packages.py

@@ -9,7 +9,7 @@
 from contextlib import contextmanager
 from dataclasses import dataclass, field
 from pathlib import Path
-from typing import Dict, Iterable, Iterator, List, Optional, Set, Tuple
+from typing import AbstractSet, Dict, Iterable, Iterator, List, Optional, Set, Tuple
 
 # importlib_metadata is gradually graduating into the importlib.metadata stdlib
 # module, however we rely on internal functions and recent (and upcoming)
@@ -181,24 +181,29 @@ class LocalPackageResolver(BasePackageResolver):
 
     def __init__(
         self,
-        pyenv_path: Optional[Path] = None,
+        pyenv_paths: AbstractSet[Path] = frozenset(),
         description_template: str = "Python env at {path}",
     ) -> None:
         """Lookup packages installed in the given virtualenv.
 
-        Default to the current python environment if `pyenv_path` is not given
-        (or None).
+        Default to the current python environment if `pyenv_paths` is empty
+        (the default).
 
         Use importlib_metadata to look up the mapping between packages and their
         provided import names.
         """
         self.description_template = description_template
-        if pyenv_path is not None:
-            self.pyenv_path = self.determine_package_dir(pyenv_path)
-            if self.pyenv_path is None:
-                raise ValueError(f"Could not find a Python env at {pyenv_path}!")
-        else:
-            self.pyenv_path = None
+        self.package_dirs: Set[Path] = set()  # empty => use sys.path instead
+        if pyenv_paths:
+            package_dirs = {
+                (path, self.determine_package_dir(path)) for path in pyenv_paths
+            }
+            for pyenv_path, package_dir in package_dirs:
+                if package_dir is None:
+                    logger.warning(f"Could not find a Python env at {pyenv_path}!")
+            self.package_dirs = {dir for _, dir in package_dirs if dir is not None}
+            if not self.package_dirs:
+                raise ValueError(f"Could not find any Python env in {pyenv_paths}!")
         # We enumerate packages for pyenv_path _once_ and cache the result here:
         self._packages: Optional[Dict[str, Package]] = None
 
@@ -233,49 +238,61 @@ def determine_package_dir(cls, path: Path) -> Optional[Path]:
         # Try again with parent directory
         return None if path.parent == path else cls.determine_package_dir(path.parent)
 
-    @property
-    @calculated_once
-    def packages(self) -> Dict[str, Package]:
-        """Return mapping of package names to Package objects.
+    def _from_one_env(
+        self, env_paths: List[str]
+    ) -> Iterator[Tuple[CustomMapping, str]]:
+        """Return package-name-to-import-names mapping from one Python env.
 
-        This enumerates the available packages in the given Python environment
-        (or the current Python environment) _once_, and caches the result for
-        the remainder of this object's life.
+        This is roughly equivalent to calling importlib_metadata's
+        packages_distributions(), except that instead of implicitly querying
+        sys.path, we query env_paths instead.
+
+        Also, we are able to return packages that map to zero import names,
+        whereas packages_distributions() cannot.
         """
-        if self.pyenv_path is None:
-            paths = sys.path  # use current Python environment
-        else:
-            paths = [str(self.pyenv_path)]
-
-        ret = {}
-        # We're reaching into the internals of importlib_metadata here,
-        # which Mypy is not overly fond of. Roughly what we're doing here
-        # is calling packages_distributions(), but on a possibly different
-        # environment than the current one (i.e. sys.path).
-        # Note that packages_distributions() is not able to return packages
-        # that map to zero import names.
-        context = DistributionFinder.Context(path=paths)  # type: ignore
+        seen = set()  # Package names (normalized) seen earlier in env_paths
+
+        # We're reaching into the internals of importlib_metadata here, which
+        # Mypy is not overly fond of, hence lots of "type: ignore"...
+        context = DistributionFinder.Context(path=env_paths)  # type: ignore
         for dist in MetadataPathFinder().find_distributions(context):  # type: ignore
             normalized_name = Package.normalize_name(dist.name)
             parent_dir = dist.locate_file("")
-            if normalized_name in ret:
+            if normalized_name in seen:
                 # We already found another instance of this package earlier in
-                # the given paths. Assume that the earlier package is what
-                # Python's import machinery will choose, and that this later
-                # package is not interesting
+                # env_paths. Assume that the earlier package is what Python's
+                # import machinery will choose, and that this later package is
+                # not interesting.
                 logger.debug(f"Skip {dist.name} {dist.version} under {parent_dir}")
                 continue
 
             logger.debug(f"Found {dist.name} {dist.version} under {parent_dir}")
-            imports = set(
+            seen.add(normalized_name)
+            imports = list(
                 _top_level_declared(dist)  # type: ignore
                 or _top_level_inferred(dist)  # type: ignore
             )
             description = self.description_template.format(path=parent_dir)
-            package = Package(dist.name, {description: imports})
-            ret[normalized_name] = package
+            yield {dist.name: imports}, description
 
-        return ret
+    @property
+    @calculated_once
+    def packages(self) -> Dict[str, Package]:
+        """Return mapping of package names to Package objects.
+
+        This enumerates the available packages in the given Python environment
+        (or the current Python environment) _once_, and caches the result for
+        the remainder of this object's life.
+        """
+
+        def _pyenvs() -> Iterator[Tuple[CustomMapping, str]]:
+            if not self.package_dirs:  # No pyenvs given, fall back to sys.path
+                yield from self._from_one_env(sys.path)
+            else:
+                for package_dir in self.package_dirs:
+                    yield from self._from_one_env([str(package_dir)])
+
+        return accumulate_mappings(_pyenvs())
 
     def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
         """Convert package names to locally available Package objects.
@@ -339,7 +356,7 @@ def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]:
         logger.info("Installing dependencies into a new temporary Python environment.")
         with self.temp_installed_requirements(sorted(package_names)) as venv_dir:
             description_template = "Temporarily pip-installed"
-            local_resolver = LocalPackageResolver(venv_dir, description_template)
+            local_resolver = LocalPackageResolver({venv_dir}, description_template)
             return local_resolver.lookup_packages(package_names)
 
 
@@ -396,7 +413,9 @@ def resolve_dependencies(
         )
     )
 
-    resolvers.append(LocalPackageResolver(pyenv_path))
+    resolvers.append(
+        LocalPackageResolver(set() if pyenv_path is None else {pyenv_path})
+    )
     if install_deps:
         resolvers += [TemporaryPipInstallResolver()]
     # Identity mapping being at the bottom of the resolvers stack ensures that

tests/test_local_env.py

@@ -81,13 +81,13 @@ def test_determine_package_dir__various_paths_in_pypackages(write_tmp_files, sub
 
 def test_local_env__empty_venv__has_no_packages(tmp_path):
     venv.create(tmp_path, with_pip=False)
-    lpl = LocalPackageResolver(tmp_path)
+    lpl = LocalPackageResolver({tmp_path})
     assert lpl.packages == {}
 
 
 def test_local_env__default_venv__contains_pip_and_setuptools(tmp_path):
     venv.create(tmp_path, with_pip=True)
-    lpl = LocalPackageResolver(tmp_path)
+    lpl = LocalPackageResolver({tmp_path})
     # We cannot do a direct comparison, as different Python/pip/setuptools
     # versions differ in exactly which packages are provided. The following
     # is a subset that we can expect across all of our supported versions.
@@ -136,6 +136,35 @@ def test_local_env__prefers_first_package_found_in_sys_path(isolate_default_reso
     }
 
 
+def test_local_env__multiple_pyenvs__can_find_packages_in_all(fake_venv):
+    venv_dir1, site_dir1 = fake_venv({"some_module": {"some_module"}})
+    venv_dir2, site_dir2 = fake_venv({"other-module": {"other_module"}})
+    lpl = LocalPackageResolver({venv_dir1, venv_dir2})
+    assert lpl.lookup_packages({"some_module", "other-module"}) == {
+        "some_module": Package(
+            "some_module", {f"Python env at {site_dir1}": {"some_module"}}
+        ),
+        "other-module": Package(
+            "other_module", {f"Python env at {site_dir2}": {"other_module"}}
+        ),
+    }
+
+
+def test_local_env__multiple_pyenvs__merges_imports_for_same_package(fake_venv):
+    venv_dir1, site_dir1 = fake_venv({"some_module": {"first_import"}})
+    venv_dir2, site_dir2 = fake_venv({"some_module": {"second_import"}})
+    lpl = LocalPackageResolver({venv_dir1, venv_dir2})
+    assert lpl.lookup_packages({"some_module"}) == {
+        "some_module": Package(
+            "some_module",
+            {
+                f"Python env at {site_dir1}": {"first_import"},
+                f"Python env at {site_dir2}": {"second_import"},
+            },
+        ),
+    }
+
+
 def test_resolve_dependencies__in_empty_venv__reverts_to_id_mapping(tmp_path):
     venv.create(tmp_path, with_pip=False)
     id_mapping = IdentityMapping()

tests/test_real_projects.py

@@ -44,7 +44,7 @@ def verify_requirements(venv_path: Path, requirements: List[str]) -> None:
         for req in requirements
         if "python_version" not in req  # we don't know how to parse these (yet)
     }
-    resolved = LocalPackageResolver(venv_path).lookup_packages(deps)
+    resolved = LocalPackageResolver({venv_path}).lookup_packages(deps)
     assert all(dep in resolved for dep in deps)
 
 

tests/test_resolver.py

@@ -76,7 +76,10 @@ def generate_expected_resolved_deps(
     if locally_installed_deps:
         ret.update(
             {
-                dep: Package(dep, {"Python env at {site_packages}": set(imports)})
+                dep: Package(
+                    Package.normalize_name(dep),
+                    {"Python env at {site_packages}": set(imports)},
+                )
                 for dep, imports in locally_installed_deps.items()
             }
         )