Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Support user defined CMAKE_INSTALL_INCLUDEDIR values #1584

Merged
merged 2 commits into from
Jan 12, 2022
Merged

Support user defined CMAKE_INSTALL_INCLUDEDIR values #1584

merged 2 commits into from
Jan 12, 2022

Conversation

robertmaynard
Copy link
Collaborator

Fixes #1541

Requires CUB PR: NVIDIA/cub#415

@robertmaynard robertmaynard added type: enhancement New feature or request. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. labels Dec 17, 2021
@alliepiper alliepiper added this to the 1.16.0 milestone Dec 17, 2021
@alliepiper alliepiper added helps: rapids Helps or needed by RAPIDS. P1: should have Necessary, but not critical. labels Dec 17, 2021
@alliepiper alliepiper self-requested a review December 17, 2021 17:43
@robertmaynard
Copy link
Collaborator Author

robertmaynard commented Dec 17, 2021 •
edited
Loading

Thinking about this more I believe we can make this easier by removing the custom INFIX variable and instead allow the user to specify a custom CMAKE_INSTALL_INCLUDEDIR and propagate that during configuration of the installed files

@robertmaynard robertmaynard changed the title Allow thrust headers to be installed into a subdirectory of include Support user defined CMAKE_INSTALL_INCLUDEDIR values Dec 20, 2021
@robertmaynard
Copy link
Collaborator Author

Thinking about this more I believe we can make this easier by removing the custom INFIX variable and instead allow the user to specify a custom CMAKE_INSTALL_INCLUDEDIR and propagate that during configuration of the installed files

Updated the logic to install based on CMAKE_INSTALL_INCLUDEDIR. Not only did this make the logic easier to follow, it matches CMake best practises.

alliepiper
alliepiper suggested changes Jan 10, 2022
View reviewed changes
cmake/ThrustInstallRules.cmake Outdated Show resolved Hide resolved
cmake/ThrustInstallRules.cmake Outdated Show resolved Hide resolved
cmake/ThrustInstallRules.cmake Outdated Show resolved Hide resolved
thrust/cmake/thrust-config-version.cmake Outdated Show resolved Hide resolved
thrust/cmake/thrust-header-search.cmake Outdated Show resolved Hide resolved
thrust/cmake/thrust-header-search.cmake Outdated Show resolved Hide resolved
thrust/cmake/thrust-header-search.cmake.in Outdated Show resolved Hide resolved
@alliepiper
Copy link
Collaborator

LGTM

run tests

@alliepiper alliepiper force-pushed the thrust_install_infix branch 2 times, most recently from 4423171 to 208f45e Compare January 12, 2022 18:59
@alliepiper
Copy link
Collaborator

Last pushes just rebased and bumped the CUB submodule to resolve conflicts.

@alliepiper alliepiper merged commit d50708f into NVIDIA:main Jan 12, 2022
@robertmaynard robertmaynard deleted the thrust_install_infix branch January 12, 2022 20:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alliepiper alliepiper alliepiper approved these changes

Assignees

@robertmaynard robertmaynard

Labels
helps: rapids Helps or needed by RAPIDS. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. P1: should have Necessary, but not critical. type: enhancement New feature or request.
Projects
None yet
Milestone
1.16.0
Development

Successfully merging this pull request may close these issues.

Allow Thrust and CUB headers to be nested in a custom directory
2 participants
@robertmaynard @alliepiper