diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 546a97c8f..70835d175 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -38,6 +38,7 @@ jobs: - name: Unit tests run: | pytest tests + pytest --doctest-modules komodo - name: Lint examples run: | diff --git a/ci/repository.yml b/ci/repository.yml index a456860f5..4385d3c55 100644 --- a/ci/repository.yml +++ b/ci/repository.yml @@ -3,16 +3,10 @@ numpy: source: pypi make: pip maintainer: ci - depends: - - setuptools - - python 1.23.5: source: pypi make: pip maintainer: ci - depends: - - setuptools - - python python: 3-builtin: @@ -26,14 +20,9 @@ setuptools: source: pypi make: pip maintainer: ci - depends: - - wheel - - python wheel: 0.42.0: source: pypi make: pip maintainer: ci - depends: - - python diff --git a/komodo/build.py b/komodo/build.py index a570fb769..3b2f61bcf 100644 --- a/komodo/build.py +++ b/komodo/build.py @@ -4,50 +4,13 @@ import os import re import stat -import sys from pathlib import Path -from typing import Dict, List, Optional, Set +from typing import Dict import requests from komodo.shell import pushd, shell - -def full_dfs( - all_packages: Dict[str, str], - repo: Dict[str, Dict], - packages_to_check: Optional[List[str]] = None, -) -> List[str]: - if packages_to_check is None: - packages_to_check = list(all_packages.keys()) - all_packages_set = set(all_packages) - - def dfs(package_name: str, pkg_order: List[str], visited: Set[str]): - if package_name in visited: - return - visited.add(package_name) - ver = all_packages[package_name] - dependencies = repo[package_name][ver].get("depends", []) - missing_depends = set(dependencies) - all_packages_set - if missing_depends: - print( - "error: " - + ",".join(missing_depends) - + f" required as dependency for {package_name}, is not in distribution", - file=sys.stderr, - ) - sys.exit(1) - for dep in dependencies: - dfs(dep, pkg_order, visited) - pkg_order.append(package_name) - - visited = set() - pkg_order = [] - for package in packages_to_check: - dfs(package, pkg_order, visited) - return pkg_order - - # When running cmake we pass the option -DDEST_PREFIX=fakeroot, this is an # absolute hack to be able to build opm-common and sunbeam with the ~fakeroot # implementation used by komodo. @@ -232,8 +195,7 @@ def make( pip="pip", fakeroot=".", ): - pkgorder = full_dfs(pkgs, repo) - assert len(set(pkgorder)) == len(pkgs) + pkgorder = list(pkgs.keys()) fakeprefix = fakeroot + prefix shell(["mkdir -p", fakeprefix]) prefix = os.path.abspath(prefix) diff --git a/komodo/check_unused_package.py b/komodo/check_unused_package.py index 8d7307205..92a287f7f 100644 --- a/komodo/check_unused_package.py +++ b/komodo/check_unused_package.py @@ -4,38 +4,39 @@ import os import sys -from komodo.build import full_dfs from komodo.prettier import load_yaml from komodo.yaml_file_types import ReleaseFile, RepositoryFile +from .pypi_dependencies import PypiDependencies + def check_for_unused_package( release_file: ReleaseFile, package_status_file: str, repository: RepositoryFile ): package_status = load_yaml(package_status_file) - private_packages = [ - pkg - for pkg in release_file.content - if package_status[pkg]["visibility"] == "private" - ] public_and_plugin_packages = [ - pkg - for pkg in release_file.content + (pkg, version) + for pkg, version in release_file.content.items() if package_status[pkg]["visibility"] in ("public", "private-plugin") ] - public_and_plugin_dependencies = full_dfs( - release_file.content, - repository.content, - public_and_plugin_packages, - ) - diff_packages = set(private_packages).difference( - set(public_and_plugin_dependencies) - ) - if diff_packages: + python_version = release_file.content["python"] + # For pypi we need to change '3.8.6-builtin' -> '3.8.6' + python_version = python_version[: python_version.rindex("-")] + dependencies = PypiDependencies(release_file.content, python_version=python_version) + for name, version in release_file.content.items(): + metadata = repository.content.get(name, {}).get(version, {}) + if metadata.get("source") != "pypi": + dependencies.add_user_specified(name, metadata.get("depends", [])) + unused_private_packages = set( + pkg + for pkg in release_file.content + if package_status[pkg]["visibility"] == "private" + ).difference(dependencies.used_packages(public_and_plugin_packages)) + if unused_private_packages: print( - f"The following {len(diff_packages)} private packages are not dependencies of any public or private-plugin packages:" + f"The following {len(unused_private_packages)} private packages are not dependencies of any public or private-plugin packages:" ) - print(", ".join(sorted(list(diff_packages)))) + print(", ".join(sorted(list(unused_private_packages)))) print( "If you have added or removed any packages check that the dependencies in repository.yml are correct." ) diff --git a/komodo/lint.py b/komodo/lint.py index 1110428c2..e6feb0757 100755 --- a/komodo/lint.py +++ b/komodo/lint.py @@ -8,7 +8,8 @@ from packaging.version import parse -from komodo.yaml_file_types import KomodoException, ReleaseFile, RepositoryFile +from .pypi_dependencies import PypiDependencies +from .yaml_file_types import KomodoException, ReleaseFile, RepositoryFile KomodoError = namedtuple( "KomodoError", @@ -66,8 +67,12 @@ def lint_version_numbers(package, version, repo): return None -def lint(release_file: ReleaseFile, repository_file: RepositoryFile): - maintainers, deps, versions = [], [], [] +def lint( + release_file: ReleaseFile, + repository_file: RepositoryFile, + check_dependencies: bool = False, +) -> Report: + maintainers, versions = [], [] for package_name, package_version in release_file.content.items(): try: lint_maintainer = repository_file.lint_maintainer( @@ -84,27 +89,55 @@ def lint(release_file: ReleaseFile, repository_file: RepositoryFile): ) if lint_version_number: versions.append(lint_version_number) - missing = [] - repository_file_package_version_data = repository_file.content.get( - package_name, - ).get(package_version) - for dependency in repository_file_package_version_data.get("depends", []): - if dependency not in release_file.content: - missing.append(dependency) - if missing: - deps.append( - _komodo_error( - package=package_name, - version=package_version, - depends=missing, - err=( - f"{MISSING_DEPENDENCY} for {package_name} {package_version}" - ), - ), - ) except KomodoException as komodo_exception: maintainers.append(komodo_exception.error) + if check_dependencies: + pypi_dependencies = { + name: version + for name, version in release_file.content.items() + if repository_file.content.get(name, {}).get(version, {}).get("source") + == "pypi" + } + + python_version = release_file.content["python"] + # For pypi we need to change '3.8.6-builtin' -> '3.8.6' + python_version = python_version[: python_version.rindex("-")] + dependencies = PypiDependencies( + pypi_dependencies, python_version=python_version + ) + for name, version in release_file.content.items(): + if ( + repository_file.content.get(name, {}).get(version, {}).get("source") + != "pypi" + ): + if ( + name not in repository_file.content + or version not in repository_file.content[name] + ): + raise ValueError( + f"Missing package in repository file: {name}=={version}" + ) + depends = repository_file.content[name][version].get("depends", []) + if depends: + dependencies.add_user_specified( + name, repository_file.content[name][version]["depends"] + ) + + failed_requirements = dependencies.failed_requirements() + if failed_requirements: + deps = [ + _komodo_error( + err="Failed requirements ", + depends=[str(r) for r in failed_requirements], + ) + ] + else: + deps = [] + dependencies.dump_cache() + else: + deps = [] + return Report( release_name=[], maintainers=maintainers, @@ -135,6 +168,13 @@ def get_args(): dest="loglevel", const=logging.INFO, ) + parser.add_argument( + "--check-pypi-dependencies", + dest="check_pypi_dependencies", + help="Checks package metadata", + action="store_true", + default=False, + ) return parser.parse_args() @@ -142,15 +182,14 @@ def lint_main(): args = get_args() logging.basicConfig(format="%(message)s", level=args.loglevel) - try: - report = lint(args.packagefile, args.repofile) - maintainers, deps, versions = ( - report.maintainers, - report.dependencies, - report.versions, - ) - except ValueError as err: - sys.exit(str(err)) + report = lint( + args.packagefile, args.repofile, check_dependencies=args.check_pypi_dependencies + ) + maintainers, deps, versions = ( + report.maintainers, + report.dependencies, + report.versions, + ) print(f"{len(maintainers)} packages") if not any(err.err for err in maintainers + deps + versions): print("No errors found") @@ -158,8 +197,9 @@ def lint_main(): for err in maintainers + deps + versions: if err.err: - dep = f": {', '.join(err.depends)}" if err.depends else "" - print(f"{err.err}{dep}") + print(f"{err.err}") + if err.depends: + print("\n ".join(err.depends)) if not any(err.err for err in maintainers + deps): sys.exit(0) # currently we allow erronous version numbers diff --git a/komodo/pypi_dependencies.py b/komodo/pypi_dependencies.py new file mode 100644 index 000000000..bf2d9ef4a --- /dev/null +++ b/komodo/pypi_dependencies.py @@ -0,0 +1,251 @@ +import os +import platform +import subprocess +import sys +from tempfile import TemporaryDirectory +from typing import Dict, List, Set, Tuple + +import pkginfo +import yaml +from packaging.requirements import Requirement +from packaging.utils import canonicalize_name +from packaging.version import InvalidVersion + +from .package_version import LATEST_PACKAGE_ALIAS + + +# From Pep 508 +def format_full_version(info) -> str: + version = f"{info.major}.{info.minor}.{info.micro}" + kind = info.releaselevel + if kind != "final": + version += kind[0] + str(info.serial) + return version + + +environment = { + "implementation_name": sys.implementation.name, + "implementation_version": format_full_version(sys.implementation.version), + "os_name": os.name, + "platform_machine": platform.machine(), + "platform_python_implementation": platform.python_implementation(), + "platform_release": platform.release(), + "platform_system": platform.system(), + "platform_version": platform.version(), + "python_full_version": platform.python_version(), + "python_version": ".".join(platform.python_version_tuple()[:2]), + "sys_platform": sys.platform, +} + + +# pylint: disable=too-many-instance-attributes +class PypiDependencies: + def __init__( + self, + to_install: Dict[str, str], + python_version: str, + cachefile: str = "./pypi_dependencies.yml", + ) -> None: + self.python_version = python_version + environment["python_full_version"] = python_version + environment["python_version"] = python_version[: python_version.rindex(".")] + self._satisfied_requirements = set() + self._failed_requirements = set() + self._used_packages = set() + + self._to_install = to_install + + self._install_names = {canonicalize_name(name): name for name in to_install} + self._to_install = { + canonicalize_name(name): version for name, version in to_install.items() + } + self._cachefile = cachefile + if os.path.exists(self._cachefile): + with open(self._cachefile, "r", encoding="utf-8") as f: + self.requirements = yaml.safe_load(f) + self.requirements = { + canonicalize_name(name): r for name, r in self.requirements.items() + } + else: + self.requirements = {} + + self._user_specified = {} + + def _update_package_sets(self, packages=None): + for package_name, version in packages or self._to_install.items(): + requirements = self._get_requirements(package_name, version) + self._used_packages.add(package_name) + for r in requirements: + _ = self.satisfied(r) + + def failed_requirements(self, packages=None): + self._update_package_sets(packages) + return self._failed_requirements + + def used_packages(self, packages=None): + self._update_package_sets(packages) + return self._used_packages + + def add_user_specified( + self, + package_name: str, + depends: List[str], + ) -> None: + canonical = canonicalize_name(package_name) + # It is necessary to set the version to the installed + # version if present as the version check considers + # beta versions as failing unless specifically specified + self._user_specified[canonicalize_name(package_name)] = [ + Requirement( + d + if self._to_install.get(canonicalize_name(d)) + in [None, LATEST_PACKAGE_ALIAS, "main", "master"] + else f"{d}=={self._to_install.get(canonicalize_name(d))}" + ) + for d in depends + if d != "python" + ] + self._install_names[canonical] = package_name + + def dump_cache(self): + with open(self._cachefile, "w", encoding="utf-8") as f: + yaml.safe_dump(self.requirements, f) + + def _get_requirements( + self, package_name: str, package_version: str + ) -> List[Requirement]: + canonical = canonicalize_name(package_name) + if canonical in self._user_specified: + return self._user_specified[canonical] + + if canonical not in self.requirements: + self.requirements[canonical] = {} + + if package_version not in self.requirements[canonical]: + with TemporaryDirectory() as tmpdir: + try: + if package_version == LATEST_PACKAGE_ALIAS: + subprocess.check_output( + [ + "pip", + "download", + package_name, + f"--python-version={self.python_version}", + "--no-deps", + ], + cwd=tmpdir, + ) + else: + subprocess.check_output( + [ + "pip", + "download", + f"{package_name}=={package_version}", + f"--python-version={self.python_version}", + "--no-deps", + ], + cwd=tmpdir, + ) + except Exception as err: + raise ValueError( + f"Could not install {package_name} {package_version} from pypi" + f"With python version {self.python_version} " + "in order to determine dependencies. " + "This may be because no wheel exists for this python version." + ) from err + + files = os.listdir(tmpdir) + if len(files) != 1: + raise ValueError( + f"Did not get one wheel for download {package_name}=={package_version}." + f"Got: {files}" + ) + file = files[0] + if file.endswith(".whl"): + dist = pkginfo.Wheel(os.path.join(tmpdir, file)) + else: + dist = pkginfo.SDist(os.path.join(tmpdir, file)) + self.requirements[canonical][package_version] = dist.requires_dist + + return [Requirement(r) for r in self.requirements[canonical][package_version]] + + def _make_install_name(self, name: str) -> str: + canonical = canonicalize_name(name) + return self._install_names.get(canonical, canonical) + + def _version_satisfied(self, version: str, requirement: Requirement) -> bool: + if version in ["main", "master", LATEST_PACKAGE_ALIAS]: + return True + try: + specifier = requirement.specifier + specifier.prereleases = True + return version in specifier + except InvalidVersion: + return True + + def _is_necessary(self, requirement, extras): + environment["extra"] = ",".join(extras) + return requirement.marker is None or requirement.marker.evaluate(environment) + + def _satisfied( + self, + requirements: List[Requirement], + ) -> Tuple[bool, List[Tuple[List[Requirement], Set[str]]]]: + installed = self._to_install + transient_requirements = [] + for requirement in requirements: + name = canonicalize_name(requirement.name) + if name not in installed: + print(f"Not installed: {name}") + return False, [] + self._used_packages.add(self._install_names[name]) + installed_version = installed[name] + + if not self._version_satisfied(installed_version, requirement): + return False, [] + + transient_requirements.append( + ( + self._get_requirements(requirement.name, installed_version), + requirement.extras, + ) + ) + return True, transient_requirements + + def satisfied( + self, + requirement: Requirement, + extra=None, + ) -> bool: + """ + >>> from packaging.requirements import Requirement + >>> PypiDependencies(to_install={}, python_version="3.8").satisfied( + ... Requirement("PyJWT[crypto] (<3, >=1.0.0)") + ... ) + Not installed: pyjwt + False + >>> PypiDependencies(to_install={"PyJWT": "2.3.0"}, python_version="3.8").satisfied( + ... Requirement("PyJWT (<3, >=1.0.0)") + ... ) + True + """ + extra = extra or set() + if not self._is_necessary(requirement, extra): + return True + if requirement in self._failed_requirements: + return False + if requirement in self._satisfied_requirements: + return True + + satisfied, transient_requirements = self._satisfied([requirement]) + if satisfied: + self._satisfied_requirements.add(requirement) + else: + self._failed_requirements.add(requirement) + return satisfied and all( + [ + self.satisfied(transient, extra) + for (transients, extra) in transient_requirements + for transient in transients + ] + ) diff --git a/pyproject.toml b/pyproject.toml index 1eb52f59c..033cf94ed 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,6 +34,7 @@ dependencies = [ "urllib3; '.el7.' not in platform_release", "ruamel.yaml", "shell", + "pkginfo", ] [project.optional-dependencies] diff --git a/tests/test_build.py b/tests/test_build.py index 355199349..6dad24177 100644 --- a/tests/test_build.py +++ b/tests/test_build.py @@ -1,6 +1,6 @@ import pytest -from komodo.build import full_dfs, make +from komodo.build import make def test_make_with_empty_pkgs(captured_shell_commands, tmpdir): @@ -27,64 +27,3 @@ def test_make_sh_does_not_accept_pypi_package_name(tmpdir): with pytest.raises(ValueError, match=r"pypi_package_name"): make(packages, repositories, {}, str(tmpdir)) - - -test_cases = [ - ( - {"dummy_package": "1.1.0"}, - { - "dummy_package": { - "1.1.0": { - "depends": [], - }, - }, - }, - [["dummy_package"]], - ), - ( - { - "package_a": "1.0.0", - "package_b": "1.0.0", - "package_c": "1.0.0", - "package_d": "1.0.0", - "package_e": "1.0.0", - }, - { - "package_a": { - "1.0.0": { - "depends": ["package_b", "package_c"], - }, - }, - "package_b": { - "1.0.0": { - "depends": ["package_d", "package_e"], - }, - }, - "package_c": { - "1.0.0": { - "depends": ["package_d"], - }, - }, - "package_d": { - "1.0.0": { - "depends": ["package_e"], - }, - }, - "package_e": { - "1.0.0": { - "depends": [], - }, - }, - }, - [ - ["package_e", "package_d", "package_c", "package_b", "package_a"], - ["package_e", "package_d", "package_b", "package_c", "package_a"], - ], - ), -] - - -@pytest.mark.parametrize("packages, repositories, expected", test_cases) -def test_installation_package_order(packages, repositories, expected): - package_order = full_dfs(packages, repositories) - assert package_order in expected diff --git a/tests/test_check_unused_package.py b/tests/test_check_unused_package.py index 319ed2fed..adc4463d3 100644 --- a/tests/test_check_unused_package.py +++ b/tests/test_check_unused_package.py @@ -9,7 +9,6 @@ { "package_a": { "1.0": { - "source": "pypi", "make": "pip", "maintainer": "scout", "depends": ["package_b", "package_c"], @@ -17,7 +16,6 @@ }, "package_b": { "1.0": { - "source": "pypi", "make": "pip", "maintainer": "scout", "depends": ["package_d", "package_e"], @@ -25,7 +23,6 @@ }, "package_c": { "1.0": { - "source": "pypi", "make": "pip", "maintainer": "scout", "depends": ["package_d"], @@ -33,7 +30,6 @@ }, "package_d": { "1.0": { - "source": "pypi", "make": "pip", "maintainer": "scout", "depends": ["package_e"], @@ -41,14 +37,12 @@ }, "package_e": { "1.0": { - "source": "pypi", "make": "pip", "maintainer": "scout", }, }, "package_f": { "1.0": { - "source": "pypi", "make": "pip", "maintainer": "scout", "depends": ["package_c"], @@ -87,6 +81,9 @@ @pytest.mark.parametrize("repo, release, package_status", test_case) def test_check_unused_package(repo, release, package_status, capsys, tmpdir): + package_status["python"] = {"visibility": "public"} + release["python"] = "3.8.6-builtin" + # Use tmpdir to create a temporary file for package status package_status_file = tmpdir.join("package_status.yml")