Skip to content
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

Revise libcufile recipe / fix tests. #1

Merged
merged 6 commits into from
Apr 12, 2023

Conversation

bdice
Copy link

@bdice bdice commented Apr 12, 2023

This PR addresses several issues in the libcufile recipe. Changes include:

  • etc/cufile.json and man were not being packaged correctly - these shouldn't go in the targets directory as I understand
  • Need to skip ppc64le when migrator is enabled (fixed skip logic)
  • Added more specific tests for package contents and moved tests to the top level (due to package name conflicts that otherwise prevent tests from running)
  • Updated summary/description a bit (description was missing)

Tests are passing locally, so I'll open this PR. However, we have one TODO item that needs addressed: if we intend to package libcufile_rdma.so (which is only available on linux64 and not aarch64), we need to figure out how to supply the missing DSOs. See PR for details.

Copy link

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Bradley! 🙏

Added some comment about RDMA/verbs support below

recipes/libcufile/meta.yaml Show resolved Hide resolved
recipes/libcufile/meta.yaml Outdated Show resolved Hide resolved
recipes/libcufile/meta.yaml Outdated Show resolved Hide resolved
recipes/libcufile/meta.yaml Outdated Show resolved Hide resolved
@bdice
Copy link
Author

bdice commented Apr 12, 2023

@mroeschke @jakirkham I think all the requests are addressed and this should be ready to merge. I updated the RDMA dependency (thanks for the tips) and also removed etc/cufile.json per our offline discussion.

You can see the CI outputs on my fork, which has Azure CI enabled: bdice#4

@mroeschke mroeschke merged commit 21053dd into mroeschke:libcufile Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants