From 4a8a5d658fb223e49fdaed3b52e0d3a8c772fc51 Mon Sep 17 00:00:00 2001 From: Michael Sauer Date: Sat, 12 Sep 2026 13:12:54 +0200 Subject: [PATCH] fix: nl_mkimage wieder Python-3.8-tauglich machen CMake ruft nl_mkimage.py bei jedem Firmware-Build ueber das plain "python3" im Docker-Build-Container auf (Python 3.8.10), unabhaengig von den Werkzeugen auf Mac/Pi, die Python >= 3.11 voraussetzen. Die zuletzt hinzugekommene Annotation "expect_device: str | None" wird dort zur Laufzeit ausgewertet und wirft "TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'", was jeden Firmware-Build abbrechen liess. "from __future__ import annotations" (wie in tools/station/server.py) sorgt dafuer, dass Annotationen nur noch als Text existieren und nie ausgewertet werden - der Rest der Datei kam ohne weitere 3.8-Stolpersteine aus. test: zwei Regressionstests ergaenzt, die ohne installiertes Python 3.8 auskommen: einer prueft per AST, dass der future-Import direkt nach dem Docstring steht, der andere laesst vermin (dev-Dependency) die Mindestversion aus Syntax/Stdlib-Nutzung ableiten. vermins "union-types"-Feature wird bewusst nicht aktiviert, weil es "str | None" unabhaengig vom future-Import als 3.10-Anforderung meldet - das wuerde ausgerechnet den korrekten Fix durchfallen lassen. Co-Authored-By: Claude Sonnet 5 --- tools/mkimage/nl_mkimage.py | 12 ++++++ tools/mkimage/pyproject.toml | 2 +- tools/mkimage/test_nl_mkimage.py | 68 ++++++++++++++++++++++++++++++++ tools/mkimage/uv.lock | 15 ++++++- 4 files changed, 95 insertions(+), 2 deletions(-) diff --git a/tools/mkimage/nl_mkimage.py b/tools/mkimage/nl_mkimage.py index 2a177ef..9a7583b 100755 --- a/tools/mkimage/nl_mkimage.py +++ b/tools/mkimage/nl_mkimage.py @@ -4,7 +4,19 @@ Layout (512 bytes): "NINL" | version(1) | device[16] | fw_version[16] | sha256[32] | 0xFF padding. The hash covers the complete `_gz.img` (T_BOOTER header + payload), which is exactly what the firmware hashes while writing the image to flash. + +Must run under Python 3.8.10: CMake invokes this script via plain `python3` inside the +x86_64 build container (see docker/Dockerfile, cmake/run_mkimage.cmake) on every firmware +build, and that container has not been upgraded past 3.8. This is independent of the +"Python >= 3.11" requirement elsewhere in this repo, which applies only to the tooling that +runs on the Mac and the Pi (tools/station, tools/mkimage's own pytest suite), never to this +file. Keep `from __future__ import annotations` below so that PEP 604 unions (`str | None`) +in annotations stay text and are never evaluated - without it, that syntax raises +`TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'` on 3.8/3.9. See +test_nl_mkimage.py::test_stays_python38_compatible, which fails the build if this drifts. """ +from __future__ import annotations + import argparse import hashlib import sys diff --git a/tools/mkimage/pyproject.toml b/tools/mkimage/pyproject.toml index aeee3bd..4c3155f 100644 --- a/tools/mkimage/pyproject.toml +++ b/tools/mkimage/pyproject.toml @@ -4,4 +4,4 @@ version = "0.1.0" requires-python = ">=3.11" [dependency-groups] -dev = ["pytest>=8"] +dev = ["pytest>=8", "vermin>=1.6"] diff --git a/tools/mkimage/test_nl_mkimage.py b/tools/mkimage/test_nl_mkimage.py index f1fa4f1..0d51b8a 100644 --- a/tools/mkimage/test_nl_mkimage.py +++ b/tools/mkimage/test_nl_mkimage.py @@ -208,3 +208,71 @@ def test_verify_with_build_arguments_is_an_error(tmp_path): ) assert r.returncode != 0 assert "cannot be combined" in r.stderr + + +# --- Python 3.8 compatibility ------------------------------------------------------------- +# +# CMake calls this script via plain `python3` inside the build container on every firmware +# build (see docker/Dockerfile, cmake/run_mkimage.cmake), and that container is stuck on +# Python 3.8.10. This is the opposite constraint from the rest of the repo's tooling (which +# needs Python >= 3.11) - see the module docstring in nl_mkimage.py for the full story, and +# git history around the "unsupported operand type(s) for |" break for what happens when it +# regresses. +# +# Two checks, because neither alone covers the failure mode we actually hit: +# 1. `from __future__ import annotations` must be the first statement after the module +# docstring. As long as it's there, PEP 604 unions in annotations (`str | None`) are +# never evaluated at runtime, so they can't raise on 3.8/3.9 no matter how many more of +# them get added later. +# 2. vermin's *default* analysis (annotations not evaluated - matching point 1) catches +# everything else 3.9+/3.10+/3.11+ that could sneak in: walrus in odd positions, match +# statements, dict `|=`, new stdlib members, etc. We deliberately do NOT enable vermin's +# `union-types` feature: it flags `str | None` as needing 3.10 unconditionally, without +# accounting for `from __future__ import annotations` deferring evaluation - which would +# make this test fail on the very fix that makes the file 3.8-safe. Check 1 above is +# what actually guards against that pattern. + + +def test_future_annotations_import_is_present(): + """Guards the PEP 604 union pattern that broke the Docker build (see module docstring).""" + import ast + + tree = ast.parse(SCRIPT.read_text()) + body = tree.body + # Skip a leading module docstring, if any. + if body and isinstance(body[0], ast.Expr) and isinstance(body[0].value, ast.Constant) \ + and isinstance(body[0].value.value, str): + body = body[1:] + assert body, "nl_mkimage.py is unexpectedly empty" + first = body[0] + assert isinstance(first, ast.ImportFrom) and first.module == "__future__" \ + and any(alias.name == "annotations" for alias in first.names), ( + "nl_mkimage.py must start with `from __future__ import annotations` right after its " + "module docstring - without it, PEP 604 unions like `str | None` in annotations are " + "evaluated eagerly and crash under the Python 3.8.10 in the Docker build container " + "(TypeError: unsupported operand type(s) for |: 'type' and 'NoneType')" + ) + + +def test_stays_python38_compatible(): + """Static check that nl_mkimage.py needs nothing newer than Python 3.8 to run. + + Runs without a Python 3.8 interpreter installed: vermin infers the minimum required + version from syntax/stdlib usage instead of actually executing the file. + """ + vermin = __import__("vermin") + + config = vermin.Config() + config.set_quiet(True) + # Match nl_mkimage.py's own strategy: with `from __future__ import annotations` present + # (checked separately above), annotations are never evaluated, so vermin shouldn't assume + # they need runtime support either. This is vermin's default, spelled out here so it's + # obvious why `union-types` (see the block comment above) is intentionally not enabled. + config.set_eval_annotations(False) + + source = SCRIPT.read_text() + py2_min, py3_min = vermin.detect(source, config, str(SCRIPT)) + assert py3_min is not None and py3_min <= (3, 8), ( + f"nl_mkimage.py now needs Python {py3_min} - it must keep working on the " + "Python 3.8.10 in the Docker build container (see module docstring)" + ) diff --git a/tools/mkimage/uv.lock b/tools/mkimage/uv.lock index 7030ede..8d0546a 100644 --- a/tools/mkimage/uv.lock +++ b/tools/mkimage/uv.lock @@ -28,12 +28,16 @@ source = { virtual = "." } [package.dev-dependencies] dev = [ { name = "pytest" }, + { name = "vermin" }, ] [package.metadata] [package.metadata.requires-dev] -dev = [{ name = "pytest", specifier = ">=8" }] +dev = [ + { name = "pytest", specifier = ">=8" }, + { name = "vermin", specifier = ">=1.6" }, +] [[package]] name = "packaging" @@ -77,3 +81,12 @@ sdist = { url = "https://files.pythonhosted.org/packages/e4/47/b9efed96c114afcfa wheels = [ { url = "https://files.pythonhosted.org/packages/24/25/1de2678b631f5a49215c6c96fff41ba892b0a34df68d6d80292b1b48aa7f/pytest-9.1.1-py3-none-any.whl", hash = "sha256:37a86b45efb9a47a61a36449063e8e18d0cab3161329fc099eb21783169c4f0c", size = 386536, upload-time = "2026-06-19T10:58:31.347Z" }, ] + +[[package]] +name = "vermin" +version = "1.8.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/24/7b/2c1b403f2a844e1acb36694fc336e323df742f7f752edb4188311ad43f9e/vermin-1.8.0.tar.gz", hash = "sha256:3621955ac2a2950175c5b4a9b2fc3bd24bd416da0388893c9eb6971264e4ca1f", size = 99913, upload-time = "2025-11-20T17:29:31.509Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/e6/fd/6d2167767488e162efd01300a679c7f7d913dbb65baa0b15760ebb369854/vermin-1.8.0-py3-none-any.whl", hash = "sha256:6f2b98ad697a16ed3442e81a0f0baabfc38c185b78aaab1b1fecd87bdd6640b8", size = 98030, upload-time = "2025-11-20T17:29:29.931Z" }, +]