From 00ecd77ef75b78dd13a127bc62b1e6dad6714892 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 8 Sep 2023 10:27:19 +0300 Subject: [PATCH 1/4] gh-109125: Run mypy on `Tools/wasm` --- .github/workflows/mypy.yml | 1 + Tools/wasm/mypy.ini | 15 ++++++++ Tools/wasm/wasm_assets.py | 9 +++-- Tools/wasm/wasm_build.py | 73 +++++++++++++++++++++--------------- Tools/wasm/wasm_webserver.py | 8 ++-- 5 files changed, 68 insertions(+), 38 deletions(-) create mode 100644 Tools/wasm/mypy.ini diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index fef7b02f47cdb7..198710bb52ac07 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -34,6 +34,7 @@ jobs: "Tools/cases_generator", "Tools/clinic", "Tools/peg_generator", + "Tools/wasm", ] name: Run mypy on ${{ matrix.target }} runs-on: ubuntu-latest diff --git a/Tools/wasm/mypy.ini b/Tools/wasm/mypy.ini new file mode 100644 index 00000000000000..176794bbb4d9a8 --- /dev/null +++ b/Tools/wasm/mypy.ini @@ -0,0 +1,15 @@ +[mypy] +files = Tools/wasm +pretty = True +show_traceback = True + +# Make sure the wasm can be run using Python 3.8: +python_version = 3.8 + +# Be strict... +strict = True +enable_error_code = truthy-bool,ignore-without-code + +# except for a few settings that can't yet be enabled: +warn_return_any = False +warn_unreachable = False diff --git a/Tools/wasm/wasm_assets.py b/Tools/wasm/wasm_assets.py index f1c8e0bf007f4c..9acfa95f1c6dd1 100755 --- a/Tools/wasm/wasm_assets.py +++ b/Tools/wasm/wasm_assets.py @@ -16,6 +16,7 @@ import sys import sysconfig import zipfile +from typing import Dict # source directory SRCDIR = pathlib.Path(__file__).parent.parent.parent.absolute() @@ -110,7 +111,7 @@ def get_builddir(args: argparse.Namespace) -> pathlib.Path: def get_sysconfigdata(args: argparse.Namespace) -> pathlib.Path: """Get path to sysconfigdata relative to build root""" - data_name = sysconfig._get_sysconfigdata_name() + data_name = sysconfig._get_sysconfigdata_name() # type: ignore[attr-defined] if not data_name.startswith(SYSCONFIG_NAMES): raise ValueError( f"Invalid sysconfig data name '{data_name}'.", SYSCONFIG_NAMES @@ -146,7 +147,7 @@ def filterfunc(filename: str) -> bool: pzf.writepy(entry, filterfunc=filterfunc) -def detect_extension_modules(args: argparse.Namespace): +def detect_extension_modules(args: argparse.Namespace) -> Dict[str, bool]: modules = {} # disabled by Modules/Setup.local ? @@ -161,7 +162,7 @@ def detect_extension_modules(args: argparse.Namespace): # disabled by configure? with open(args.sysconfig_data) as f: data = f.read() - loc = {} + loc: Dict[str, Dict[str, str]] = {} exec(data, globals(), loc) for key, value in loc["build_time_vars"].items(): @@ -195,7 +196,7 @@ def path(val: str) -> pathlib.Path: ) -def main(): +def main() -> None: args = parser.parse_args() relative_prefix = args.prefix.relative_to(pathlib.Path("/")) diff --git a/Tools/wasm/wasm_build.py b/Tools/wasm/wasm_build.py index c9947057a90754..3558ecd869dfc5 100755 --- a/Tools/wasm/wasm_build.py +++ b/Tools/wasm/wasm_build.py @@ -40,7 +40,17 @@ import webbrowser # for Python 3.8 -from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Union +from typing import ( + cast, + Any, + Callable, + Dict, + Iterable, + List, + Optional, + Tuple, + Union, +) logger = logging.getLogger("wasm_build") @@ -64,7 +74,7 @@ (3, 1, 16): "https://github.com/emscripten-core/emscripten/issues/17393", (3, 1, 20): "https://github.com/emscripten-core/emscripten/issues/17720", } -_MISSING = pathlib.PurePath("MISSING") +_MISSING = pathlib.Path("MISSING") WASM_WEBSERVER = WASMTOOLS / "wasm_webserver.py" @@ -109,7 +119,7 @@ def parse_emconfig( emconfig: pathlib.Path = EM_CONFIG, -) -> Tuple[pathlib.PurePath, pathlib.PurePath]: +) -> Tuple[pathlib.Path, pathlib.Path]: """Parse EM_CONFIG file and lookup EMSCRIPTEN_ROOT and NODE_JS. The ".emscripten" config file is a Python snippet that uses "EM_CONFIG" @@ -150,11 +160,11 @@ def read_python_version(configure: pathlib.Path = CONFIGURE) -> str: class ConditionError(ValueError): - def __init__(self, info: str, text: str): + def __init__(self, info: str, text: str) -> None: self.info = info self.text = text - def __str__(self): + def __str__(self) -> str: return f"{type(self).__name__}: '{self.info}'\n{self.text}" @@ -180,19 +190,19 @@ class Platform: name: str pythonexe: str config_site: Optional[pathlib.PurePath] - configure_wrapper: Optional[pathlib.PurePath] + configure_wrapper: Optional[pathlib.Path] make_wrapper: Optional[pathlib.PurePath] - environ: dict + environ: Dict[str, Any] check: Callable[[], None] # Used for build_emports(). ports: Optional[pathlib.PurePath] cc: Optional[pathlib.PurePath] - def getenv(self, profile: "BuildProfile") -> dict: + def getenv(self, profile: "BuildProfile") -> Dict[str, Any]: return self.environ.copy() -def _check_clean_src(): +def _check_clean_src() -> None: candidates = [ SRCDIR / "Programs" / "python.o", SRCDIR / "Python" / "frozen_modules" / "importlib._bootstrap.h", @@ -202,7 +212,7 @@ def _check_clean_src(): raise DirtySourceDirectory(os.fspath(candidate), CLEAN_SRCDIR) -def _check_native(): +def _check_native() -> None: if not any(shutil.which(cc) for cc in ["cc", "gcc", "clang"]): raise MissingDependency("cc", INSTALL_NATIVE) if not shutil.which("make"): @@ -234,12 +244,12 @@ def _check_native(): ) -def _check_emscripten(): +def _check_emscripten() -> None: if EMSCRIPTEN_ROOT is _MISSING: raise MissingDependency("Emscripten SDK EM_CONFIG", INSTALL_EMSDK) # sanity check emconfigure = EMSCRIPTEN.configure_wrapper - if not emconfigure.exists(): + if emconfigure is not None and not emconfigure.exists(): raise MissingDependency(os.fspath(emconfigure), INSTALL_EMSDK) # version check version_txt = EMSCRIPTEN_ROOT / "emscripten-version.txt" @@ -250,7 +260,10 @@ def _check_emscripten(): if version.endswith("-git"): # git / upstream / tot-upstream installation version = version[:-4] - version_tuple = tuple(int(v) for v in version.split(".")) + version_tuple = cast( + Tuple[int, int, int], + tuple(int(v) for v in version.split(".")) + ) if version_tuple < EMSDK_MIN_VERSION: raise ConditionError( os.fspath(version_txt), @@ -293,7 +306,7 @@ def _check_emscripten(): ) -def _check_wasi(): +def _check_wasi() -> None: wasm_ld = WASI_SDK_PATH / "bin" / "wasm-ld" if not wasm_ld.exists(): raise MissingDependency(os.fspath(wasm_ld), INSTALL_WASI_SDK) @@ -400,7 +413,7 @@ class EmscriptenTarget(enum.Enum): node_debug = "node-debug" @property - def is_browser(self): + def is_browser(self) -> bool: cls = type(self) return self in {cls.browser, cls.browser_debug} @@ -421,7 +434,7 @@ class SupportLevel(enum.Enum): experimental = "experimental, may be broken" broken = "broken / unavailable" - def __bool__(self): + def __bool__(self) -> bool: cls = type(self) return self in {cls.supported, cls.working} @@ -500,7 +513,7 @@ def make_cmd(self) -> List[str]: cmd.insert(0, os.fspath(platform.make_wrapper)) return cmd - def getenv(self) -> dict: + def getenv(self) -> Dict[str, Any]: """Generate environ dict for platform""" env = os.environ.copy() env.setdefault("MAKEFLAGS", f"-j{os.cpu_count()}") @@ -529,7 +542,7 @@ def _run_cmd( cmd: Iterable[str], args: Iterable[str] = (), cwd: Optional[pathlib.Path] = None, - ): + ) -> int: cmd = list(cmd) cmd.extend(args) if cwd is None: @@ -541,46 +554,46 @@ def _run_cmd( env=self.getenv(), ) - def _check_execute(self): + def _check_execute(self) -> None: if self.is_browser: raise ValueError(f"Cannot execute on {self.target}") - def run_build(self, *args): + def run_build(self, *args: str) -> None: """Run configure (if necessary) and make""" if not self.makefile.exists(): logger.info("Makefile not found, running configure") self.run_configure(*args) self.run_make("all", *args) - def run_configure(self, *args): + def run_configure(self, *args: str) -> int: """Run configure script to generate Makefile""" os.makedirs(self.builddir, exist_ok=True) return self._run_cmd(self.configure_cmd, args) - def run_make(self, *args): + def run_make(self, *args: str) -> int: """Run make (defaults to build all)""" return self._run_cmd(self.make_cmd, args) - def run_pythoninfo(self, *args): + def run_pythoninfo(self, *args: str) -> int: """Run 'make pythoninfo'""" self._check_execute() return self.run_make("pythoninfo", *args) - def run_test(self, target: str, testopts: Optional[str] = None): + def run_test(self, target: str, testopts: Optional[str] = None) -> int: """Run buildbottests""" self._check_execute() if testopts is None: testopts = self.default_testopts return self.run_make(target, f"TESTOPTS={testopts}") - def run_py(self, *args): + def run_py(self, *args: str) -> int: """Run Python with hostrunner""" self._check_execute() - self.run_make( + return self.run_make( "--eval", f"run: all; $(HOSTRUNNER) ./$(PYTHON) {shlex.join(args)}", "run" ) - def run_browser(self, bind="127.0.0.1", port=8000): + def run_browser(self, bind: str = "127.0.0.1", port: int = 8000) -> None: """Run WASM webserver and open build in browser""" relbuilddir = self.builddir.relative_to(SRCDIR) url = f"http://{bind}:{port}/{relbuilddir}/python.html" @@ -611,7 +624,7 @@ def run_browser(self, bind="127.0.0.1", port=8000): except KeyboardInterrupt: pass - def clean(self, all: bool = False): + def clean(self, all: bool = False) -> None: """Clean build directory""" if all: if self.builddir.exists(): @@ -619,7 +632,7 @@ def clean(self, all: bool = False): elif self.makefile.exists(): self.run_make("clean") - def build_emports(self, force: bool = False): + def build_emports(self, force: bool = False) -> None: """Pre-build emscripten ports.""" platform = self.host.platform if platform.ports is None or platform.cc is None: @@ -829,7 +842,7 @@ def build_emports(self, force: bool = False): ) -def main(): +def main() -> None: args = parser.parse_args() logging.basicConfig( level=logging.INFO if args.verbose else logging.ERROR, diff --git a/Tools/wasm/wasm_webserver.py b/Tools/wasm/wasm_webserver.py index 186bd57fc2067d..3d1d5d42a1e8c4 100755 --- a/Tools/wasm/wasm_webserver.py +++ b/Tools/wasm/wasm_webserver.py @@ -21,21 +21,21 @@ class MyHTTPRequestHandler(server.SimpleHTTPRequestHandler): } ) - def end_headers(self): + def end_headers(self) -> None: self.send_my_headers() super().end_headers() - def send_my_headers(self): + def send_my_headers(self) -> None: self.send_header("Cross-Origin-Opener-Policy", "same-origin") self.send_header("Cross-Origin-Embedder-Policy", "require-corp") -def main(): +def main() -> None: args = parser.parse_args() if not args.bind: args.bind = None - server.test( + server.test( # type: ignore[attr-defined] HandlerClass=MyHTTPRequestHandler, protocol="HTTP/1.1", port=args.port, From fe0dad32f98e64621f5a890b61f81dac36b5acb8 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Fri, 8 Sep 2023 10:33:35 +0300 Subject: [PATCH 2/4] Fix actions --- .github/workflows/mypy.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index 198710bb52ac07..e3c63a40250cd8 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -10,6 +10,7 @@ on: - "Tools/clinic/**" - "Tools/cases_generator/**" - "Tools/peg_generator/**" + - "Tools/wasm/**" - "Tools/requirements-dev.txt" - ".github/workflows/mypy.yml" workflow_dispatch: From a1725b4f30fa228b0c359bdc32fa215b39f8056a Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 11 Sep 2023 14:02:59 +0300 Subject: [PATCH 3/4] Address review --- Tools/wasm/mypy.ini | 5 ++--- Tools/wasm/wasm_assets.py | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tools/wasm/mypy.ini b/Tools/wasm/mypy.ini index 176794bbb4d9a8..c62598f89eba69 100644 --- a/Tools/wasm/mypy.ini +++ b/Tools/wasm/mypy.ini @@ -10,6 +10,5 @@ python_version = 3.8 strict = True enable_error_code = truthy-bool,ignore-without-code -# except for a few settings that can't yet be enabled: -warn_return_any = False -warn_unreachable = False +# except for incomplete defs, which are useful for module authors: +disallow_incomplete_defs = False diff --git a/Tools/wasm/wasm_assets.py b/Tools/wasm/wasm_assets.py index 9acfa95f1c6dd1..ffa5e303412c46 100755 --- a/Tools/wasm/wasm_assets.py +++ b/Tools/wasm/wasm_assets.py @@ -111,7 +111,8 @@ def get_builddir(args: argparse.Namespace) -> pathlib.Path: def get_sysconfigdata(args: argparse.Namespace) -> pathlib.Path: """Get path to sysconfigdata relative to build root""" - data_name = sysconfig._get_sysconfigdata_name() # type: ignore[attr-defined] + assert isinstance(args.builddir, pathlib.Path) + data_name: str = sysconfig._get_sysconfigdata_name() # type: ignore[attr-defined] if not data_name.startswith(SYSCONFIG_NAMES): raise ValueError( f"Invalid sysconfig data name '{data_name}'.", SYSCONFIG_NAMES From 35fa459b819a46c6d81560e2e4752df6cb11594e Mon Sep 17 00:00:00 2001 From: Nikita Sobolev Date: Sat, 16 Sep 2023 11:18:56 +0300 Subject: [PATCH 4/4] ABCDEF sorting --- .github/workflows/mypy.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/mypy.yml b/.github/workflows/mypy.yml index e3c63a40250cd8..405511ca6820b3 100644 --- a/.github/workflows/mypy.yml +++ b/.github/workflows/mypy.yml @@ -7,12 +7,12 @@ on: - main pull_request: paths: - - "Tools/clinic/**" + - ".github/workflows/mypy.yml" - "Tools/cases_generator/**" + - "Tools/clinic/**" - "Tools/peg_generator/**" - - "Tools/wasm/**" - "Tools/requirements-dev.txt" - - ".github/workflows/mypy.yml" + - "Tools/wasm/**" workflow_dispatch: permissions: