Export numa via a bundled FindNuma module instead of an absolute path#145
Conversation
pentschev
left a comment
There was a problem hiding this comment.
@vyasr would you mind having a look at this? This change came from #141 , and I'm no CMake expert to determine whether this change is correct for all situations we support downstream (like conda-forge/wheels). At the very least CMakeLists.txt seems to my inexperienced eyes suspicious because apparently linking by numa name works, however, NUMA_LIB is supposed to be defined prior to that in the same file.
vyasr
left a comment
There was a problem hiding this comment.
This solution will solve the proximate problem, but it doesn't go far enough. The current solution won't guarantee that the numa.h header is available, and it won't propagate any compiler/linker flags or dependencies. What we ought to do is define a find module that produces a suitable NUMA target that we can link to. Then we bundle that file with our package so that the same find logic can run for consumers of cuCascade. Pytorch's module is one example that does a more comprehensive search for components and handles the proper find_package arguments. It doesn't create a target though. cudf's old cufile module is a good example for that. cudf also demonstrates how you can use a rapids-cmake function to ensure that find module gets installed for consumers. cuCascade doesn't use rapids-cmake, but the rapids-cmake function in question can be replicated easily enough, it adds the file in question to a tracked list and later copies them into the build/install tree.
find_library resolves numa to an absolute path (e.g. /usr/lib64/libnuma.so) that was passed as a PUBLIC link dependency and serialized verbatim into the exported cuCascadeTargets.cmake. Consumers on systems where libnuma lives elsewhere (e.g. a manylinux-built wheel consumed on Ubuntu arm64, where it is at /usr/lib/aarch64-linux-gnu/libnuma.so) then fail to link. Add a FindNuma module that locates libnuma's header and library and produces a Numa::Numa imported target, and link that target instead of a raw path. The module is bundled into the installed CMake package and rerun from cuCascadeConfig.cmake, so the exported targets reference the Numa::Numa target name and each consumer resolves libnuma against its own system. This also propagates the numa.h include directory, which a bare library path did not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
19fd163 to
8206077
Compare
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vyasr
left a comment
There was a problem hiding this comment.
Looks right to me, thanks!
|
/merge |
Summary
Fixes #118.
find_library(NUMA_LIB numa REQUIRED)resolvesnumato an absolute path (e.g./usr/lib64/libnuma.so). That path was passed as aPUBLIClink dependency via${NUMA_LIB}, so CMake serialized it verbatim into the exportedcuCascadeTargets.cmake. Consumers on systems where libnuma lives at a different path then fail to link — for example a manylinux-built wheel referencing/usr/lib64/libnuma.soconsumed on Ubuntu arm64 (where it is at/usr/lib/aarch64-linux-gnu/libnuma.so):Changes
Rather than embed a library path in the export, this introduces a proper find module that produces an imported target, bundled with the package so the same find logic runs on both sides (per review feedback).
cmake/Modules/FindNuma.cmake(new): locatesnuma.hand thenumalibrary, validates viafind_package_handle_standard_args, and produces aNuma::Numaimported target carrying both the include directory and the library location. This also propagates thenuma.hinclude path, which a bare library reference did not.CMakeLists.txt: usefind_package(Numa REQUIRED)and link theNuma::Numatarget instead of a raw path; installFindNuma.cmakeinto the package'scmake/cuCascade/Modulesdir (skipped for topology-only builds, which don't depend on libnuma).cmake/cuCascadeConfig.cmake.in: prepend the bundledModulesdir toCMAKE_MODULE_PATHand rerunfind_dependency(Numa REQUIRED), so consumers reconstructNuma::Numaagainst their own libnuma — no path baked into the exported targets, and a clear configure-time error if libnuma is missing.Verification
Built, installed to a temp prefix, and configured a standalone consumer project against the install:
cuCascadeTargets.cmakereferences the target, not a path:FindNuma.cmakeis installed underlib/cmake/cuCascade/Modules/.find_package(cuCascade)reconstructsNuma::Numa, resolving libnuma at its own system path.🤖 This PR was authored by an AI agent (Claude Code), under human review.
🤖 Generated with Claude Code