[PATCH] D115535: [libunwind] Provide a way to conveniently install libunwind headers
Louis Dionne via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 15 14:01:48 PST 2021
ldionne added inline comments.
================
Comment at: libunwind/include/CMakeLists.txt:19-23
+ install(FILES mach-o/compact_unwind_encoding.h
+ DESTINATION include/mach-o
+ COMPONENT unwind-headers
+ PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
+ )
----------------
rZhBoYao wrote:
> ldionne wrote:
> > Instead, why don't we simply do
> >
> > ```
> > set(files
> > __libunwind_config.h
> > libunwind.h
> > mach-o/compact_unwind_encoding.h
> > unwind_arm_ehabi.h
> > unwind_itanium.h
> > unwind.h
> > )
> > ```
> >
> > above and always include that header?
> At first, I actually tried that, only to find out `mach-o/compact_unwind_encoding.h` would then be installed as `<CMAKE_INSTALL_PREFIX>/include/compact_unwind_encoding.h` instead of `<CMAKE_INSTALL_PREFIX>/include/mach-o/compact_unwind_encoding.h`, which I think is undesirable.
I would replace it with this:
```
foreach(file ${files})
get_filename_component(dir ${file} DIRECTORY)
install(FILES ${file}
DESTINATION "include/${dir}"
COMPONENT unwind-headers
PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
)
endforeach()
```
That's what we do for libc++. That way, we don't have platform specific logic and it should work without modification if new headers are added in the future.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115535/new/
https://reviews.llvm.org/D115535
More information about the llvm-commits
mailing list