[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