From c25116245dd67c10f526c587b56f47b189d67b63 Mon Sep 17 00:00:00 2001 From: DavHau Date: Thu, 3 Feb 2022 16:36:19 +0700 Subject: [PATCH] improve unwanted dependency removal - retrieve dependency information for nixpkgs packages only from sdist and not from wheel provider - skip dependency removal if package dependencies are unknown --- mach_nix/data/providers.py | 38 +++++++++++------ mach_nix/generators/overides_generator.py | 52 ++++++++++++++--------- mach_nix/resolver/__init__.py | 23 ++++++++-- mach_nix/resolver/resolvelib_resolver.py | 19 ++++++--- 4 files changed, 90 insertions(+), 42 deletions(-) diff --git a/mach_nix/data/providers.py b/mach_nix/data/providers.py index 378c40f..09450c1 100644 --- a/mach_nix/data/providers.py +++ b/mach_nix/data/providers.py @@ -8,7 +8,7 @@ import sys from abc import ABC, abstractmethod from dataclasses import dataclass from operator import itemgetter -from typing import List, Tuple, Iterable +from typing import List, Optional, Tuple, Iterable import distlib.markers from pkg_resources import RequirementParseError @@ -36,6 +36,14 @@ class ProviderInfo: url: str = None hash: str = None + def toDict(self): + return dict( + provider=self.provider.name, + wheel_fname=self.wheel_fname, + url=self.url, + hash=self.hash, + ) + def normalize_name(key: str) -> str: return key.replace('_', '-').lower() @@ -106,8 +114,12 @@ class DependencyProviderBase(ABC): pass def get_reqs_for_extras(self, pkg_name, pkg_ver, extras): - install_reqs_wo_extras, setup_reqs_wo_extras = self.get_pkg_reqs(pkg_name, pkg_ver) - install_reqs_w_extras, setup_reqs_w_extras = self.get_pkg_reqs(pkg_name, pkg_ver, extras=extras) + install_reqs_wo_extras, _ = self.get_pkg_reqs(pkg_name, pkg_ver) + install_reqs_w_extras, _ = self.get_pkg_reqs(pkg_name, pkg_ver, extras=extras) + if install_reqs_wo_extras is None: + install_reqs_wo_extras = [] + if install_reqs_w_extras is None: + install_reqs_w_extras = [] install_reqs = set(install_reqs_w_extras) - set(install_reqs_wo_extras) return list(install_reqs) @@ -116,7 +128,7 @@ class DependencyProviderBase(ABC): @abstractmethod @cached() - def get_pkg_reqs(self, candidate: Candidate) -> Tuple[List[Requirement], List[Requirement]]: + def get_pkg_reqs(self, candidate: Candidate) -> Tuple[Optional[List[Requirement]], Optional[List[Requirement]]]: """ Get all requirements of a candidate for the current platform and the specified extras returns two lists: install_requires, setup_requires @@ -177,7 +189,7 @@ class CombinedDependencyProvider(DependencyProviderBase): if pkg_version in (c.ver for c in provider.all_candidates(pkg_name, build=build)): return provider - def get_pkg_reqs(self, c: Candidate) -> Tuple[List[Requirement], List[Requirement]]: + def get_pkg_reqs(self, c: Candidate) -> Tuple[Optional[List[Requirement]], Optional[List[Requirement]]]: for provider in self.allowed_providers_for_pkg(c.name).values(): if c in provider.all_candidates(c.name, c.selected_extras, c.build): return provider.get_pkg_reqs(c) @@ -243,19 +255,21 @@ class NixpkgsDependencyProvider(DependencyProviderBase): self.wheel_provider = wheel_provider self.sdist_provider = sdist_provider - def get_pkg_reqs(self, c: Candidate) -> Tuple[List[Requirement], List[Requirement]]: + def get_pkg_reqs(self, c: Candidate) -> Tuple[Optional[List[Requirement]], Optional[List[Requirement]]]: if not self.nixpkgs.exists(c.name, c.ver): raise Exception(f"Cannot find {c.name}:{c.ver} in nixpkgs") requirements = self.nixpkgs.get_requirements(c.name, c.ver) if requirements is not None: - return list(parse_reqs(requirements)), [] + return list(parse_reqs(requirements)), None install_reqs, setup_reqs = [], [] - for provider in (self.sdist_provider, self.wheel_provider): + try: + return self.sdist_provider.get_pkg_reqs(c) + except PackageNotFound: try: - install_reqs, setup_reqs = provider.get_pkg_reqs(c) - except PackageNotFound: - pass - return install_reqs, setup_reqs + # wheel provider onyl knows install deps + return self.wheel_provider.get_pkg_reqs(c)[0], None + except: + return None, None def all_candidates(self, pkg_name, extras=None, build=None) -> Iterable[Candidate]: if build: diff --git a/mach_nix/generators/overides_generator.py b/mach_nix/generators/overides_generator.py index 6a8d469..2734b03 100644 --- a/mach_nix/generators/overides_generator.py +++ b/mach_nix/generators/overides_generator.py @@ -37,7 +37,9 @@ class OverridesGenerator(ExpressionGenerator): pkgs = dict(sorted(((p.name, p) for p in pkgs), key=lambda x: x[1].name)) return self._gen_python_env(pkgs) - def _gen_imports(self, all_pnames): + def _gen_imports(self, pkgs): + all_pnames = list(pkgs.keys()) + pkgsJSON = json.dumps({pname: pkg.toDict() for pname, pkg in pkgs.items()}) out = f""" {{ pkgs, python, ... }}: with builtins; @@ -52,6 +54,7 @@ class OverridesGenerator(ExpressionGenerator): pypiFetcher = import pypi_fetcher_src {{ inherit pkgs; }}; fetchPypi = pypiFetcher.fetchPypi; fetchPypiWheel = pypiFetcher.fetchPypiWheel; + pkgsData = fromJSON ''{pkgsJSON}''; isPyModule = pkg: isAttrs pkg && hasAttr "pythonModule" pkg; normalizeName = name: (replaceStrings ["_"] ["-"] (toLower name)); @@ -73,18 +76,22 @@ class OverridesGenerator(ExpressionGenerator): "propagatedBuildInputs" "buildInputs" ]; - pnamesEnv = - genAttrs - [ "{'" "'.join(all_pnames)}" ] - (pname: null); - removeUnwantedPythonDeps = pname: inputs: - filter - (dep: - if ! isPyModule dep || pnamesEnv ? "${{normalizeName (get_pname dep)}}" then - true - else - trace "removing dependency ${{dep.name}} from ${{pname}}" false) - inputs; + removeUnwantedPythonDeps = pythonSelf: pname: inputs: + # Do not remove any deps if provider is nixpkgs and actual dependencies are unknown. + # Otherwise we risk removing dependencies which are needed. + if pkgsData."${{pname}}".provider_info.provider == "nixpkgs" + && + (pkgsData."${{pname}}".build_inputs == null + || pkgsData."${{pname}}".prop_build_inputs == null) then + inputs + else + filter + (dep: + if ! isPyModule dep || pkgsData ? "${{normalizeName (get_pname dep)}}" then + true + else + trace "removing dependency ${{dep.name}} from ${{pname}}" false) + inputs; updatePythonDeps = newPkgs: pkg: if ! isPyModule pkg then pkg else let @@ -100,7 +107,7 @@ class OverridesGenerator(ExpressionGenerator): in newP; updateAndRemoveDeps = pythonSelf: name: inputs: - removeUnwantedPythonDeps name (map (dep: updatePythonDeps pythonSelf dep) inputs); + removeUnwantedPythonDeps pythonSelf name (map (dep: updatePythonDeps pythonSelf dep) inputs); cleanPythonDerivationInputs = pythonSelf: name: oldAttrs: mapAttrs (n: v: if elem n depNamesAll then updateAndRemoveDeps pythonSelf name v else v ) oldAttrs; override = pkg: @@ -317,10 +324,16 @@ class OverridesGenerator(ExpressionGenerator): if pkg.name not in overrides_keys: continue overlays_required = True - build_inputs_local = {b for b in pkg.build_inputs if b in overrides_keys} - build_inputs_nixpkgs = set(pkg.build_inputs) - build_inputs_local - prop_build_inputs_local = {b for b in pkg.prop_build_inputs if b in overrides_keys} - prop_build_inputs_nixpkgs = set(pkg.prop_build_inputs) - prop_build_inputs_local + build_inputs, prop_build_inputs = pkg.build_inputs, pkg.prop_build_inputs + if build_inputs is None: + build_inputs = [] + if prop_build_inputs is None: + prop_build_inputs = [] + + build_inputs_local = {b for b in build_inputs if b in overrides_keys} + build_inputs_nixpkgs = set(build_inputs) - build_inputs_local + prop_build_inputs_local = {b for b in prop_build_inputs if b in overrides_keys} + prop_build_inputs_nixpkgs = set(prop_build_inputs) - prop_build_inputs_local # convert build inputs to string build_inputs_str = self._gen_build_inputs(build_inputs_local, build_inputs_nixpkgs, ).strip() # convert prop build inputs to string @@ -394,8 +407,7 @@ class OverridesGenerator(ExpressionGenerator): def _gen_python_env(self, pkgs: Dict[str, ResolvedPkg]): overrides_keys = {p.name for p in pkgs.values()} - all_pnames = list(pkgs.keys()) - out = self._gen_imports(all_pnames) + self._gen_overrides(pkgs, overrides_keys) + out = self._gen_imports(pkgs) + self._gen_overrides(pkgs, overrides_keys) python_with_packages = f""" in {{ diff --git a/mach_nix/resolver/__init__.py b/mach_nix/resolver/__init__.py index 2d50ae1..dc79165 100644 --- a/mach_nix/resolver/__init__.py +++ b/mach_nix/resolver/__init__.py @@ -1,18 +1,20 @@ from abc import ABC, abstractmethod from dataclasses import dataclass, field -from typing import List, Iterable, Set +from typing import List, Optional, Iterable, Set from mach_nix.data.providers import ProviderInfo from mach_nix.requirements import Requirement from mach_nix.versions import Version +from json import JSONEncoder + @dataclass -class ResolvedPkg: +class ResolvedPkg(JSONEncoder): name: str ver: Version - build_inputs: List[str] - prop_build_inputs: List[str] + build_inputs: Optional[List[str]] + prop_build_inputs: Optional[List[str]] is_root: bool provider_info: ProviderInfo extras_selected: List[str] @@ -20,6 +22,19 @@ class ResolvedPkg: removed_circular_deps: Set[str] = field(default_factory=set) build: str = None + def toDict(self): + return dict( + name=self.name, + ver=str(self.ver), + build_inputs=self.build_inputs, + prop_build_inputs=self.prop_build_inputs, + is_root=self.is_root, + provider_info=self.provider_info.toDict(), + extras_selected=self.extras_selected, + removed_circular_deps=list(self.removed_circular_deps), + build=self.build, + ) + class Resolver(ABC): diff --git a/mach_nix/resolver/resolvelib_resolver.py b/mach_nix/resolver/resolvelib_resolver.py index cbf6a2c..b83b6c4 100644 --- a/mach_nix/resolver/resolvelib_resolver.py +++ b/mach_nix/resolver/resolvelib_resolver.py @@ -37,6 +37,10 @@ class Provider(resolvelib.providers.AbstractProvider): def get_dependencies(self, candidate): install_requires, setup_requires = self.provider.get_pkg_reqs(candidate) + if install_requires is None: + install_requires = [] + if setup_requires is None: + setup_requires = [] deps = install_requires + setup_requires return deps @@ -56,12 +60,15 @@ class ResolvelibResolver(Resolver): candidate = result.mapping[name] ver = candidate.ver install_requires, setup_requires = self.deps_provider.get_pkg_reqs(candidate) - prop_build_inputs = list(filter( - lambda name: not name.startswith('-'), - list({req.key for req in install_requires}))) - build_inputs = list(filter( - lambda name: not name.startswith('-'), - list({req.key for req in setup_requires}))) + build_inputs, prop_build_inputs = None, None + if install_requires is not None: + prop_build_inputs = list(filter( + lambda name: not name.startswith('-'), + list({req.key for req in install_requires}))) + if setup_requires is not None: + build_inputs = list(filter( + lambda name: not name.startswith('-'), + list({req.key for req in setup_requires}))) is_root = name in result.graph._forwards[None] nix_py_pkgs.append(ResolvedPkg( name=name,