-
Notifications
You must be signed in to change notification settings - Fork 27
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 |
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.