[PATCH] D151503: [CUDA] correctly install cuda_wrappers/bits/shared_ptr_base.h

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/lib/Headers/CMakeLists.txt:516
   COMPONENT cuda-resource-headers)
 
 install(
----------------
qiongsiwu1 wrote:
> Do we need an install target for `${cuda_wrapper_bits_files}` for the `cuda-resource-headers` component as well? It seems to be the case because this patch is treating `${cuda_wrapper_bits_files}` as part of `cuda-resource-headers`.
> 
> ```
> add_header_target("cuda-resource-headers" "${cuda_files};${cuda_wrapper_files};${cuda_wrapper_bits_files}")
> ```
> 
> 
I'm not sure I understand the question. Are you saying that a separate `install()` for the 'bits' is not necessary and we could just install all headers with a single `install` above?

If that's the case, then, AFAICT, the answer is that we do need a separate `install`. 
`install(FILES)` does not preserve the directory structure and dumps all files listed in `FILES`, regardless if they are in different directories into the same DESTINATION directory.
That is exactly the problem this patch is intended to fix. We do need to place the file under `cuda_wrappers/bits/` directory and that's why we have separate `install(DESTINATION ${header_install_dir}/cuda_wrappers/bits)` here.

`install(DIRECTORY)` would presumably preserve the source directory structure, but we lose per-file granularity. It may work for the files under cuda_wrappers for now, but I think there's some merit in explicitly controlling which headers we ship and where we put them. While we do have 1:1 mapping between the source tree and install tree, it may not always be the case.





Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151503/new/

https://reviews.llvm.org/D151503