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

Introduce RAPIDS.cmake a better way to fetch rapids-cmake #45

Merged

Conversation

robertmaynard
Copy link
Contributor

The original approach of using FetchContent naively to acquire
rapids-cmake has a subtle bug, which can easily be resolved
by using the new and improved RAPIDS.cmake approach.

Now to acquire rapids-cmake projects can do the following:

file(DOWNLOAD
    https://github.com/rapidsai/rapids-cmake/blob/branch-<Major>.<Minor>/RAPIDS.cmake
    ${CMAKE_BINARY_DIR})
include(${CMAKE_BINARY_DIR}/RAPIDS.cmake)

This will automatically fetch rapids-cmake if that hasn't happened
before. Otherwise unlike the old approach it will ensure that
rapids-cmake is in CMAKE_MODULE_PATH, solving a subtle bug.
When multiple adjacent projects included rapids-cmake via CPM they
would hit a bug where the second project couldn't find any rapids-cmake
modules.

@robertmaynard robertmaynard added improvement Improves an existing functionality non-breaking Introduces a non-breaking change 3 - Ready for Review Ready for review by team Refactor labels Jul 20, 2021
@robertmaynard robertmaynard requested a review from a team as a code owner July 20, 2021 20:00
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

@robertmaynard
Copy link
Contributor Author

rerun tests

The original approach of using FetchContent naively to acquire
rapids-cmake has a subtle bug, which can easily be resolved
by using the new and improved RAPIDS.cmake approach.

Now to acquire rapids-cmake projects can do the following:
```cmake
file(DOWNLOAD
    https://github.com/rapidsai/rapids-cmake/blob/branch-<Major>.<Minor>/RAPIDS.cmake
    ${CMAKE_BINARY_DIR})
include(${CMAKE_BINARY_DIR}/RAPIDS.cmake)
```

This will automatically fetch rapids-cmake if that hasn't happened
before. Otherwise unlike the old approach it will ensure that
rapids-cmake is in CMAKE_MODULE_PATH, solving a subtle bug.
When multiple adjacent projects included rapids-cmake via CPM they
would hit a bug where the second project couldn't find any rapids-cmake
modules.
@robertmaynard
Copy link
Contributor Author

rerun tests

@robertmaynard
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8cdc275 into rapidsai:branch-21.10 Jul 23, 2021
@robertmaynard robertmaynard deleted the fea/introduce_rapids.cmake branch July 23, 2021 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team improvement Improves an existing functionality non-breaking Introduces a non-breaking change Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants