Use pathlib in cuda.pathfinder._binaries and _utils (part 3 of #2410) - #2494
Use pathlib in cuda.pathfinder._binaries and _utils (part 3 of #2410)#2494LeSingh1 wants to merge 2 commits into
Conversation
Part 3 of the series proposed in NVIDIA#2410. _binaries/find_nvidia_binary_utility.py now works in Path internally: _is_executable_candidate, _ctk_bin_subdirs and _resolve_in_trusted_dirs take and return Path. str() is applied once, on the public return of find_nvidia_binary_utility(), which is unchanged. SITE_PACKAGES_BINDIRS holds path components instead of joined strings. The caller immediately did sub_dir.split(os.sep) to undo the join, and find_sub_dirs_all_sitepackages wants components anyway. find_sub_dirs_no_cache walks in Path. Its return type stays list[str]: _binaries, _dynamic_libs, _headers and _static_libs all consume it, so flipping it is better done on its own once those have moved. Its directory test goes through a small _is_dir() helper rather than calling Path.is_dir() directly. The two are not interchangeable here: os.path.isdir() returns False for any stat error, while Path.is_dir() only swallows the errnos in pathlib's ignore list and propagates the rest. This function walks site-packages trees that nobody here controls, so a single unreadable directory would have turned a clean "not found" into a PermissionError. _utils/env_vars.py uses Path.exists/Path.samefile. Both calls are already inside the existing try/except OSError, so the same error-handling difference does not apply. os.path.normcase and os.path.normpath stay in _paths_differ: PurePath does not collapse "..", which test_paths_differ_text_only depends on, and Path.resolve() would also follow symlinks. os.path.abspath likewise stays in _resolve_in_trusted_dirs, since Path.absolute() does not normalize. test_find_nvidia_binaries.py moves with the module: it asserts on the exact values passed to and returned by these private helpers, so it cannot be separated from the signature change. Verified by differential fuzzing of find_sub_dirs_no_cache against the previous implementation: 3000 calls over randomized trees comparing result order, plus 720 calls over trees containing unreadable directories. Identical, except that a parent dir spelled with redundant separators now yields the normalized form. Signed-off-by: LeSingh1 <sshaurya914@gmail.com>
github-actions
Bot
added
the
cuda.pathfinder
mdboom
left a comment
mdboom
left a comment
There was a problem hiding this comment.
Looks good, other than the _is_dir helper not being needed.
| from pathlib import Path | ||
|
|
||
|
|
||
| def _is_dir(path: Path) -> bool: |
There was a problem hiding this comment.
As discussed in #2410, let's not worry about this behavioral change. No need for this helper function -- just use is_dir() directly.
Per review: accept the behavioral change from os.path.isdir (False on any stat error) to Path.is_dir() (propagates EACCES/ENAMETOOLONG).
|
Done —
That covers the file you commented on; CI is the real validation for the rest. |
2 participants
find_nvidia_binary_utilityworks inPathinternally; its public return is unchanged.SITE_PACKAGES_BINDIRSholds path components instead of joined strings, since the caller immediately split them again.find_sub_dirs_no_cachetests directories through a small_is_dir()helper rather thanPath.is_dir()directly. The two are not interchangeable:os.path.isdir()returnsFalsefor any stat error, whilePath.is_dir()propagates the ones outside pathlib's ignore list (EACCES,ENAMETOOLONG). This walks site-packages trees we do not control, so one unreadable directory would otherwise turn a clean "not found" into aPermissionError. Raised separately on #2410, since it applies beyond this chunk.test_find_nvidia_binaries.pyis included here rather than in part 4, because it asserts on the exact values these private helpers receive and return.Verified on Linux CI: full pytest output byte-identical to the base commit apart from elapsed time. Fuzzed
find_sub_dirs_no_cacheagainst the old implementation — 3000 calls comparing result order, plus 720 over trees containing unreadable directories.Part 3 of #2410.