diff --git a/fawltydeps/packages.py b/fawltydeps/packages.py index 2eacca7b..a5b6261f 100644 --- a/fawltydeps/packages.py +++ b/fawltydeps/packages.py @@ -205,26 +205,109 @@ def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]: } -class LocalPackageResolver(BasePackageResolver): +class InstalledPackageResolver(BasePackageResolver): """Lookup imports exposed by packages installed in a Python environment.""" - def __init__( - self, - srcs: AbstractSet[PyEnvSource] = frozenset(), - use_current_env: bool = False, - ) -> None: - """Lookup packages installed in the given Python environments. + def __init__(self) -> None: + """Lookup packages installed in some Python environments. + + Uses importlib_metadata to look up the mapping between packages and + their provided import names. + """ + # We enumerate packages _once_ and cache the result here: + self._packages: Optional[Dict[str, Package]] = None + + 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 is roughly equivalent to calling importlib_metadata's + packages_distributions(), except that instead of implicitly querying + sys.path, we query the given env_paths instead. + + Also, we are able to return packages that map to zero import names, + whereas packages_distributions() cannot. + """ + 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): + normalized_name = Package.normalize_name(dist.name) + parent_dir = dist.locate_file("") + if normalized_name in seen: + # We already found another instance of this package earlier in + # 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}") + seen.add(normalized_name) + imports = list( + _top_level_declared(dist) # type: ignore + or _top_level_inferred(dist) # type: ignore + ) + yield {dist.name: imports}, str(parent_dir) + + @property + @abstractmethod + def packages(self) -> Dict[str, Package]: + """Return mapping of package names to Package objects.""" + raise NotImplementedError + + def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]: + """Convert package names to locally available Package objects. + + (Although this function generally works with _all_ locally available + packages, we apply it only to the subset that is the dependencies of + the current project.) + + Return a dict mapping package names to the Package objects that + encapsulate the package-name-to-import-names mappings. + + Only return dict entries for the packages that we manage to find in the + local environment. Omit any packages for which we're unable to determine + what imports names they provide. This applies to packages that are + missing from the local environment, or packages where we fail to + determine its provided import names. + """ + return { + name: self.packages[Package.normalize_name(name)] + for name in package_names + if Package.normalize_name(name) in self.packages + } + - If 'use_current_env' is enabled, then the current python environment - (aka. sys.path) will also be included in the lookup. +class SysPathPackageResolver(InstalledPackageResolver): + """Lookup imports exposed by packages installed in sys.path.""" + + @property + @calculated_once + def packages(self) -> Dict[str, Package]: + """Return mapping of package names to Package objects. + + This enumerates the available packages in the current Python environment + (aka. sys.path) _once_, and caches the result for the remainder of this + object's life. + """ + return accumulate_mappings(self.__class__, self._from_one_env(sys.path)) + + +class LocalPackageResolver(InstalledPackageResolver): + """Lookup imports packages installed in the given Python environments.""" + + def __init__(self, srcs: AbstractSet[PyEnvSource] = frozenset()) -> None: + """Lookup packages installed in the given Python environments. Use importlib_metadata to look up the mapping between packages and their provided import names. """ + super().__init__() self.package_dirs: Set[Path] = set(src.path for src in srcs) - self.use_current_env: bool = use_current_env - # We enumerate packages for pyenv_path _once_ and cache the result here: - self._packages: Optional[Dict[str, Package]] = None @classmethod def find_package_dirs(cls, path: Path) -> Iterator[Path]: @@ -270,42 +353,6 @@ def find_package_dirs(cls, path: Path) -> Iterator[Path]: package_dir.relative_to(path) # ValueError if not relative yield package_dir - 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 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. - """ - 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): - normalized_name = Package.normalize_name(dist.name) - parent_dir = dist.locate_file("") - if normalized_name in seen: - # We already found another instance of this package earlier in - # 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}") - seen.add(normalized_name) - imports = list( - _top_level_declared(dist) # type: ignore - or _top_level_inferred(dist) # type: ignore - ) - yield {dist.name: imports}, str(parent_dir) - @property @calculated_once def packages(self) -> Dict[str, Package]: @@ -319,33 +366,9 @@ def packages(self) -> Dict[str, Package]: def _pyenvs() -> Iterator[Tuple[CustomMapping, str]]: for package_dir in self.package_dirs: yield from self._from_one_env([str(package_dir)]) - if self.use_current_env: - yield from self._from_one_env(sys.path) return accumulate_mappings(self.__class__, _pyenvs()) - def lookup_packages(self, package_names: Set[str]) -> Dict[str, Package]: - """Convert package names to locally available Package objects. - - (Although this function generally works with _all_ locally available - packages, we apply it only to the subset that is the dependencies of - the current project.) - - Return a dict mapping package names to the Package objects that - encapsulate the package-name-to-import-names mappings. - - Only return dict entries for the packages that we manage to find in the - local environment. Omit any packages for which we're unable to determine - what imports names they provide. This applies to packages that are - missing from the local environment, or packages where we fail to - determine its provided import names. - """ - return { - name: self.packages[Package.normalize_name(name)] - for name in package_names - if Package.normalize_name(name) in self.packages - } - def pyenv_sources(*pyenv_paths: Path) -> Set[PyEnvSource]: """Helper for converting Python environment paths into PyEnvSources. @@ -506,10 +529,10 @@ def setup_resolvers( ) if pyenv_srcs: - yield LocalPackageResolver(pyenv_srcs, False) + yield LocalPackageResolver(pyenv_srcs) if use_current_env: - yield LocalPackageResolver(frozenset(), True) + yield SysPathPackageResolver() if install_deps: yield TemporaryPipInstallResolver() diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py index 2c08ca8a..3e5c0072 100644 --- a/tests/test_cmdline.py +++ b/tests/test_cmdline.py @@ -949,7 +949,7 @@ def test_check_json__no_pyenvs_found__falls_back_to_current_env(fake_project): "pip-requirements-parser": { "package_name": "pip_requirements_parser", "import_names": ["packaging_legacy_version", "pip_requirements_parser"], - "resolved_with": "LocalPackageResolver", + "resolved_with": "SysPathPackageResolver", "debug_info": { f"{site_packages}": [ "packaging_legacy_version", diff --git a/tests/test_local_env.py b/tests/test_local_env.py index d3920107..d372cd15 100644 --- a/tests/test_local_env.py +++ b/tests/test_local_env.py @@ -10,6 +10,7 @@ IdentityMapping, LocalPackageResolver, Package, + SysPathPackageResolver, pyenv_sources, resolve_dependencies, setup_resolvers, @@ -135,7 +136,7 @@ def test_local_env__default_venv__contains_pip(tmp_path): assert str(expect_location) in pip.debug_info -def test_local_env__current_venv__contains_prepared_packages(isolate_default_resolver): +def test_sys_path_env__contains_prepared_packages(isolate_default_resolver): isolate_default_resolver( { "pip": {"pip"}, @@ -145,13 +146,13 @@ def test_local_env__current_venv__contains_prepared_packages(isolate_default_res "pytest": {"pytest"}, } ) - lpl = LocalPackageResolver(use_current_env=True) + sys_path = SysPathPackageResolver() expect_package_names = ["pip", "setuptools", "isort", "pydantic", "pytest"] for package_name in expect_package_names: - assert package_name in lpl.packages + assert package_name in sys_path.packages -def test_local_env__prefers_first_package_found_in_sys_path(isolate_default_resolver): +def test_sys_path_env__prefers_first_package_found(isolate_default_resolver): # Add the same package twice, The one that ends up _first_ in sys.path is # the one that Python would end up importing, and it is therefore also the # one that we should resolve to. @@ -160,10 +161,10 @@ def test_local_env__prefers_first_package_found_in_sys_path(isolate_default_reso site_dir2 = isolate_default_resolver({"other": {"actual"}}) assert site_dir1 != site_dir2 assert sys.path[0] == str(site_dir2) - actual = LocalPackageResolver(use_current_env=True).lookup_packages({"other"}) + actual = SysPathPackageResolver().lookup_packages({"other"}) assert actual == { "other": Package( - "other", {"actual"}, LocalPackageResolver, {str(site_dir2): {"actual"}} + "other", {"actual"}, SysPathPackageResolver, {str(site_dir2): {"actual"}} ), } @@ -272,19 +273,16 @@ def test_resolve_dependencies__when_no_env_found__fallback_to_current(): # enables the use_current_env flag to setup_resolvers() in this case. resolvers = list(setup_resolvers(use_current_env=True)) - # The resulting resolvers should include a single LocalPackageResolver whose - # .package_dirs is empty and .use_current_env is True. - local_resolvers = [r for r in resolvers if isinstance(r, LocalPackageResolver)] - assert len(local_resolvers) == 1 - lpr = local_resolvers[0] - assert lpr.package_dirs == set() - assert lpr.use_current_env is True + # The resulting resolvers should include a single SysPathPackageResolver. + syspath_resolvers = [r for r in resolvers if isinstance(r, SysPathPackageResolver)] + assert len(syspath_resolvers) == 1 + spr = syspath_resolvers[0] # The only thing we can assume about the _current_ env (in which FD runs) - # is that "fawltydeps" is installed (hence resolved via our 'lpr'), and that - # "other_module" is not installed (and thus resolved with id mapping). + # is that "fawltydeps" is installed (hence resolved via our 'spr'), and that + # "other_module" is not installed (and thus resolved with IdentityMapping). actual = resolve_dependencies(["fawltydeps", "other_module"], resolvers) assert actual == { - "fawltydeps": lpr.lookup_packages({"fawltydeps"})["fawltydeps"], + "fawltydeps": spr.lookup_packages({"fawltydeps"})["fawltydeps"], "other_module": Package("other_module", {"other_module"}, IdentityMapping), } diff --git a/tests/test_packages.py b/tests/test_packages.py index 1f1fc9d6..50d34784 100644 --- a/tests/test_packages.py +++ b/tests/test_packages.py @@ -9,6 +9,7 @@ IdentityMapping, LocalPackageResolver, Package, + SysPathPackageResolver, UserDefinedMapping, resolve_dependencies, setup_resolvers, @@ -278,12 +279,12 @@ def test_user_defined_mapping__no_input__returns_empty_mapping(): ), ], ) -def test_LocalPackageResolver_lookup_packages( +def test_SysPathPackageResolver_lookup_packages( isolate_default_resolver, dep_name, expect_import_names ): isolate_default_resolver(default_sys_path_env_for_tests) - lpl = LocalPackageResolver(use_current_env=True) - actual = lpl.lookup_packages({dep_name}) + sys_path = SysPathPackageResolver() + actual = sys_path.lookup_packages({dep_name}) if expect_import_names is None: assert actual == {} else: @@ -307,7 +308,7 @@ def test_resolve_dependencies__informs_once_when_id_mapping_is_used( dep_names = ["some-foo", "pip", "some-foo"] isolate_default_resolver(default_sys_path_env_for_tests) expect = { - "pip": Package("pip", {"pip"}, LocalPackageResolver), + "pip": Package("pip", {"pip"}, SysPathPackageResolver), "some-foo": Package("some-foo", {"some_foo"}, IdentityMapping), } expect_log = [ diff --git a/tests/test_resolver.py b/tests/test_resolver.py index 9a48e243..fdef6d2d 100644 --- a/tests/test_resolver.py +++ b/tests/test_resolver.py @@ -10,8 +10,8 @@ from fawltydeps.packages import ( IdentityMapping, - LocalPackageResolver, Package, + SysPathPackageResolver, TemporaryPipInstallResolver, UserDefinedMapping, resolve_dependencies, @@ -86,7 +86,7 @@ def generate_expected_resolved_deps( ret = {} ret.update( { - dep: Package(dep, imports, LocalPackageResolver) + dep: Package(dep, imports, SysPathPackageResolver) for dep, imports in locally_installed_deps.items() } ) diff --git a/tests/utils.py b/tests/utils.py index 417bf9de..9c25adb4 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,7 +10,7 @@ from typing import Any, Dict, Iterable, Iterator, List, Optional, Tuple, Union from fawltydeps.main import main -from fawltydeps.packages import IdentityMapping, LocalPackageResolver, Package +from fawltydeps.packages import IdentityMapping, Package, SysPathPackageResolver from fawltydeps.types import ( DeclaredDependency, Location, @@ -79,7 +79,7 @@ def resolved_factory(*deps: str) -> Dict[str, Package]: def make_package(dep: str) -> Package: imports = default_sys_path_env_for_tests.get(dep, None) if imports is not None: # exists in local env - return Package(dep, imports, LocalPackageResolver) + return Package(dep, imports, SysPathPackageResolver) # fall back to identity mapping return Package(dep, {dep}, IdentityMapping) @@ -285,8 +285,8 @@ class FDTestVector: # pylint: disable=too-many-instance-attributes DeclaredDependency(name="pip", source=Location(Path("requirements2.txt"))), ], expect_resolved_deps={ - "Pip": Package("pip", {"pip"}, LocalPackageResolver), - "pip": Package("pip", {"pip"}, LocalPackageResolver), + "Pip": Package("pip", {"pip"}, SysPathPackageResolver), + "pip": Package("pip", {"pip"}, SysPathPackageResolver), }, expect_unused_deps=[ UnusedDependency("Pip", [Location(Path("requirements1.txt"))]),