From 8cc62793f3dfdd82b84183746eee47812ea7076b Mon Sep 17 00:00:00 2001 From: "kingstom.chen" Date: Thu, 4 Jun 2026 02:46:24 -0600 Subject: [PATCH] fix(doctor): report ABI-incompatible renderdoc module instead of "not found" A renderdoc module that exists on a search path but is built for a different Python (ABI mismatch -> clean ImportError) was reported by `rdc doctor` as "renderdoc-module: not found in search paths", with a "build from source" hint -- even though a build already existed. Root cause: find_renderdoc() recorded the module-level _diagnostic only for SUCCESS / CRASH_PRONE / TIMEOUT probe outcomes; IMPORT_FAILED was dropped, so doctor._import_renderdoc() saw no diagnostic and fell through to the generic "not found" message. - discover.py: record an IMPORT_FAILED diagnostic, but only when the candidate dir actually holds a renderdoc artifact (renderdoc*.so/.pyd/.py) -- a bare import failure (e.g. the renderdoccmd sibling dir with no module) must not masquerade as "found but failed to import". - doctor.py: add an IMPORT_FAILED branch naming the path and the likely cause (built for a different Python; rebuild). The no-diagnostic case still reports "not found in search paths". - tests: positive (module present -> diagnostic) and negative (no module -> no diagnostic) cases in test_discover.py, plus the doctor message. --- src/rdc/commands/doctor.py | 8 +++++ src/rdc/discover.py | 14 +++++++++ tests/unit/test_discover.py | 58 +++++++++++++++++++++++++++++++++++++ tests/unit/test_doctor.py | 26 +++++++++++++++++ 4 files changed, 106 insertions(+) diff --git a/src/rdc/commands/doctor.py b/src/rdc/commands/doctor.py index 026db926..dfda2e68 100644 --- a/src/rdc/commands/doctor.py +++ b/src/rdc/commands/doctor.py @@ -61,6 +61,14 @@ def _import_renderdoc() -> tuple[Any | None, CheckResult]: False, f"incompatible at {diag.candidate_path} -- rebuild renderdoc for current Python", ) + if diag is not None and diag.result == ProbeResult.IMPORT_FAILED: + return None, CheckResult( + "renderdoc-module", + False, + f"found at {diag.candidate_path} but failed to import" + " -- likely built for a different Python (ABI mismatch);" + " rebuild for current Python", + ) return None, CheckResult("renderdoc-module", False, "not found in search paths") version = getattr(module, "GetVersionString", lambda: "unknown")() diff --git a/src/rdc/discover.py b/src/rdc/discover.py index 1e939678..c7bd2ffc 100644 --- a/src/rdc/discover.py +++ b/src/rdc/discover.py @@ -56,6 +56,16 @@ def _is_arm_studio_dir(directory: str) -> bool: return any("arm-performance-studio" in p.lower() for p in parts) +def _has_renderdoc_module(directory: str) -> bool: + """Return True if *directory* holds an importable renderdoc module artifact.""" + d = Path(directory) + return ( + (d / "renderdoc.py").is_file() + or bool(list(d.glob("renderdoc*.so"))) + or bool(list(d.glob("renderdoc*.pyd"))) + ) + + def _preload_librenderdoc(directory: str) -> None: """Preload librenderdoc.so with RTLD_GLOBAL for ARM PS patched module. @@ -174,6 +184,10 @@ def find_renderdoc() -> ModuleType | None: _diagnostic = outcome elif outcome.result == ProbeResult.TIMEOUT: _diagnostic = outcome + elif outcome.result == ProbeResult.IMPORT_FAILED and _has_renderdoc_module(path): + # A module file is present but won't load (e.g. built for another + # Python); keep it so the caller can tell this from "not found". + _diagnostic = outcome if crash_prone_candidates: _diagnostic = ProbeOutcome( diff --git a/tests/unit/test_discover.py b/tests/unit/test_discover.py index e71406a4..04b44d61 100644 --- a/tests/unit/test_discover.py +++ b/tests/unit/test_discover.py @@ -234,6 +234,64 @@ def mock_which(cmd: str): assert diag.result == ProbeResult.CRASH_PRONE assert diag.candidate_path == str(crash_dir) + def test_diagnostic_set_after_import_failed( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """A module that exists but fails to import (ABI mismatch) is recorded. + + Without this, an import-failed candidate left no diagnostic and the + caller could not distinguish it from "nothing found at all". + """ + bad_dir = tmp_path / "abi-mismatch" + bad_dir.mkdir() + # A module artifact is present -- the import failure is a real load + # failure (ABI mismatch), not "no module here". + (bad_dir / "renderdoc.so").write_text("fake") + + def mock_renderdoc_search_paths(): + return [str(bad_dir)] + + def mock_which(cmd: str): + return None + + monkeypatch.setattr("rdc.discover._try_import", lambda: None) + monkeypatch.setattr( + "rdc.discover._probe_candidate", + lambda path, timeout=5.0: ProbeOutcome(ProbeResult.IMPORT_FAILED, str(bad_dir)), + ) + monkeypatch.setattr("rdc.discover._try_import_from", lambda d: None) + monkeypatch.setattr("rdc._platform.renderdoc_search_paths", mock_renderdoc_search_paths) + monkeypatch.setattr("rdc.discover.shutil.which", mock_which) + + result = find_renderdoc() + + assert result is None + diag = _get_diagnostic() + assert diag is not None + assert diag.result == ProbeResult.IMPORT_FAILED + assert diag.candidate_path == str(bad_dir) + + def test_import_failed_without_module_leaves_no_diagnostic( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """A dir with no module (e.g. the renderdoccmd sibling /usr/bin) probes + as IMPORT_FAILED but must not be reported as "found but failed to import". + """ + empty_dir = tmp_path / "no-module" + empty_dir.mkdir() + + monkeypatch.setattr("rdc.discover._try_import", lambda: None) + monkeypatch.setattr( + "rdc.discover._probe_candidate", + lambda path, timeout=5.0: ProbeOutcome(ProbeResult.IMPORT_FAILED, str(empty_dir)), + ) + monkeypatch.setattr("rdc.discover._try_import_from", lambda d: None) + monkeypatch.setattr("rdc._platform.renderdoc_search_paths", lambda: [str(empty_dir)]) + monkeypatch.setattr("rdc.discover.shutil.which", lambda cmd: None) + + assert find_renderdoc() is None + assert _get_diagnostic() is None + class TestArmStudioDir: """_is_arm_studio_dir detects ARM PS directory layout.""" diff --git a/tests/unit/test_doctor.py b/tests/unit/test_doctor.py index 1054c23c..0b9a083c 100644 --- a/tests/unit/test_doctor.py +++ b/tests/unit/test_doctor.py @@ -650,6 +650,32 @@ def test_crash_prone_shows_path_and_rebuild_hint( assert crash_path in result.detail assert "rebuild" in result.detail.lower() or "incompatible" in result.detail.lower() + def test_import_failed_shows_path_and_abi_hint( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + """When a module exists but fails to import, name the path and ABI cause. + + Regression: an ABI-mismatched module (built for another Python) used to + be reported as the generic "not found in search paths". + """ + bad_path = str(tmp_path / "abi-mismatch") + monkeypatch.setattr( + "rdc.commands.doctor.find_renderdoc", + lambda: None, + ) + monkeypatch.setattr( + "rdc.commands.doctor._get_diagnostic", + lambda: ProbeOutcome(ProbeResult.IMPORT_FAILED, bad_path), + ) + + _, result = _import_renderdoc() + + assert result.ok is False + assert bad_path in result.detail + assert "import" in result.detail.lower() + assert "python" in result.detail.lower() + assert "not found" not in result.detail.lower() + def test_no_diagnostic_shows_not_found(self, monkeypatch: pytest.MonkeyPatch) -> None: """When no diagnostic available, show generic not found message.""" monkeypatch.setattr(