From d58525b9a0aa131e824a023bae5ba4b920fcfbf1 Mon Sep 17 00:00:00 2001 From: Anthony Shoumikhin Date: Mon, 31 Aug 2026 21:29:33 -0700 Subject: [PATCH 1/3] Stop importing the SDK downloader to load the Qualcomm package Deferring the SDK setup meant six modules now do this at import: from executorch.backends.qualcomm import setup_qnn_sdk which loads the package `__init__`, which imported the downloader from the sibling `scripts` directory. That directory is not part of the installed package everywhere the backend is built, so on those builds every one of those six modules fails to import: ModuleNotFoundError: No module named 'executorch.backends.qualcomm.scripts' Before this backend deferred its setup nothing imported the package `__init__` from inside the package, so the missing directory never mattered. That is why it went unnoticed. Nothing about loading the package needs the downloader. It is used in one branch of setup, the one that fetches a prebuilt SDK, and that branch is unreachable when `QNN_SDK_ROOT` is already set or the platform has no published SDK. So it is now imported inside that branch. `is_linux_x86` moved here rather than being resolved lazily, because setup asks it before the branch that needs a downloader, and the answer is a two line platform test. `install_qnn_sdk` and `QNN_ZIP_URL` resolve through a module level `__getattr__`, so they are still ordinary module attributes that callers and tests can replace. Test plan: package import, scripts present ok package import, scripts absent ok, and setup_qnn_sdk() completes QNN_SDK_ROOT set, scripts absent early return, downloader never imported The new test covers the third case, which is the one that broke. It fails on the previous revision with the ModuleNotFoundError above. --- backends/qualcomm/__init__.py | 41 +++++++++++++++---- .../tests/test_import_side_effects.py | 22 ++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/backends/qualcomm/__init__.py b/backends/qualcomm/__init__.py index 4b45dcdb67c..7ef2860a9cf 100644 --- a/backends/qualcomm/__init__.py +++ b/backends/qualcomm/__init__.py @@ -1,4 +1,6 @@ import os +import platform +import sys import threading # The Qualcomm SDK setup below is deferred rather than run here, so that importing this @@ -11,11 +13,33 @@ # Nothing about loading this package needs any of that. The native adaptor does not link # the SDK; it resolves QNN symbols with dlopen when a model is actually compiled, and says # so plainly when they are missing. So setup happens on the first call that needs it. -from .scripts.download_qnn_sdk import ( # noqa: F401 - install_qnn_sdk, - is_linux_x86, - QNN_ZIP_URL, -) + +# Resolved on first use rather than at import, because the downloader lives in a sibling +# directory that some builds do not package, and importing this package must not require it. +# A module level __getattr__ keeps these as ordinary attributes, so callers and tests can still +# reach and replace them. +_LAZY_NAMES = ("install_qnn_sdk", "QNN_ZIP_URL") + + +def is_linux_x86() -> bool: + """True when a prebuilt Qualcomm SDK is published for this platform.""" + return platform.system().lower() == "linux" and platform.machine().lower() in ( + "x86_64", + "amd64", + "i386", + "i686", + ) + + +def __getattr__(name): + if name in _LAZY_NAMES: + from .scripts.download_qnn_sdk import install_qnn_sdk, QNN_ZIP_URL + + globals()["install_qnn_sdk"] = install_qnn_sdk + globals()["QNN_ZIP_URL"] = QNN_ZIP_URL + return globals()[name] + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") + _sdk_ready = False # Guards the flag above. Python's import lock does not, because the setup is now called from @@ -57,16 +81,17 @@ def _setup_qnn_sdk_locked() -> None: return # Downloading a prebuilt SDK is only possible for the platform it is published for. - if not is_linux_x86(): + if not sys.modules[__name__].is_linux_x86(): _sdk_ready = True return - if not install_qnn_sdk(): + module = sys.modules[__name__] + if not module.install_qnn_sdk(): raise RuntimeError( "Failed to set up QNN SDK.\n\n" "To resolve, try one of:\n" " 1. Download the SDK manually from:\n" - f" {QNN_ZIP_URL}\n" + f" {module.QNN_ZIP_URL}\n" " Or go to step 2 if QNN SDK already exists.\n" " 2. Set QNN_SDK_ROOT to an existing SDK installation:\n" " export QNN_SDK_ROOT=/path/to/qualcomm/sdk\n" diff --git a/backends/qualcomm/tests/test_import_side_effects.py b/backends/qualcomm/tests/test_import_side_effects.py index 20d3528afce..f52d49742fc 100644 --- a/backends/qualcomm/tests/test_import_side_effects.py +++ b/backends/qualcomm/tests/test_import_side_effects.py @@ -84,6 +84,28 @@ def test_importing_the_package_does_not_set_up_the_sdk(): ) +def test_importing_the_package_does_not_need_the_downloader(monkeypatch): + """Importing must not require the sibling scripts directory. + + Some builds package this backend without it, and a module level import of the downloader + made every module that calls setup_qnn_sdk fail to import there with + ModuleNotFoundError. Nothing about loading the package needs it: it is only used to + download an SDK, which is one branch of setup. + """ + monkeypatch.setitem( + sys.modules, "executorch.backends.qualcomm.scripts.download_qnn_sdk", None + ) + monkeypatch.setitem(sys.modules, "executorch.backends.qualcomm.scripts", None) + + importlib.reload(qnn) + + assert qnn.setup_qnn_sdk is not None + assert qnn.disable_mkldnn_on_amd is not None + # is_linux_x86 has to answer without the downloader, because setup asks it before the + # branch that needs one. + assert isinstance(qnn.is_linux_x86(), bool) + + def test_setup_is_idempotent(monkeypatch): """The compile paths each call it, so only the first call may do the work.""" calls = [] From 86c8ee71d669b46f497e3c271f34fa0831d04027 Mon Sep 17 00:00:00 2001 From: Anthony Shoumikhin Date: Mon, 31 Aug 2026 21:44:51 -0700 Subject: [PATCH 2/3] Exercise the case that broke, and keep the failure actionable Two review points, both right. The new test only checked that attributes existed after reloading with the downloader missing. It never called `setup_qnn_sdk()`, so it did not cover the case the change is about: a Linux x86 host with a preinstalled SDK, where setup takes its early return. It now forces that path and calls setup, which is what would raise if the downloader were imported again. And on a build with no downloader, where `QNN_SDK_ROOT` happens to be unset on Linux x86, the lazy attribute leaked a `ModuleNotFoundError` about a missing sibling directory. That tells the caller nothing they can act on. It is now a `RuntimeError` naming the variable to set, chained to the original so the cause is still visible. --- backends/qualcomm/__init__.py | 15 ++++++++++++++- .../qualcomm/tests/test_import_side_effects.py | 13 +++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/backends/qualcomm/__init__.py b/backends/qualcomm/__init__.py index 7ef2860a9cf..295e72a3270 100644 --- a/backends/qualcomm/__init__.py +++ b/backends/qualcomm/__init__.py @@ -86,7 +86,20 @@ def _setup_qnn_sdk_locked() -> None: return module = sys.modules[__name__] - if not module.install_qnn_sdk(): + try: + installed = module.install_qnn_sdk() + except ModuleNotFoundError as error: + # This build does not carry the downloader, so an SDK cannot be fetched here. Say what to + # do rather than surfacing a missing module from a packaging detail. + raise RuntimeError( + "This build cannot download a QNN SDK. Set QNN_SDK_ROOT to an existing " + "installation:\n" + " export QNN_SDK_ROOT=/path/to/qualcomm/sdk\n" + " export LD_LIBRARY_PATH=" + "$QNN_SDK_ROOT/lib/x86_64-linux-clang/:$LD_LIBRARY_PATH" + ) from error + + if not installed: raise RuntimeError( "Failed to set up QNN SDK.\n\n" "To resolve, try one of:\n" diff --git a/backends/qualcomm/tests/test_import_side_effects.py b/backends/qualcomm/tests/test_import_side_effects.py index f52d49742fc..01814698ea4 100644 --- a/backends/qualcomm/tests/test_import_side_effects.py +++ b/backends/qualcomm/tests/test_import_side_effects.py @@ -99,11 +99,16 @@ def test_importing_the_package_does_not_need_the_downloader(monkeypatch): importlib.reload(qnn) - assert qnn.setup_qnn_sdk is not None + # The case that actually broke: a Linux x86 host with a preinstalled SDK, where setup runs + # its early return. That must not touch the downloader. + monkeypatch.setattr(qnn, "is_linux_x86", lambda: True) + monkeypatch.setattr(qnn, "_sdk_ready", False) + monkeypatch.setenv("QNN_SDK_ROOT", "/opt/qcom/sdk") + monkeypatch.delenv("EXECUTORCH_BUILDING_WHEEL", raising=False) + + qnn.setup_qnn_sdk() + assert qnn.disable_mkldnn_on_amd is not None - # is_linux_x86 has to answer without the downloader, because setup asks it before the - # branch that needs one. - assert isinstance(qnn.is_linux_x86(), bool) def test_setup_is_idempotent(monkeypatch): From feb8b0bc44c27cdb561f25bf39789580011127a9 Mon Sep 17 00:00:00 2001 From: Anthony Shoumikhin Date: Mon, 31 Aug 2026 22:03:00 -0700 Subject: [PATCH 3/3] Only rewrap a missing downloader, not a missing dependency of it The catch was too wide. The downloader imports `requests`, so on a build where that is not installed the missing dependency was reported as "This build cannot download a QNN SDK", pointing the reader at packaging instead of at the real cause. Reproduced by hiding `requests`. Narrowed to the downloader module itself, by name. A `ModuleNotFoundError` from anything else is re-raised untouched: requests missing ModuleNotFoundError naming requests downloader missing RuntimeError naming QNN_SDK_ROOT --- backends/qualcomm/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/backends/qualcomm/__init__.py b/backends/qualcomm/__init__.py index 295e72a3270..5b2734c8b83 100644 --- a/backends/qualcomm/__init__.py +++ b/backends/qualcomm/__init__.py @@ -89,6 +89,10 @@ def _setup_qnn_sdk_locked() -> None: try: installed = module.install_qnn_sdk() except ModuleNotFoundError as error: + # Only when the downloader itself is absent. A dependency missing from inside it is a + # different problem and is left to speak for itself. + if not (error.name or "").startswith(f"{__name__}.scripts"): + raise # This build does not carry the downloader, so an SDK cannot be fetched here. Say what to # do rather than surfacing a missing module from a packaging detail. raise RuntimeError(