[b4 PATCH RESEND 0/5] Resolve some static typing errors

[b4 PATCH RESEND 0/5] Resolve some static typing errors

[Tamir Duberstein]

This series slightly improves the quality of type annotations in the
project. It also adds development dependencies so they can more easily
be installed.

Unfortunately this is only a drop in the bucket; there are still 356
errors as reported by pyright. Without minimal tooling to ensure these
don't gress, progress will be difficult.

I'd be happy to contribute such tooling, but I didn't find where to put
it - there's no obvious place from which tests are run, for example.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Tamir Duberstein (5):
      Add development dependencies
      Avoid file descriptor leak
      Correctly type annotate generators
      Provide overloads for git_run_command
      Add missing imports

 pyproject.toml             |  8 ++++++-
 src/b4/__init__.py         | 52 ++++++++++++++++++++++++++++++----------------
 src/b4/diff.py             |  7 ++++---
 src/b4/ez.py               |  2 ++
 src/b4/pr.py               |  2 ++
 src/b4/ty.py               |  1 +
 src/tests/test___init__.py |  1 +
 7 files changed, 51 insertions(+), 22 deletions(-)
---
base-commit: 2a6338e451a0c1e81f214f48c820c1e52d76b2f1
change-id: 20241025-better-type-annotations-e732fbdb2ff5

Best regards,
-----BEGIN SSH SIGNATURE-----
U1NIU0lHAAAAAQAAADMAAAALc3NoLWVkMjU1MTkAAAAgtYz36g7iDMSkY5K7Ab51ksGX7h
JgsMRt+XVZTrIzMVIAAAADZ2l0AAAAAAAAAAZzaGE1MTIAAABTAAAAC3NzaC1lZDI1NTE5
AAAAQDyp0EuH8MRQJAmLtKb9/nsjczbFhwM2g0XcY70EEkGF87t28QCADDPBXv4SIqesgc
b8/3E18A09Qv9cHgcldw8=
-----END SSH SIGNATURE-----
-- 
Tamir Duberstein <tamird@gmail.com>

[PATCH RESEND b4 1/5] Add development dependencies

[Tamir Duberstein]

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 pyproject.toml | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/pyproject.toml b/pyproject.toml
index 31bfe3033650b2b251fcdcbab0456f4a67e914df..7c65c30efdb95fa40cc1f22933bcd62073db4f26 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -22,7 +22,13 @@ classifiers = [
     "Topic :: Software Development",
     "Topic :: Utilities",
 ]
-dynamic = ["dependencies", "optional-dependencies"]
+dynamic = ["dependencies"]
+
+[project.optional-dependencies]
+dev = [
+    "pytest",
+    "types-requests",
+]
 
 [tool.setuptools.dynamic]
 dependencies = { file = ["requirements.in"] }

-- 
2.47.0

[PATCH RESEND b4 2/5] Avoid file descriptor leak

[Tamir Duberstein]

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 src/b4/diff.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/b4/diff.py b/src/b4/diff.py
index 63e60f2fc82c95145a897c2dd319283996ed5a8a..743127ef0a1922eace81686e27cd0366f8e42f9c 100644
--- a/src/b4/diff.py
+++ b/src/b4/diff.py
@@ -174,8 +174,8 @@ def main(cmdargs: argparse.Namespace) -> None:
         sys.exit(1)
     if cmdargs.outdiff is not None:
         logger.info('Writing %s', cmdargs.outdiff)
-        fh = open(cmdargs.outdiff, 'w')
+        with open(cmdargs.outdiff, 'w') as fh:
+            fh.write(rdiff)
     else:
         logger.info('---')
-        fh = sys.stdout
-    fh.write(rdiff)
+        sys.stdout.write(rdiff)

-- 
2.47.0

[PATCH RESEND b4 3/5] Correctly type annotate generators

[Tamir Duberstein]

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 src/b4/__init__.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index 5115a9cd9b8746f84eb104a2be69fe121724167e..d1f0fc5b56693d7d234bc4aced65166244928728 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -36,7 +36,7 @@ import requests
 
 from pathlib import Path
 from contextlib import contextmanager
-from typing import Optional, Tuple, Set, List, BinaryIO, Union, Sequence, Literal, Iterator, Dict
+from typing import Optional, Tuple, Set, List, BinaryIO, Union, Sequence, Literal, Iterator, Dict, overload
 
 from email import charset
 
@@ -2758,7 +2758,7 @@ def git_get_repo_status(gitdir: Optional[str] = None, untracked: bool = False) -
 
 
 @contextmanager
-def git_temp_worktree(gitdir: Optional[str] = None, commitish: Optional[str] = None) -> Optional[Iterator[Path]]:
+def git_temp_worktree(gitdir: Optional[str] = None, commitish: Optional[str] = None) -> Generator[str]:
     """Context manager that creates a temporary work tree and chdirs into it. The
     worktree is deleted when the contex manager is closed. Taken from gj_tools."""
     dfn = None
@@ -2776,7 +2776,7 @@ def git_temp_worktree(gitdir: Optional[str] = None, commitish: Optional[str] = N
 
 
 @contextmanager
-def git_temp_clone(gitdir: Optional[str] = None) -> Optional[Iterator[Path]]:
+def git_temp_clone(gitdir: Optional[str] = None) -> Generator[str]:
     """Context manager that creates a temporary shared clone."""
     if gitdir is None:
         topdir = git_get_toplevel()
@@ -2794,7 +2794,7 @@ def git_temp_clone(gitdir: Optional[str] = None) -> Optional[Iterator[Path]]:
 
 
 @contextmanager
-def in_directory(dirname: str) -> Iterator[bool]:
+def in_directory(dirname: str) -> Generator[bool]:
     """Context manager that chdirs into a directory and restores the original
     directory when closed. Taken from gj_tools."""
     cdir = os.getcwd()

-- 
2.47.0

[PATCH RESEND b4 4/5] Provide overloads for git_run_command

[Tamir Duberstein]

This requires explicit use of kwargs to allow decode to be a required
parameter in the negative case (which is necessary to disambiguate the
overloads).

Callers now know for certain whether they get str or bytes.

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 src/b4/__init__.py | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index d1f0fc5b56693d7d234bc4aced65166244928728..e71fbe7d8ea8d4ca19228690ce7c186cc91b81ad 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -36,7 +36,7 @@ import requests
 
 from pathlib import Path
 from contextlib import contextmanager
-from typing import Optional, Tuple, Set, List, BinaryIO, Union, Sequence, Literal, Iterator, Dict, overload
+from typing import Optional, Tuple, Set, List, BinaryIO, Union, Sequence, Literal, Iterator, Dict, TypeVar, overload
 
 from email import charset
 
@@ -2697,14 +2697,27 @@ def gpg_run_command(args: List[str], stdin: Optional[bytes] = None) -> Tuple[int
     return _run_command(cmdargs, stdin=stdin)
 
 
+@overload
+def git_run_command(gitdir: Optional[Union[str, Path]], args: List[str], stdin: Optional[bytes] = ...,
+                    *, logstderr: bool = ..., decode: Literal[False],
+                    rundir: Optional[str] = ...) -> Tuple[int, bytes]:
+    ...
+
+
+@overload
+def git_run_command(gitdir: Optional[Union[str, Path]], args: List[str], stdin: Optional[bytes] = ...,
+                    *, logstderr: bool = ..., decode: Literal[True] = ...,
+                    rundir: Optional[str] = ...) -> Tuple[int, str]:
+    ...
+
 def git_run_command(gitdir: Optional[Union[str, Path]], args: List[str], stdin: Optional[bytes] = None,
-                    logstderr: bool = False, decode: bool = True,
+                    *, logstderr: bool = False, decode: bool = True,
                     rundir: Optional[str] = None) -> Tuple[int, Union[str, bytes]]:
     cmdargs = ['git', '--no-pager']
     if gitdir:
         if os.path.exists(os.path.join(gitdir, '.git')):
             gitdir = os.path.join(gitdir, '.git')
-        cmdargs += ['--git-dir', gitdir]
+        cmdargs += ['--git-dir', str(gitdir)]
 
     # counteract some potential local settings
     if args[0] == 'log':
@@ -2714,16 +2727,18 @@ def git_run_command(gitdir: Optional[Union[str, Path]], args: List[str], stdin:
 
     ecode, out, err = _run_command(cmdargs, stdin=stdin, rundir=rundir)
 
-    if decode:
-        out = out.decode(errors='replace')
+    U = TypeVar('U', str, bytes)
+
+    def _handle(out: U, err: U) -> Tuple[int, Union[str, bytes]]:
+        if logstderr and len(err.strip()):
+            logger.debug('Stderr: %s', err)
+            out += err
 
-    if logstderr and len(err.strip()):
-        if decode:
-            err = err.decode(errors='replace')
-        logger.debug('Stderr: %s', err)
-        out += err
+        return ecode, out
 
-    return ecode, out
+    if decode:
+        return _handle(out.decode(errors='replace'), err.decode(errors='replace'))
+    return _handle(out, err)
 
 
 def git_credential_fill(gitdir: Optional[str], protocol: str, host: str, username: str) -> Optional[str]:

-- 
2.47.0

[PATCH RESEND b4 5/5] Add missing imports

[Tamir Duberstein]

Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
 src/b4/__init__.py         | 9 +++++----
 src/b4/diff.py             | 1 +
 src/b4/ez.py               | 2 ++
 src/b4/pr.py               | 2 ++
 src/b4/ty.py               | 1 +
 src/tests/test___init__.py | 1 +
 6 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/b4/__init__.py b/src/b4/__init__.py
index e71fbe7d8ea8d4ca19228690ce7c186cc91b81ad..7b1925da93e1b3feadf669cc188ab6fb030f67f4 100644
--- a/src/b4/__init__.py
+++ b/src/b4/__init__.py
@@ -8,12 +8,13 @@ import sys
 import gzip
 import os
 import fnmatch
-import email.utils
-import email.policy
-import email.parser
-import email.header
 import email.generator
+import email.header
+import email.message
+import email.parser
+import email.policy
 import email.quoprimime
+import email.utils
 import tempfile
 import pathlib
 import argparse
diff --git a/src/b4/diff.py b/src/b4/diff.py
index 743127ef0a1922eace81686e27cd0366f8e42f9c..1de3063ea0b3c73e4e3d8bfd4ec91b4c6f90cce6 100644
--- a/src/b4/diff.py
+++ b/src/b4/diff.py
@@ -11,6 +11,7 @@ import b4
 import b4.mbox
 import mailbox
 import email
+import email.message
 import email.parser
 import shutil
 import pathlib
diff --git a/src/b4/ez.py b/src/b4/ez.py
index 970631592d2139b12ee4a6a9a112081eee1de721..9494b005a4f47a7be9cb053cc6075dede29d4cb0 100644
--- a/src/b4/ez.py
+++ b/src/b4/ez.py
@@ -17,6 +17,8 @@ import datetime
 import json
 import shlex
 import email
+import email.policy
+import email.utils
 import pathlib
 import base64
 import textwrap
diff --git a/src/b4/pr.py b/src/b4/pr.py
index b6eb5d3933d7e0bb1224877f071bb19facd8dbd8..60f46f1e3fa96a52a9d667bffb43c2a75b89cc4e 100644
--- a/src/b4/pr.py
+++ b/src/b4/pr.py
@@ -13,7 +13,9 @@ import b4
 import re
 import json
 import email
+import email.message
 import email.parser
+import email.utils
 import argparse
 
 import urllib.parse
diff --git a/src/b4/ty.py b/src/b4/ty.py
index e1afb61280d2161a054d40ae78df0d4b33dbb41d..021588ff9e559cfd8aa7c6818cdeba750931e8f6 100644
--- a/src/b4/ty.py
+++ b/src/b4/ty.py
@@ -13,6 +13,7 @@ import re
 import email
 import email.message
 import email.policy
+import email.utils
 import json
 import argparse
 
diff --git a/src/tests/test___init__.py b/src/tests/test___init__.py
index 297e5120a8c8adf241660f06e9a83c1ff4a0fd3a..3e01c2f5e127cd1123d0fb8503493e396148ee73 100644
--- a/src/tests/test___init__.py
+++ b/src/tests/test___init__.py
@@ -35,6 +35,7 @@ def test_save_git_am_mbox(sampledir, tmp_path, source, regex, flags, ismbox):
             msgs = list(mbx)
         else:
             import email
+            import email.parser
             with open(f'{sampledir}/{source}.txt', 'rb') as fh:
                 msg = email.parser.BytesParser(policy=b4.emlpolicy, _class=email.message.EmailMessage).parse(fh)
             msgs = [msg]

-- 
2.47.0

Re: [b4 PATCH RESEND 0/5] Resolve some static typing errors

[Tamir Duberstein]

Gentle ping. Kostantin, how can I help move this forward?

Re: [b4 PATCH RESEND 0/5] Resolve some static typing errors

[Konstantin Ryabitsev]

On Tue, Dec 03, 2024 at 12:04:10PM -0500, Tamir Duberstein wrote:
> Gentle ping. Kostantin, how can I help move this forward?

I hope to be able to get back to doing b4 work this week or next! Sorry about
the delay -- the holidays and other priorities were keeping me occupied
elsewhere.

-K

Re: [b4 PATCH RESEND 0/5] Resolve some static typing errors

[Tamir Duberstein]

On Tue, Dec 3, 2024 at 1:44 PM Konstantin Ryabitsev
<konstantin@linuxfoundation.org> wrote:
>
> I hope to be able to get back to doing b4 work this week or next! Sorry about
> the delay -- the holidays and other priorities were keeping me occupied
> elsewhere.
>
> -K

Hi Konstantin, hope you had a good holiday break and happy new year.
Gentle ping to have a look at this, if you could.

Cheers.
Tamir