From d6fedddc5e3811358ac53fed36ee11fc8c033329 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Mon, 13 May 2024 14:10:51 +0200 Subject: [PATCH] Enable typeguard during build time and tests, fix issues found --- .github/workflows/tests.yaml | 1 + contrib/osc.spec | 10 ++++++++++ osc-wrapper.py | 23 +++++++++++++++++++++++ osc/commandline.py | 29 +++++++++++++++-------------- osc/core.py | 30 +++++++++++++++--------------- osc/meter.py | 1 - osc/output/output.py | 6 ++++-- osc/util/ar.py | 2 +- osc/util/models.py | 2 +- osc/util/safewriter.py | 19 ++++++++++++++++++- tests/__init__.py | 14 ++++++++++++++ 11 files changed, 102 insertions(+), 35 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index f23cf75617..bf54c0f27e 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -134,6 +134,7 @@ jobs: sudo apt-get -y --no-install-recommends install git python3-behave diffstat diffutils python3 python3-cryptography python3-pip python3-rpm python3-ruamel.yaml python3-setuptools python3-urllib3 obs-build obs-service-set-version # obs-scm-bridge is not available as a package at the moment, install it from github sudo pip3 config set global.break-system-packages 1 + sudo pip3 install typeguard sudo pip3 install git+https://github.com/openSUSE/obs-scm-bridge sudo chmod a+x /usr/local/lib/*/*/obs_scm_bridge sudo mkdir -p /usr/lib/obs/service diff --git a/contrib/osc.spec b/contrib/osc.spec index 2659754d3d..79fbb998a0 100644 --- a/contrib/osc.spec +++ b/contrib/osc.spec @@ -30,6 +30,13 @@ %bcond_with fdupes %endif +# use typeguard during build on distros where typeguard is available +%if (0%{?suse_version} > 1500 || 0%{?fedora} >= 37) +%bcond_without typeguard +%else +%bcond_with typeguard +%endif + # the macro exists only on openSUSE based distros %if %{undefined python3_fix_shebang} %define python3_fix_shebang %nil @@ -76,6 +83,9 @@ BuildRequires: %{use_python_pkg}-cryptography BuildRequires: %{use_python_pkg}-devel >= 3.6 BuildRequires: %{use_python_pkg}-rpm BuildRequires: %{use_python_pkg}-setuptools +%if %{with typeguard} +BuildRequires: %{use_python_pkg}-typeguard +%endif BuildRequires: %{use_python_pkg}-urllib3 BuildRequires: %{ruamel_yaml_pkg} BuildRequires: diffstat diff --git a/osc-wrapper.py b/osc-wrapper.py index 952f69cf62..4cc02a26b4 100755 --- a/osc-wrapper.py +++ b/osc-wrapper.py @@ -4,6 +4,29 @@ This wrapper allows osc to be called from the source directory during development. """ + +import os + + +USE_TYPEGUARD = os.environ.get("OSC_TYPEGUARD", "1").lower() in ("1", "true", "on") + +if USE_TYPEGUARD: + try: + from typeguard import install_import_hook + except ImportError: + install_import_hook = None + + if install_import_hook is None: + try: + from typeguard.importhook import install_import_hook + except ImportError: + install_import_hook = None + + if install_import_hook: + # install typeguard import hook only if available + install_import_hook("osc") + + import osc.babysitter osc.babysitter.main() diff --git a/osc/commandline.py b/osc/commandline.py index ca8350d2f5..793b265ada 100644 --- a/osc/commandline.py +++ b/osc/commandline.py @@ -24,6 +24,7 @@ from pathlib import Path from tempfile import NamedTemporaryFile from typing import List +from typing import Optional from urllib.parse import urlsplit from urllib.error import HTTPError @@ -316,10 +317,10 @@ def pop_args( args, arg1_name: str = None, arg1_is_optional: bool = False, - arg1_default: str = None, + arg1_default: Optional[str] = None, arg2_name: str = None, arg2_is_optional: bool = False, - arg2_default: str = None, + arg2_default: Optional[str] = None, ): """ Pop 2 arguments from `args`. @@ -391,9 +392,9 @@ def pop_args( def pop_project_package_from_args( args: List[str], project_is_optional: bool = False, - default_project: str = None, + default_project: Optional[str] = None, package_is_optional: bool = False, - default_package: str = None, + default_package: Optional[str] = None, ): """ Pop project and package from given `args`. @@ -464,9 +465,9 @@ def pop_project_package_from_args( def pop_repository_arch_from_args( args: List[str], repository_is_optional: bool = False, - default_repository: str = None, + default_repository: Optional[str] = None, arch_is_optional: bool = False, - default_arch: str = None, + default_arch: Optional[str] = None, ): """ Pop repository and arch from given `args`. @@ -503,13 +504,13 @@ def pop_repository_arch_from_args( def pop_project_package_repository_arch_from_args( args: List[str], project_is_optional: bool = False, - default_project: str = None, + default_project: Optional[str] = None, package_is_optional: bool = False, - default_package: str = None, + default_package: Optional[str] = None, repository_is_optional: bool = False, - default_repository: str = None, + default_repository: Optional[str] = None, arch_is_optional: bool = False, - default_arch: str = None, + default_arch: Optional[str] = None, ): """ Pop project, package, repository and arch from given `args`. @@ -589,13 +590,13 @@ def pop_project_package_repository_arch_from_args( def pop_project_package_targetproject_targetpackage_from_args( args: List[str], project_is_optional: bool = False, - default_project: str = None, + default_project: Optional[str] = None, package_is_optional: bool = False, - default_package: str = None, + default_package: Optional[str] = None, target_project_is_optional: bool = False, - default_target_project: str = None, + default_target_project: Optional[str] = None, target_package_is_optional: bool = False, - default_target_package: str = None, + default_target_package: Optional[str] = None, ): """ Pop project, package, target project and target package from given `args`. diff --git a/osc/core.py b/osc/core.py index b6868667ed..6904026751 100644 --- a/osc/core.py +++ b/osc/core.py @@ -2320,7 +2320,7 @@ def get_request_collection( package=None, states=None, review_states=None, - types: List[str] = None, + types: Optional[List[str]] = None, ids=None, withfullhistory=False ): @@ -2863,12 +2863,12 @@ def get_source_file_diff(dir, filename, rev, oldfilename=None, olddir=None, orig def server_diff( apiurl: str, - old_project: str, - old_package: str, - old_revision: str, + old_project: Optional[str], + old_package: Optional[str], + old_revision: Optional[str], new_project: str, new_package: str, - new_revision: str, + new_revision: Optional[str], unified=False, missingok=False, meta=False, @@ -2876,7 +2876,7 @@ def server_diff( onlyissues=False, full=True, xml=False, - files: list = None, + files: Optional[list] = None, ): query: Dict[str, Union[str, int]] = {"cmd": "diff"} if expand: @@ -2929,19 +2929,19 @@ def server_diff( def server_diff_noex( apiurl: str, - old_project: str, - old_package: str, - old_revision: str, + old_project: Optional[str], + old_package: Optional[str], + old_revision: Optional[str], new_project: str, new_package: str, - new_revision: str, + new_revision: Optional[str], unified=False, missingok=False, meta=False, expand=True, onlyissues=False, xml=False, - files: list = None, + files: Optional[list] = None, ): try: return server_diff(apiurl, @@ -3096,7 +3096,7 @@ def checkout_package( pathname=None, prj_obj=None, expand_link=False, - prj_dir: Path=None, + prj_dir: Optional[Path] = None, server_service_files=None, service_files=None, native_obs_package=False, @@ -3808,7 +3808,7 @@ def copy_pac( return 'Done.' -def lock(apiurl: str, project: str, package: str, msg: str = None): +def lock(apiurl: str, project: str, package: str, msg: Optional[str] = None): url_path = ["source", project] if package: url_path += [package] @@ -5158,7 +5158,7 @@ def owner( return res -def set_link_rev(apiurl: str, project: str, package: str, revision="", expand=False, msg: str=None, vrev: str=None): +def set_link_rev(apiurl: str, project: str, package: str, revision="", expand=False, msg: Optional[str] = None, vrev: Optional[str] = None): url = makeurl(apiurl, ["source", project, package, "_link"]) try: f = http_GET(url) @@ -5179,7 +5179,7 @@ def set_link_rev(apiurl: str, project: str, package: str, revision="", expand=Fa return revision -def _set_link_rev(apiurl: str, project: str, package: str, root, revision="", expand=False, setvrev: str=None): +def _set_link_rev(apiurl: str, project: str, package: str, root, revision="", expand=False, setvrev: Optional[str] = None): """ Updates the rev attribute of the _link xml. If revision is set to None the rev and vrev attributes are removed from the _link xml. diff --git a/osc/meter.py b/osc/meter.py index 466a581860..d8bc152d00 100644 --- a/osc/meter.py +++ b/osc/meter.py @@ -102,7 +102,6 @@ def create_text_meter(*args, **kwargs) -> TextMeterBase: use_pb_fallback = kwargs.pop("use_pb_fallback", False) - meter_class: TextMeterBase if config.quiet: meter_class = NoTextMeter elif not have_pb_module or not config.show_download_progress or not sys.stdout.isatty() or use_pb_fallback: diff --git a/osc/output/output.py b/osc/output/output.py index 50aa98b598..75b5f80165 100644 --- a/osc/output/output.py +++ b/osc/output/output.py @@ -5,7 +5,9 @@ import subprocess import sys import tempfile +from typing import BinaryIO from typing import Dict +from typing import Generator from typing import List from typing import Optional from typing import TextIO @@ -137,7 +139,7 @@ def safe_print(*args, **kwargs): print(*args, **kwargs) -def safe_write(file: TextIO, text: Union[str, bytes], *, add_newline: bool = False): +def safe_write(file: Union[BinaryIO, TextIO], text: Union[str, bytes], *, add_newline: bool = False): """ Run sanitize_text() on ``text`` and write it to ``file``. @@ -211,7 +213,7 @@ def run_pager(message: Union[bytes, str], tmp_suffix: str = ""): run_external(*cmd, env=env) -def pipe_to_pager(lines: Union[List[bytes], List[str]], *, add_newlines=False): +def pipe_to_pager(lines: Union[List[bytes], List[str], Generator[bytes, None, None], Generator[str, None, None]], *, add_newlines=False): """ Pipe ``lines`` to the pager. If running in a non-interactive terminal, print the data instead. diff --git a/osc/util/ar.py b/osc/util/ar.py index 8c501e66c7..6578b7fc13 100644 --- a/osc/util/ar.py +++ b/osc/util/ar.py @@ -37,7 +37,7 @@ def __str__(self): class ArHdr: """Represents an ar header entry""" - def __init__(self, fn: bytes, date: bytes, uid: bytes, gid: bytes, mode: bytes, size: bytes, fmag: bytes, off: bytes): + def __init__(self, fn: bytes, date: bytes, uid: bytes, gid: bytes, mode: bytes, size: bytes, fmag: bytes, off: int): self.file = fn.strip() self.date = date.strip() self.uid = uid.strip() diff --git a/osc/util/models.py b/osc/util/models.py index caff6abd00..5e9cb0bada 100644 --- a/osc/util/models.py +++ b/osc/util/models.py @@ -782,7 +782,7 @@ def xml_request( apiurl: str, path: List[str], query: Optional[dict] = None, - headers: Optional[str] = None, + headers: Optional[dict] = None, data: Optional[str] = None, ) -> urllib3.response.HTTPResponse: from ..connection import http_request diff --git a/osc/util/safewriter.py b/osc/util/safewriter.py index 817948c0e5..75ef5cf9e4 100644 --- a/osc/util/safewriter.py +++ b/osc/util/safewriter.py @@ -1,6 +1,8 @@ +import io + # be careful when debugging this code: # don't add print statements when setting sys.stdout = SafeWriter(sys.stdout)... -class SafeWriter: +class SafeWriter(io.TextIOBase): """ Safely write an (unicode) str. In case of an "UnicodeEncodeError" the the str is encoded with the "encoding" encoding. @@ -8,15 +10,30 @@ class SafeWriter: """ def __init__(self, writer, encoding='unicode_escape'): + super().__init__() self._writer = writer self._encoding = encoding + # TextIOBase requires overriding the following stub methods: detach, read, readline, and write + + def detach(self, *args, **kwargs): + return self._writer.detach(*args, **kwargs) + + def read(self, *args, **kwargs): + return self._writer.read(args, **kwargs) + + def readline(self, *args, **kwargs): + return self._writer.readline(args, **kwargs) + def write(self, s): try: self._writer.write(s) except UnicodeEncodeError as e: self._writer.write(s.encode(self._encoding)) + def fileno(self, *args, **kwargs): + return self._writer.fileno(*args, **kwargs) + def __getattr__(self, name): return getattr(self._writer, name) diff --git a/tests/__init__.py b/tests/__init__.py index e69de29bb2..2740d5773f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -0,0 +1,14 @@ +try: + from typeguard import install_import_hook +except ImportError: + install_import_hook = None + +if not install_import_hook: + try: + from typeguard.importhook import install_import_hook + except ImportError: + install_import_hook = None + +if install_import_hook: + # install typeguard import hook only if available + install_import_hook("osc")