Enable typeguard during build time and tests, fix issues found

It is also possible to use typeguard in osc-wrapper.py
by setting OSC_TYPEGUARD=1.

Author: dmach

.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
@@ -149,4 +150,4 @@ jobs:
       - name: "Run tests"
         run: |
           cd behave
-          behave -Dosc=../osc-wrapper.py -Dgit-obs=../git-obs.py -Dpodman_max_containers=2
+          OSC_TYPEGUARD=1 behave -Dosc=../osc-wrapper.py -Dgit-obs=../git-obs.py -Dpodman_max_containers=2

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

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()

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`.
@@ -4914,6 +4915,8 @@ def do_rdiff(self, subcmd, opts, *args):
                     rev2 = -rev - 1
                 else:
                     return
+                rev1 = str(rev1)
+                rev2 = str(rev2)
             except:
                 print(f'Revision \'{opts.change}\' not an integer', file=sys.stderr)
                 return

osc/core.py

@@ -1325,7 +1325,7 @@ def show_package_trigger_reason(apiurl: str, prj: str, pac: str, repo: str, arch
         raise
 
 
-def show_package_meta(apiurl: str, prj: str, pac: str, meta=False, blame=None):
+def show_package_meta(apiurl: str, prj: str, pac: str, meta=False, blame=None) -> List[bytes]:
     query: Dict[str, Union[str, int]] = {}
     if meta:
         query['meta'] = 1
@@ -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,20 +2863,20 @@ 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,
     expand=True,
     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,
@@ -3214,9 +3214,9 @@ def checkout_package(
 
 
 def replace_pkg_meta(
-    pkgmeta, new_name: str, new_prj: str, keep_maintainers=False, dst_userid=None, keep_develproject=False,
+    pkgmeta: List[bytes], new_name: str, new_prj: str, keep_maintainers=False, dst_userid=None, keep_develproject=False,
     keep_lock: bool = False, keep_scmsync: bool = True,
-):
+) -> str:
     """
     update pkgmeta with new new_name and new_prj and set calling user as the
     only maintainer (unless keep_maintainers is set). Additionally remove the
@@ -3450,7 +3450,7 @@ def aggregate_pac(
 
     if meta_change:
         src_meta = show_package_meta(apiurl, src_project, src_package_meta)
-        dst_meta = replace_pkg_meta(src_meta, dst_package_meta, dst_project)
+        dst_meta = replace_pkg_meta(src_meta, dst_package_meta, dst_project).split("\n")
         meta_change = True
 
     if disable_publish:
@@ -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]
@@ -4756,25 +4756,26 @@ def get_commitlog(
             # revision is srcmd5
             revision_list = [i for i in revision_list if i.srcmd5 == revision]
         else:
-            revision = int(revision)
+            assert revision is not None
+            revision_int = int(revision)
             if revision_is_empty(revision_upper):
-                revision_list = [i for i in revision_list if i.rev == revision]
+                revision_list = [i for i in revision_list if i.rev == revision_int]
             else:
-                revision_upper = int(revision_upper)
-                revision_list = [i for i in revision_list if i.rev <= revision_upper and i.rev >= revision]
+                revision_upper_int = int(revision_upper)
+                revision_list = [i for i in revision_list if i.rev <= revision_upper_int and i.rev >= revision_int]
 
     if format == "csv":
         f = io.StringIO()
         writer = csv.writer(f, dialect="unix")
-        for revision in reversed(revision_list):
+        for i in reversed(revision_list):
             writer.writerow(
                 (
-                    revision.rev,
-                    revision.user,
-                    revision.get_time_str(),
-                    revision.srcmd5,
-                    revision.comment,
-                    revision.requestid,
+                    i.rev,
+                    i.user,
+                    i.get_time_str(),
+                    i.srcmd5,
+                    i.comment,
+                    i.requestid,
                 )
             )
         f.seek(0)
@@ -4783,42 +4784,42 @@ def get_commitlog(
 
     if format == "xml":
         root = ET.Element("log")
-        for revision in reversed(revision_list):
+        for i in reversed(revision_list):
             entry = ET.SubElement(root, "logentry")
-            entry.attrib["revision"] = str(revision.rev)
-            entry.attrib["srcmd5"] = revision.srcmd5
-            ET.SubElement(entry, "author").text = revision.user
-            ET.SubElement(entry, "date").text = revision.get_time_str()
-            ET.SubElement(entry, "requestid").text = str(revision.requestid) if revision.requestid else ""
-            ET.SubElement(entry, "msg").text = revision.comment or ""
+            entry.attrib["revision"] = str(i.rev)
+            entry.attrib["srcmd5"] = i.srcmd5
+            ET.SubElement(entry, "author").text = i.user
+            ET.SubElement(entry, "date").text = i.get_time_str()
+            ET.SubElement(entry, "requestid").text = str(i.requestid) if i.requestid else ""
+            ET.SubElement(entry, "msg").text = i.comment or ""
         xmlindent(root)
         yield from ET.tostring(root, encoding="utf-8").decode("utf-8").splitlines()
         return
 
     if format == "text":
-        for revision in reversed(revision_list):
+        for i in reversed(revision_list):
             entry = (
-                f"r{revision.rev}",
-                revision.user,
-                revision.get_time_str(),
-                revision.srcmd5,
-                revision.version,
-                f"rq{revision.requestid}" if revision.requestid else ""
+                f"r{i.rev}",
+                i.user,
+                i.get_time_str(),
+                i.srcmd5,
+                i.version,
+                f"rq{i.requestid}" if i.requestid else ""
             )
             yield 76 * "-"
             yield " | ".join(entry)
             yield ""
-            yield revision.comment or "<no message>"
+            yield i.comment or "<no message>"
             yield ""
             if patch:
                 rdiff = server_diff_noex(
                     apiurl,
                     prj,
                     package,
-                    revision.rev - 1,
+                    str(i.rev - 1),
                     prj,
                     package,
-                    revision.rev,
+                    str(i.rev),
                     meta=meta,
                 )
                 yield highlight_diff(rdiff).decode("utf-8", errors="replace")
@@ -5158,7 +5159,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 +5180,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.

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: