From d9551025449981ecbe618b7ac430c5662a8f5535 Mon Sep 17 00:00:00 2001 From: Jacob Roberts Date: Fri, 4 Nov 2022 21:02:37 -0700 Subject: [PATCH 1/6] Add mypy typings to project compatible with python 3.10 --- make_wheels.py | 36 +++++++++++++++++++++--------------- mypy.ini | 27 +++++++++++++++++++++++++++ nodejs-cmd/nodejs_cmd.py | 6 +++--- requirements.txt | 5 ++++- tests/test_comand_line.py | 22 ++++++++++++---------- tests/test_node.py | 17 +++++++++-------- tests/test_npm.py | 10 ++++++---- 7 files changed, 82 insertions(+), 41 deletions(-) create mode 100644 mypy.ini diff --git a/make_wheels.py b/make_wheels.py index ed21e0d..6435e72 100644 --- a/make_wheels.py +++ b/make_wheels.py @@ -1,6 +1,8 @@ import os import hashlib import urllib.request +import urllib.error +from typing import Dict, List, Any import libarchive from email.message import EmailMessage from wheel.wheelfile import WheelFile @@ -48,8 +50,8 @@ } -class ReproducibleWheelFile(WheelFile): - def writestr(self, zinfo, *args, **kwargs): +class ReproducibleWheelFile(WheelFile): # type: ignore[no-any-unimported] + def writestr(self, zinfo: ZipInfo, *args: Any, **kwargs: Any) -> None: if not isinstance(zinfo, ZipInfo): raise ValueError("ZipInfo required") zinfo.date_time = (1980, 1, 1, 0, 0, 0) @@ -57,7 +59,7 @@ def writestr(self, zinfo, *args, **kwargs): super().writestr(zinfo, *args, **kwargs) -def make_message(headers, payload=None): +def make_message(headers: Dict[str, str | List[str]], payload: str | None =None) -> EmailMessage: msg = EmailMessage() for name, value in headers.items(): if isinstance(value, list): @@ -69,8 +71,9 @@ def make_message(headers, payload=None): msg.set_payload(payload) return msg +WheelContents = Dict[ZipInfo | str, bytes | EmailMessage] -def write_wheel_file(filename, contents): +def write_wheel_file(filename: str, contents: WheelContents) -> str: with ReproducibleWheelFile(filename, 'w') as wheel: for member_info, member_source in contents.items(): if not isinstance(member_info, ZipInfo): @@ -82,15 +85,18 @@ def write_wheel_file(filename, contents): return filename -def write_wheel(out_dir, *, name, version, tag, metadata, description, contents, entry_points): +def write_wheel(out_dir: str, *, name: str, version: str, tag: str, metadata: Dict[str, str | List[str]], description: str, contents: WheelContents, entry_points: Dict[str, str]) -> str: name_snake = name.replace('-', '_') wheel_name = f'{name_snake}-{version}-{tag}.whl' dist_info = f'{name_snake}-{version}.dist-info' if entry_points: - contents[f'{dist_info}/entry_points.txt'] = (cleandoc(""" + entry_points_entries = '\n'.join([f'{k} = {v}' for k, v in entry_points.items()] if entry_points else []) + entry_points_file_contents = cleandoc(""" [console_scripts] {entry_points} - """).format(entry_points='\n'.join([f'{k} = {v}' for k, v in entry_points.items()] if entry_points else []))).encode('ascii'), + """).format(entry_points=entry_points_entries) + contents[f'{dist_info}/entry_points.txt'] = entry_points_file_contents.encode('ascii') + return write_wheel_file(os.path.join(out_dir, wheel_name), { **contents, f'{dist_info}/METADATA': make_message({ @@ -108,12 +114,12 @@ def write_wheel(out_dir, *, name, version, tag, metadata, description, contents, }) -def write_nodejs_wheel(out_dir, *, node_version, version, platform, archive): - contents = {} - entry_points = {} - init_imports = [] +def write_nodejs_wheel(out_dir: str, *, node_version: str, version: str, platform: str, archive_contents: bytes) -> str: + contents: WheelContents = {} + entry_points: Dict[str, str] = {} + init_imports: List[str] = [] - with libarchive.memory_reader(archive) as archive: + with libarchive.memory_reader(archive_contents) as archive: for entry in archive: entry_name = '/'.join(entry.name.split('/')[1:]) if entry.isdir or not entry_name: @@ -286,7 +292,7 @@ def main() -> None: ) -def make_nodejs_version(node_version, suffix=''): +def make_nodejs_version(node_version: str, suffix: str='') -> None: wheel_version = f'{node_version}{suffix}' print('--') print('Making Node.js Wheels for version', node_version) @@ -312,12 +318,12 @@ def make_nodejs_version(node_version, suffix=''): node_version=node_version, version=wheel_version, platform=python_platform, - archive=node_archive) + archive_contents=node_archive) with open(wheel_path, 'rb') as wheel: print(f' {wheel_path}') print(f' {hashlib.sha256(wheel.read()).hexdigest()}') -def main(): +def main() -> None: for node_version in BUILD_VERSIONS: make_nodejs_version(node_version, suffix=BUILD_SUFFIX) diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 0000000..0a4157d --- /dev/null +++ b/mypy.ini @@ -0,0 +1,27 @@ +[mypy] + +files = **/*.py + +exclude = (?x)( + venv + ) + +check_untyped_defs = True +disallow_untyped_defs = True +disallow_incomplete_defs = True +disallow_untyped_decorators = True +disallow_any_unimported = True +warn_return_any = True +warn_unused_ignores = True +show_error_codes = True +enable_error_code = ignore-without-code +follow_imports = normal + +[mypy-libarchive] +ignore_missing_imports = True + +[mypy-wheel.*] +ignore_missing_imports = True + +[mypy-nodejs] +ignore_missing_imports = True \ No newline at end of file diff --git a/nodejs-cmd/nodejs_cmd.py b/nodejs-cmd/nodejs_cmd.py index 4f638f9..0e7cfc7 100644 --- a/nodejs-cmd/nodejs_cmd.py +++ b/nodejs-cmd/nodejs_cmd.py @@ -1,10 +1,10 @@ from nodejs import node, npm, npx -def node_main(): +def node_main() -> None: node.main() -def npm_main(): +def npm_main() -> None: npm.main() -def npx_main(): +def npx_main() -> None: npx.main() diff --git a/requirements.txt b/requirements.txt index af65072..b620951 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,7 @@ wheel twine libarchive-c -pytest \ No newline at end of file +pytest +typing-extensions +mypy +types-setuptools \ No newline at end of file diff --git a/tests/test_comand_line.py b/tests/test_comand_line.py index 35b5f9c..2d92922 100644 --- a/tests/test_comand_line.py +++ b/tests/test_comand_line.py @@ -1,61 +1,63 @@ "Test nodejs command line" import os, sys, subprocess +import pathlib +from pytest import CaptureFixture THIS_DIR = os.path.dirname(os.path.abspath(__file__)) -def test_runs(): +def test_runs() -> None: assert subprocess.call([sys.executable, "-m", "nodejs", "--version"]) == 0 -def test_version(capfd): +def test_version(capfd: CaptureFixture) -> None: subprocess.call([sys.executable, "-m", "nodejs", "--version"]) out, err = capfd.readouterr() assert out.startswith('v') -def test_eval(capfd): +def test_eval(capfd: CaptureFixture) -> None: subprocess.call([sys.executable, "-m", "nodejs", "--eval", "console.log('hello')"]) out, err = capfd.readouterr() assert out.strip() == 'hello' -def test_eval_error(capfd): +def test_eval_error(capfd: CaptureFixture) -> None: subprocess.call([sys.executable, "-m", "nodejs", "--eval", "console.error('error')"]) out, err = capfd.readouterr() assert err.strip() == 'error' -def test_eval_error_exit(): +def test_eval_error_exit() -> None: ret = subprocess.call([sys.executable, "-m", "nodejs", "--eval", "process.exit(1)"]) assert ret == 1 -def test_script(capfd): +def test_script(capfd: CaptureFixture) -> None: subprocess.call([sys.executable, "-m", "nodejs", os.path.join(THIS_DIR, "test_node", "test_script.js")]) out, err = capfd.readouterr() assert out.strip() == 'hello' -def test_args(capfd): +def test_args(capfd: CaptureFixture) -> None: subprocess.call([sys.executable, "-m", "nodejs", os.path.join(THIS_DIR, "test_node", "test_args.js"), "hello"]) out, err = capfd.readouterr() assert out.strip() == 'hello' -def test_npm_runs(): +def test_npm_runs() -> None: assert subprocess.call([sys.executable, "-m", "nodejs.npm", "--version"]) == 0 -def test_npm_version(capfd): +def test_npm_version(capfd: CaptureFixture) -> None: subprocess.call([sys.executable, "-m", "nodejs.npm", "--version"]) out, err = capfd.readouterr() assert isinstance(out, str) -def test_install_package(tmp_path, capfd): +def test_install_package(tmp_path: pathlib.Path, capfd: CaptureFixture) -> None: os.chdir(tmp_path) subprocess.call([sys.executable, "-m", "nodejs.npm", "init", "-y"]) assert (tmp_path / 'package.json').exists() diff --git a/tests/test_node.py b/tests/test_node.py index 345e6e8..f8e3b77 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,22 +1,23 @@ "Test nodejs.node" import os +from pytest import CaptureFixture THIS_DIR = os.path.dirname(os.path.abspath(__file__)) -def test_package_installed(): +def test_package_installed() -> None: import nodejs assert nodejs.__version__ is not None -def test_runs(): +def test_runs() -> None: from nodejs import node assert node.call(['--version']) is 0 -def test_version(capfd): +def test_version(capfd: CaptureFixture) -> None: from nodejs import node, node_version node.call(['--version']) out, err = capfd.readouterr() @@ -24,34 +25,34 @@ def test_version(capfd): assert out.strip() == f'v{node_version}' -def test_eval(capfd): +def test_eval(capfd: CaptureFixture) -> None: from nodejs import node node.call(['--eval', 'console.log("hello")']) out, err = capfd.readouterr() assert out.strip() == 'hello' -def test_eval_error(capfd): +def test_eval_error(capfd: CaptureFixture) -> None: from nodejs import node node.call(['--eval', 'console.error("error")']) out, err = capfd.readouterr() assert err.strip() == 'error' -def test_eval_error_exit(): +def test_eval_error_exit() -> None: from nodejs import node ret = node.call(['--eval', 'process.exit(1)']) assert ret == 1 -def test_script(capfd): +def test_script(capfd: CaptureFixture) -> None: from nodejs import node node.call([os.path.join(THIS_DIR, 'test_node', 'test_script.js')]) out, err = capfd.readouterr() assert out.strip() == 'hello' -def test_args(capfd): +def test_args(capfd: CaptureFixture) -> None: from nodejs import node node.call([os.path.join(THIS_DIR, 'test_node', 'test_args.js'), 'hello']) out, err = capfd.readouterr() diff --git a/tests/test_npm.py b/tests/test_npm.py index c8c5dc0..90b3975 100644 --- a/tests/test_npm.py +++ b/tests/test_npm.py @@ -1,23 +1,25 @@ "Test nodejs.npm" import os +import pathlib +from pytest import CaptureFixture -def test_runs(): +def test_runs() -> None: from nodejs import npm assert npm.call(['--version']) is 0 -def test_version(capfd): +def test_version(capfd: CaptureFixture) -> None: from nodejs import npm npm.call(['--version']) out, err = capfd.readouterr() assert isinstance(out, str) -def test_install_package(tmp_path, capfd): +def test_install_package(tmp_path: pathlib.Path, capfd: CaptureFixture) -> None: from nodejs import npm, node - import json + os.chdir(tmp_path) npm.call(['init', '-y']) assert (tmp_path / 'package.json').exists() From a929f30d1bd75ce3d6cbc4be87ce6f917539f5b8 Mon Sep 17 00:00:00 2001 From: Jacob Roberts Date: Fri, 4 Nov 2022 21:09:29 -0700 Subject: [PATCH 2/6] Add type checking and running of the tests to CI --- .github/workflows/test.yaml | 10 ++++++++++ .gitignore | 6 +++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 420d116..1d38999 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -28,6 +28,16 @@ jobs: - name: Show built files run: | ls -l dist/* + - name: Type Checking + run: | + python -m mypy + - name: Tests + run: | + for wheel in $(find dist -name "*manylinux*_x86_64.whl"); do + echo "Testing installation with wheel: ${wheel}" + pip install ${wheel} + python -m pytest + done - uses: actions/upload-artifact@v3 with: name: nodejs-pip-wheels diff --git a/.gitignore b/.gitignore index d23485d..4d119c4 100644 --- a/.gitignore +++ b/.gitignore @@ -5,4 +5,8 @@ nodejs-cmd/*.egg-info .DS_Store env*/ __pycache__/ -*.py[cod] \ No newline at end of file +*.py[cod] +venv +node_modules +package-lock.json +package.json From 9f3dac274d4a5caedadeac4be73efd8e6fc63515 Mon Sep 17 00:00:00 2001 From: Jacob Roberts Date: Fri, 4 Nov 2022 21:16:01 -0700 Subject: [PATCH 3/6] Temporarily pin wheel less than version 38 to deal with botched release --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b620951..b3850c0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -wheel +wheel < 0.38 twine libarchive-c pytest From 05a843090e29c33de950aac4740cd395ff8fab68 Mon Sep 17 00:00:00 2001 From: Jacob Roberts Date: Sat, 5 Nov 2022 10:26:59 -0700 Subject: [PATCH 4/6] Add slightly more coverage for npx and corepack --- .github/workflows/test.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1d38999..82f422a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -25,7 +25,7 @@ jobs: run: | mkdir dist python make_wheels.py - - name: Show built files + - name: Show built wheels run: | ls -l dist/* - name: Type Checking @@ -89,5 +89,5 @@ jobs: run: python -m nodejs --version python -m nodejs.npm --version - - + python -m nodejs.npx --version + python -m nodejs.corepack --version From 4a2dbdb192e4672d6409f9029f68c79acd3e6236 Mon Sep 17 00:00:00 2001 From: Jacob Roberts Date: Sat, 5 Nov 2022 11:19:33 -0700 Subject: [PATCH 5/6] Revert locking the wheel dependency since the issue is fixed in the latest 0.38.2 release --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b3850c0..b620951 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -wheel < 0.38 +wheel twine libarchive-c pytest From d5ba9207a73c8d1b6aab55737802f64756077903 Mon Sep 17 00:00:00 2001 From: Jacob Roberts Date: Mon, 7 Nov 2022 09:58:37 -0800 Subject: [PATCH 6/6] Enfoce that the project is built using python 3.10 by developers --- make_wheels.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/make_wheels.py b/make_wheels.py index 6435e72..23ae2dc 100644 --- a/make_wheels.py +++ b/make_wheels.py @@ -1,5 +1,6 @@ import os import hashlib +import platform import urllib.request import urllib.error from typing import Dict, List, Any @@ -9,6 +10,14 @@ from zipfile import ZipInfo, ZIP_DEFLATED from inspect import cleandoc +# To support our internal typings, we require Python3.10+, but to KISS +# for now we only support building this project with Python3.10 (this is +# a developer only requirement) +(python_major, python_minor, _python_patch) = platform.python_version_tuple() +if not (python_major == '3' and python_minor == '10'): + breakpoint() + raise Exception(f"This build script is expected to run on Python 3.10, but you are using {platform.python_version()}") + # Versions to build if run as a script: BUILD_VERSIONS = ('14.19.3', '16.15.1', '18.4.0')