Use cuda-pathfinder in build-system.requires#1817
Conversation
Pin cuda-pathfinder>=1.5 in both build-system.requires and project.dependencies. Made-with: Cursor
The previous 1.3.4a0 was a stale value that could confuse someone. Made-with: Cursor
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
|
rwgk
added
enhancement
rwgk
added
P1
| COMPILE_FOR_COVERAGE = bool(int(os.environ.get("CUDA_PYTHON_COVERAGE", "0"))) | ||
|
|
||
|
|
||
| # Please keep in sync with the copy in cuda_bindings/build_hooks.py. |
There was a problem hiding this comment.
We should avoid duplicating implementations. Could we not hoist this into a separate file and import from both?
There was a problem hiding this comment.
We should avoid duplicating implementations.
Agreed
Could we not hoist this into a separate file and import from both?
No 😭
In the words of Cursor Opus 4.6 1M Thinking:
There's no reasonable way to share code between the two build_hooks.py files:
- Each is a PEP 517 build backend loaded via
backend-path=["."]from its own package directory (cuda_bindings/orcuda_core/). They can't import from the monorepo root or from each other. - The function exists because
cuda.pathfindercan't be imported normally in this context — so it can't live insidecuda-pathfindereither. - A shared helper package in
build-system.requireswould be massive overkill for ~15 lines of stable code.
The only alternatives are worse: a new PyPI package just for this, importlib file-path hacks, or copying a shared file into both directories at CI time.
The "please keep in sync" comments are the pragmatic solution here.
There was a problem hiding this comment.
We have other existing cases of "please keep in sync" that are impractical to avoid.
It just crossed my mind: a pre-commit check to enforce that they stay in sync is probably pretty easy to implement.
| if os.path.isdir(os.path.join(sp_cuda, "pathfinder")): | ||
| cuda.__path__ = list(cuda.__path__) + [sp_cuda] | ||
| break | ||
| import cuda.pathfinder |
There was a problem hiding this comment.
Should we include some message explaining that we iterated through sys.path looking for pathfinder and couldn't find it? That seems to be important information that would be missing from just throwing we couldn't import pathfinder module.
There was a problem hiding this comment.
for p in sys.path:
sp_cuda = os.path.join(p, "cuda")
if os.path.isdir(os.path.join(sp_cuda, "pathfinder")):
cuda.__path__ = list(cuda.__path__) + [sp_cuda]
break
else:
raise ModuleNotFoundError(
"cuda-pathfinder is not installed in the build environment. "
"Ensure 'cuda-pathfinder>=1.5' is in build-system.requires."
)
import cuda.pathfinder ```
cpcloud
left a comment
cpcloud
left a comment
There was a problem hiding this comment.
The path hacking needs some more justification and explicit examples of in what common scenarios a plain import isn't going to work.
| for p in sys.path: | ||
| sp_cuda = os.path.join(p, "cuda") | ||
| if os.path.isdir(os.path.join(sp_cuda, "pathfinder")): | ||
| cuda.__path__ = list(cuda.__path__) + [sp_cuda] | ||
| break |
There was a problem hiding this comment.
Path hacking of any sort like this is a code smell. At the very least the docstring is missing a concrete example of why this is needed.
isolated build environments
is doing a ton of work there. Isolated how? What is backend-path and where does that come from? Where are all these nouns like backend-path and build-env coming from?
In what real-world scenarios does this actually happen where there's not a viable alternative?
I'm really skeptical that path hacking like this is necessary.
Made-with: Cursor
3 participants
Follow-up to #1801. Closes the build-time integration gap identified in #1803.
The build hooks in
cuda_bindingsandcuda_corenow usecuda.pathfinder.get_cuda_path_or_home()to resolveCUDA_PATH/CUDA_HOME, replacing the previous inlineos.environ.get()fallback.This firmly establishes that
cuda-pathfinderis usable as a tool at build time, not just at runtime, although a helper function (_import_get_cuda_path_or_home()) is needed becausecudais a Python namespace package; see #1824 for details.cuda-pathfinder>=1.5in bothbuild-system.requiresandproject.dependenciesforcuda_bindingsandcuda_core.backend-path=["."]causes thecudanamespace to shadow the installedcuda-pathfinder. The new_import_get_cuda_path_or_home()helper detects this and fixescuda.__path__by replacing the_NamespacePathwith a plain list that includes the site-packagescuda/directory.cuda_core/tests/test_build_hooks.pyaddsget_cuda_path_or_home.cache_clear()calls so monkeypatched env vars take effect.cuda_pathfinder/pixi.tomlpackage version from1.3.4a0to1.5.0(just to avoid confusing someone looking at the version in passing).