#5667: dont install std algorithm headers multiple times #5670
Conversation
cmake/kokkos_tribits.cmake
Outdated
| @@ -534,7 +534,7 @@ ELSE() | |||
| CMAKE_PARSE_ARGUMENTS(PARSE | |||
| "" | |||
| "" | |||
| "HEADERS;SOURCES" | |||
| "HEADERS;SOURCES;NOINSTALLHEADERS" | |||
There was a problem hiding this comment.
I don't think that change is needed. Please undo.
There was a problem hiding this comment.
Are the parsed arguments used at all?
There was a problem hiding this comment.
It is, sadly required because I need to pass it in to tribits, but our function will throw a cmake error on an unrecognized argument in non-tribits mode 🤷
The argument is not used in non-tribits mode. This is yet another thing we will have to strike when we don't ahve to support tribits.
There was a problem hiding this comment.
Where is the error emitted from? I thought cmake_parse_arguments allowed for unrecognized arguments.
There was a problem hiding this comment.
Maybe not with multi value keyword args? The documentation doesn't seem to say. I get:
CMake Error at cmake/kokkos_tribits.cmake:18 (if):
if given arguments:
"NOINSTALLHEADERS" <all the headers here>
Unknown arguments specified
Call Stack (most recent call first):
cmake/kokkos_tribits.cmake:127 (VERIFY_EMPTY)
cmake/kokkos_tribits.cmake:542 (KOKKOS_INTERNAL_ADD_LIBRARY_INSTALL)
algorithms/src/CMakeLists.txt:26 (KOKKOS_ADD_INTERFACE_LIBRARY)
There was a problem hiding this comment.
I suppose that is fine. Thanks for getting to the bottom of it
There was a problem hiding this comment.
I still fail to see where the parsed arguments are used below. It seems that only NAME is relevant.
There was a problem hiding this comment.
Oh I see, yes I think we could actually remove the parse arguments. I'm not 100% sure why it is there.
|
btw, this is not going to affect only the std algorithms but everything under algorithms since you are modifying the algorithm-level cmakefile. right? |
|
retest this please |
|
retest this please |
|
There was no need to re-run. I would have ignored the Cuda timing based failure :/ |
:_: |
|
Cherry-pick to 3.7.02 submitted with #5711 |
[3.7.02] Dont install std algorithm headers multiple times (#5670)
Note that this will install them under
<install_dir>/include/std_algorithms. Any code that depends on them being in<install_dir>/includewill break.Fixes #5667