Skip to content

Commit 76bceed

Browse files
mdboomrwgk
andauthored
Make pre-commit work on Windows (#2327)
* Make pre-commit work on Windows * Update .pre-commit-config.yaml * Address some of the comments in the PR * Simplify type-checking * Address comments in PR * Simplifications * Fix simplifications * Fix type check * Add comment about stubgen-pyx issues * Update CONTRIBUTING.md Co-authored-by: Ralf W. Grosse-Kunstleve <rwgkio@gmail.com> * Update cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py --------- Co-authored-by: Ralf W. Grosse-Kunstleve <rwgkio@gmail.com> Co-authored-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
1 parent 3887f91 commit 76bceed

9 files changed

Lines changed: 166 additions & 46 deletions

File tree

.github/workflows/ci.yml

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,38 @@ jobs:
501501
with:
502502
is-release: ${{ github.ref_type == 'tag' }}
503503

504+
precommit-windows:
505+
name: Pre-commit on Windows
506+
runs-on: windows-latest
507+
if: ${{ github.repository_owner == 'nvidia' && !fromJSON(needs.should-skip.outputs.skip) }}
508+
needs:
509+
- should-skip
510+
permissions:
511+
contents: read
512+
steps:
513+
- name: Checkout repository
514+
uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
515+
with:
516+
fetch-depth: 1
517+
persist-credentials: false
518+
519+
- name: Set up Python
520+
uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0
521+
with:
522+
python-version: '3.13'
523+
524+
- name: Install pre-commit
525+
shell: bash
526+
run: |
527+
set -euxo pipefail
528+
python -m pip install --upgrade pip pre-commit
529+
530+
- name: Run pre-commit
531+
shell: bash
532+
run: |
533+
set -euxo pipefail
534+
SKIP=lychee pre-commit run --all-files
535+
504536
checks:
505537
name: Check job status
506538
if: always()
@@ -514,6 +546,7 @@ jobs:
514546
- test-linux-aarch64
515547
- test-windows
516548
- doc
549+
- precommit-windows
517550
steps:
518551
- name: Exit
519552
run: |
@@ -546,6 +579,7 @@ jobs:
546579
check_result "should-skip" "success" "${{ needs.should-skip.result }}"
547580
check_result "detect-changes" "success" "${{ needs.detect-changes.result }}"
548581
check_result "doc" "success" "${{ needs.doc.result }}"
582+
check_result "precommit-windows" "success" "${{ needs.precommit-windows.result }}"
549583
550584
# [doc-only] flips these from 'success' to 'skipped'
551585
if [[ "$doc_only" == "true" ]]; then expected="skipped"; else expected="success"; fi

.pre-commit-config.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ repos:
6969

7070
- id: stubgen-pyx-cuda-core
7171
name: Generate .pyi stubs for cuda_core
72-
entry: stubgen-pyx cuda_core/cuda --continue-on-error --include-private
72+
entry: python ./toolshed/run_stubgen_pyx.py
7373
language: python
7474
files: ^cuda_core/cuda/.*\.(pyx|pxd)$
7575
pass_filenames: false
@@ -103,7 +103,7 @@ repos:
103103
- id: check-yaml
104104
- id: debug-statements
105105
- id: end-of-file-fixer
106-
exclude: &gen_exclude '^(?:cuda_python/README\.md|cuda_bindings/cuda/bindings/.*\.in?|cuda_bindings/docs/source/module/.*\.rst?|.*\.pyi)$'
106+
exclude: &gen_exclude '^(?:cuda_python/README\.md|(?:.*/)?CLAUDE\.md|(?:.*/)?\.git_archival\.txt|cuda_bindings/cuda/bindings/.*\.in?|cuda_bindings/docs/source/module/.*\.rst?|.*\.pyi)$'
107107
- id: mixed-line-ending
108108
- id: trailing-whitespace
109109
exclude: |

CONTRIBUTING.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ Thank you for your interest in contributing to CUDA Python! Based on the type of
2525
- [Symptoms of a bad clone](#symptoms-of-a-bad-clone)
2626
- [Type stubs for cuda.core](#type-stubs-for-cudacore)
2727
- [Pre-commit](#pre-commit)
28+
- [Pre-commit on Windows](#pre-commit-on-windows)
2829
- [Signing Your Work](#signing-your-work)
2930
- [Code signing](#code-signing)
3031
- [Developer Certificate of Origin (DCO)](#developer-certificate-of-origin-dco)
@@ -166,6 +167,17 @@ between commits, leaving stale headers or out-of-date stubs in the history.
166167
If the hook isn't installed, `pre-commit run` (and CI) will print a visible
167168
warning reminding you to run `pre-commit install`.
168169

170+
### Pre-commit on Windows
171+
172+
For development on Windows (not WSL), the `lychee` pre-commit task will not work
173+
when running `pre-commit run --all-files`. This problem does not occur if you
174+
install the pre-commit hook and run it automatically as part of your `git
175+
commit` workflow. To resolve this, you can either:
176+
177+
1. Run `pre-commit` in Git Bash, rather than directly in PowerShell or cmd
178+
179+
2. Skip it by setting the environment variable `SKIP` to `lychee`. This would
180+
be `$env:SKIP = "lychee"` in PowerShell or `set SKIP=lychee` in cmd.
169181

170182
## Signing Your Work
171183

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_linux.py

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
# SPDX-License-Identifier: Apache-2.0
33

44
from __future__ import annotations
@@ -7,14 +7,18 @@
77
import ctypes
88
import ctypes.util
99
import os
10+
import sys
1011
from typing import TYPE_CHECKING, cast
1112

1213
from cuda.pathfinder._dynamic_libs.load_dl_common import LoadedDL
1314

1415
if TYPE_CHECKING:
1516
from cuda.pathfinder._dynamic_libs.lib_descriptor import LibDescriptor
1617

17-
CDLL_MODE = os.RTLD_NOW | os.RTLD_GLOBAL
18+
if sys.platform == "linux":
19+
CDLL_MODE = os.RTLD_NOW | os.RTLD_GLOBAL
20+
else:
21+
CDLL_MODE = 0
1822

1923

2024
def _load_libdl() -> ctypes.CDLL:
@@ -132,27 +136,35 @@ def _candidate_sonames(desc: LibDescriptor) -> list[str]:
132136
return candidates
133137

134138

135-
def check_if_already_loaded_from_elsewhere(desc: LibDescriptor, _have_abs_path: bool) -> LoadedDL | None:
136-
for soname in _candidate_sonames(desc):
137-
try:
138-
handle = ctypes.CDLL(soname, mode=os.RTLD_NOLOAD)
139-
except OSError:
140-
continue
141-
else:
142-
return LoadedDL(
143-
abs_path_for_dynamic_library(desc.name, handle),
144-
True,
145-
handle._handle,
146-
"was-already-loaded-from-elsewhere",
147-
)
148-
return None
149-
150-
151-
def _load_lib(desc: LibDescriptor, filename: str) -> ctypes.CDLL:
152-
cdll_mode = CDLL_MODE
153-
if desc.requires_rtld_deepbind:
154-
cdll_mode |= os.RTLD_DEEPBIND
155-
return ctypes.CDLL(filename, cdll_mode)
139+
if sys.platform == "linux":
140+
141+
def check_if_already_loaded_from_elsewhere(desc: LibDescriptor, _have_abs_path: bool) -> LoadedDL | None:
142+
for soname in _candidate_sonames(desc):
143+
try:
144+
handle = ctypes.CDLL(soname, mode=os.RTLD_NOLOAD)
145+
except OSError:
146+
continue
147+
else:
148+
return LoadedDL(
149+
abs_path_for_dynamic_library(desc.name, handle),
150+
True,
151+
handle._handle,
152+
"was-already-loaded-from-elsewhere",
153+
)
154+
return None
155+
156+
def _load_lib(desc: LibDescriptor, filename: str) -> ctypes.CDLL:
157+
cdll_mode = CDLL_MODE
158+
if desc.requires_rtld_deepbind:
159+
cdll_mode |= os.RTLD_DEEPBIND
160+
return ctypes.CDLL(filename, cdll_mode)
161+
else:
162+
163+
def check_if_already_loaded_from_elsewhere(_desc: LibDescriptor, _have_abs_path: bool) -> LoadedDL | None:
164+
raise RuntimeError(f"check_if_already_loaded_from_elsewhere() is not supported on platform {sys.platform!r}")
165+
166+
def _load_lib(_desc: LibDescriptor, _filename: str) -> ctypes.CDLL:
167+
raise RuntimeError(f"_load_lib() is not supported on platform {sys.platform!r}")
156168

157169

158170
def load_with_system_search(desc: LibDescriptor) -> LoadedDL | None:

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
# SPDX-License-Identifier: Apache-2.0
33

44
from __future__ import annotations
@@ -7,6 +7,7 @@
77
import ctypes.wintypes
88
import os
99
import struct
10+
import sys
1011
import warnings
1112
from typing import TYPE_CHECKING
1213

@@ -22,7 +23,10 @@
2223
POINTER_ADDRESS_SPACE = 2 ** (struct.calcsize("P") * 8)
2324

2425
# Set up kernel32 functions with proper types
25-
kernel32 = ctypes.windll.kernel32 # type: ignore[attr-defined]
26+
windll = getattr(ctypes, "windll", None)
27+
if windll is None:
28+
raise RuntimeError("ctypes.windll is required on Windows")
29+
kernel32 = windll.kernel32
2630

2731
# GetModuleHandleW
2832
kernel32.GetModuleHandleW.argtypes = [ctypes.wintypes.LPCWSTR]
@@ -45,6 +49,11 @@
4549
kernel32.GetModuleFileNameW.restype = ctypes.wintypes.DWORD
4650

4751

52+
# GetLastError
53+
kernel32.GetLastError.argtypes = []
54+
kernel32.GetLastError.restype = ctypes.wintypes.DWORD
55+
56+
4857
def ctypes_handle_to_unsigned_int(handle: ctypes.wintypes.HMODULE) -> int:
4958
"""Convert ctypes HMODULE to unsigned int."""
5059
handle_uint = int(handle)
@@ -73,7 +82,8 @@ def add_dll_directory(dll_abs_path: str) -> None:
7382
# the directory must stay on the search path for the process lifetime, and
7483
# the handle has no finalizer, so dropping it does not remove the directory.
7584
try:
76-
os.add_dll_directory(dirpath) # type: ignore[attr-defined]
85+
if sys.platform == "win32":
86+
os.add_dll_directory(dirpath)
7787
except OSError as e:
7888
# Warn instead of failing silently; the PATH update below is a weaker
7989
# fallback that newer loaders may ignore.
@@ -96,15 +106,15 @@ def abs_path_for_dynamic_library(libname: str, handle: ctypes.wintypes.HMODULE)
96106
length = kernel32.GetModuleFileNameW(handle, buffer, len(buffer))
97107

98108
if length == 0:
99-
error_code = ctypes.GetLastError() # type: ignore[attr-defined]
109+
error_code = kernel32.GetLastError()
100110
raise RuntimeError(f"GetModuleFileNameW failed for {libname!r} (error code: {error_code})")
101111

102112
# If buffer was too small, try with larger buffer
103113
if length == len(buffer):
104114
buffer = ctypes.create_unicode_buffer(32768) # Extended path length
105115
length = kernel32.GetModuleFileNameW(handle, buffer, len(buffer))
106116
if length == 0:
107-
error_code = ctypes.GetLastError() # type: ignore[attr-defined]
117+
error_code = kernel32.GetLastError()
108118
raise RuntimeError(f"GetModuleFileNameW failed for {libname!r} (error code: {error_code})")
109119

110120
return buffer.value
@@ -170,7 +180,7 @@ def load_with_abs_path(desc: LibDescriptor, found_path: str, found_via: str | No
170180
handle = kernel32.LoadLibraryExW(found_path, None, flags)
171181

172182
if not handle:
173-
error_code = ctypes.GetLastError() # type: ignore[attr-defined]
183+
error_code = kernel32.GetLastError()
174184
raise RuntimeError(f"Failed to load DLL at {found_path}: Windows error {error_code}")
175185

176186
return LoadedDL(found_path, False, ctypes_handle_to_unsigned_int(handle), found_via)

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/platform_loader.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
1+
# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
22
# SPDX-License-Identifier: Apache-2.0
33

44
"""Platform loader seam for OS-specific dynamic linking.
@@ -16,11 +16,11 @@
1616

1717
from __future__ import annotations
1818

19+
import sys
1920
from typing import Protocol
2021

2122
from cuda.pathfinder._dynamic_libs.lib_descriptor import LibDescriptor
2223
from cuda.pathfinder._dynamic_libs.load_dl_common import LoadedDL
23-
from cuda.pathfinder._utils.platform_aware import IS_WINDOWS
2424

2525

2626
class PlatformLoader(Protocol):
@@ -31,7 +31,7 @@ def load_with_system_search(self, desc: LibDescriptor) -> LoadedDL | None: ...
3131
def load_with_abs_path(self, desc: LibDescriptor, found_path: str, found_via: str | None = None) -> LoadedDL: ...
3232

3333

34-
if IS_WINDOWS:
34+
if sys.platform == "win32":
3535
from cuda.pathfinder._dynamic_libs import load_dl_windows as _impl
3636
else:
3737
from cuda.pathfinder._dynamic_libs import load_dl_linux as _impl

cuda_pathfinder/cuda/pathfinder/_utils/driver_info.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55

66
import ctypes
77
import functools
8+
import sys
89
from collections.abc import Callable
910
from dataclasses import dataclass
1011

1112
from cuda.pathfinder._dynamic_libs.load_nvidia_dynamic_lib import (
1213
load_nvidia_dynamic_lib as _load_nvidia_dynamic_lib,
1314
)
14-
from cuda.pathfinder._utils.platform_aware import IS_WINDOWS
1515

1616

1717
class QueryDriverCudaVersionError(RuntimeError):
@@ -60,16 +60,16 @@ def query_driver_cuda_version() -> DriverCudaVersion:
6060
raise QueryDriverCudaVersionError("Failed to query the CUDA driver version.") from exc
6161

6262

63+
if sys.platform == "win32":
64+
_DRIVER_LIB_LOADER: Callable[[str], ctypes.CDLL] = ctypes.WinDLL
65+
else:
66+
_DRIVER_LIB_LOADER = ctypes.CDLL
67+
68+
6369
def _query_driver_cuda_version_int() -> int:
6470
"""Return the encoded CUDA driver version from ``cuDriverGetVersion()``."""
6571
loaded_cuda = _load_nvidia_dynamic_lib("cuda")
66-
if IS_WINDOWS:
67-
# `ctypes.WinDLL` exists on Windows at runtime. The ignore is only for
68-
# Linux mypy runs, where the platform stubs do not define that attribute.
69-
loader_cls: Callable[[str], ctypes.CDLL] = ctypes.WinDLL # type: ignore[attr-defined]
70-
else:
71-
loader_cls = ctypes.CDLL
72-
driver_lib = loader_cls(loaded_cuda.abs_path)
72+
driver_lib = _DRIVER_LIB_LOADER(loaded_cuda.abs_path)
7373
cu_driver_get_version = driver_lib.cuDriverGetVersion
7474
cu_driver_get_version.argtypes = [ctypes.POINTER(ctypes.c_int)]
7575
cu_driver_get_version.restype = ctypes.c_int

cuda_pathfinder/tests/test_utils_driver_info.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ def test_query_driver_cuda_version_uses_windll_on_windows(monkeypatch):
4646
fake_driver_lib = _FakeDriverLib(status=0, version=12080)
4747
loaded_paths: list[str] = []
4848

49-
monkeypatch.setattr(driver_info, "IS_WINDOWS", True)
5049
monkeypatch.setattr(
5150
driver_info,
5251
"_load_nvidia_dynamic_lib",
@@ -57,7 +56,7 @@ def fake_windll(abs_path: str):
5756
loaded_paths.append(abs_path)
5857
return fake_driver_lib
5958

60-
monkeypatch.setattr(driver_info.ctypes, "WinDLL", fake_windll, raising=False)
59+
monkeypatch.setattr(driver_info, "_DRIVER_LIB_LOADER", fake_windll)
6160

6261
assert driver_info._query_driver_cuda_version_int() == 12080
6362
assert loaded_paths == [r"C:\Windows\System32\nvcuda.dll"]
@@ -93,9 +92,8 @@ def fail_query_driver_cuda_version_int() -> int:
9392
def test_query_driver_cuda_version_int_raises_when_cuda_call_fails(monkeypatch):
9493
fake_driver_lib = _FakeDriverLib(status=1, version=0)
9594

96-
monkeypatch.setattr(driver_info, "IS_WINDOWS", False)
9795
monkeypatch.setattr(driver_info, "_load_nvidia_dynamic_lib", lambda _libname: _loaded_cuda("/usr/lib/libcuda.so.1"))
98-
monkeypatch.setattr(driver_info.ctypes, "CDLL", lambda _abs_path: fake_driver_lib)
96+
monkeypatch.setattr(driver_info, "_DRIVER_LIB_LOADER", lambda _abs_path: fake_driver_lib)
9997

10098
with pytest.raises(RuntimeError, match=r"cuDriverGetVersion\(\) \(status=1\)"):
10199
driver_info._query_driver_cuda_version_int()

0 commit comments

Comments
 (0)