Skip to content

Conversation

@wence-
Copy link
Contributor

@wence- wence- commented Oct 22, 2025

Description

Since #18235, cudf has not needed CCCL patches. However, that change missed reordering the get_XXX includes, so we have still always been cloning and using a separate CCCL version. Fix that by reordering. We also move the search for RMM before searching for NVTX since an RMM build can also supply that.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Since rapidsai#18235, cudf has not needed CCCL patches. However, that change missed
reordering the `get_XXX` includes, so we have still always been cloning and
using a separate CCCL version. Fix that by reordering. We also move the
search for RMM before searching for NVTX since an RMM build can also supply
that.
@wence- wence- requested a review from a team as a code owner October 22, 2025 10:43
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue labels Oct 22, 2025
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Even without patches, the current ordering is still relevant. We want to be able to override CCCL versions for testing purposes, which requires RMM after CCCL. Is there a problem you wanted to solve with this, or is it just an attempt at fixing the comment?

@wence-
Copy link
Contributor Author

wence- commented Oct 22, 2025

Even without patches, the current ordering is still relevant. We want to be able to override CCCL versions for testing purposes, which requires RMM after CCCL. Is there a problem you wanted to solve with this, or is it just an attempt at fixing the comment?

Fixing the comment. Also, I noticed that a main build of cudf was downloading another (identical) copy of cccl.

Presumably we can still try new versions, but I guess we must remember to change two files, rather than one.

@bdice
Copy link
Contributor

bdice commented Oct 22, 2025

That’s expected behavior (and consistent across RAPIDS) but we can fix the comment so that it doesn’t mention patches. We don’t really want to rely on transitive dependency fetching, because it makes it confusing to override. We’ve had lots of difficulties with that in the past.

@wence- wence- closed this Oct 22, 2025
@bdice bdice reopened this Oct 22, 2025
@bdice bdice added doc Documentation non-breaking Non-breaking change labels Oct 22, 2025
@wence-
Copy link
Contributor Author

wence- commented Oct 22, 2025

/merge

@rapids-bot rapids-bot bot merged commit a431dbe into rapidsai:main Oct 25, 2025
502 of 508 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CMake CMake build issue doc Documentation libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants