From 3cb26147cd4c470177e1755c254f6f7a03878526 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Tue, 20 Oct 2026 11:10:26 +0530 Subject: [PATCH] fix(bindings): name the Python root package's public surface in __all__ The package ships a PEP 561 marker since #586. Under it a type checker treats a bare `from .x import Y` in `__init__.py` as private, so the marker alone made pyright (and so Pylance) refuse the README's own `from offline_protocol_sdk import ProtocolManager` with reportPrivateImportUsage, and `mypy --strict` report that the module "does not explicitly export" it. Before the marker both accepted it. The root package now lists its public surface in `__all__`: the generated module's own `__all__` plus the eleven hand-written managers, using only the `+=` forms the typing spec requires every checker to read. tests/test_public_surface.py pins the list to the attributes the package exposes, so a manager added without a line in `__all__` fails in CI rather than in a user's editor. The wheel harness gains the refusal case for a package without the marker, which #586 had checked by hand. Measured against the wheel built from this branch: pyright 1.1.407 and mypy 2.4.0 --strict both accept the README import with full types. --- CHANGELOG.md | 8 ++- .../python/offline_protocol_sdk/__init__.py | 28 +++++++- bindings/python/tests/test_public_surface.py | 68 +++++++++++++++++++ scripts/tests/test-build-wheel.sh | 13 ++++ 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 bindings/python/tests/test_public_surface.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 98804ae75..55f074ffd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -254,7 +254,13 @@ archived by series under [docs/changelog/](docs/changelog/); see the ("missing library stubs or py.typed marker") and every value it returned was `Any` in the application, although nearly every public function is annotated. The wheel now carries the marker, and the wheel build fails - if it goes missing. + if it goes missing. The root package also names its public surface in + `__all__`: under PEP 561 a type checker treats a bare `from .x import Y` + in `__init__.py` as private, so with the marker alone pyright (and so + Pylance) refused the README's own `from offline_protocol_sdk import + ProtocolManager`, and `mypy --strict` reported that the module "does not + explicitly export" it. A test pins the list to the attributes the package + exposes, and the wheel harness refuses a wheel without the marker. - **Python: a Linux box that could no longer advertise recovers on its own.** Linux loses track of the controller's advertising when a central-role connection completes while advertising is on: the controller keeps diff --git a/bindings/python/offline_protocol_sdk/__init__.py b/bindings/python/offline_protocol_sdk/__init__.py index 78c2f7695..e14259b4f 100644 --- a/bindings/python/offline_protocol_sdk/__init__.py +++ b/bindings/python/offline_protocol_sdk/__init__.py @@ -8,7 +8,6 @@ from ._callback_reentrancy import make_handle_maps_reentrant as _make_reentrant _make_reentrant(_generated) -del _generated, _make_reentrant # Platform managers from .secure_storage import SecureStorage # noqa: F401 @@ -20,3 +19,30 @@ from .ble_manager import BleManager # noqa: F401 from .ble_peripheral import BlePeripheral # noqa: F401 from .protocol_manager import ProtocolManager # noqa: F401 + +# The public surface, by name. The package ships a PEP 561 marker, and under +# it a type checker treats a bare `from .x import Y` above as private: pyright +# (and so Pylance) refuses `from offline_protocol_sdk import ProtocolManager`, +# the README's own quick start, and `mypy --strict` reports that the module +# "does not explicitly export" it. Only the `+=` forms below are ones every +# checker is required to read, so no unpacking or `list()` here. +# tests/test_public_surface.py pins this list to the attributes the package +# exposes, so a manager added above without a line here fails in CI rather +# than in a user's editor. +__all__ = [] +__all__ += _generated.__all__ +__all__ += [ + "SecureStorage", + "AppStateStorage", + "account_storage_namespace", + "TransportManager", + "TransportState", + "InternetManager", + "PeerEntry", + "PeerStreamManager", + "BleManager", + "BlePeripheral", + "ProtocolManager", +] + +del _generated, _make_reentrant diff --git a/bindings/python/tests/test_public_surface.py b/bindings/python/tests/test_public_surface.py new file mode 100644 index 000000000..cbfa6c8e5 --- /dev/null +++ b/bindings/python/tests/test_public_surface.py @@ -0,0 +1,68 @@ +"""Pins the root package's ``__all__`` to the attributes it exposes. + +The package ships a PEP 561 marker, and under it a type checker treats a +bare ``from .x import Y`` in ``__init__.py`` as private: pyright (and so +Pylance) refuses ``from offline_protocol_sdk import ProtocolManager``, the +README's own quick start, and ``mypy --strict`` reports that the module +"does not explicitly export" it. Only a name in ``__all__`` is public. A +manager added to the root without a line in ``__all__`` imports fine at +runtime and fails only in a user's editor, so the list is checked here +against what the package really exposes. +""" + +from __future__ import annotations + +import inspect + +import offline_protocol_sdk as sdk + +# Written out, not derived: the names the README and the example import from +# the root, which the generated module's own ``__all__`` does not cover. +HAND_WRITTEN_EXPORTS = ( + "SecureStorage", + "AppStateStorage", + "account_storage_namespace", + "TransportManager", + "TransportState", + "InternetManager", + "PeerEntry", + "PeerStreamManager", + "BleManager", + "BlePeripheral", + "ProtocolManager", +) + + +def _public_attributes() -> set[str]: + # The imports in __init__.py also bind each submodule as an attribute. + # Those stay reachable as ``offline_protocol_sdk.secure_storage`` whatever + # ``__all__`` says, and are not part of the star-import surface. + return { + name + for name, value in vars(sdk).items() + if not name.startswith("_") and not inspect.ismodule(value) + } + + +def test_all_names_every_public_attribute_and_nothing_else() -> None: + listed = set(sdk.__all__) + exposed = _public_attributes() + assert listed == exposed, ( + f"missing from __all__: {sorted(exposed - listed)}; " + f"listed but not exposed: {sorted(listed - exposed)}" + ) + + +def test_all_has_no_duplicates() -> None: + assert len(sdk.__all__) == len(set(sdk.__all__)) + + +def test_the_hand_written_managers_are_listed() -> None: + for name in HAND_WRITTEN_EXPORTS: + assert name in sdk.__all__, name + + +def test_the_generated_surface_is_listed() -> None: + from offline_protocol_sdk import offline_protocol as generated + + assert set(generated.__all__) <= set(sdk.__all__) diff --git a/scripts/tests/test-build-wheel.sh b/scripts/tests/test-build-wheel.sh index f0bfdb2bf..0571716b4 100755 --- a/scripts/tests/test-build-wheel.sh +++ b/scripts/tests/test-build-wheel.sh @@ -123,6 +123,19 @@ if [ "$HAVE_ZLIB" = 1 ]; then else echo " skip - no zlib headers to build the refusal fixture" fi +# The PEP 561 marker: setuptools wheels a package without it happily, so the +# required-files check at the end of the script is the only thing that +# notices, and this is the only place that proves it does. +copy_package "$WORK/nomarker" +rm "$WORK/nomarker/bindings/python/offline_protocol_sdk/py.typed" +if (cd "$WORK/nomarker" && bash bindings/python/scripts/build-wheel.sh "linux-$ARCH" "$WORK/libfixture.so" 1.2.3) >"$WORK/nomarker.log" 2>&1; then + fail "a package without the PEP 561 marker was wheeled" +elif grep -q "is missing \['offline_protocol_sdk/py.typed'\]" "$WORK/nomarker.log"; then + pass "a package without the PEP 561 marker is refused" +else + fail "a package without the PEP 561 marker was refused for another reason" + cat "$WORK/nomarker.log" >&2 +fi copy_package "$WORK/badversion" if (cd "$WORK/badversion" && bash bindings/python/scripts/build-wheel.sh "linux-$ARCH" "$WORK/libfixture.so" 1.2.3+local) >/dev/null 2>&1; then fail "a version with no Python spelling was wheeled"