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

Fix exporting of include directories #540

Merged

Conversation

KyleFromNVIDIA
Copy link
Contributor

The CUDA and cufile include directories should not be exported as absolute paths when installing, only in the build tree. Add a $<BUILD_INTERFACE> generator expression.

The CUDA and cufile include directories should not be exported as
absolute paths when installing, only in the build tree. Add a
$<BUILD_INTERFACE> generator expression.
@KyleFromNVIDIA KyleFromNVIDIA requested a review from a team as a code owner November 6, 2024 17:50
@KyleFromNVIDIA KyleFromNVIDIA added bug Something isn't working non-breaking Introduces a non-breaking change labels Nov 6, 2024
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I think that this is a reasonable minimum fix to unblock downstream builds that are currently receiving invalid includes when linking to the kvikio target. I'll open an issue for the longer-term fixes that I'd like to see after this.

@vyasr
Copy link
Contributor

vyasr commented Nov 6, 2024

OK so now the C++ wheel isn't exporting sufficient information for the Python wheel to know it needs to find CUDA. I thought that we could fix that by addressing point 2 from #541 and linking to the cuda target directly, but then I remembered that kvikio's cuda support is optional and we handle cuda via a dlopen shim. I think it should still be OK to target_link_libraries(cuda* though, right? The linker will prune any libraries that aren't actually used so extra libs shouldn't be a problem, and we'll get the correct includes added.

@KyleFromNVIDIA
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 130db96 into rapidsai:branch-24.12 Nov 6, 2024
57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants