-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate libucxx build and ucxx build #257
Separate libucxx build and ucxx build #257
Conversation
The import test for ucxx/conda/recipes/ucxx/meta.yaml Lines 258 to 259 in 95f4eda
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @KyleFromNVIDIA , left a few change requests.
Thanks for the review @pentschev. This PR is still very much a work in progress. I'll address whatever portions of your review are still relevant once I've actually got it working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks mostly good to me, left a few comments/suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, thanks @KyleFromNVIDIA .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took a quick look, I don't have any additional comments. Thanks!
/merge |
This PR fixes a number of issues in the manifest: - cudf's dependency on rmm was accidentally removed in #221. Since rmm is header-only I guess nobody noticed the issue since it doesn't increase build times and only shows up if you attempt to make local changes to rmm and test them in cudf. - UCXX_ENABLE_PYTHON was removed in rapidsai/ucxx#257 - cuml depends on cuvs as of rapidsai/cuml#6085 - cugraph does not depend on cugraph-ops as of rapidsai/cugraph#4744 - Many packages were relying on transitively getting rmm or raft as dependencies, which causes problems when the intermediate dependency is removed. Extra `-D<package_name>_ROOT` variables in the CMake configure don't cause any problems so I made everything explicit. --------- Co-authored-by: Paul Taylor <[email protected]> Co-authored-by: Bradley Dice <[email protected]>
Rather than building
libucxx_python.so
in thelibucxx
build, and transferring it to theucxx
package, build it in theucxx
build.Nothing other than the Python wheel is currently using
ucxx::python
, so stop exporting it and installing the headers as well.