Motivation
Follow up on PR #2586 with a semantics-neutral internal cleanup proposed in this review comment.
Standalone Windows Nsight discovery currently places product-specific registry and executable-layout knowledge in find_nvidia_binary_utility.py. Extracting that logic would leave the generic finder focused on search policy and orchestration while giving the Windows Nsight details a clear owner.
This refactor should be done after PR #2586 is merged.
Proposed design
Add:
cuda_pathfinder/cuda/pathfinder/_binaries/windows_nsight.py
The module would own the Nsight-specific registry and layout definitions and expose internal lazy candidate iterators:
_REGISTRY_ROOT = ...
_NSYS_TARGET_DIR_BY_ARCH = ...
_NCU_TARGET_DIR_BY_ARCH = ...
def _installed_product_root(product: str) -> str | None: ...
def nsys_candidate_paths() -> Iterator[str]: ...
def ncu_candidate_paths() -> Iterator[str]: ...
The generic finder would continue to own:
- Search orchestration and caching
- Executable validation
- Absolute-path normalization
- Candidate resolution
Its Nsight call sites would become approximately:
from cuda.pathfinder._binaries import windows_nsight
return _resolve_candidate_paths(windows_nsight.nsys_candidate_paths())
return _resolve_candidate_paths(windows_nsight.ncu_candidate_paths())
Lazy candidate iterators avoid a dependency from windows_nsight.py back to the generic finder. They must also preserve the current ncu behavior: yield ncu.bat first and determine the native machine architecture only if the resolver requests the direct executable fallback. A valid launcher should continue to work without invoking architecture detection.
Behavior to preserve
- Keep
winreg dynamically imported inside _installed_product_root() so importing cuda.pathfinder remains safe on non-Windows hosts.
- Import
windows_nsight itself unconditionally because tests simulate Windows on Linux by patching IS_WINDOWS after import.
- Treat an absent product registration as a normal miss.
- Raise a contextual
RuntimeError for incomplete or invalid registry data, and allow access errors to propagate.
- Preserve native-machine selection and the lack of cross-architecture fallback for direct
nsys and ncu executables.
- Keep all new names internal, with no public API or observable search-order change.
- Leave Compute Sanitizer in the generic finder for now because its special path is a Windows CUDA Toolkit layout rather than standalone Nsight product discovery.
If more Windows-specific CUDA Toolkit utility layouts are added later, that may justify a separate windows_ctk.py boundary.
Testing
- Move direct registry and Nsight-layout tests to
test_windows_nsight.py.
- Keep public-finder, wheel/Conda precedence, terminal-miss, caching, and overall search-order tests in
test_find_nvidia_binaries.py.
- Move existing test-authorship markers with their tests unchanged.
- Verify that lazy
ncu candidate evaluation does not call native architecture detection when ncu.bat resolves successfully.
Acceptance criteria
find_nvidia_binary_utility.py owns generic discovery policy and orchestration.
windows_nsight.py owns standalone Windows Nsight registry and layout knowledge.
- The public API, candidate precedence, caching, error behavior, path normalization, and observable search order remain unchanged.
- Existing tests continue to pass after being reorganized.
Motivation
Follow up on PR #2586 with a semantics-neutral internal cleanup proposed in this review comment.
Standalone Windows Nsight discovery currently places product-specific registry and executable-layout knowledge in
find_nvidia_binary_utility.py. Extracting that logic would leave the generic finder focused on search policy and orchestration while giving the Windows Nsight details a clear owner.This refactor should be done after PR #2586 is merged.
Proposed design
Add:
The module would own the Nsight-specific registry and layout definitions and expose internal lazy candidate iterators:
The generic finder would continue to own:
Its Nsight call sites would become approximately:
Lazy candidate iterators avoid a dependency from
windows_nsight.pyback to the generic finder. They must also preserve the currentncubehavior: yieldncu.batfirst and determine the native machine architecture only if the resolver requests the direct executable fallback. A valid launcher should continue to work without invoking architecture detection.Behavior to preserve
winregdynamically imported inside_installed_product_root()so importingcuda.pathfinderremains safe on non-Windows hosts.windows_nsightitself unconditionally because tests simulate Windows on Linux by patchingIS_WINDOWSafter import.RuntimeErrorfor incomplete or invalid registry data, and allow access errors to propagate.nsysandncuexecutables.If more Windows-specific CUDA Toolkit utility layouts are added later, that may justify a separate
windows_ctk.pyboundary.Testing
test_windows_nsight.py.test_find_nvidia_binaries.py.ncucandidate evaluation does not call native architecture detection whenncu.batresolves successfully.Acceptance criteria
find_nvidia_binary_utility.pyowns generic discovery policy and orchestration.windows_nsight.pyowns standalone Windows Nsight registry and layout knowledge.