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 <noreply@anthropic.com>
2026-09-12 13:12:54 +02:00
parent d23b0d4e02
commit 4a8a5d658f
4 changed files with 95 additions and 2 deletions
@@ -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
@@ -4,4 +4,4 @@ version = "0.1.0"
 requires-python = ">=3.11"

 [dependency-groups]
-dev = ["pytest>=8"]
+dev = ["pytest>=8", "vermin>=1.6"]
@@ -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)"
+    )
@@ -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" },
+]