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

rapids_cpm_find is more invariant as one would expect #51

Conversation

robertmaynard
Copy link
Contributor

Fixes #49

rapids_cpm_find now looks at the GLOBAL_TARGETS entries and will not call CPMFindPackage if any of them already exist.

This makes it safe to call rapids_cpm_find no matter what existing targets already exist. therefore users of rapids-cmake don't need to do manual if(TARGET ...) checks, resulting in missing packages in the export set.

@robertmaynard robertmaynard added bug Something isn't working non-breaking Introduces a non-breaking change 3 - Ready for Review Ready for review by team labels Jul 30, 2021
@robertmaynard
Copy link
Contributor Author

Need to add tests

Fixes rapidsai#49

rapids_cpm_find now looks at the `GLOBAL_TARGETS` entries and
will not call CPMFindPackage if any of them already exist.

This makes it safe to call `rapids_cpm_find` no matter what
existing targets already exist. therefore users of rapids-cmake
don't need to do manual `if(TARGET ...)` checks, resulting
in missing packages in the export set.
@robertmaynard robertmaynard force-pushed the rapids_cpm_find_more_invariant branch from bfbf274 to e362d2f Compare August 2, 2021 14:57
@robertmaynard
Copy link
Contributor Author

@gpucibot merge

1 similar comment
@robertmaynard
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit f367fe6 into rapidsai:branch-21.10 Aug 2, 2021
@robertmaynard robertmaynard deleted the rapids_cpm_find_more_invariant branch August 2, 2021 16:20
rapids-bot bot pushed a commit that referenced this pull request Aug 18, 2021
Requires #48 and #51

Adds rapids_cpm_<PackageName> for common packages

Fixes #32

The following packages are now are easier to user for RAPIDS projects
as rapids-cmake offers a pre-configured setup for each project.

-   GTest
-   NVBench
-   RMM
-   SpdLog
-   Thrust

On top of providing a consistent version of these packages to all RAPIDS projects, rapids-cmake now is able to deduce when these projects should also be installed.

```cmake

rapids_cpm_gtest(BUILD_EXPORT_SET myproject)
rapdis_cpm_rmm(BUILD_EXPORT_SET myproject
               INSTALL_EXPORT_SET myproject)
```
Given the above snippet when RMM is built as a subcomponent of `myproject` it will be installed as well. This is done since
RMM is part of the INSTALL export set, and therefore must be available from an installed version of `myproject`.

Authors:
  - Robert Maynard (https://github.com/robertmaynard)

Approvers:
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #52
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 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.

[BUG] rapids_cpm_find isn't as invariant as one would expect
1 participant