Skip to content

Commit 5d5f21a

Browse files
LeSingh1LeSingh1
committed
Use pathlib.Path for internals and replace glob.glob with Path.glob
Follow-up to review feedback on #2489. Narrow the str-compatibility constraint to the public API. The internal helpers and the SearchPlatform protocol now pass and return Path; str() is applied only where the value leaves this module: - FindResult.abs_path, which reaches users unchanged as LoadedDL.abs_path - derive_ctk_root(), whose result is consumed by load_nvidia_dynamic_lib and the header search Replace all four glob.glob() call sites with Path.glob(). Sorting is done on the string form via a small sorted_glob() helper: PurePath ordering is case-insensitive on Windows, so plain sorted() over Path objects would reorder mixed-case filenames relative to the byte-wise ordering these call sites have always used, and the newest-first policy from #1732 rides on that ordering. Two sites deliberately stay on os.path: - _find_lib_dir_using_anchor() keeps os.path.normpath(): PurePath does not collapse "..", and Path.resolve() would additionally follow symlinks. - _derive_ctk_root_windows() keeps ntpath. derive_ctk_root() also reaches it on Linux, where PureWindowsPath would rewrite "/opt/foo/bin/libx.so" to "\opt\foo" instead of "/opt/foo". Differential fuzzing against the previous revision (400k randomized path shapes, 600 randomized directory trees) shows one intended change: LinuxSearchPlatform.conda_anchor_point() now returns a normalized Path rather than $CONDA_PREFIX verbatim. That is not observable downstream, because _find_lib_dir_using_anchor() already normpath'd its result; resolved paths, error messages and listdir attachments are byte-identical. Signed-off-by: LeSingh1 <[email protected]>
1 parent 2eb02cb commit 5d5f21a

3 files changed

Lines changed: 91 additions & 70 deletions

File tree

‎cuda_pathfinder/cuda/pathfinder/_dynamic_libs/search_platform.py‎

Lines changed: 57 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
from __future__ import annotations
1212

13-
import glob
1413
from collections.abc import Sequence
1514
from dataclasses import dataclass
1615
from pathlib import Path, PurePath
@@ -23,13 +22,30 @@
2322
from cuda.pathfinder._utils.windows_arch import windows_pe_matches_arch, windows_python_arch
2423

2524

25+
def sorted_glob(directory: Path, pattern: str, *, reverse: bool = False) -> list[Path]:
26+
"""Return ``directory.glob(pattern)`` matches in a deterministic order.
27+
28+
The ordering is deliberately taken from the string form rather than from
29+
``Path`` comparison: ``PurePath`` ordering is case-insensitive on Windows,
30+
so plain ``sorted()`` over ``Path`` objects would reorder mixed-case
31+
filenames relative to the byte-wise ordering used up to now. Issue #1732
32+
tracks the newest-first policy that rides on this ordering, so it is kept
33+
unchanged on both platforms.
34+
"""
35+
return sorted(directory.glob(pattern), key=str, reverse=reverse)
36+
37+
38+
def _sorted_dir_entry_names(directory: Path) -> list[str]:
39+
return sorted(entry.name for entry in directory.iterdir())
40+
41+
2642
def _no_such_file_in_sub_dirs(
2743
sub_dirs: Sequence[str], file_wild: str, error_messages: list[str], attachments: list[str]
2844
) -> None:
2945
error_messages.append(f"No such file: {file_wild}")
3046
for sub_dir in find_sub_dirs_all_sitepackages(sub_dirs):
3147
attachments.append(f' listdir("{sub_dir}"):')
32-
for node in sorted(node_path.name for node_path in Path(sub_dir).iterdir()):
48+
for node in _sorted_dir_entry_names(Path(sub_dir)):
3349
attachments.append(f" {node}")
3450

3551

@@ -38,7 +54,7 @@ def _find_so_in_rel_dirs(
3854
so_basename: str,
3955
error_messages: list[str],
4056
attachments: list[str],
41-
) -> str | None:
57+
) -> Path | None:
4258
sub_dirs_searched: list[tuple[str, ...]] = []
4359
file_wild = so_basename + "*"
4460
for rel_dir in rel_dirs:
@@ -50,29 +66,29 @@ def _find_so_in_rel_dirs(
5066
# multiple coexist, matching the newest-first bias elsewhere in pathfinder
5167
# (see LinuxSearchPlatform.find_in_lib_dir and load_dl_linux._candidate_sonames).
5268
# Issue #1732 tracks the deferred question of raising on true ambiguity.
53-
so_path = Path(abs_dir, so_basename)
69+
abs_dir_path = Path(abs_dir)
70+
so_path = abs_dir_path / so_basename
5471
if so_path.is_file():
55-
return str(so_path)
56-
for match in sorted(glob.glob(str(Path(abs_dir, file_wild))), reverse=True):
57-
so_path = Path(match)
72+
return so_path
73+
for so_path in sorted_glob(abs_dir_path, file_wild, reverse=True):
5874
if so_path.is_file():
59-
return str(so_path)
75+
return so_path
6076
sub_dirs_searched.append(sub_dir)
6177
for sub_dir in sub_dirs_searched:
6278
_no_such_file_in_sub_dirs(sub_dir, file_wild, error_messages, attachments)
6379
return None
6480

6581

66-
def _find_dll_under_dir(dirpath: str, file_wild: str, target_arch: str | None = None) -> str | None:
67-
for match in sorted(glob.glob(str(Path(dirpath, file_wild)))):
68-
path = Path(match)
82+
def _find_dll_under_dir(dirpath: Path, file_wild: str, target_arch: str | None = None) -> Path | None:
83+
for path in sorted_glob(dirpath, file_wild):
6984
if not path.is_file():
7085
continue
7186
if is_suppressed_dll_file(path.name):
7287
continue
88+
# windows_pe_matches_arch() lives in _utils and is still str-typed.
7389
if target_arch is not None and not windows_pe_matches_arch(str(path), target_arch):
7490
continue
75-
return str(path)
91+
return path
7692
return None
7793

7894

@@ -81,14 +97,14 @@ def _find_dll_in_rel_dirs(
8197
lib_searched_for: str,
8298
error_messages: list[str],
8399
attachments: list[str],
84-
) -> str | None:
100+
) -> Path | None:
85101
sub_dirs_searched: list[tuple[str, ...]] = []
86102
for rel_dir in rel_dirs:
87103
sub_dir = PurePath(rel_dir).parts
88104
for abs_dir in find_sub_dirs_all_sitepackages(sub_dir):
89-
dll_name = _find_dll_under_dir(abs_dir, lib_searched_for)
90-
if dll_name is not None:
91-
return dll_name
105+
dll_path = _find_dll_under_dir(Path(abs_dir), lib_searched_for)
106+
if dll_path is not None:
107+
return dll_path
92108
sub_dirs_searched.append(sub_dir)
93109
for sub_dir in sub_dirs_searched:
94110
_no_such_file_in_sub_dirs(sub_dir, lib_searched_for, error_messages, attachments)
@@ -100,7 +116,7 @@ def lib_searched_for(self, libname: str) -> str: ...
100116

101117
def site_packages_rel_dirs(self, desc: LibDescriptor) -> tuple[str, ...]: ...
102118

103-
def conda_anchor_point(self, conda_prefix: str) -> str: ...
119+
def conda_anchor_point(self, conda_prefix: str) -> Path: ...
104120

105121
def anchor_rel_dirs(self, desc: LibDescriptor) -> tuple[str, ...]: ...
106122

@@ -110,16 +126,16 @@ def find_in_site_packages(
110126
lib_searched_for: str,
111127
error_messages: list[str],
112128
attachments: list[str],
113-
) -> str | None: ...
129+
) -> Path | None: ...
114130

115131
def find_in_lib_dir(
116132
self,
117-
lib_dir: str,
133+
lib_dir: Path,
118134
desc: LibDescriptor,
119135
lib_searched_for: str,
120136
error_messages: list[str],
121137
attachments: list[str],
122-
) -> str | None: ...
138+
) -> Path | None: ...
123139

124140

125141
@dataclass(frozen=True, slots=True)
@@ -130,8 +146,8 @@ def lib_searched_for(self, libname: str) -> str:
130146
def site_packages_rel_dirs(self, desc: LibDescriptor) -> tuple[str, ...]:
131147
return cast(tuple[str, ...], desc.site_packages_linux)
132148

133-
def conda_anchor_point(self, conda_prefix: str) -> str:
134-
return conda_prefix
149+
def conda_anchor_point(self, conda_prefix: str) -> Path:
150+
return Path(conda_prefix)
135151

136152
def anchor_rel_dirs(self, desc: LibDescriptor) -> tuple[str, ...]:
137153
return cast(tuple[str, ...], desc.anchor_rel_dirs_linux)
@@ -142,39 +158,37 @@ def find_in_site_packages(
142158
lib_searched_for: str,
143159
error_messages: list[str],
144160
attachments: list[str],
145-
) -> str | None:
161+
) -> Path | None:
146162
return _find_so_in_rel_dirs(rel_dirs, lib_searched_for, error_messages, attachments)
147163

148164
def find_in_lib_dir(
149165
self,
150-
lib_dir: str,
166+
lib_dir: Path,
151167
_desc: LibDescriptor,
152168
lib_searched_for: str,
153169
error_messages: list[str],
154170
attachments: list[str],
155-
) -> str | None:
156-
lib_dir_path = Path(lib_dir)
171+
) -> Path | None:
157172
# Most libraries have both unversioned and versioned files/symlinks (exact match first)
158-
so_path = lib_dir_path / lib_searched_for
173+
so_path = lib_dir / lib_searched_for
159174
if so_path.is_file():
160-
return str(so_path)
175+
return so_path
161176
# Some libraries only exist as versioned files (e.g., libcupti.so.13 in conda),
162177
# so the glob fallback is needed
163178
file_wild = lib_searched_for + "*"
164179
# Only one match is expected, but to ensure deterministic behavior in unexpected
165180
# situations, and to be internally consistent, we sort in reverse order with the
166181
# intent to return the newest version first. Issue #1732 tracks the deferred
167182
# question of raising on true ambiguity.
168-
for match in sorted(glob.glob(str(lib_dir_path / file_wild)), reverse=True):
169-
so_path = Path(match)
183+
for so_path in sorted_glob(lib_dir, file_wild, reverse=True):
170184
if so_path.is_file():
171-
return str(so_path)
185+
return so_path
172186
error_messages.append(f"No such file: {file_wild}")
173187
attachments.append(f' listdir("{lib_dir}"):')
174-
if not lib_dir_path.is_dir():
188+
if not lib_dir.is_dir():
175189
attachments.append(" DIRECTORY DOES NOT EXIST")
176190
else:
177-
for node in sorted(node_path.name for node_path in lib_dir_path.iterdir()):
191+
for node in _sorted_dir_entry_names(lib_dir):
178192
attachments.append(f" {node}")
179193
return None
180194

@@ -189,8 +203,8 @@ def lib_searched_for(self, libname: str) -> str:
189203
def site_packages_rel_dirs(self, desc: LibDescriptor) -> tuple[str, ...]:
190204
return cast(tuple[str, ...], desc.site_packages_windows.for_arch(self.target_arch))
191205

192-
def conda_anchor_point(self, conda_prefix: str) -> str:
193-
return str(Path(conda_prefix, "Library"))
206+
def conda_anchor_point(self, conda_prefix: str) -> Path:
207+
return Path(conda_prefix, "Library")
194208

195209
def anchor_rel_dirs(self, desc: LibDescriptor) -> tuple[str, ...]:
196210
return cast(tuple[str, ...], desc.anchor_rel_dirs_windows.for_arch(self.target_arch))
@@ -201,32 +215,31 @@ def find_in_site_packages(
201215
lib_searched_for: str,
202216
error_messages: list[str],
203217
attachments: list[str],
204-
) -> str | None:
218+
) -> Path | None:
205219
return _find_dll_in_rel_dirs(rel_dirs, lib_searched_for, error_messages, attachments)
206220

207221
def find_in_lib_dir(
208222
self,
209-
lib_dir: str,
223+
lib_dir: Path,
210224
desc: LibDescriptor,
211225
_lib_searched_for: str,
212226
error_messages: list[str],
213227
attachments: list[str],
214-
) -> str | None:
228+
) -> Path | None:
215229
file_wild = desc.name + "*.dll"
216230
target_arch = self.target_arch if desc.requires_windows_binary_arch_check else None
217-
dll_name = _find_dll_under_dir(lib_dir, file_wild, target_arch)
218-
if dll_name is not None:
219-
return dll_name
231+
dll_path = _find_dll_under_dir(lib_dir, file_wild, target_arch)
232+
if dll_path is not None:
233+
return dll_path
220234
if target_arch is None:
221235
error_messages.append(f"No such file: {file_wild}")
222236
else:
223237
error_messages.append(f"No {target_arch}-compatible PE file: {file_wild}")
224-
lib_dir_path = Path(lib_dir)
225238
attachments.append(f' listdir("{lib_dir}"):')
226-
if not lib_dir_path.is_dir():
239+
if not lib_dir.is_dir():
227240
attachments.append(" DIRECTORY DOES NOT EXIST")
228241
else:
229-
for node in sorted(node_path.name for node_path in lib_dir_path.iterdir()):
242+
for node in _sorted_dir_entry_names(lib_dir):
230243
attachments.append(f" {node}")
231244
return None
232245

‎cuda_pathfinder/cuda/pathfinder/_dynamic_libs/search_steps.py‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
:data:`~cuda.pathfinder._dynamic_libs.search_platform.PLATFORM` instance.
2020
"""
2121

22-
import glob
2322
import os
2423
from collections.abc import Callable
2524
from dataclasses import dataclass, field
@@ -28,7 +27,7 @@
2827

2928
from cuda.pathfinder._dynamic_libs.lib_descriptor import LibDescriptor
3029
from cuda.pathfinder._dynamic_libs.load_dl_common import DynamicLibNotFoundError
31-
from cuda.pathfinder._dynamic_libs.search_platform import PLATFORM, SearchPlatform
30+
from cuda.pathfinder._dynamic_libs.search_platform import PLATFORM, SearchPlatform, sorted_glob
3231
from cuda.pathfinder._utils.env_vars import get_cuda_path_or_home
3332

3433
# ---------------------------------------------------------------------------
@@ -38,7 +37,12 @@
3837

3938
@dataclass
4039
class FindResult:
41-
"""A library file located on disk (not yet loaded)."""
40+
"""A library file located on disk (not yet loaded).
41+
42+
``abs_path`` is a ``str``: it is handed to the platform loaders and surfaces
43+
unchanged as the public ``LoadedDL.abs_path``. Everything upstream of this
44+
dataclass works with ``Path``.
45+
"""
4246

4347
abs_path: str
4448
found_via: str
@@ -71,26 +75,26 @@ def raise_not_found(self) -> NoReturn:
7175
FindStep = Callable[[SearchContext], FindResult | None]
7276

7377

74-
def _find_lib_dir_using_anchor(desc: LibDescriptor, platform: SearchPlatform, anchor_point: str) -> str | None:
78+
def _find_lib_dir_using_anchor(desc: LibDescriptor, platform: SearchPlatform, anchor_point: Path) -> Path | None:
7579
"""Find the library directory under *anchor_point* using the descriptor's relative paths."""
7680
rel_dirs = platform.anchor_rel_dirs(desc)
77-
anchor = Path(anchor_point)
7881
for rel_path in rel_dirs:
79-
for match in sorted(glob.glob(str(anchor / rel_path))):
80-
if Path(match).is_dir():
82+
for match in sorted_glob(anchor_point, rel_path):
83+
if match.is_dir():
8184
# os.path.normpath has no pathlib equivalent: PurePath deliberately does
82-
# not collapse "..", so keeping it preserves the exact path reported for
83-
# an anchor such as a CUDA_PATH containing "..".
84-
return os.path.normpath(match)
85+
# not collapse "..", and Path.resolve() would also follow symlinks. Keeping
86+
# normpath preserves the exact path reported for an anchor such as a
87+
# CUDA_PATH containing "..".
88+
return Path(os.path.normpath(match))
8589
return None
8690

8791

88-
def _find_using_lib_dir(ctx: SearchContext, lib_dir: str | None) -> str | None:
92+
def _find_using_lib_dir(ctx: SearchContext, lib_dir: Path | None) -> Path | None:
8993
"""Find a library file in a resolved lib directory."""
9094
if lib_dir is None:
9195
return None
9296
return cast(
93-
str | None,
97+
Path | None,
9498
ctx.platform.find_in_lib_dir(
9599
lib_dir,
96100
ctx.desc,
@@ -151,7 +155,11 @@ def _derive_ctk_root_windows(resolved_lib_path: str) -> str | None:
151155

152156

153157
def derive_ctk_root(resolved_lib_path: str) -> str | None:
154-
"""Derive CTK root from a resolved canary library path."""
158+
"""Derive CTK root from a resolved canary library path.
159+
160+
Returns a ``str`` because the result crosses into ``load_nvidia_dynamic_lib``
161+
and the header search, which are outside this module.
162+
"""
155163
ctk_root = _derive_ctk_root_linux(resolved_lib_path)
156164
if ctk_root is not None:
157165
return ctk_root
@@ -160,11 +168,11 @@ def derive_ctk_root(resolved_lib_path: str) -> str | None:
160168

161169
def find_via_ctk_root(ctx: SearchContext, ctk_root: str) -> FindResult | None:
162170
"""Find a library under a previously derived CTK root."""
163-
lib_dir = _find_lib_dir_using_anchor(ctx.desc, ctx.platform, ctk_root)
171+
lib_dir = _find_lib_dir_using_anchor(ctx.desc, ctx.platform, Path(ctk_root))
164172
abs_path = _find_using_lib_dir(ctx, lib_dir)
165173
if abs_path is None:
166174
return None
167-
return FindResult(abs_path, "system-ctk-root")
175+
return FindResult(str(abs_path), "system-ctk-root")
168176

169177

170178
# ---------------------------------------------------------------------------
@@ -179,7 +187,7 @@ def find_in_site_packages(ctx: SearchContext) -> FindResult | None:
179187
return None
180188
abs_path = ctx.platform.find_in_site_packages(rel_dirs, ctx.lib_searched_for, ctx.error_messages, ctx.attachments)
181189
if abs_path is not None:
182-
return FindResult(abs_path, "site-packages")
190+
return FindResult(str(abs_path), "site-packages")
183191
return None
184192

185193

@@ -192,7 +200,7 @@ def find_in_conda(ctx: SearchContext) -> FindResult | None:
192200
lib_dir = _find_lib_dir_using_anchor(ctx.desc, ctx.platform, anchor)
193201
abs_path = _find_using_lib_dir(ctx, lib_dir)
194202
if abs_path is not None:
195-
return FindResult(abs_path, "conda")
203+
return FindResult(str(abs_path), "conda")
196204
return None
197205

198206

@@ -210,10 +218,10 @@ def find_in_cuda_path(ctx: SearchContext) -> FindResult | None:
210218
cuda_home = get_cuda_path_or_home()
211219
if cuda_home is None:
212220
return None
213-
lib_dir = _find_lib_dir_using_anchor(ctx.desc, ctx.platform, cuda_home)
221+
lib_dir = _find_lib_dir_using_anchor(ctx.desc, ctx.platform, Path(cuda_home))
214222
abs_path = _find_using_lib_dir(ctx, lib_dir)
215223
if abs_path is not None:
216-
return FindResult(abs_path, "CUDA_PATH")
224+
return FindResult(str(abs_path), "CUDA_PATH")
217225
return None
218226

219227

0 commit comments

Comments
 (0)