From 661c8fdf4f3b2982dbb92fe8e644d6cd5a13c5cf Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Mon, 16 Jan 2023 17:22:11 -0300 Subject: [PATCH 1/5] linters: set disallow_untyped_defs in mypy This forces type annotations for functions in craft_parts/. --- craft_parts/actions.py | 2 +- craft_parts/callbacks.py | 4 +- craft_parts/ctl.py | 4 +- craft_parts/errors.py | 37 ++++++++++-------- craft_parts/executor/executor.py | 4 +- craft_parts/executor/filesets.py | 2 +- craft_parts/executor/migration.py | 4 +- craft_parts/executor/part_handler.py | 18 +++++---- craft_parts/executor/step_handler.py | 4 +- craft_parts/infos.py | 10 ++--- craft_parts/lifecycle_manager.py | 2 +- craft_parts/main.py | 2 +- craft_parts/overlays/chroot.py | 2 +- craft_parts/overlays/layers.py | 6 +-- craft_parts/overlays/overlay_manager.py | 15 +++++--- craft_parts/packages/__init__.py | 6 ++- craft_parts/packages/apt_cache.py | 8 ++-- craft_parts/packages/base.py | 6 +-- craft_parts/packages/deb.py | 14 +++---- craft_parts/packages/errors.py | 8 ++-- craft_parts/packages/normalize.py | 2 +- craft_parts/packages/platform.py | 3 +- craft_parts/packages/snaps.py | 12 +++--- craft_parts/parts.py | 8 ++-- craft_parts/permissions.py | 4 +- craft_parts/plugins/ant_plugin.py | 6 ++- craft_parts/plugins/autotools_plugin.py | 2 +- craft_parts/plugins/cmake_plugin.py | 2 +- craft_parts/plugins/dotnet_plugin.py | 6 ++- craft_parts/plugins/dump_plugin.py | 2 +- craft_parts/plugins/go_plugin.py | 6 ++- craft_parts/plugins/make_plugin.py | 2 +- craft_parts/plugins/meson_plugin.py | 6 ++- craft_parts/plugins/npm_plugin.py | 6 ++- craft_parts/plugins/python_plugin.py | 2 +- craft_parts/plugins/rust_plugin.py | 4 +- craft_parts/plugins/scons_plugin.py | 4 +- craft_parts/sequencer.py | 4 +- craft_parts/sources/base.py | 8 ++-- craft_parts/sources/cache.py | 2 +- craft_parts/sources/deb_source.py | 2 +- craft_parts/sources/git_source.py | 44 +++++++++++----------- craft_parts/sources/local_source.py | 21 +++++++---- craft_parts/sources/sources.py | 4 +- craft_parts/sources/tar_source.py | 6 +-- craft_parts/sources/zip_source.py | 2 +- craft_parts/state_manager/state_manager.py | 4 +- craft_parts/state_manager/step_state.py | 2 +- craft_parts/steps.py | 2 +- craft_parts/utils/os_utils.py | 16 ++++---- craft_parts/utils/url_utils.py | 9 ++++- setup.cfg | 6 ++- tests/unit/test_errors.py | 2 +- 53 files changed, 209 insertions(+), 160 deletions(-) diff --git a/craft_parts/actions.py b/craft_parts/actions.py index 177cac8fd..0bcb12a0e 100644 --- a/craft_parts/actions.py +++ b/craft_parts/actions.py @@ -49,7 +49,7 @@ class ActionType(enum.IntEnum): UPDATE = 3 REAPPLY = 4 - def __repr__(self): + def __repr__(self) -> str: return f"{self.__class__.__name__}.{self.name}" diff --git a/craft_parts/callbacks.py b/craft_parts/callbacks.py index b20222b6d..26c2bbc03 100644 --- a/craft_parts/callbacks.py +++ b/craft_parts/callbacks.py @@ -126,13 +126,13 @@ def run_post_step(step_info: StepInfo) -> None: return _run_step(hook_list=_POST_STEP_HOOKS, step_info=step_info) -def _run_step(*, hook_list: List[CallbackHook], step_info: StepInfo): +def _run_step(*, hook_list: List[CallbackHook], step_info: StepInfo) -> None: for hook in hook_list: if not hook.step_list or step_info.step in hook.step_list: hook.function(step_info) -def _ensure_not_defined(func: Callback, hook_list: List[CallbackHook]): +def _ensure_not_defined(func: Callback, hook_list: List[CallbackHook]) -> None: for hook in hook_list: if func == hook.function: raise errors.CallbackRegistrationError( diff --git a/craft_parts/ctl.py b/craft_parts/ctl.py index a0319193f..d0407d1b7 100644 --- a/craft_parts/ctl.py +++ b/craft_parts/ctl.py @@ -52,7 +52,7 @@ def run(cls, cmd: str, args: List[str]) -> Optional[str]: raise RuntimeError(f"invalid command {cmd!r}") -def _client(cmd: str, args: List[str]): +def _client(cmd: str, args: List[str]) -> Optional[str]: """Execute a command in the running step processor. The control protocol client allows a user scriptlet to execute @@ -101,7 +101,7 @@ def _client(cmd: str, args: List[str]): return retval -def main(): +def main() -> None: """Run the ctl client cli.""" if len(sys.argv) < 2: print(f"usage: {sys.argv[0]} [arguments]") diff --git a/craft_parts/errors.py b/craft_parts/errors.py index 5c327fe78..e54201c9f 100644 --- a/craft_parts/errors.py +++ b/craft_parts/errors.py @@ -17,10 +17,11 @@ """Craft parts errors.""" import dataclasses +import pathlib from typing import TYPE_CHECKING, List, Optional, Set if TYPE_CHECKING: - from pydantic.error_wrappers import ErrorDict + from pydantic.error_wrappers import ErrorDict, Loc @dataclasses.dataclass(repr=True) @@ -120,7 +121,9 @@ def __init__(self, *, part_name: str, message: str): super().__init__(brief=brief, details=details, resolution=resolution) @classmethod - def from_validation_error(cls, *, part_name: str, error_list: List["ErrorDict"]): + def from_validation_error( + cls, *, part_name: str, error_list: List["ErrorDict"] + ) -> "PartSpecificationError": """Create a PartSpecificationError from a pydantic error list. :param part_name: The name of the part being processed. @@ -146,7 +149,7 @@ def from_validation_error(cls, *, part_name: str, error_list: List["ErrorDict"]) return cls(part_name=part_name, message="\n".join(formatted_errors)) @classmethod - def _format_loc(cls, loc): + def _format_loc(cls, loc: "Loc") -> str: """Format location.""" loc_parts = [] for loc_part in loc: @@ -160,11 +163,11 @@ def _format_loc(cls, loc): else: raise RuntimeError(f"unhandled loc: {loc_part}") - loc = ".".join(loc_parts) + loc_str = ".".join(loc_parts) # Filter out internal __root__ detail. - loc = loc.replace(".__root__", "") - return loc + loc_str = loc_str.replace(".__root__", "") + return loc_str class CopyTreeError(PartsError): @@ -266,7 +269,7 @@ def __init__(self, plugin_name: str, *, part_name: str): class OsReleaseIdError(PartsError): """Failed to determine the host operating system identification string.""" - def __init__(self): + def __init__(self) -> None: brief = "Unable to determine the host operating system ID." super().__init__(brief=brief) @@ -275,7 +278,7 @@ def __init__(self): class OsReleaseNameError(PartsError): """Failed to determine the host operating system name.""" - def __init__(self): + def __init__(self) -> None: brief = "Unable to determine the host operating system name." super().__init__(brief=brief) @@ -284,7 +287,7 @@ def __init__(self): class OsReleaseVersionIdError(PartsError): """Failed to determine the host operating system version.""" - def __init__(self): + def __init__(self) -> None: brief = "Unable to determine the host operating system version ID." super().__init__(brief=brief) @@ -293,7 +296,7 @@ def __init__(self): class OsReleaseCodenameError(PartsError): """Failed to determine the host operating system version codename.""" - def __init__(self): + def __init__(self) -> None: brief = "Unable to determine the host operating system codename." super().__init__(brief=brief) @@ -306,7 +309,7 @@ class FilesetError(PartsError): :param message: The error message. """ - def __init__(self, *, name: str, message: str): + def __init__(self, *, name: str, message: str) -> None: self.name = name self.message = message brief = f"{name!r} fileset error: {message}." @@ -321,7 +324,7 @@ class FilesetConflict(PartsError): :param conflicting_files: A set containing the conflicting file names. """ - def __init__(self, conflicting_files: Set[str]): + def __init__(self, conflicting_files: Set[str]) -> None: self.conflicting_files = conflicting_files brief = "Failed to filter files: inconsistent 'stage' and 'prime' filesets." details = ( @@ -342,7 +345,7 @@ class FileOrganizeError(PartsError): :param message: The error message. """ - def __init__(self, *, part_name, message): + def __init__(self, *, part_name: str, message: str) -> None: self.part_name = part_name self.message = message brief = f"Failed to organize part {part_name!r}: {message}." @@ -360,7 +363,7 @@ class PartFilesConflict(PartsError): def __init__( self, *, part_name: str, other_part_name: str, conflicting_files: List[str] - ): + ) -> None: self.part_name = part_name self.other_part_name = other_part_name self.conflicting_files = conflicting_files @@ -386,7 +389,7 @@ class StageFilesConflict(PartsError): :param conflicting_files: The list of confictling files. """ - def __init__(self, *, part_name: str, conflicting_files: List[str]): + def __init__(self, *, part_name: str, conflicting_files: List[str]) -> None: self.part_name = part_name self.conflicting_files = conflicting_files indented_conflicting_files = (f" {i}" for i in conflicting_files) @@ -546,7 +549,9 @@ def __init__(self) -> None: class DebError(PartsError): """A "deb"-related command failed.""" - def __init__(self, deb_path, command, exit_code): + def __init__( + self, deb_path: pathlib.Path, command: List[str], exit_code: int + ) -> None: brief = ( f"Failed when handling {deb_path}: " f"command {command!r} exited with code {exit_code}." diff --git a/craft_parts/executor/executor.py b/craft_parts/executor/executor.py index cd562c507..cfb39aba0 100644 --- a/craft_parts/executor/executor.py +++ b/craft_parts/executor/executor.py @@ -19,7 +19,7 @@ import logging import shutil from pathlib import Path -from typing import Dict, List, Optional, Union +from typing import Any, Dict, List, Optional, Union from craft_parts import callbacks, overlays, packages, parts, plugins from craft_parts.actions import Action, ActionType @@ -280,7 +280,7 @@ def __enter__(self) -> "ExecutionContext": self._executor.prologue() return self - def __exit__(self, *exc): + def __exit__(self, *exc: Any) -> None: self._executor.epilogue() def execute( diff --git a/craft_parts/executor/filesets.py b/craft_parts/executor/filesets.py index 5ee8d6991..d3daed93b 100644 --- a/craft_parts/executor/filesets.py +++ b/craft_parts/executor/filesets.py @@ -30,7 +30,7 @@ def __init__(self, entries: List[str], *, name: str = ""): self._name = name self._list = entries - def __repr__(self): + def __repr__(self) -> str: return f"Fileset({self._list!r}, name={self._name!r})" @property diff --git a/craft_parts/executor/migration.py b/craft_parts/executor/migration.py index 0f2aa0c94..c5faea4e0 100644 --- a/craft_parts/executor/migration.py +++ b/craft_parts/executor/migration.py @@ -19,7 +19,7 @@ import logging import os from pathlib import Path -from typing import Dict, List, Optional, Set, Tuple +from typing import Callable, Dict, List, Optional, Set, Tuple from craft_parts import overlays from craft_parts.permissions import Permissions, filter_permissions @@ -38,7 +38,7 @@ def migrate_files( missing_ok: bool = False, follow_symlinks: bool = False, oci_translation: bool = False, - fixup_func=lambda *args: None, + fixup_func: Callable[..., None] = lambda *args: None, permissions: Optional[List[Permissions]] = None, ) -> Tuple[Set[str], Set[str]]: """Copy or link files from a directory to another. diff --git a/craft_parts/executor/part_handler.py b/craft_parts/executor/part_handler.py index 2e2987597..e91b1e8b1 100644 --- a/craft_parts/executor/part_handler.py +++ b/craft_parts/executor/part_handler.py @@ -22,7 +22,7 @@ import shutil from glob import iglob from pathlib import Path -from typing import Any, Callable, Dict, List, Optional, Set, cast +from typing import Any, Callable, Dict, List, Optional, Sequence, Set, cast from typing_extensions import Protocol @@ -269,7 +269,7 @@ def _run_build( *, stdout: Stream, stderr: Stream, - update=False, + update: bool = False, ) -> StepState: """Execute the build step for this part. @@ -648,7 +648,9 @@ def _reapply_action( f"cannot reapply step {step_name!r} of {self._part.name!r}" ) - def _reapply_overlay(self, step_info, *, stdout: Stream, stderr: Stream) -> None: + def _reapply_overlay( + self, step_info: StepInfo, *, stdout: Stream, stderr: Stream + ) -> None: """Clean and repopulate the current part's layer, keeping its state.""" shutil.rmtree(self._part.part_layer_dir) self._run_overlay(step_info, stdout=stdout, stderr=stderr) @@ -847,7 +849,7 @@ def _clean_shared(self, step: Step, *, shared_dir: Path) -> None: ) overlay_migration_state_path.unlink() - def _make_dirs(self): + def _make_dirs(self) -> None: dirs = [ self._part.part_src_dir, self._part.part_build_dir, @@ -861,7 +863,7 @@ def _make_dirs(self): for dir_name in dirs: os.makedirs(dir_name, exist_ok=True) - def _organize(self, *, overwrite=False): + def _organize(self, *, overwrite: bool = False) -> None: mapping = self._part.spec.organize_files organize_files( part_name=self._part.name, @@ -894,7 +896,7 @@ def _fetch_stage_packages(self, *, step_info: StepInfo) -> Optional[List[str]]: return fetched_packages - def _fetch_stage_snaps(self): + def _fetch_stage_snaps(self) -> Optional[Sequence[str]]: """Download snap packages to the part's snap directory.""" stage_snaps = self._part.spec.stage_snaps if not stage_snaps: @@ -923,7 +925,7 @@ def _fetch_overlay_packages(self) -> None: part_name=self._part.name, package_name=err.package_name ) - def _unpack_stage_packages(self): + def _unpack_stage_packages(self) -> None: """Extract stage packages contents to the part's install directory.""" pulled_packages = None @@ -938,7 +940,7 @@ def _unpack_stage_packages(self): stage_packages=pulled_packages, ) - def _unpack_stage_snaps(self): + def _unpack_stage_snaps(self) -> None: """Extract stage snap contents to the part's install directory.""" stage_snaps = self._part.spec.stage_snaps if not stage_snaps: diff --git a/craft_parts/executor/step_handler.py b/craft_parts/executor/step_handler.py index d52bff92b..78d408118 100644 --- a/craft_parts/executor/step_handler.py +++ b/craft_parts/executor/step_handler.py @@ -158,14 +158,14 @@ def _builtin_stage(self) -> StepContents: srcdir = str(self._part.part_install_dir) files, dirs = filesets.migratable_filesets(stage_fileset, srcdir) - def pkgconfig_fixup(file_path): + def pkgconfig_fixup(file_path: str) -> None: if os.path.islink(file_path): return if not file_path.endswith(".pc"): return packages.fix_pkg_config( prefix_prepend=self._part.stage_dir, - pkg_config_file=file_path, + pkg_config_file=Path(file_path), prefix_trim=self._part.part_install_dir, ) diff --git a/craft_parts/infos.py b/craft_parts/infos.py index 74c8dc2e4..49e5960dd 100644 --- a/craft_parts/infos.py +++ b/craft_parts/infos.py @@ -80,7 +80,7 @@ def __init__( project_name: Optional[str] = None, project_vars_part_name: Optional[str] = None, project_vars: Optional[Dict[str, str]] = None, - **custom_args, # custom passthrough args + **custom_args: Any, # custom passthrough args ): if not project_dirs: project_dirs = ProjectDirs() @@ -101,7 +101,7 @@ def __init__( self.execution_finished = False - def __getattr__(self, name): + def __getattr__(self, name: str) -> Any: if hasattr(self._dirs, name): return getattr(self._dirs, name) @@ -255,7 +255,7 @@ def _ensure_valid_variable_name(self, name: str) -> None: if name not in self._project_vars: raise ValueError(f"{name!r} not in project variables") - def _set_machine(self, arch: Optional[str]): + def _set_machine(self, arch: Optional[str]) -> None: """Initialize host and target machine information based on the architecture. :param arch: The architecture to use. If empty, assume the @@ -302,7 +302,7 @@ def __init__( self._part_state_dir = part.part_state_dir self.build_attributes = part.spec.build_attributes.copy() - def __getattr__(self, name): + def __getattr__(self, name: str) -> Any: # Use composition and attribute cascading to avoid setting attributes # cumulatively in the init method. if hasattr(self._project_info, name): @@ -404,7 +404,7 @@ def __init__( self.step_environment: Dict[str, str] = {} self.state: "Optional[states.StepState]" = None - def __getattr__(self, name): + def __getattr__(self, name: str) -> Any: if hasattr(self._part_info, name): return getattr(self._part_info, name) diff --git a/craft_parts/lifecycle_manager.py b/craft_parts/lifecycle_manager.py index a29707530..b401958ab 100644 --- a/craft_parts/lifecycle_manager.py +++ b/craft_parts/lifecycle_manager.py @@ -94,7 +94,7 @@ def __init__( base_layer_hash: Optional[bytes] = None, project_vars_part_name: Optional[str] = None, project_vars: Optional[Dict[str, str]] = None, - **custom_args, # custom passthrough args + **custom_args: Any, # custom passthrough args ): # pylint: disable=too-many-locals diff --git a/craft_parts/main.py b/craft_parts/main.py index 15a917994..8e33299ab 100644 --- a/craft_parts/main.py +++ b/craft_parts/main.py @@ -37,7 +37,7 @@ from craft_parts import ActionType, Step -def main(): +def main() -> None: """Run the command-line interface.""" options = _parse_arguments() diff --git a/craft_parts/overlays/chroot.py b/craft_parts/overlays/chroot.py index cbc58dd96..b3bcc57fc 100644 --- a/craft_parts/overlays/chroot.py +++ b/craft_parts/overlays/chroot.py @@ -32,7 +32,7 @@ logger = logging.getLogger(__name__) -def chroot(path: Path, target: Callable, *args, **kwargs) -> Any: +def chroot(path: Path, target: Callable, *args: Any, **kwargs: Any) -> Any: """Execute a callable in a chroot environment. :param path: The new filesystem root. diff --git a/craft_parts/overlays/layers.py b/craft_parts/overlays/layers.py index ba9576e80..599b087b1 100644 --- a/craft_parts/overlays/layers.py +++ b/craft_parts/overlays/layers.py @@ -18,7 +18,7 @@ import hashlib import logging -from typing import Dict, List, Optional +from typing import Any, Dict, List, Optional from craft_parts.parts import Part @@ -31,10 +31,10 @@ class LayerHash: def __init__(self, layer_hash: bytes): self.digest = layer_hash - def __repr__(self): + def __repr__(self) -> str: return self.hex() - def __eq__(self, other): + def __eq__(self, other: Any) -> bool: if not isinstance(other, LayerHash): return False diff --git a/craft_parts/overlays/overlay_manager.py b/craft_parts/overlays/overlay_manager.py index b10f7158f..827e2f543 100644 --- a/craft_parts/overlays/overlay_manager.py +++ b/craft_parts/overlays/overlay_manager.py @@ -20,7 +20,9 @@ import os import sys from pathlib import Path -from typing import List, Optional +from typing import Any, List, Optional + +from typing_extensions import Literal from craft_parts import packages from craft_parts.infos import ProjectInfo @@ -127,7 +129,8 @@ def refresh_packages_list(self) -> None: mount_dir = self._project_info.overlay_mount_dir # Ensure we always run refresh_packages_list by resetting the cache - packages.Repository.refresh_packages_list.cache_clear() + if hasattr(packages.Repository.refresh_packages_list, "cache_clear"): + packages.Repository.refresh_packages_list.cache_clear() # type: ignore chroot.chroot(mount_dir, packages.Repository.refresh_packages_list) def download_packages(self, package_names: List[str]) -> None: @@ -178,14 +181,14 @@ def __init__( self._pkg_cache = pkg_cache self._pid = os.getpid() - def __enter__(self): + def __enter__(self) -> "LayerMount": self._overlay_manager.mount_layer( self._top_part, pkg_cache=self._pkg_cache, ) return self - def __exit__(self, *exc): + def __exit__(self, *exc: Any) -> Literal[False]: # prevent pychroot process leak if os.getpid() != self._pid: sys.exit() @@ -211,11 +214,11 @@ def __init__(self, overlay_manager: OverlayManager): self._overlay_manager.mkdirs() self._pid = os.getpid() - def __enter__(self): + def __enter__(self) -> "PackageCacheMount": self._overlay_manager.mount_pkg_cache() return self - def __exit__(self, *exc): + def __exit__(self, *exc: Any) -> Literal[False]: # prevent pychroot process leak if os.getpid() != self._pid: sys.exit() diff --git a/craft_parts/packages/__init__.py b/craft_parts/packages/__init__.py index a33fe4ed2..81ef6aae8 100644 --- a/craft_parts/packages/__init__.py +++ b/craft_parts/packages/__init__.py @@ -15,16 +15,20 @@ # along with this program. If not, see . """Operations with platform-specific package repositories.""" +from typing import TYPE_CHECKING, Type from . import errors # noqa: F401 from . import snaps # noqa: F401 from .normalize import fix_pkg_config # noqa: F401 from .platform import is_deb_based +if TYPE_CHECKING: + from .base import BaseRepository + # pylint: disable=import-outside-toplevel -def _get_repository_for_platform(): +def _get_repository_for_platform() -> Type["BaseRepository"]: if is_deb_based(): from .deb import Ubuntu diff --git a/craft_parts/packages/apt_cache.py b/craft_parts/packages/apt_cache.py index a1c0b1ddf..d2d192611 100644 --- a/craft_parts/packages/apt_cache.py +++ b/craft_parts/packages/apt_cache.py @@ -24,7 +24,7 @@ import shutil from contextlib import ContextDecorator from pathlib import Path -from typing import Dict, List, Optional, Set, Tuple +from typing import Any, Dict, List, Optional, Set, Tuple import apt import apt.cache @@ -47,7 +47,7 @@ class LogProgress(apt.progress.base.AcquireProgress): """Internal Base class for text progress classes.""" - def __init__(self): + def __init__(self) -> None: self._id = 1 def fail(self, item: apt_pkg.AcquireItemDesc) -> None: @@ -101,7 +101,7 @@ def __enter__(self) -> AptCache: # pylint: enable=attribute-defined-outside-init - def __exit__(self, *exc) -> None: + def __exit__(self, *exc: Any) -> None: self.cache.close() @classmethod @@ -362,7 +362,7 @@ def unmark_packages(self, unmark_names: Set[str]) -> None: self._autokeep_packages() -def _verify_marked_install(package: apt.package.Package): +def _verify_marked_install(package: apt.package.Package) -> None: if package.installed or package.marked_install: return diff --git a/craft_parts/packages/base.py b/craft_parts/packages/base.py index 26ffa2407..49c7209a0 100644 --- a/craft_parts/packages/base.py +++ b/craft_parts/packages/base.py @@ -21,7 +21,7 @@ import logging import os from pathlib import Path -from typing import List, Optional, Set, Tuple, Type +from typing import Any, List, Optional, Set, Tuple, Type from craft_parts import xattrs @@ -223,8 +223,8 @@ def get_installed_packages(cls) -> List[str]: @classmethod def fetch_stage_packages( - cls, - **kwargs, # pylint: disable=unused-argument + cls, # pylint: disable=unused-argument + **kwargs: Any, ) -> List[str]: """Fetch stage packages to stage_packages_path.""" return [] diff --git a/craft_parts/packages/deb.py b/craft_parts/packages/deb.py index 91a1f9862..97030544b 100644 --- a/craft_parts/packages/deb.py +++ b/craft_parts/packages/deb.py @@ -27,7 +27,7 @@ import tempfile from io import StringIO from pathlib import Path -from typing import Dict, List, Optional, Sequence, Set, Tuple +from typing import Any, Callable, Dict, List, Optional, Sequence, Set, Tuple from craft_parts.utils import deb_utils, file_utils, os_utils @@ -252,11 +252,11 @@ } -def _apt_cache_wrapper(method): +def _apt_cache_wrapper(method: Any) -> Callable[..., Any]: """Decorate a method to handle apt availability.""" @functools.wraps(method) - def wrapped(*args, **kwargs): + def wrapped(*args: Any, **kwargs: Any) -> Any: if not _APT_CACHE_AVAILABLE: raise errors.PackageBackendNotSupported("apt") return method(*args, **kwargs) @@ -369,7 +369,7 @@ def get_package_libraries(cls, package_name: str) -> Set[str]: @classmethod @_apt_cache_wrapper - def get_packages_for_source_type(cls, source_type): + def get_packages_for_source_type(cls, source_type: str) -> Set[str]: """Return a list of packages required to to work with source_type.""" if source_type == "bzr": packages = {"bzr"} @@ -720,7 +720,7 @@ def _unpack_stage_slices( @classmethod @_apt_cache_wrapper - def is_package_installed(cls, package_name) -> bool: + def is_package_installed(cls, package_name: str) -> bool: """Inform if a package is installed on the host system.""" with AptCache() as apt_cache: return apt_cache.get_installed_version(package_name) is not None @@ -747,7 +747,7 @@ def _extract_deb_name_version(cls, deb_path: pathlib.Path) -> str: return output.decode().strip() -def get_cache_dirs(cache_dir: Path): +def get_cache_dirs(cache_dir: Path) -> Tuple[Path, Path]: """Return the paths to the stage and deb cache directories.""" stage_cache_dir = cache_dir / "stage-packages" deb_cache_dir = cache_dir / "download" @@ -756,7 +756,7 @@ def get_cache_dirs(cache_dir: Path): # XXX: this will be removed when user messages support is implemented. -def process_run(command: List[str], **kwargs) -> None: +def process_run(command: List[str], **kwargs: Any) -> None: """Run a command and log its output.""" # Pass logger so messages can be logged as originating from this package. os_utils.process_run(command, logger.debug, **kwargs) diff --git a/craft_parts/packages/errors.py b/craft_parts/packages/errors.py index 11e6ea433..6e40c9cf7 100644 --- a/craft_parts/packages/errors.py +++ b/craft_parts/packages/errors.py @@ -126,7 +126,7 @@ class BuildPackageNotFound(PackagesError): :param package: The name of the missing package. """ - def __init__(self, package): + def __init__(self, package: str): self.package = package brief = f"Cannot find package listed in 'build-packages': {package}" @@ -205,7 +205,7 @@ class SnapInstallError(PackagesError): :param snap_channel: The snap channel. """ - def __init__(self, *, snap_name, snap_channel): + def __init__(self, *, snap_name: str, snap_channel: str): self.snap_name = snap_name self.snap_channel = snap_channel brief = f"Error installing snap {snap_name!r} from channel {snap_channel!r}." @@ -220,7 +220,7 @@ class SnapDownloadError(PackagesError): :param snap_channel: The snap channel. """ - def __init__(self, *, snap_name, snap_channel): + def __init__(self, *, snap_name: str, snap_channel: str): self.snap_name = snap_name self.snap_channel = snap_channel brief = f"Error downloading snap {snap_name!r} from channel {snap_channel!r}." @@ -235,7 +235,7 @@ class SnapRefreshError(PackagesError): :param snap_channel: The snap channel. """ - def __init__(self, *, snap_name, snap_channel): + def __init__(self, *, snap_name: str, snap_channel: str): self.snap_name = snap_name self.snap_channel = snap_channel brief = f"Error refreshing snap {snap_name!r} to channel {snap_channel!r}." diff --git a/craft_parts/packages/normalize.py b/craft_parts/packages/normalize.py index c9931e01f..9b5ac3d2a 100644 --- a/craft_parts/packages/normalize.py +++ b/craft_parts/packages/normalize.py @@ -218,7 +218,7 @@ def _fix_filemode(path: Path) -> None: path.chmod(mode & 0o1777) -def _rewrite_python_shebangs(root_dir: Path): +def _rewrite_python_shebangs(root_dir: Path) -> None: """Recursively change #!/usr/bin/pythonX shebangs to #!/usr/bin/env pythonX. :param str root_dir: Directory that will be crawled for shebangs. diff --git a/craft_parts/packages/platform.py b/craft_parts/packages/platform.py index f3a960c31..f98ccab7f 100644 --- a/craft_parts/packages/platform.py +++ b/craft_parts/packages/platform.py @@ -15,6 +15,7 @@ # along with this program. If not, see . """Helpers to determine the repository for the platform.""" +from typing import Optional from craft_parts import errors from craft_parts.utils import os_utils @@ -22,7 +23,7 @@ _DEB_BASED_PLATFORM = ["ubuntu", "debian", "elementary OS", "elementary", "neon"] -def is_deb_based(distro=None) -> bool: +def is_deb_based(distro: Optional[str] = None) -> bool: """Verify the distribution packaging system. :param distro: The distribution name. diff --git a/craft_parts/packages/snaps.py b/craft_parts/packages/snaps.py index 54933d296..d1215981c 100644 --- a/craft_parts/packages/snaps.py +++ b/craft_parts/packages/snaps.py @@ -21,7 +21,7 @@ import os import subprocess import sys -from typing import Any, Dict, List, Optional, Sequence, Set, Tuple, Union +from typing import Any, Dict, Iterator, List, Optional, Sequence, Set, Tuple, Union from urllib import parse import requests_unixsocket # type: ignore @@ -195,7 +195,7 @@ def is_valid(self) -> bool: store_channels = self._get_store_channels() return self.channel in store_channels.keys() - def download(self, *, directory: Optional[str] = None): + def download(self, *, directory: Optional[str] = None) -> None: """Download a given snap.""" # We use the `snap download` command here on recommendation # of the snapd team. @@ -216,7 +216,7 @@ def download(self, *, directory: Optional[str] = None): snap_name=self.name, snap_channel=self.channel ) from err - def install(self): + def install(self) -> None: """Installs the snap onto the system.""" logger.debug("Installing snap: %s", self.name) snap_install_cmd = ["snap", "install", self.name] @@ -244,7 +244,7 @@ def install(self): # Now that the snap is installed, invalidate the data we had on it. self._is_installed = None - def refresh(self): + def refresh(self) -> None: """Refresh a snap onto a channel on the system.""" logger.debug("Refreshing snap: %s (channel %s)", self.name, self.channel) snap_refresh_cmd = ["snap", "refresh", self.name, "--channel", self.channel] @@ -339,12 +339,12 @@ def _get_parsed_snap(snap: str) -> Tuple[str, str]: return snap_name, snap_channel -def get_snapd_socket_path_template(): +def get_snapd_socket_path_template() -> str: """Return the template for the snapd socket URI.""" return "http+unix://%2Frun%2Fsnapd.socket/v2/{}" -def _get_local_snap_file_iter(snap_name: str, *, chunk_size: int): +def _get_local_snap_file_iter(snap_name: str, *, chunk_size: int) -> Iterator: slug = f'snaps/{parse.quote(snap_name, safe="")}/file' url = get_snapd_socket_path_template().format(slug) try: diff --git a/craft_parts/parts.py b/craft_parts/parts.py index 6f85e65ea..f250bb91d 100644 --- a/craft_parts/parts.py +++ b/craft_parts/parts.py @@ -72,7 +72,7 @@ class Config: # pylint: disable=no-self-argument @validator("stage_files", "prime_files", each_item=True) - def validate_relative_path_list(cls, item): + def validate_relative_path_list(cls, item: str) -> str: """Check if the list does not contain empty of absolute paths.""" assert item != "", "path cannot be empty" assert ( @@ -81,13 +81,13 @@ def validate_relative_path_list(cls, item): return item @root_validator(pre=True) - def validate_root(cls, values): + def validate_root(cls, values: Dict[str, Any]) -> Dict[str, Any]: """Check if the part spec has a valid configuration of packages and slices.""" if not platform.is_deb_based(): # This check is only relevant in deb systems. return values - def is_slice(name): + def is_slice(name: str) -> bool: return "_" in name # Detect a mixture of .deb packages and chisel slices. @@ -202,7 +202,7 @@ def __init__( part_name=name, error_list=err.errors() ) - def __repr__(self): + def __repr__(self) -> str: return f"Part({self.name!r})" @property diff --git a/craft_parts/permissions.py b/craft_parts/permissions.py index 867447a9a..cf64a09e6 100644 --- a/craft_parts/permissions.py +++ b/craft_parts/permissions.py @@ -19,7 +19,7 @@ import os from fnmatch import fnmatch from pathlib import Path -from typing import List, Optional, Union +from typing import Any, Dict, List, Optional, Union from pydantic import BaseModel, root_validator @@ -49,7 +49,7 @@ class Permissions(BaseModel): # pylint: disable=no-self-argument @root_validator(pre=True) - def validate_root(cls, values): + def validate_root(cls, values: Dict[Any, Any]) -> Dict[Any, Any]: """Validate that "owner" and "group" are correctly specified.""" has_owner = "owner" in values has_group = "group" in values diff --git a/craft_parts/plugins/ant_plugin.py b/craft_parts/plugins/ant_plugin.py index d97dd6239..f8177e9a3 100644 --- a/craft_parts/plugins/ant_plugin.py +++ b/craft_parts/plugins/ant_plugin.py @@ -38,7 +38,7 @@ class AntPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "AntPluginProperties": """Populate make properties from the part specification. :param data: A dictionary containing part properties. @@ -60,7 +60,9 @@ class AntPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment contains dependencies needed by the plugin. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/plugins/autotools_plugin.py b/craft_parts/plugins/autotools_plugin.py index 261d01a2a..ee2788d7b 100644 --- a/craft_parts/plugins/autotools_plugin.py +++ b/craft_parts/plugins/autotools_plugin.py @@ -31,7 +31,7 @@ class AutotoolsPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "AutotoolsPluginProperties": """Populate autotools properties from the part specification. :param data: A dictionary containing part properties. diff --git a/craft_parts/plugins/cmake_plugin.py b/craft_parts/plugins/cmake_plugin.py index 56bafe87f..7f4add5a0 100644 --- a/craft_parts/plugins/cmake_plugin.py +++ b/craft_parts/plugins/cmake_plugin.py @@ -32,7 +32,7 @@ class CMakePluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "CMakePluginProperties": """Populate class attributes from the part specification. :param data: A dictionary containing part properties. diff --git a/craft_parts/plugins/dotnet_plugin.py b/craft_parts/plugins/dotnet_plugin.py index 2a8e66681..3c641b123 100644 --- a/craft_parts/plugins/dotnet_plugin.py +++ b/craft_parts/plugins/dotnet_plugin.py @@ -36,7 +36,7 @@ class DotnetPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "DotnetPluginProperties": """Populate make properties from the part specification. :param data: A dictionary containing part properties. @@ -58,7 +58,9 @@ class DotPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment contains dependencies needed by the plugin. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/plugins/dump_plugin.py b/craft_parts/plugins/dump_plugin.py index 405ce7a1b..322bc7134 100644 --- a/craft_parts/plugins/dump_plugin.py +++ b/craft_parts/plugins/dump_plugin.py @@ -29,7 +29,7 @@ class DumpPluginProperties(PluginProperties): """The part properties used by the dump plugin.""" @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "DumpPluginProperties": """Populate dump properties from the part specification. 'source' is a required part property. diff --git a/craft_parts/plugins/go_plugin.py b/craft_parts/plugins/go_plugin.py index 3c082784d..1401eec4d 100644 --- a/craft_parts/plugins/go_plugin.py +++ b/craft_parts/plugins/go_plugin.py @@ -38,7 +38,7 @@ class GoPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "GoPluginProperties": """Populate make properties from the part specification. :param data: A dictionary containing part properties. @@ -60,7 +60,9 @@ class GoPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment contains dependencies needed by the plugin. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/plugins/make_plugin.py b/craft_parts/plugins/make_plugin.py index a8f84cc99..a9ae00bd6 100644 --- a/craft_parts/plugins/make_plugin.py +++ b/craft_parts/plugins/make_plugin.py @@ -31,7 +31,7 @@ class MakePluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "MakePluginProperties": """Populate make properties from the part specification. :param data: A dictionary containing part properties. diff --git a/craft_parts/plugins/meson_plugin.py b/craft_parts/plugins/meson_plugin.py index 35f90adb9..419ccdc56 100644 --- a/craft_parts/plugins/meson_plugin.py +++ b/craft_parts/plugins/meson_plugin.py @@ -37,7 +37,7 @@ class MesonPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "MesonPluginProperties": """Populate make properties from the part specification. :param data: A dictionary containing part properties. @@ -59,7 +59,9 @@ class MesonPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment contains dependencies needed by the plugin. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/plugins/npm_plugin.py b/craft_parts/plugins/npm_plugin.py index 6f75ca691..fd9281e67 100644 --- a/craft_parts/plugins/npm_plugin.py +++ b/craft_parts/plugins/npm_plugin.py @@ -53,7 +53,7 @@ class NpmPluginProperties(PluginProperties, PluginModel): @root_validator @classmethod - def node_version_defined(cls, values): + def node_version_defined(cls, values: Dict[str, Any]) -> Dict[str, Any]: """If npm-include-node is true, then npm-node-version must be defined.""" if values.get("npm_include_node") and not values.get("npm_node_version"): raise ValueError("npm-node-version is required if npm-include-node is true") @@ -82,7 +82,9 @@ class NpmPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment has the dependencies to build npm applications. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/plugins/python_plugin.py b/craft_parts/plugins/python_plugin.py index 685e8dee5..4d525e736 100644 --- a/craft_parts/plugins/python_plugin.py +++ b/craft_parts/plugins/python_plugin.py @@ -35,7 +35,7 @@ class PythonPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "PythonPluginProperties": """Populate make properties from the part specification. :param data: A dictionary containing part properties. diff --git a/craft_parts/plugins/rust_plugin.py b/craft_parts/plugins/rust_plugin.py index 65724e1a3..a506d04ee 100644 --- a/craft_parts/plugins/rust_plugin.py +++ b/craft_parts/plugins/rust_plugin.py @@ -66,7 +66,9 @@ class RustPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment has the dependencies to build Rust applications. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/plugins/scons_plugin.py b/craft_parts/plugins/scons_plugin.py index e202a35e1..660e8ef2e 100644 --- a/craft_parts/plugins/scons_plugin.py +++ b/craft_parts/plugins/scons_plugin.py @@ -55,7 +55,9 @@ class SConsPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment contains dependencies needed by the plugin. :param part_dependencies: A list of the parts this part depends on. diff --git a/craft_parts/sequencer.py b/craft_parts/sequencer.py index 1b16755fb..8a0073010 100644 --- a/craft_parts/sequencer.py +++ b/craft_parts/sequencer.py @@ -306,7 +306,7 @@ def _update_step( reason: Optional[str] = None, outdated_files: Optional[List[str]] = None, outdated_dirs: Optional[List[str]] = None, - ): + ) -> None: """Set the step state as reexecuted by updating its timestamp.""" logger.debug("update step %s:%s", part.name, step) properties = ActionProperties( @@ -332,7 +332,7 @@ def _update_step( def _reapply_layer( self, part: Part, layer_hash: LayerHash, *, reason: Optional[str] = None - ): + ) -> None: """Update the layer hash without changing the step state.""" logger.debug("reapply layer %s: hash=%s", part.name, layer_hash) self._layer_state.set_layer_hash(part, layer_hash) diff --git a/craft_parts/sources/base.py b/craft_parts/sources/base.py index 681d124ff..095e24ea5 100644 --- a/craft_parts/sources/base.py +++ b/craft_parts/sources/base.py @@ -22,7 +22,7 @@ import shutil import subprocess from pathlib import Path -from typing import List, Optional, Sequence, Tuple +from typing import Any, List, Optional, Sequence, Tuple import requests from overrides import overrides @@ -77,7 +77,7 @@ def __init__( self.source_branch = source_branch self.source_depth = source_depth self.source_checksum = source_checksum - self.source_details = None + self.source_details: Any = None self.source_submodules = source_submodules self.command = command self._dirs = project_dirs @@ -118,7 +118,7 @@ def get_outdated_files(self) -> Tuple[List[str], List[str]]: """ raise errors.SourceUpdateUnsupported(self.__class__.__name__) - def update(self): + def update(self) -> None: """Update pulled source. :raise errors.SourceUpdateUnsupported: If the source can't update its files. @@ -126,7 +126,7 @@ def update(self): raise errors.SourceUpdateUnsupported(self.__class__.__name__) @classmethod - def _run(cls, command: List[str], **kwargs): + def _run(cls, command: List[str], **kwargs: Any) -> None: try: os_utils.process_run(command, logger.debug, **kwargs) except subprocess.CalledProcessError as err: diff --git a/craft_parts/sources/cache.py b/craft_parts/sources/cache.py index 7e9a3f387..f1ae6b1fb 100644 --- a/craft_parts/sources/cache.py +++ b/craft_parts/sources/cache.py @@ -67,6 +67,6 @@ def get(self, *, key: str) -> Optional[Path]: return None - def clean(self): + def clean(self) -> None: """Remove all files from the cache namespace.""" shutil.rmtree(self.file_cache) diff --git a/craft_parts/sources/deb_source.py b/craft_parts/sources/deb_source.py index 85bd07dea..b2b8a9402 100644 --- a/craft_parts/sources/deb_source.py +++ b/craft_parts/sources/deb_source.py @@ -82,7 +82,7 @@ def provision( dst: Path, keep: bool = False, src: Optional[Path] = None, - ): + ) -> None: """Extract deb file contents to the part source dir.""" if src: deb_file = src diff --git a/craft_parts/sources/git_source.py b/craft_parts/sources/git_source.py index f50ee2072..3a31763a3 100644 --- a/craft_parts/sources/git_source.py +++ b/craft_parts/sources/git_source.py @@ -21,7 +21,7 @@ import subprocess import sys from pathlib import Path -from typing import List, Optional +from typing import Dict, List, Optional, cast from overrides import overrides @@ -164,12 +164,13 @@ def __init__( # pylint: disable=too-many-arguments source_type="git", option="source-checksum" ) - def _fetch_origin_commit(self): + def _fetch_origin_commit(self) -> None: """Fetch from origin, using source-commit if defined.""" + command_str = cast(str, self.command) command = [ - self.command, + command_str, "-C", - self.part_src_dir, + str(self.part_src_dir), "fetch", "origin", ] @@ -178,7 +179,7 @@ def _fetch_origin_commit(self): self._run(command) - def _get_current_branch(self): + def _get_current_branch(self) -> str: """Get current git branch.""" command = [ self.command, @@ -190,7 +191,7 @@ def _get_current_branch(self): return self._run_output(command) - def _pull_existing(self): + def _pull_existing(self) -> None: """Pull data for an existing repository. For an existing (initialized) local git repository, pull from origin @@ -212,10 +213,12 @@ def _pull_existing(self): else: refspec = "refs/remotes/origin/" + self._get_current_branch() - command = [ - self.command, + command_prefix = [ + cast(str, self.command), "-C", - self.part_src_dir, + str(self.part_src_dir), + ] + command = command_prefix + [ "fetch", "--prune", ] @@ -224,15 +227,12 @@ def _pull_existing(self): command.append("--recurse-submodules=yes") self._run(command) - command = [self.command, "-C", self.part_src_dir, "reset", "--hard", refspec] + command = command_prefix + ["reset", "--hard", refspec] self._run(command) if self.source_submodules is None or len(self.source_submodules) > 0: - command = [ - self.command, - "-C", - self.part_src_dir, + command = command_prefix + [ "submodule", "update", "--recursive", @@ -243,16 +243,18 @@ def _pull_existing(self): command.append(submodule) self._run(command) - def _clone_new(self): + def _clone_new(self) -> None: """Clone a git repository, using submodules, branch, and depth if defined.""" - command = [self.command, "clone"] + command = [cast(str, self.command), "clone"] if self.source_submodules is None: command.append("--recursive") else: for submodule in self.source_submodules: command.append("--recursive=" + submodule) if self.source_tag or self.source_branch: - command.extend(["--branch", self.source_tag or self.source_branch]) + command.extend( + ["--branch", cast(str, self.source_tag or self.source_branch)] + ) if self.source_depth: command.extend(["--depth", str(self.source_depth)]) @@ -260,14 +262,14 @@ def _clone_new(self): command.append(self._format_source()) logger.debug("Executing: %s", " ".join([str(i) for i in command])) - self._run(command + [self.part_src_dir]) + self._run(command + [str(self.part_src_dir)]) if self.source_commit: self._fetch_origin_commit() command = [ - self.command, + cast(str, self.command), "-C", - self.part_src_dir, + str(self.part_src_dir), "checkout", self.source_commit, ] @@ -303,7 +305,7 @@ def pull(self) -> None: self._clone_new() self.source_details = self._get_source_details() - def _get_source_details(self): + def _get_source_details(self) -> Dict[str, Optional[str]]: """Return a dictionary containing current source parameters.""" tag = self.source_tag commit = self.source_commit diff --git a/craft_parts/sources/local_source.py b/craft_parts/sources/local_source.py index 5a7de62ed..1d822a142 100644 --- a/craft_parts/sources/local_source.py +++ b/craft_parts/sources/local_source.py @@ -22,7 +22,7 @@ import logging import os from pathlib import Path -from typing import List, Optional, Tuple +from typing import Any, Callable, List, Optional, Set, Tuple from overrides import overrides @@ -39,7 +39,12 @@ class LocalSource(SourceHandler): """The local source handler.""" - def __init__(self, *args, copy_function=file_utils.link_or_copy, **kwargs): + def __init__( + self, + *args: Any, + copy_function: Callable[..., None] = file_utils.link_or_copy, + **kwargs: Any + ): super().__init__(*args, **kwargs) self.source_abspath = os.path.abspath(self.source) self.copy_function = copy_function @@ -61,11 +66,11 @@ def __init__(self, *args, copy_function=file_utils.link_or_copy, **kwargs): self._ignore = functools.partial( _ignore, self.source_abspath, os.getcwd(), self._ignore_patterns ) - self._updated_files = set() - self._updated_directories = set() + self._updated_files: Set[str] = set() + self._updated_directories: Set[str] = set() @overrides - def pull(self): + def pull(self) -> None: """Retrieve the local source files.""" if not Path(self.source_abspath).exists(): raise errors.SourceNotFound(self.source) @@ -147,7 +152,7 @@ def get_outdated_files(self) -> Tuple[List[str], List[str]]: return (sorted(self._updated_files), sorted(self._updated_directories)) @overrides - def update(self): + def update(self) -> None: """Update pulled source. Call method :meth:`check_if_outdated` before updating to populate the @@ -174,8 +179,8 @@ def _ignore( source: str, current_directory: str, patterns: List[str], - directory, - files, + directory: str, + _files: Any, also_ignore: Optional[List[str]] = None, ) -> List[str]: if also_ignore: diff --git a/craft_parts/sources/sources.py b/craft_parts/sources/sources.py index 963f03414..b6e3d3512 100644 --- a/craft_parts/sources/sources.py +++ b/craft_parts/sources/sources.py @@ -147,7 +147,9 @@ def get_source_handler( return source_handler -def _get_source_handler_class(source, *, source_type: str = "") -> SourceHandlerType: +def _get_source_handler_class( + source: str, *, source_type: str = "" +) -> SourceHandlerType: """Return the appropriate handler class for the given source. :param source: The source specification. diff --git a/craft_parts/sources/tar_source.py b/craft_parts/sources/tar_source.py index bf7d8aa2a..ce7102857 100644 --- a/craft_parts/sources/tar_source.py +++ b/craft_parts/sources/tar_source.py @@ -20,7 +20,7 @@ import re import tarfile from pathlib import Path -from typing import List, Optional +from typing import Iterator, List, Optional from overrides import overrides @@ -82,7 +82,7 @@ def provision( dst: Path, keep: bool = False, src: Optional[Path] = None, - ): + ) -> None: """Extract tarball contents to the part source dir.""" if src: tarball = src @@ -98,7 +98,7 @@ def provision( def _extract(tarball: Path, dst: Path) -> None: with tarfile.open(tarball) as tar: - def filter_members(tar): + def filter_members(tar: tarfile.TarFile) -> Iterator[tarfile.TarInfo]: """Strip common prefix and ban dangerous names.""" members = tar.getmembers() common = os.path.commonprefix([m.name for m in members]) diff --git a/craft_parts/sources/zip_source.py b/craft_parts/sources/zip_source.py index 3884283f8..3b4738176 100644 --- a/craft_parts/sources/zip_source.py +++ b/craft_parts/sources/zip_source.py @@ -78,7 +78,7 @@ def provision( dst: Path, keep: bool = False, src: Optional[Path] = None, - ): + ) -> None: """Extract zip file contents to the part source dir.""" if src: zip_file = src diff --git a/craft_parts/state_manager/state_manager.py b/craft_parts/state_manager/state_manager.py index 5847879fb..42b1434e3 100644 --- a/craft_parts/state_manager/state_manager.py +++ b/craft_parts/state_manager/state_manager.py @@ -47,7 +47,7 @@ class _StateWrapper: serial: int step_updated: bool = False - def is_newer_than(self, other: "_StateWrapper"): + def is_newer_than(self, other: "_StateWrapper") -> bool: """Verify if this state is newer than the specified state. :param other: The wrapped state to compare this state to. @@ -58,7 +58,7 @@ def is_newer_than(self, other: "_StateWrapper"): class _StateDB: """A dictionary-backed simple database manager for wrapped states.""" - def __init__(self): + def __init__(self) -> None: self._state: Dict[Tuple[str, Step], _StateWrapper] = {} self._serial_gen = itertools.count(1) diff --git a/craft_parts/state_manager/step_state.py b/craft_parts/state_manager/step_state.py index 830312291..83103b72e 100644 --- a/craft_parts/state_manager/step_state.py +++ b/craft_parts/state_manager/step_state.py @@ -128,7 +128,7 @@ def diff_project_options_of_interest( ) @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> Any: """Create and populate a new state object from dictionary data.""" raise RuntimeError("this must be implemented by the step-specific class.") diff --git a/craft_parts/steps.py b/craft_parts/steps.py index a46d3da0d..f54cc45a2 100644 --- a/craft_parts/steps.py +++ b/craft_parts/steps.py @@ -41,7 +41,7 @@ class Step(enum.IntEnum): STAGE = 4 PRIME = 5 - def __repr__(self): + def __repr__(self) -> str: return f"{self.__class__.__name__}.{self.name}" def previous_steps(self) -> List["Step"]: diff --git a/craft_parts/utils/os_utils.py b/craft_parts/utils/os_utils.py index c91f70039..ed168180e 100644 --- a/craft_parts/utils/os_utils.py +++ b/craft_parts/utils/os_utils.py @@ -23,7 +23,7 @@ import sys import time from pathlib import Path -from typing import Callable, Dict, List, Optional +from typing import Any, Callable, Dict, List, Optional from craft_parts import errors @@ -72,7 +72,7 @@ def write_text( cls._last_write_time = time.time() -def get_bin_paths(*, root: Path, existing_only=True) -> List[str]: +def get_bin_paths(*, root: Path, existing_only: bool = True) -> List[str]: """List common system executable paths. :param root: A path to prepend to each entry in the list. @@ -109,7 +109,7 @@ def get_include_paths(*, root: Path, arch_triplet: str) -> List[str]: def get_library_paths( - *, root: Path, arch_triplet: str, existing_only=True + *, root: Path, arch_triplet: str, existing_only: bool = True ) -> List[str]: """List common library paths. @@ -224,7 +224,7 @@ def get_system_info() -> str: return uname -def mount(device: str, mountpoint: str, *args) -> None: +def mount(device: str, mountpoint: str, *args: str) -> None: """Mount a filesystem. :param device: The device to mount. @@ -237,7 +237,7 @@ def mount(device: str, mountpoint: str, *args) -> None: subprocess.check_call(["/bin/mount", *args, device, mountpoint]) -def mount_overlayfs(mountpoint: str, *args) -> None: +def mount_overlayfs(mountpoint: str, *args: str) -> None: """Mount an overlay filesystem using fuse-overlayfs. :param mountpoint: Where the device will be mounted. @@ -252,7 +252,7 @@ def mount_overlayfs(mountpoint: str, *args) -> None: _UMOUNT_RETRIES = 5 -def umount(mountpoint: str, *args) -> None: +def umount(mountpoint: str, *args: str) -> None: """Unmount a filesystem. :param mountpoint: The mount point or device to unmount. @@ -351,7 +351,9 @@ def version_codename(self) -> str: raise errors.OsReleaseCodenameError() -def process_run(command: List[str], log_func: Callable[[str], None], **kwargs) -> None: +def process_run( + command: List[str], log_func: Callable[[str], None], **kwargs: Any +) -> None: """Run a command and handle its output.""" with subprocess.Popen( command, diff --git a/craft_parts/utils/url_utils.py b/craft_parts/utils/url_utils.py index c57b70d70..3e0504138 100644 --- a/craft_parts/utils/url_utils.py +++ b/craft_parts/utils/url_utils.py @@ -22,6 +22,8 @@ import urllib.request from typing import Optional +import requests + from craft_parts.utils import os_utils logger = logging.getLogger(__name__) @@ -38,8 +40,11 @@ def is_url(url: str) -> bool: def download_request( - request, destination: str, message: Optional[str] = None, total_read: int = 0 -): + request: requests.Response, + destination: str, + message: Optional[str] = None, + total_read: int = 0, +) -> None: """Download a request with nice progress bars. :param request: The URL download request. diff --git a/setup.cfg b/setup.cfg index cbd752bbe..d92055e5c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -13,7 +13,11 @@ max-line-length = 88 extend-ignore = E203, E501 [mypy] -python_version = 3.8 +python_version = 3.7 +plugins = pydantic.mypy + +[mypy-craft_parts.*] +disallow_untyped_defs = True [pydocstyle] # D105 Missing docstring in magic method (reason: magic methods already have definitions) diff --git a/tests/unit/test_errors.py b/tests/unit/test_errors.py index 08a1d28ea..901f8a470 100644 --- a/tests/unit/test_errors.py +++ b/tests/unit/test_errors.py @@ -88,7 +88,7 @@ def test_part_specification_error(): assert err.resolution == "Review part 'foo' and make sure it's correct." -def test_part_specification_error_from_validation_error(): +def test_part_specification_error_from_validation_error() -> None: error_list: List["ErrorDict"] = [ {"loc": ("field-1", 0), "msg": "something is wrong", "type": "value_error"}, {"loc": ("field-2",), "msg": "field required", "type": "value_error"}, From 0406a709472402ea7d9c9d478e86c655106941d8 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Tue, 17 Jan 2023 09:32:54 -0300 Subject: [PATCH 2/5] fix tests that expected a Path instead of a str SourceHandler._run() takes a List[str], so it's incorrect to pass a Path. --- tests/unit/sources/test_git_source.py | 66 +++++++++++++-------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/tests/unit/sources/test_git_source.py b/tests/unit/sources/test_git_source.py index 697de1922..1a47cfdd1 100644 --- a/tests/unit/sources/test_git_source.py +++ b/tests/unit/sources/test_git_source.py @@ -122,7 +122,7 @@ def test_pull(self, fake_run, new_dir): git.pull() fake_run.assert_called_once_with( - ["git", "clone", "--recursive", "git://my-source", Path("source_dir")] + ["git", "clone", "--recursive", "git://my-source", "source_dir"] ) def test_pull_with_depth(self, fake_run, new_dir): @@ -140,7 +140,7 @@ def test_pull_with_depth(self, fake_run, new_dir): "--depth", "2", "git://my-source", - Path("source_dir"), + "source_dir", ] ) @@ -161,7 +161,7 @@ def test_pull_branch(self, fake_run, new_dir): "--branch", "my-branch", "git://my-source", - Path("source_dir"), + "source_dir", ] ) @@ -179,7 +179,7 @@ def test_pull_tag(self, fake_run, new_dir): "--branch", "tag", "git://my-source", - Path("source_dir"), + "source_dir", ] ) @@ -200,14 +200,14 @@ def test_pull_commit(self, fake_run, new_dir): "clone", "--recursive", "git://my-source", - Path("source_dir"), + "source_dir", ] ), mock.call( [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "origin", "2514f9533ec9b45d07883e10a561b248497a8e3c", @@ -217,7 +217,7 @@ def test_pull_commit(self, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "checkout", "2514f9533ec9b45d07883e10a561b248497a8e3c", ] @@ -230,7 +230,7 @@ def test_pull_with_submodules_default(self, fake_run, new_dir): git.pull() fake_run.assert_called_once_with( - ["git", "clone", "--recursive", "git://my-source", Path("source_dir")] + ["git", "clone", "--recursive", "git://my-source", "source_dir"] ) def test_pull_with_submodules_empty(self, fake_run, new_dir): @@ -243,7 +243,7 @@ def test_pull_with_submodules_empty(self, fake_run, new_dir): git.pull() fake_run.assert_called_once_with( - ["git", "clone", "git://my-source", Path("source_dir")] + ["git", "clone", "git://my-source", "source_dir"] ) def test_pull_with_submodules(self, fake_run, new_dir): @@ -262,7 +262,7 @@ def test_pull_with_submodules(self, fake_run, new_dir): "--recursive=submodule_1", "--recursive=dir/submodule_2", "git://my-source", - Path("source_dir"), + "source_dir", ] ) @@ -282,7 +282,7 @@ def test_pull_local(self, fake_run, new_dir): "clone", "--recursive", f"file://{new_dir}/path/to/repo.git", - Path("source_dir"), + "source_dir", ] ) @@ -316,7 +316,7 @@ def test_pull_repository_syntax(self, fake_run, new_dir, repository): "clone", "--recursive", repository, - Path("source_dir"), + "source_dir", ] ) @@ -332,7 +332,7 @@ def test_pull_existing(self, mocker, fake_run, fake_get_current_branch, new_dir) [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", "--recurse-submodules=yes", @@ -342,7 +342,7 @@ def test_pull_existing(self, mocker, fake_run, fake_get_current_branch, new_dir) [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "refs/remotes/origin/test-branch", @@ -352,7 +352,7 @@ def test_pull_existing(self, mocker, fake_run, fake_get_current_branch, new_dir) [ "git", "-C", - Path("source_dir"), + "source_dir", "submodule", "update", "--recursive", @@ -376,7 +376,7 @@ def test_pull_existing_with_tag(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", "--recurse-submodules=yes", @@ -386,7 +386,7 @@ def test_pull_existing_with_tag(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "refs/tags/tag", @@ -396,7 +396,7 @@ def test_pull_existing_with_tag(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "submodule", "update", "--recursive", @@ -423,7 +423,7 @@ def test_pull_existing_with_commit(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", "--recurse-submodules=yes", @@ -433,7 +433,7 @@ def test_pull_existing_with_commit(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "2514f9533ec9b45d07883e10a561b248497a8e3c", @@ -443,7 +443,7 @@ def test_pull_existing_with_commit(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "submodule", "update", "--recursive", @@ -470,7 +470,7 @@ def test_pull_existing_with_branch(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", "--recurse-submodules=yes", @@ -480,7 +480,7 @@ def test_pull_existing_with_branch(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "refs/remotes/origin/my-branch", @@ -490,7 +490,7 @@ def test_pull_existing_with_branch(self, mocker, fake_run, new_dir): [ "git", "-C", - Path("source_dir"), + "source_dir", "submodule", "update", "--recursive", @@ -590,7 +590,7 @@ def test_pull_existing_with_submodules_default( [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", "--recurse-submodules=yes", @@ -600,7 +600,7 @@ def test_pull_existing_with_submodules_default( [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "refs/remotes/origin/test-branch", @@ -610,7 +610,7 @@ def test_pull_existing_with_submodules_default( [ "git", "-C", - Path("source_dir"), + "source_dir", "submodule", "update", "--recursive", @@ -639,7 +639,7 @@ def test_pull_existing_with_submodules_empty( [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", ] @@ -648,7 +648,7 @@ def test_pull_existing_with_submodules_empty( [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "refs/remotes/origin/test-branch", @@ -676,7 +676,7 @@ def test_pull_existing_with_submodules( [ "git", "-C", - Path("source_dir"), + "source_dir", "fetch", "--prune", "--recurse-submodules=yes", @@ -686,7 +686,7 @@ def test_pull_existing_with_submodules( [ "git", "-C", - Path("source_dir"), + "source_dir", "reset", "--hard", "refs/remotes/origin/test-branch", @@ -696,7 +696,7 @@ def test_pull_existing_with_submodules( [ "git", "-C", - Path("source_dir"), + "source_dir", "submodule", "update", "--recursive", @@ -770,7 +770,7 @@ def test_pull_failure(self, mocker, new_dir): "clone", "--recursive", "git://my-source", - Path("source_dir"), + "source_dir", ] assert raised.value.exit_code == 1 From 232370e1c59acdeffa88676699448ca56c4742ef Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Wed, 18 Jan 2023 10:26:51 -0300 Subject: [PATCH 3/5] changes from review --- craft_parts/packages/snaps.py | 2 +- craft_parts/sources/base.py | 4 ++-- craft_parts/sources/git_source.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/craft_parts/packages/snaps.py b/craft_parts/packages/snaps.py index d1215981c..46aee79ba 100644 --- a/craft_parts/packages/snaps.py +++ b/craft_parts/packages/snaps.py @@ -344,7 +344,7 @@ def get_snapd_socket_path_template() -> str: return "http+unix://%2Frun%2Fsnapd.socket/v2/{}" -def _get_local_snap_file_iter(snap_name: str, *, chunk_size: int) -> Iterator: +def _get_local_snap_file_iter(snap_name: str, *, chunk_size: int) -> Iterator[bytes]: slug = f'snaps/{parse.quote(snap_name, safe="")}/file' url = get_snapd_socket_path_template().format(slug) try: diff --git a/craft_parts/sources/base.py b/craft_parts/sources/base.py index 095e24ea5..cf01f3242 100644 --- a/craft_parts/sources/base.py +++ b/craft_parts/sources/base.py @@ -22,7 +22,7 @@ import shutil import subprocess from pathlib import Path -from typing import Any, List, Optional, Sequence, Tuple +from typing import Any, Dict, List, Optional, Sequence, Tuple import requests from overrides import overrides @@ -77,7 +77,7 @@ def __init__( self.source_branch = source_branch self.source_depth = source_depth self.source_checksum = source_checksum - self.source_details: Any = None + self.source_details: Optional[Dict[str, Optional[str]]] = None self.source_submodules = source_submodules self.command = command self._dirs = project_dirs diff --git a/craft_parts/sources/git_source.py b/craft_parts/sources/git_source.py index 3a31763a3..4d90e3aec 100644 --- a/craft_parts/sources/git_source.py +++ b/craft_parts/sources/git_source.py @@ -184,7 +184,7 @@ def _get_current_branch(self) -> str: command = [ self.command, "-C", - self.part_src_dir, + str(self.part_src_dir), "branch", "--show-current", ] @@ -315,7 +315,7 @@ def _get_source_details(self) -> Dict[str, Optional[str]]: if not tag and not branch and not commit: commit = self._run_output( - ["git", "-C", self.part_src_dir, "rev-parse", "HEAD"] + ["git", "-C", str(self.part_src_dir), "rev-parse", "HEAD"] ) return { From d30336704ed3eda9bf32ca419acd10ad33f34c8f Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Wed, 18 Jan 2023 10:40:29 -0300 Subject: [PATCH 4/5] add missing annotations in MavenPlugin --- craft_parts/plugins/maven_plugin.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/craft_parts/plugins/maven_plugin.py b/craft_parts/plugins/maven_plugin.py index e7249a014..ba586a4c0 100644 --- a/craft_parts/plugins/maven_plugin.py +++ b/craft_parts/plugins/maven_plugin.py @@ -39,7 +39,7 @@ class MavenPluginProperties(PluginProperties, PluginModel): source: str @classmethod - def unmarshal(cls, data: Dict[str, Any]): + def unmarshal(cls, data: Dict[str, Any]) -> "MavenPluginProperties": """Populate class attributes from the part specification. :param data: A dictionary containing part properties. @@ -61,7 +61,9 @@ class MavenPluginEnvironmentValidator(validator.PluginEnvironmentValidator): :param env: A string containing the build step environment setup. """ - def validate_environment(self, *, part_dependencies: Optional[List[str]] = None): + def validate_environment( + self, *, part_dependencies: Optional[List[str]] = None + ) -> None: """Ensure the environment contains dependencies needed by the plugin. :param part_dependencies: A list of the parts this part depends on. From 3b0a6885031f3a41b9a93421446f6e39d60e6cb3 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Thu, 19 Jan 2023 14:06:50 -0300 Subject: [PATCH 5/5] add a TODO instead of a hasattr() --- craft_parts/overlays/overlay_manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/craft_parts/overlays/overlay_manager.py b/craft_parts/overlays/overlay_manager.py index 827e2f543..e5980213e 100644 --- a/craft_parts/overlays/overlay_manager.py +++ b/craft_parts/overlays/overlay_manager.py @@ -129,8 +129,8 @@ def refresh_packages_list(self) -> None: mount_dir = self._project_info.overlay_mount_dir # Ensure we always run refresh_packages_list by resetting the cache - if hasattr(packages.Repository.refresh_packages_list, "cache_clear"): - packages.Repository.refresh_packages_list.cache_clear() # type: ignore + # TODO: this assumes an Ubuntu repository, we must improve this API + packages.Repository.refresh_packages_list.cache_clear() # type: ignore chroot.chroot(mount_dir, packages.Repository.refresh_packages_list) def download_packages(self, package_names: List[str]) -> None: