From 61fb33021e0aa207680a46f56a24584436d83362 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Mon, 14 Aug 2023 13:25:38 -0500 Subject: [PATCH 01/15] Add typing for `_wait_for_object.py` --- trio/_core/_windows_cffi.py | 21 ++++++++++++++++----- trio/_wait_for_object.py | 13 ++++++++++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index 639e75b50e..37bed14c50 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -1,7 +1,14 @@ +from __future__ import annotations + import enum import re +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from typing_extensions import NoReturn import cffi +from cffi.FFI import CData as CData ################################################################ # Functions and types @@ -302,20 +309,24 @@ class IoControlCodes(enum.IntEnum): ################################################################ -def _handle(obj): +def _handle(obj: int | CData) -> CData: # For now, represent handles as either cffi HANDLEs or as ints. If you # try to pass in a file descriptor instead, it's not going to work # out. (For that msvcrt.get_osfhandle does the trick, but I don't know if # we'll actually need that for anything...) For sockets this doesn't # matter, Python never allocates an fd. So let's wait until we actually # encounter the problem before worrying about it. - if type(obj) is int: + if isinstance(obj, int): return ffi.cast("HANDLE", obj) - else: - return obj + return obj -def raise_winerror(winerror=None, *, filename=None, filename2=None): +def raise_winerror( + winerror: int | None = None, + *, + filename: str | None = None, + filename2: str | None = None, +) -> NoReturn: if winerror is None: winerror, msg = ffi.getwinerror() else: diff --git a/trio/_wait_for_object.py b/trio/_wait_for_object.py index 32a88e5398..254d07561c 100644 --- a/trio/_wait_for_object.py +++ b/trio/_wait_for_object.py @@ -2,10 +2,17 @@ import trio -from ._core._windows_cffi import ErrorCodes, _handle, ffi, kernel32, raise_winerror +from ._core._windows_cffi import ( + CData, + ErrorCodes, + _handle, + ffi, + kernel32, + raise_winerror, +) -async def WaitForSingleObject(obj): +async def WaitForSingleObject(obj: int | CData) -> None: """Async and cancellable variant of WaitForSingleObject. Windows only. Args: @@ -45,7 +52,7 @@ async def WaitForSingleObject(obj): kernel32.CloseHandle(cancel_handle) -def WaitForMultipleObjects_sync(*handles): +def WaitForMultipleObjects_sync(*handles: int | CData) -> None: """Wait for any of the given Windows handles to be signaled.""" n = len(handles) handle_arr = ffi.new(f"HANDLE[{n}]") From 9ff1bed38174ad7fa625578310e3dffbc87b2412 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Mon, 14 Aug 2023 13:33:14 -0500 Subject: [PATCH 02/15] Fix CData type --- trio/_core/_windows_cffi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index 37bed14c50..7649bf920f 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -8,7 +8,6 @@ from typing_extensions import NoReturn import cffi -from cffi.FFI import CData as CData ################################################################ # Functions and types @@ -223,6 +222,7 @@ LIB = re.sub(r"\bPASCAL\b", "__stdcall", LIB) ffi = cffi.FFI() +CData = ffi.CData ffi.cdef(LIB) kernel32 = ffi.dlopen("kernel32.dll") From 4e8a2c20ec52ecca233c432eb9842f6eb87df85d Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Mon, 14 Aug 2023 20:06:55 -0500 Subject: [PATCH 03/15] Future annotations --- trio/_wait_for_object.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/trio/_wait_for_object.py b/trio/_wait_for_object.py index 254d07561c..50a9d13ff2 100644 --- a/trio/_wait_for_object.py +++ b/trio/_wait_for_object.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import math import trio From f119f027fa1de120fb41a798d84a3e6802de37e1 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Mon, 14 Aug 2023 20:13:30 -0500 Subject: [PATCH 04/15] Use explicit type alias --- trio/_core/_windows_cffi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index 7649bf920f..152d7941d4 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from typing_extensions import NoReturn + from typing_extensions import NoReturn, TypeAlias import cffi @@ -222,7 +222,7 @@ LIB = re.sub(r"\bPASCAL\b", "__stdcall", LIB) ffi = cffi.FFI() -CData = ffi.CData +CData: TypeAlias = ffi.CData ffi.cdef(LIB) kernel32 = ffi.dlopen("kernel32.dll") From fe151df09b171645c05cb6ed1e4de150da0a28a7 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Mon, 14 Aug 2023 22:14:53 -0500 Subject: [PATCH 05/15] Maybe saying it's `cffi.api.FFI` will fix it? --- trio/_core/_windows_cffi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index 152d7941d4..bab7d4ada5 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -221,7 +221,7 @@ # being _MSC_VER >= 800) LIB = re.sub(r"\bPASCAL\b", "__stdcall", LIB) -ffi = cffi.FFI() +ffi = cffi.api.FFI() CData: TypeAlias = ffi.CData ffi.cdef(LIB) From 0ea0464087c8af2fa78fda550f8758eca49cdc9e Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Tue, 15 Aug 2023 17:33:52 +1000 Subject: [PATCH 06/15] Fix this type alias --- trio/_core/_windows_cffi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index bab7d4ada5..a042dfae9f 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -222,7 +222,7 @@ LIB = re.sub(r"\bPASCAL\b", "__stdcall", LIB) ffi = cffi.api.FFI() -CData: TypeAlias = ffi.CData +CData: TypeAlias = cffi.api.FFI.CData ffi.cdef(LIB) kernel32 = ffi.dlopen("kernel32.dll") From d8f5306ae30b177c12ebfef736db6c0118a2aeae Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Thu, 17 Aug 2023 18:28:34 +1000 Subject: [PATCH 07/15] Handle None in raise_winerror() --- trio/_core/_windows_cffi.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index a042dfae9f..596284025b 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -328,8 +328,14 @@ def raise_winerror( filename2: str | None = None, ) -> NoReturn: if winerror is None: - winerror, msg = ffi.getwinerror() + err = ffi.getwinerror() + if err is None: + raise OSError("No error set?") + winerror, msg = err else: - _, msg = ffi.getwinerror(winerror) + err = ffi.getwinerror() + if err is None: + raise OSError("No error set?") + _, msg = err # https://docs.python.org/3/library/exceptions.html#OSError raise OSError(0, msg, filename, winerror, filename2) From fd26a24512c3ab5b846172cea887b1f837c0a8ef Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Thu, 17 Aug 2023 20:34:43 +1000 Subject: [PATCH 08/15] Fix copy-paste error Interestingly not caught by tests, maybe we need specific tests for this. --- trio/_core/_windows_cffi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index 596284025b..973eaeb912 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -333,7 +333,7 @@ def raise_winerror( raise OSError("No error set?") winerror, msg = err else: - err = ffi.getwinerror() + err = ffi.getwinerror(winerror) if err is None: raise OSError("No error set?") _, msg = err From b9aa4d1e709f5ee76bd7b95cebcc8beff02a6af2 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Thu, 17 Aug 2023 19:21:16 -0500 Subject: [PATCH 09/15] Update `pyproject.toml` --- pyproject.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 073e2508d1..d272f0d6f6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,7 +62,7 @@ module = [ "trio._core._thread_cache", "trio._core._traps", "trio._core._unbounded_queue", - "trio._core._unbounded_queue", + "trio._core._windows_cffi", "trio._deprecate", "trio._dtls", "trio._file_io", @@ -78,6 +78,7 @@ module = [ "trio._threads", "trio._tools.gen_exports", "trio._util", + "trio._wait_for_object", ] disallow_incomplete_defs = true disallow_untyped_defs = true From 5748203bce3fe12edbcfb4c7e810aa4955921d36 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Fri, 18 Aug 2023 13:19:17 +1000 Subject: [PATCH 10/15] Add a test for raise_winerror() --- trio/_core/_tests/test_windows.py | 38 +++++++++++++++++++++++++++++++ trio/_core/_windows_cffi.py | 4 ++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/trio/_core/_tests/test_windows.py b/trio/_core/_tests/test_windows.py index 0dac94543c..6f40be1b24 100644 --- a/trio/_core/_tests/test_windows.py +++ b/trio/_core/_tests/test_windows.py @@ -1,6 +1,7 @@ import os import tempfile from contextlib import contextmanager +from unittest.mock import create_autospec import pytest @@ -22,6 +23,43 @@ ) +def test_winerror(monkeypatch) -> None: + mock = create_autospec(ffi.getwinerror) + monkeypatch.setattr(ffi, "getwinerror", mock) + + # Returning none = no error, should not happen. + mock.return_value = None + with pytest.raises(RuntimeError, match="No error set"): + raise_winerror() + mock.assert_called_once_with() + mock.reset_mock() + + with pytest.raises(RuntimeError, match="No error set"): + raise_winerror(38) + mock.assert_called_once_with(38) + mock.reset_mock() + + mock.return_value = (12, "test error") + with pytest.raises(OSError) as exc: + raise_winerror(filename="file_1", filename2="file_2") + mock.assert_called_once_with() + mock.reset_mock() + assert exc.value.winerror == 12 + assert exc.value.strerror == "test error" + assert exc.value.filename == "file_1" + assert exc.value.filename2 == "file_2" + + # With an explicit number passed in, it overrides what getwinerror() returns. + with pytest.raises(OSError) as exc: + raise_winerror(18, filename="afile", filename2="bfile") + mock.assert_called_once_with(18) + mock.reset_mock() + assert exc.value.winerror == 18 + assert exc.value.strerror == "test error" + assert exc.value.filename == "afile" + assert exc.value.filename2 == "bfile" + + # The undocumented API that this is testing should be changed to stop using # UnboundedQueue (or just removed until we have time to redo it), but until # then we filter out the warning. diff --git a/trio/_core/_windows_cffi.py b/trio/_core/_windows_cffi.py index 973eaeb912..a65a332c2f 100644 --- a/trio/_core/_windows_cffi.py +++ b/trio/_core/_windows_cffi.py @@ -330,12 +330,12 @@ def raise_winerror( if winerror is None: err = ffi.getwinerror() if err is None: - raise OSError("No error set?") + raise RuntimeError("No error set?") winerror, msg = err else: err = ffi.getwinerror(winerror) if err is None: - raise OSError("No error set?") + raise RuntimeError("No error set?") _, msg = err # https://docs.python.org/3/library/exceptions.html#OSError raise OSError(0, msg, filename, winerror, filename2) From 8d46654f32cd360d4b7863d78a96877333fdee6f Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Fri, 18 Aug 2023 13:22:44 +1000 Subject: [PATCH 11/15] Change names --- trio/_core/_tests/test_windows.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/trio/_core/_tests/test_windows.py b/trio/_core/_tests/test_windows.py index 6f40be1b24..69b95b15ce 100644 --- a/trio/_core/_tests/test_windows.py +++ b/trio/_core/_tests/test_windows.py @@ -51,13 +51,13 @@ def test_winerror(monkeypatch) -> None: # With an explicit number passed in, it overrides what getwinerror() returns. with pytest.raises(OSError) as exc: - raise_winerror(18, filename="afile", filename2="bfile") + raise_winerror(18, filename="a/file", filename2="b/file") mock.assert_called_once_with(18) mock.reset_mock() assert exc.value.winerror == 18 assert exc.value.strerror == "test error" - assert exc.value.filename == "afile" - assert exc.value.filename2 == "bfile" + assert exc.value.filename == "a/file" + assert exc.value.filename2 == "b/file" # The undocumented API that this is testing should be changed to stop using From 31c038fe8773b5e46095b751a3ef66b799d6a2b8 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Fri, 18 Aug 2023 21:49:33 -0500 Subject: [PATCH 12/15] Fix mypy CI failure on linux and macos --- trio/_core/_tests/test_windows.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/trio/_core/_tests/test_windows.py b/trio/_core/_tests/test_windows.py index 69b95b15ce..d2231cc8a8 100644 --- a/trio/_core/_tests/test_windows.py +++ b/trio/_core/_tests/test_windows.py @@ -44,7 +44,8 @@ def test_winerror(monkeypatch) -> None: raise_winerror(filename="file_1", filename2="file_2") mock.assert_called_once_with() mock.reset_mock() - assert exc.value.winerror == 12 + if sys.platform == "win32": # mypy complains otherwise + assert exc.value.winerror == 12 assert exc.value.strerror == "test error" assert exc.value.filename == "file_1" assert exc.value.filename2 == "file_2" @@ -54,7 +55,8 @@ def test_winerror(monkeypatch) -> None: raise_winerror(18, filename="a/file", filename2="b/file") mock.assert_called_once_with(18) mock.reset_mock() - assert exc.value.winerror == 18 + if sys.platform == "win32": # mypy complains otherwise + assert exc.value.winerror == 18 assert exc.value.strerror == "test error" assert exc.value.filename == "a/file" assert exc.value.filename2 == "b/file" From 3e9c132a6b3202f0e42405f1320cecbabbd21644 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Fri, 18 Aug 2023 21:50:59 -0500 Subject: [PATCH 13/15] Remove `_wait_for_object` and `_core._windows_cffi` fully typed ignores --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9a505b2674..98917df4c1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,9 +62,6 @@ module = [ "trio/_highlevel_serve_listeners", "trio/_highlevel_ssl_helpers", "trio/_highlevel_socket", -# 2755 -"trio/_core/_windows_cffi", -"trio/_wait_for_object", # 2761 "trio/_core/_generated_io_windows", "trio/_core/_io_windows", From 5d3f5583200110ec402894fa2f53bfb30a69a0c0 Mon Sep 17 00:00:00 2001 From: CoolCat467 <52022020+CoolCat467@users.noreply.github.com> Date: Fri, 18 Aug 2023 23:36:00 -0500 Subject: [PATCH 14/15] Import sys for new mypy CI fix --- trio/_core/_tests/test_windows.py | 1 + 1 file changed, 1 insertion(+) diff --git a/trio/_core/_tests/test_windows.py b/trio/_core/_tests/test_windows.py index d2231cc8a8..4732cd95c3 100644 --- a/trio/_core/_tests/test_windows.py +++ b/trio/_core/_tests/test_windows.py @@ -1,4 +1,5 @@ import os +import sys import tempfile from contextlib import contextmanager from unittest.mock import create_autospec From b268935152dcf8a0a36fe93b8d7ff64fcf397713 Mon Sep 17 00:00:00 2001 From: Spencer Brown Date: Sat, 19 Aug 2023 16:43:39 +1000 Subject: [PATCH 15/15] Entirely skip type checking test_windows on other platforms --- trio/_core/_tests/test_windows.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/trio/_core/_tests/test_windows.py b/trio/_core/_tests/test_windows.py index 4732cd95c3..99bb97284b 100644 --- a/trio/_core/_tests/test_windows.py +++ b/trio/_core/_tests/test_windows.py @@ -2,6 +2,7 @@ import sys import tempfile from contextlib import contextmanager +from typing import TYPE_CHECKING from unittest.mock import create_autospec import pytest @@ -10,6 +11,8 @@ # Mark all the tests in this file as being windows-only pytestmark = pytest.mark.skipif(not on_windows, reason="windows only") +assert sys.platform == "win32" or not TYPE_CHECKING # Skip type checking on Windows + from ... import _core, sleep from ...testing import wait_all_tasks_blocked from .tutil import gc_collect_harder, restore_unraisablehook, slow @@ -45,8 +48,7 @@ def test_winerror(monkeypatch) -> None: raise_winerror(filename="file_1", filename2="file_2") mock.assert_called_once_with() mock.reset_mock() - if sys.platform == "win32": # mypy complains otherwise - assert exc.value.winerror == 12 + assert exc.value.winerror == 12 assert exc.value.strerror == "test error" assert exc.value.filename == "file_1" assert exc.value.filename2 == "file_2" @@ -56,8 +58,7 @@ def test_winerror(monkeypatch) -> None: raise_winerror(18, filename="a/file", filename2="b/file") mock.assert_called_once_with(18) mock.reset_mock() - if sys.platform == "win32": # mypy complains otherwise - assert exc.value.winerror == 18 + assert exc.value.winerror == 18 assert exc.value.strerror == "test error" assert exc.value.filename == "a/file" assert exc.value.filename2 == "b/file"