Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-109125: Run mypy on Tools/wasm #109126

Merged
merged 5 commits into from
Sep 19, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/mypy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ on:
- "Tools/clinic/**"
- "Tools/cases_generator/**"
- "Tools/peg_generator/**"
- "Tools/wasm/**"
sobolevn marked this conversation as resolved.
Show resolved Hide resolved
- "Tools/requirements-dev.txt"
- ".github/workflows/mypy.yml"
workflow_dispatch:
Expand All @@ -34,6 +35,7 @@ jobs:
"Tools/cases_generator",
"Tools/clinic",
"Tools/peg_generator",
"Tools/wasm",
]
name: Run mypy on ${{ matrix.target }}
runs-on: ubuntu-latest
Expand Down
14 changes: 14 additions & 0 deletions Tools/wasm/mypy.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[mypy]
files = Tools/wasm
pretty = True
show_traceback = True

# Make sure the wasm can be run using Python 3.8:
python_version = 3.8
brettcannon marked this conversation as resolved.
Show resolved Hide resolved

# Be strict...
strict = True
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this mean all code in this directory must be fully typed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can ignore any code that is not typed:

  1. Via adding global disallow_incomplete_defs = false
  2. Via per-file ignores
  3. Via inline # type: ignore

For now, yes, all code must be typed. Do you want to add disallow_incomplete_defs = false?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to add disallow_incomplete_defs = false?

Yes please.

enable_error_code = truthy-bool,ignore-without-code

# except for incomplete defs, which are useful for module authors:
disallow_incomplete_defs = False
10 changes: 6 additions & 4 deletions Tools/wasm/wasm_assets.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import sys
import sysconfig
import zipfile
from typing import Dict

# source directory
SRCDIR = pathlib.Path(__file__).parent.parent.parent.absolute()
Expand Down Expand Up @@ -110,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()
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
Expand Down Expand Up @@ -146,7 +148,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 ?
Expand All @@ -161,7 +163,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():
Expand Down Expand Up @@ -195,7 +197,7 @@ def path(val: str) -> pathlib.Path:
)


def main():
def main() -> None:
args = parser.parse_args()

relative_prefix = args.prefix.relative_to(pathlib.Path("/"))
Expand Down
73 changes: 43 additions & 30 deletions Tools/wasm/wasm_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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"

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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}"


Expand All @@ -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",
Expand All @@ -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"):
Expand Down Expand Up @@ -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"
Expand All @@ -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),
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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}

Expand All @@ -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}

Expand Down Expand Up @@ -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()}")
Expand Down Expand Up @@ -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:
Expand All @@ -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"
Expand Down Expand Up @@ -611,15 +624,15 @@ 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():
shutil.rmtree(self.builddir)
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:
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions Tools/wasm/wasm_webserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading